Spring Security SAML Certificate/credentials rollover support

Context: Credential rollover

A SAML 2.0 configuration uses anywhere between 1 to 4 different types of certificates. All of these need to be configured by both the Asserting Party (Identity Provider) and Relying Party (Asserting Party). When one of the certificates expires, this needs to be changed over to a new certificate, preferably without any downtime for the users. This is generally done by configuring two certificates during a rollover window, letting the other party update the certificate and then removing the old one.

The certificate types are: - Encryption (optional) - Provided by the Identity Provider. Used to encrypt the authentication request. Currently unsupported by Spring Security. Assuming the Identity Provider has configured both credentials configured already, might break the moment the Identity Provider stops supporting the old one. - Verification (required) - Provided by the Identity Provider. Used to verify the assertions sent by the Identity Provider. Goes correct if both are present in the Metadata XML. Spring Security will pick the one that's been used to sign with. - Decryption (optional) - Provided by the Service Provider. Used to decrypt encrypted assertions (if used). Goes correct if both are configured in Spring Security. It will pick the one that works. - Signing (optional) - Provided by the Service Provider. Used to sign the authentication request, if desired by the Identity Provider. This is the main one that currently goes wrong.

Current Behavior

When multiple signing credentials are configured, OpenSAML just picks the first one that satisfies all conditions and signs with it. Since the credentials are stored in a HashSet (see RelyingPartyRegistration.java) there is no control over which one it is. If the new signing credentials are used before they are configured on the Identity Provider, this will lead to authentication failures.

Expected Behavior

It should be possible to control which credentials are used for encryption and signing if multiple are known.

