exception while parsing IdP metadata

[huancz]

   InvalidOperationException: The specified type was not recognized: name='SecurityTokenServiceType', namespace='http://docs.oasis-open.org/wsfed/federation/200706', at <RoleDescriptor xmlns='urn:oasis:names:tc:SAML:2.0:metadata'>
   at SAML2.Utils.Serialization.Deserialize[T](XmlReader reader) in src\SAML2\Utils\Serialization.cs:line 37
   at SAML2.Utils.Serialization.DeserializeFromXmlString[T](String xml) in src\SAML2\Utils\Serialization.cs:line 51
   at SAML2.Saml20MetadataDocument..ctor(XmlDocument entityDescriptor) in src\SAML2\Saml20MetadataDocument.cs:line 91
   at SAML2.Config.IdentityProviderCollection.ParseFile(String file) in src\SAML2\Config\IdentityProviderCollection.cs:line 358
   at SAML2.Config.IdentityProviderCollection.Refresh() in src\SAML2\Config\IdentityProviderCollection.cs:line 161
   at SAML2.Config.Saml2Config.Init(Saml2Config config) in src\SAML2\Config\Saml2Config.cs:line 143
   at SAML2.Config.Saml2Config.InitFromConfigFile() in src\SAML2\Config\Saml2Config.cs:line 131
   at SAML2.Config.Saml2Config.get_Current() in src\SAML2\Config\Saml2Config.cs:line 51
   at SAML2.Logging.LoggerProvider..cctor() in src\SAML2\Logging\LoggerProvider.cs:line 26

