Attribute Statement List being incompletely read

[TheDarkTrumpet]

Hello,

I came across a recent issue, and am looking into it a bit more but figured I'd report it here first.

While I got the library to work with what I need, the big issue that I'm running into is an incomplete list of assertions being pulled.

The assertions being read from the breakpoint are, in order:
eduPersonEntitlement
uid
eduPersonScopedAffiliation
eduPersonTargetedID

The others available are in the raw xml below, but it stops around DisplayName. My guess is that is being seen as an invalid attribute, and stops reading.

The assertion list, that comes from SAMLTracer is below (some private information removed):

<saml2:AttributeStatement>
     <saml2:Attribute FriendlyName="eduPersonEntitlement"
          Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.7"
          NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
           >
            <saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
               xsi:type="xsd:string"
               >urn:mace:dir:entitlement:common-lib-terms</saml2:AttributeValue>
      </saml2:Attribute>
      <saml2:Attribute FriendlyName="uid"
            Name="urn:oid:0.9.2342.19200300.100.1.1"
            NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
            >
            <saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                     xsi:type="xsd:string"
                                      >dthole</saml2:AttributeValue>
            </saml2:Attribute>
        <saml2:Attribute FriendlyName="eduPersonScopedAffiliation"
             Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.9"
             NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
             >
            <saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                      xsi:type="xsd:string"
                      >affiliate@unc.edu</saml2:AttributeValue>
               <saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                      xsi:type="xsd:string"
                      >member@unc.edu</saml2:AttributeValue>
            </saml2:Attribute>
       <saml2:Attribute FriendlyName="eduPersonTargetedID"
               Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.10"
               NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
               >
                <saml2:AttributeValue>
                    <saml2:NameID Format="urn:oasis:names:tc:SAML:2.0:nameid-format:persistent"
                                  NameQualifier="https://sso.unc.edu/idp"
                                  SPNameQualifier="https://dev.newview.io/saml"
                                  >REMOVED</saml2:NameID>
                </saml2:AttributeValue>
            </saml2:Attribute>
            <saml2:Attribute FriendlyName="displayName"
                             Name="urn:oid:2.16.840.1.113730.3.1.241"
                             NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
                             >
                <saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                                      xsi:type="xsd:string"
                                      >David Thole</saml2:AttributeValue>
            </saml2:Attribute>
            <saml2:Attribute FriendlyName="sn"
                             Name="urn:oid:2.5.4.4"
                             NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
                             >
                <saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                                      xsi:type="xsd:string"
                                      >Thole</saml2:AttributeValue>
            </saml2:Attribute>
            <saml2:Attribute FriendlyName="givenName"
                             Name="urn:oid:2.5.4.42"
                             NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
                             >
                <saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                                      xsi:type="xsd:string"
                                      >David</saml2:AttributeValue>
            </saml2:Attribute>
            <saml2:Attribute FriendlyName="pid"
                             Name="urn:oid:1.3.6.1.4.1.10411.3103.1.1.1.1"
                             NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
                             >
                <saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                                      xsi:type="xsd:string"
                                      >NUMBERREMOVED</saml2:AttributeValue>
            </saml2:Attribute>
            <saml2:Attribute FriendlyName="eduPersonPrincipalName"
                             Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.6"
                             NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
                             >
                <saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                                      xsi:type="xsd:string"
                                      >dthole@unc.edu</saml2:AttributeValue>
            </saml2:Attribute>
        </saml2:AttributeStatement>

[i8beef]

Is that the ACTUAL assertion or an example? Are you sure those attributes are being SENT in the assertion at all?