One solution is to store the list of credentials in a LinkedHashSet (I don't really understand why HashSets are the default anyway, but I digress). This way the first credential added that satisfies all conditions will be used.

For encryption credentials the newest credentials (with latest expiry) will likely be the best candidate, assuming the Identity Provider has already configured them. If the data is loaded from a metadata XML, this feels like a likely assumption.

For signing credentials the oldest credentials should be used until they either expire, or the Identity Provider has loaded the new credentials, in which case it's safe to remove the old ones.

Context

Spring Security version: 5.5.3, but looking through the git sourcecode, there have been no significant changes here. OpenSAML version: 3.4.6. OpenSAML 4 also does not seem to have changed as far as I can tell from the source.

• How has this issue affected you? This issue has not affected us yet, but we'd like to have this fixed before the first users can no longer login due to expired certificates.
• What are you trying to accomplish? Implementing a SAML 2.0 credentials rollover mechanism that does not affect users.
• Are you aware of any workarounds? If request signing is disabled, there is no issue. If the Identity Provider can configure multiple verification credentials it's possible to configure the new credentials on the Identity Provider first, and afterwards configure Spring Security. This does mean the metadata XML needs to be bypassed.

Comment From: svschouw-bb

There might be a more convoluted workaround that also works: use a different RelyingPartyRegistration definition for the Saml2MetadataFilter compared to the one used for the OpenSamlAuthenticationProvider. Have the metadata version expose both signing credentials already, but let the authentication provider version use the older version for now. Then after rollover use the new signing credentials for both.

Note that if the same credentials are used for decryption the reverse should be done: the metadata version should use the new one already, and the authentication provider version should use both.

This workaround does require splitting the RelyingPartyRegistrationRepository and relyingPartyRegistrationResolver.

Comment From: svschouw-bb

Maybe the "convoluted" workaround from the previous comment is actually the proper way to do it. If so, that might make it mostly a documentation issue. I have actually no experience with how Spring Boot configures all this.

Comment From: jzheaux

@svschouw-bb, thanks for your insights. I think using LinkedHashSet makes sense, would you be able to provide a PR that makes this change?

As for introducing a custom sorting mechanism, I wonder if there is an OpenSAML credentials resolver or criteria that Spring Security should be using that follows your intuitions about which to use first. If not, I think the easiest way is to 1. rely on the metadata to affirm the preferred order or 2. rely on application developers to add them in their needed order.

Comment From: svschouw-bb

I could probably provide a Pull Request to change the HashSet to a LinkedHashSet , although I would probably have to see how hard it is to provide a unittest for it (a HashSet might accidentally provide the right order).

I'm also having a lot of trouble getting the Spring Security tests to run. I'm not super familiar with Gradle.

But the more I think about it, the more I think this is mostly a metadata problem. As long as both parties provide the correct metadata, there is no real ambiguity. The Identity Provider (Asserting Party) should provide the newest encryption certificate and all verification certificates. The Service Provider (Relying Party / Spring Security) should provide only the newest (active?) certificate for encryption, but use all (non-expired) credentials for decryption. It should use an active credential (oldest non-expired?*) for signing, but provide the new (as well as the current) certificate ahead of time for validation.

Assuming the Asserting Party does things properly as well the above an already by accomplished by splitting the RelyingPartyRegistrations into a metadata version and an authentication version, like I mentioned in https://github.com/spring-projects/spring-security/issues/10799#issuecomment-1028342310, but this requires splitting the RelyingPartyRegistrationRepository and relyingPartyRegistrationResolver as well. If the RelyingPartRegistration could have an active signing and active encryption credential (not necessarily the same!) the Saml2MetadataFilter and OpenSamlAuthenticationProvider could make use of this.

Then only the issue remains where Identity Providers provide all certificates for encryption instead of just the newest. But Spring Security does not support request encryption anyway, and I don't know if there are any Identity Providers that do this wrong.

(*) Be sure not to use the oldest non-expired credentials to the very last second. By the time the request arrives at the Identity Provider, the credentials could have expired!

Comment From: jzheaux

Good points all around, @svschouw-bb.

Maybe the "convoluted" workaround from the previous comment is actually the proper way to do it. If so, that might make it mostly a documentation issue.

I'm not sure that setting up different registrations in this way was the intent of the design. It seems your approach works because the registration used by the authentication provider has only one credential, thus guaranteeing the order. I think the use of HashSet was simply an oversight.

But the more I think about it, the more I think this is mostly a metadata problem. As long as both parties provide the correct metadata, there is no real ambiguity.

Agreed, though to have a RelyingPartyRegistration reflect the same order as specified by the metadata, the underlying datatype will have to guarantee that order is preserved.

I could probably provide a Pull Request to change the HashSet to a LinkedHashSet

Great!

although I would probably have to see how hard it is to provide a unittest for it (a HashSet might accidentally provide the right order).

I can see that. What I'd probably try is add the creds in one order, then add them in another order, and ensure that the iteration comes out as entered either way. For example, the following test will fail:

Set<String> s = new HashSet<>();
s.add("bob");
s.add("bill");
assertThat(s).containsExactly("bob", "bill");
s.clear();
s.add("bill");
s.add("bob");
assertThat(s).containsExactly("bill", "bob");

Comment From: svschouw-bb

Ah yes, that's a good way to test it of course.

Still, I think I would prefer a way to say in the metadata "start using this and only this certificate for encryption", but to still accept the old certificate while the Identity Provider is being updated. Because if you provide both/all certificates there is no guarantee that the asserting party will use the newest one. Of course, using the workaround by splitting the RelyingPartyRegistration works.

Agreed, though to have a RelyingPartyRegistration reflect the same order as specified by the metadata, the underlying datatype will have to guarantee that order is preserved.

There is nothing in the SAML 2.0 spec that I could find that says anything about the order. Only that all keys provided must be supported:

[E68]The inclusion of multiple elements with the same use attribute (or no such attribute) indicates that any of the included keys may be used by the containing role or affiliation. A relying party SHOULD allow for the use of any of the included keys. When possible the signing or encrypting party SHOULD indicate as specifically as possible which key it used to enable more efficient processing.

Source: https://www.oasis-open.org/committees/download.php/35391/sstc-saml-metadata-errata-2.0-wd-04-diff.pdf * Source: http://saml.xml.org/saml-specifications

So switching from HashSet to LinkedHashSet, other than sanity reasons, will really only solve the "Signing" case with RelyingPartyRegistration.Builder.signingX509Credentials(c -> { c.add(oldKey); c.add(newKey); }). We have no guarantee that the Identity Provider will provide the best encryption key first, if they provide multiple.

My suggestion: split the signingX509Credentials into activeSigningX509Credentials and additionalSigningX509Credentials and split the decryptionX509Credentials into activeDecryptionX509Credentials and additionalDecryptionX509Credentials. Document that activeSigningX509Credentials is for the current signing case and additionalSigningX509Credentials is for future signing keys. Then activeDecryptionX509Credentials should be the newest decryption credentials and additionalDecryptionX509Credentials is for older still supported credentials.

Technically the signingX509Credentials doesn't need to be split if it's a LinkedHashSet, but for symmetry reasons I would split them. Make sure the old ones stay deprecated but supported.

The biggest question is whether activeSigningX509Credentials and activeDecryptionX509Credentials should be collections or single elements. I don't know if there are esoteric reasons to have multiple of them (the OpenSAML credentials resolver picks the first matching one, but I don't know what that means effectively). On the other hand, there might also be no reason not to make them collections.