Corresponding line from IdP (whole file is available at https://tnia.eidentita.cz/FPSTS/FederationMetadata/2007-06/FederationMetadata.xml):

<RoleDescriptor xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:fed="http://docs.oasis-open.org/wsfed/federation/200706" xsi:type="fed:SecurityTokenServiceType" [...]

If I should guess, reason why this element is present may be because this IdP claims to support both ws-federation and SAML2 and developers can choose which to implement. But they only provide one metadata file. The RoleDescriptor seems like it belongs to the other spec, and is useless for SAML 2.0 use case. This library has class with the same name, but

• it's an abstract class, it was never expected to encounter RoleDescriptor as raw element and the library doesn't use it directly (and if I fiddle with the IdP metadata and remove the xsi:type declaration, xml parser DOES throw because we are asking for an instance of an abstract class -> ignoring it shouldn't break any previously working uses)
• the content provided by NIA has very little overlap with the class structure (KeyDescriptor is present, but none of the fed: elements are)

I found two workarounds while investigating this, both horrible for general usage. I the end, all that is needed is this:

diff --git a/src/SAML2/Schema/Metadata/EntityDescriptor.cs b/src/SAML2/Schema/Metadata/EntityDescriptor.cs
index e03bb5b..31d542a 100644
--- a/src/SAML2/Schema/Metadata/EntityDescriptor.cs
+++ b/src/SAML2/Schema/Metadata/EntityDescriptor.cs
@@ -128,7 +128,6 @@ namespace SAML2.Schema.Metadata
         [XmlElement(AuthnAuthorityDescriptor.ElementName, typeof(AuthnAuthorityDescriptor), Order = 3)]
         [XmlElement(IdpSsoDescriptor.ElementName, typeof(IdpSsoDescriptor), Order = 3)]
         [XmlElement(PdpDescriptor.ElementName, typeof(PdpDescriptor), Order = 3)]
-        [XmlElement(RoleDescriptor.ElementName, typeof(RoleDescriptor), Order = 3)]
         [XmlElement(SpSsoDescriptor.ElementName, typeof(SpSsoDescriptor), Order = 3)]
         public object[] Items { get; set; }

Unknown element is silently skipped, and I can get as far as redirect to login endpoint on IdP with something that looks like valid assertion request (big base64-urlencoded string. I didn't yet go as far as trying to decode it back to XML. The request fails, but likely for unrelated reason).

[i8beef]

I think you're right, its due to the combined IDP metadata file.

I will admit, my skills with the default Microsoft XML serializer are lacking, and this whole section was inherited from OIOSAML.NET (and I think THEY inherited from somewhere else too if I remember right). I don't think that was ever meant to support anything outside of the SAML spec, and I seem to remember that Microsoft's serializer here is fairly light on options to control multiform item lists like this... IF I UNDERSTAND CORRECTLY removing the XmlElement by itself is going cause lots of other issues...

Its been a minute since I've used SAML, but if you just edit the IDP metadata file and remove that RoleDescriptor piece altogether (and probably the signature) does it still accept it without the signature? If you trust the metadata source, and you aren't using the metadata fetcher (that's just a nice to have, you can just as easily supply the static metadata file yourself), that might work to get you past this without modification.

[huancz]

if you just edit the IDP metadata file and remove that RoleDescriptor piece altogether (and probably the signature) does it still accept it without the signature

This is similar to the first of my "horrible workarounds". I didn't remove the signature from file, I disabled signature checking instead, removed abstract keyword from the RoleDescriptor class, and removed the xsi:type="fed:SecurityTokenServiceType" from the metadata (I left rest of the RoleDescriptor intact).

I just tried your suggestion and it works too. I'd rather avoid both, because of maintenance burden. I probably won't be the one who will be deploying it to production, and if I have to choose what to modify, I'll include modified SAML2 library in the project rather than documenting how one should remove signature and RoleDescriptor elements from IdP metadata.

Anyway, I have several solutions how to go forward, this may be more of a PSA type of bugreport - others might run into the same issue.

[i8beef]

Ultimately the XML deserializer is just namespace sensitive here. I will look around and see if there's a way to get it to ignore elements with a namespace that isn't in a known good (SAML) list, but so far I'm coming up blank on this. That would be the BEST solution, but the Microsoft XML serializer is pretty limited in some of those options without implementing your own custom XML serializer for every type...

I would think maintaining a separate fork is more of a maintenance headache than maintaining the XML document modifications as a workaround... if you remove the signature node, you should be able to modify it without failing the signature check, and you'd basically just be "un-merging" that metadata document so you have ONLY a SAML metadata document. Alternatively, you might look and see if your provider has a "SAML only" option somewhere.

[i8beef]

Note that my advice here seems to be rather common when an IDP provider provides a metadata document with ws-fed extensions to be used by pure SAML solutions (another example telling you to do exactly what I'm suggesting).

Also note that this IDP metadata as provided doesn't pass regular XML metadata validations (https://www.samltool.com/validate_xml.php)

Basically it looks like in the spec it allows for implementations to SUPPORT arbitrary extensions like this AT THE CHOICE OF THE IMPLEMENTATION. A generic SAML implementation (i.e., this library) has no obligation to support arbitrary extensions like this... if you needed to, creating the entire RoleDescriptor XML serialization classes in a fork of this library to properly support this would be necessary...

Or you modify the IDP metadata to remove the extensions you aren't using anyway. Thanks for pointing this out, its a useful piece that hopefully others can find now, but I think my recommendation remains the same: modify the IDP metadata.

If you can find a way to tell the XML serializer to ignore the unknown element and still respect the Order tags, etc., I'm open to adding that though. I've just come up blank in ways to do that.

[huancz]

Thanks for the links, you may be right here, and editing metadata may be the right to deal with this after all. But I found something else today (http://docs.oasis-open.org/security/saml/Post2.0/sstc-saml-metadata-ext-query-os.html):

2.3 Element The element defined in [SAML2Meta] is an abstract extension point that contains descriptive information common across various entity roles. New roles can be defined by extending its abstract md:RoleDescriptorType complex type, which is the approach taken here.

Spec says that RoleDescriptor is abstract. The validator you linked confirms it:

Line: 1 | Column: 0 --> Element '{urn:oasis:names:tc:SAML:2.0:metadata}RoleDescriptor', attribute '{http://www.w3.org/2001/XMLSchema-instance}type': The QName value '{http://docs.oasis-open.org/wsfed/federation/200706}SecurityTokenServiceType' of the xsi:type attribute does not resolve to a type definition.

Line: 1 | Column: 0 --> Element '{urn:oasis:names:tc:SAML:2.0:metadata}RoleDescriptor': The type definition is abstract.

The way I read all this, you should never encounter RoleDescriptor in valid SAML2 metadata. Only types derived from it, and even then it's only interesting if you want to implement some of the extensions. So, it should be safe to just ignore it, and it's inclusion as one of deserializable items was probably error in original OIOSAML.NET. Same as I said in OP, but now supported by little more than just my gut feeling.

If you can find a way to tell the XML serializer to ignore the unknown element and still respect the Order tags, etc., I'm open to adding that though. I've just come up blank in ways to do that.

See the proposed patch in OP, it does exactly that. I don't understand the part about keeing order tags, my knowledge of microsoft XML serializer is about as weak as yours, probably even weaker.

[i8beef]

Ok, Im about convinced.

Microsoft's XML serializer does some odd things when you include the Order attribute option (as I do). I had to refresh myself here, but basically it defines an order to serialize properties (why I use it to get proper generation order), but ALSO uses it when DESERIALIZING (unfortunate consequence) so that if an element appears out of order it is SKIPPED and left NULL. I had to refresh myself because I couldn't remember if it would skip elements not in the SPECIFIC ordinal location defined: that's not the case, it just makes sure its in the right order IN RELATION to the other elements. It's weird, and always throws me, so I just wanted to make sure removing this wouldn't break all other deserialization when there are multiple EntityDescriptors caused by those Order params.

I am willing to make this change, but might need to revert if anyone comes along with a use case that DOES require it here.

[i8beef]

Try 3.2.0.60

[huancz]

I tested it today... and apparently I managed to botch my previous tests. 3.2.0.60 doesn't break parsing of valid metadata, but it still cannot parse my IdP. It doesn't directly throw, I can access metadata.ashx. But when there is RoleDescriptor present, it will cause the parser to ignore ANY further elements. Like IDPSSODescriptor. And it will of course throw later when attempting to redirect.

This should be the proper fix, based on this answer

diff --git a/src/SAML2/Schema/Metadata/EntityDescriptor.cs b/src/SAML2/Schema/Metadata/EntityDescriptor.cs
index 31d542a..e9cc8d3 100644
--- a/src/SAML2/Schema/Metadata/EntityDescriptor.cs
+++ b/src/SAML2/Schema/Metadata/EntityDescriptor.cs
@@ -129,6 +129,7 @@ namespace SAML2.Schema.Metadata
         [XmlElement(IdpSsoDescriptor.ElementName, typeof(IdpSsoDescriptor), Order = 3)]
         [XmlElement(PdpDescriptor.ElementName, typeof(PdpDescriptor), Order = 3)]
         [XmlElement(SpSsoDescriptor.ElementName, typeof(SpSsoDescriptor), Order = 3)]
+        [XmlAnyElement(Order = 3)]
         public object[] Items { get; set; }

         /// <summary>

I thoroughly tested this change with both original (invalid) MD and MD with removed RoleDescriptor. My first guess would be that it should break when I move RoleDescriptor to another place (like after element and place RoleDescriptor before it). But I placed dummy elements before and after any SAML2 element and the configuration is still parsed correctly.

Looks like [XmlAnyElement] will just become dump for every unrecognized element, regardless of Order.

[i8beef]

Unfortunately XmlAnyElement needs to be the last item in the object Order wise. You'll find this messes up the elements that can FOLLOW this location in the XML. I haven't been able to figure out how to allow arbitrary RoleDescriptors... I might ask around because its an interesting problem, but one I think I need some help on...

In the mean time, I have created a unit test around this, and I have a method that... works. It basically makes a dummy object SPECIFICALLY for the wsfed extension, so it will deserialize, with a basically useless object that will be ignored for the most part by my code (I think it might import the key information, but it just wouldn't be used for anything).

This isn't ideal, since basically any extension type would have to be explicitly "supported" with such a type.

I just pushed another version that I think should (a) let you keep moving, (b) keep the backwards compat for RoleDescriptor in case there are cases we don't see right now, and (c) adds testing method that allows us to try other things and test quicker. See if that gets you past where you are, its a little bit of a hack, but not the worst thing I've ever written...

[i8beef]

Also, if you would, make sure YOUR metadata SERIALIZES correctly by hitting the metadata endpoint... those EntityDescriptors are used in a few places and while Deserialization works here, I'm not sure if the tests cover enough of the serialization validation...

[i8beef]

Wow, this is an ugly solution... but it would be one way to allow you to keep everything, signature and all, and basically strip it and modify it on read in to eliminate any elements of an unknown namespace: https://www.reddit.com/r/dotnet/comments/a5j73v/handling_missing_classes_in_xml_serialization/ebpg0gl/

Gonna think about this over the weekend a bit and see if I can come up with anything I hate less than the dummy element.

[huancz]

Sorry for not getting back to you earlier, right now I have to divide time between several projects.

It basically makes a dummy object SPECIFICALLY for the wsfed extension, so it will deserialize, with a basically useless object that will be ignored for the most part by my code (I think it might import the key information, but it just wouldn't be used for anything).

This isn't ideal, since basically any extension type would have to be explicitly "supported" with such a type.

I agree, this was something I was trying to avoid too. Writing "support" like this for nonstandard extensions feels wrong. And it will still have problem with order - if metadata contains wsfed element at the wrong place, it will still blow up. If there is one popular tool that generates md like this it might be worthwhile thing to do, but... icky.

Also, if you would, make sure YOUR metadata SERIALIZES correctly by hitting the metadata endpoint... those EntityDescriptors are used in a few places and while Deserialization works here, I'm not sure if the tests cover enough of the serialization validation...

As far as I can tell, it still works, and looks like it contains everything important. But I can't check against the real IdP, the registration process is taking way longer than anticipated.

And another "but" - after comparing standard authn requests with the example, I noticed that NIA requires some modifications to authn exchange too. They mostly don't use SP metadata (they can import SP cert from it during setup as one option, but that's it). Instead of caching requested attributes from metadata, they require SP to send them as an extension in authn requests.

So, even if we somehow convince XmlSerializer to ignore unknown elements in generic and elegant way, I'll still have to make a fork and implement the NIA quirks. There is "quirks mode" hidden in the code, but it looks like a workaround for one very specific problem with XML signatures, this would be whole another level.

Right now I'm still considering how to approach it. Best idea yet would be some kind of configurable interceptor class. It would get access to untyped XmlDocuments at the right points in lifetime and be able to modify them. With authn requests it could inject the extensions I need before signing. With metadata, if I put call to this between signature checking and XmlSerializer call, it could be elegant way to deal with <RoleDescriptor>, or any other nonstandard stuff - just remove it. And the maintenance burden would be at the proper place - people like me trying to talk to "mostly SAML but still nonstandard implementations". I thought of reusing <actions> configuration and adding few more extensibility points, but the IAction interface isn't suitable, so... something similar.

[i8beef]

The problem I think I have here is that the Xml serialization is pretty strict by design. Opening up direct access to the XmlDocuments isn't something I would do because it breaks with the entire underlying methodology of the library. Add in the fact that the SAML spec is fairly strict as well about order of elements, etc., and people could easily get into trouble, and there's just too much of the library predicated on the XML handling all being rather tightly coupled here.

If you can figure out how to work in extension support within that framework, I'd be open to it, but I don't think direct exposure of the Xml documents instead of working through the serialization library is a generic solution I can support... unfortunately I ALSO haven't used this library myself in over a decade now as most of my projects are OAuth driven these days, I'm more of a glorified caretaker right now here.

If you'd like to take that on, I definitely accept PRs if you can test it real well. I'm a little conservative on modifications here given that I don't have a good test bed to verify things. The small test suite is about as close as I really have to verify stuff.

Another option is to wrap up changes in an EXTENSION library like I did with the OIOSAML.NET validation specifics (https://github.com/i8beef/SAML2.Profiles.DKSAML20). What YOU need I think is pretty far beyond the extension points that that provides though... so it'd probably require quite a number of hooks to be added to allow for overriding default behavior with external implementations.