Also do you have a RequestedAttributes (https://github.com/i8beef/SAML2/wiki/Metadata-Element) element in your config specifying these extra attributes you want? SAML let's you specify what attributes your application requires from the IDP, and if you aren't specifying these, it's possible they are only sending you their "defaults".

[TheDarkTrumpet]

Yeah, this is pretty much the exact thing sent by the IDP. I removed some very private information, but described the element type it was. To describe that process, I used Firefox and SamlTracer to pull the SAML from the redirect, and what you see pasted above is a slight modification of exactly what they provided. So yeah, I'm sure that's the actual assertion.

Regarding the Metadata element, I actually tried that before pasting here. I tried removing all but eduPersonPrincipalName, since that's the one I actually need - and no change. The 4 that came through originally were the 4 that came through after changing the Web.conf. One thing I haven't tried in this case is trying to specify the name specifically (urn:oid:1.3.6.1.4.1.5923.1.1.1.6) to see if that'd come through.

I did end up stepping through a fair amount of the code looking into this before I asked UNC to update their attributes and where I left off is

SAML2/src/SAML2/Validation/Saml20StatementValidator.cs

Line 62 in c07b7a4

foreach (var o in statement.Items)

- which didn't have the extra attributes (only the first 4). Really comparing the DisplayName and others that parsed, the only thing I could think of that was different was the space. Looking in the code, I didn't see any default number to pull in.

They, since this morning, modified the attributes being sent back, so I'm not able to test it with that instance. The example app I made you a contributor on is where I'll continue testing this on. I asked the uiowa identity team (my group) to expose those same attributes to my test app, which has a lot more logging in place for me to go further on.

Which, leads me to the other issue I submitted. I apologize for not getting back with you on these in a timely manner. I'm the only developer working on this product, and I have a lot of hard deadlines that won't ease up til the end of the month. I have been keeping a log of items that may need addressing (at least in documentation), and I have a few code fixes I may make pull requests for. I have every intention to really help out on this, right now I'm more or less in crunch time so just keeping notes of what's going on right now.

As an aside, I'm working with my university and Internet2 to create some documentation on Shibboleth authentication in Azure that references your library and getting it all setup. My intention is to take that example app, and through a wiki, include this guide. I'm not sure if Internet2 will update their documentation to include my stuff, or provide a link, or whatever. Still, I recently had Wisconsin asking for help, which I provided the example app and they got Shibboleth working. If you're interested in seeing this document, I can share it with you while I work on generalizing it and getting the example app totally finished.

[TheDarkTrumpet]

One thing I forgot to mention that's worth mentioning. This isn't a high priority item exactly, and the task is as much for me as it is for you. I'm hoping to dive into the code and try and fix this issue and include a pull request, assuming you don't do it first. I'm finding that I'm learning quite a bit from the way you handle XML and the deserializing from the assertion, and as a programmer - I want to learn how this stuff works. This, right now, isn't blocking anything.

[i8beef]

I suspect that you have a serialization issue around eduPersonTargetedID. Notice there is a saml2:NameID element within the AttributeValue. That isn't supported by the SAML spec. You sure you pasted that right?

[TheDarkTrumpet]

I was pretty sure I pasted it exactly from SamlTrace, and I didn't notice this til you mentioned it.

Once my university (uiowa) updates the attributes released to my test app, I'll try it again with them. If it successfully pulls everything in, then I'll see if UNC minds updating the released attributes they send to me and go from there.

I'll need to be a bit careful. I don't want to inconvenience our partner university too much, but if this is a bug in how they are sending information out of the shibboleth idp, then I would assume they want to know about it and would want to fix it. I'll be in touch on that. I think my university will have this updated this week, so by the end of the week I should have an update once they release the attributes.

Thanks again for your help. I'll be in touch as soon as they get this setup, earliest tomorrow after this immediate deadline.

[sosumi]

So, saml2:NameID as the AttributeValue of eduPersonTargetedID is perfectly valid per the SAML spec. In fact, it's literally the definition of the the attribute.

The other university (who I won't name here) isn't doing anything wrong. It's the same way my institution releases the attribute.

If that's the issue, then that's a deserialization bug in this library.

[i8beef]

That may be what they are sending, but according to the OASIS SAML 2 specification, I'm not seeing that as a valid option: https://www.oasis-open.org/committees/download.php/35711/sstc-saml-core-errata-2.0-wd-06-diff.pdf

That particular element typically occurs in the saml:Subject element, not under Attributes, kind of like these examples: https://www.samltool.com/generic_sso_res.php

Can you point to the spec location that says complex types are supported under AttributeValue?

[i8beef]

If this is indeed the issue, I'd expect you would be throwing an InvalidOperation exception during the deserialization step of the assertion load. If this is to be supported, I suspect some sort of custom serialization would have to be put in place to explicitely treat all sub-nodes of AttributeValue as a string value instead of attempting deserialize any found XML as an object. I'm not sure how to do that, so I'd have to look a bit and see if I can figure out how to do that without some drastic changes to the serialization approach.

[sosumi]

Per the latest version of the standard (with errata included): https://www.oasis-open.org/committees/download.php/56777/sstc-saml-core-errata-2.0-wd-07-diff.pdf

Lines 1313 - 1314:

The element supplies the value of a specified SAML attribute. It is of the
xs:anyType type, which allows any well-formed XML to appear as the content of the element.

eduPersonTargetedID is formally defined in the eduPerson schema here: http://software.internet2.edu/eduperson/internet2-mace-dir-eduperson-201602.html#eduPersonTargetedID

It's more specifically defined in a SAML context in the MACE-Dir SAML Attribute Profile: http://macedir.org/docs/internet2-mace-dir-saml-attributes-latest.pdf

A definition put together by the Swiss Identity and Access Federation (with links to relevant documentation) can be found here: https://www.switch.ch/aai/support/documents/attributes/edupersontargetedid/

For Scott Cantor's take on the history of the eduPersonTargetedID attribute and how it came about (you'll note that he's the lead editor of the OASIS SAML 2.0 standard), see here: https://wiki.shibboleth.net/confluence/display/SHIB2/NativeSPTargetedID

I'm not saying it's a fantastic solution, but it's nearly universal practice in the higher education SAML 2.0 space, and is perfectly allowable per the specification.

[i8beef]

Yeah, that's interesting. Kind throws a wrench in a few things, but I think we could probably handle this by taking a slightly longer route around the problem... Right now this library assumes that everything passed to it as an AttributeValue will be a string, but the serializer is going to hate having XML in those fields. I can't find an easy way to make it just take the content and treat it as a string value during serialize / deserialize, so I think the best bet is probably going to be to make AttributeValue and actual type in Schema/Core, with an XmlText property as well as an XmlAnyElement property to catch both cases.

We don't use any of the attributes DIRECTLY in code (aside from attribute queries, which I still need to look at), and it actually just slaps the attribute element directly into the Principal (something we inherited...) That would let anyone using the Saml20Principal set by the included SamlPrincpalAction read the value of the attribute as EITHER an Attribute.Value property (which would be null in this case), or an Attribute.AnyElement property (which would contain the XmlElement structure passed in the attribute).

I can put together those changes this week.

What actually makes me real curious though is how you don't have an exception getting thrown that's blowing up the whole process. This totally should cause an XmlSerialization issue, and I can't for the life of me find anywhere that I SWALLOW said exception... in the Saml20AbstractEndpoint class there's a try catch wrapping the handlers, but on Exception it rethrows (it only catches for logging purposes).

[i8beef]

Can you guys try that branch and see if it gets you over the hump here? I still need to review the rest of the pieces this has an effect on, but I think this will make your assertion deserialize at least. When you go to get the values that are passed, if a compelx object is passed like the NameId above, you will have to access the Attribute.AttributeValues and check if it's Value is null. You will have access to an XmlElement structure though in there as "AnyElements" for any pieces you want.

[TheDarkTrumpet]

I can try it around Thursday, I have a few major issues I need to get out by tomorrow afternoon.