The OpenSamlMetadataResolver will use the activeDecryptionX509Credentials and all the signing credentials. The OpenSamlSigningUtils will use the activeSigningX509Credentials and the OpenSamlDecryptionUtils will use all the decryption credentials.

I will try to make a PR for the simple HashSet to LinkedHashSet change (does that need a CLA? If so, that might take a little while).

Additionally I could make a PR for the credentials split. Although maybe the solution needs to be discussed more first.

Comment From: jzheaux

Good points, @svschouw-bb. I can see the advantage you are describing to publish as metadata a more constrained set of keys than the ones that will be used when authenticating.

We have no guarantee that the Identity Provider will provide the best encryption key first, if they provide multiple.

I'm not certain what a framework can do to help with this scenario. We have to pick one, and relying on the asserting party to use order of preference seems the most logical. If an asserting party can't do that, then applications can provide custom implementations for more sophisticated selection criteria. Or they can adapt the asserting party's metadata response into a RelyingPartyRegistration with the correctly-ordered keys.

My suggestion: split the signingX509Credentials into activeSigningX509Credentials

I'm inclined to favor convention in these cases. Spring Security can continue to pick the first one in the list that matches the signing criteria and can continue to iterate over decryption keys to attempt decryption.

I'd recommend starting with changing the datatype to preserve ordering. Yes, a CLA will be required since it is not an Obvious Fix. If you are not able to provide this, I can make the change instead, just let me know.

Comment From: svschouw-bb

I'm inclined to favor convention in these cases. Spring Security can continue to pick the first one in the list that matches the signing criteria and can continue to iterate over decryption keys to attempt decryption.

That would work for the authentication case, but not necessarily for the metadata case. Unless you also only publish the first certificate for encryption. But this must then be documented, since it is not at all intuitive. My first intuition (in fact, my implementation before I started to really think about this) would be to just add all keys in chronological order (which is what you would want for signing anyway).

I'd recommend starting with changing the datatype to preserve ordering. Yes, a CLA will be required since it is not an Obvious Fix. If you are not able to provide this, I can make the change instead, just let me know.

CLA is being worked on. I have a branch ready to go as soon as that is done.

Comment From: svschouw-bb

In theory the CLA should be signed? I'm not seeing it show up yet. Although on the CLA page it says "Thank you for signing the Agreement!" I'm not entirely sure where I'm supposed to look.

Comment From: jzheaux

Closed in f13320ba5659ea5527e72af9e7e0b54f643d7a57