SAML-Toolkits / ruby-saml

SAML SSO for Ruby
https://developers.onelogin.com/v1.0/page/saml-toolkit-for-ruby-on-rails
MIT License
898 stars 561 forks source link

Add sp_cert_multi to facilitate SP cert/key rotation #673

Open johnnyshields opened 7 months ago

johnnyshields commented 7 months ago

Fixes https://github.com/SAML-Toolkits/ruby-saml/issues/560

This PR introduces sp_cert_multi parameter which is analogous to idp_cert_multi. It allows developers to have fine-grained control over SP certs and private keys, including:

  1. Allow SP to have separate SP signing and encryption keys (required by some governmental regulations.)
  2. Allow SP to rotate SP private keys, not just certificates (for example, if key becomes compromised.)
  3. Allow SP to have more than two concurrent pairs of SP certs/keys in rotation at once.

The changes are summarized as follows:

  1. Add SamlSettings sp_cert_multi parameter. It has the following shape:

    sp_cert_multi = {
     signing: [
       { certificate: cert1, private_key: private_key1 },
       { certificate: cert2, private_key: private_key2 }
     ],
     encryption: [
       { certificate: cert1, private_key: private_key1 },
       { certificate: cert3, private_key: private_key3 }
     ],
    }

    (Note: You can use same certs for signing/encryption, and same PK everywhere. It's completely backward compatible with current functionality.)

  2. sp_cert_multi is mutually exclusive with the following: certificate, certificate_new, private_key.

  3. If security[:check_sp_cert_expiry] is true, Ruby Saml automatically uses the first non-expired certificate in sp_cert_multi[:signing] for signing, and only uses private keys associated with non-expired certs in sp_cert_multi[:encryption] for decryption. This is evaluated in realtime, so as soon as your old cert expires your app automatically starts signing with the new one.

  4. The validation error :check_sp_cert_expiration is now raised only if ALL SP certs are expired. This is a slight behavior change;

    • Previously if Settings.certificate was expired but Settings.certificate_new was not, an error would be raised.
    • In this PR this case does not raise an error and starts automatically using certificate_new for signing. (This case was not previously in the tests, but I've now added a test for it with the new logic.)
  5. :check_sp_cert_expiration now also validates the certificate not_before condition; previously it was only validating not_after.

  6. If :check_sp_cert_expiration is true, we now no longer include expired certs in the generated SP metadata. This is a good practice because having expired certs may cause the IdP system to throw an error, depending on how strictly it does its validation.

    • Point of discussion: It's debatable if we want to include "not_before" certs here (i.e. not yet ready to use, but ok for IdP to pre-cache and use when ready.) For simplicity's sake I've excluded them, especially because "not_before" is relatively rare in the wild.
  7. Refactor so that internal references to get_sp_cert, get_sp_private_key, etc. now point to the new structure of multiple certs.

  8. When performing decryption, we now try all private keys under sp_cert_multi[:encryption] (this is analogous to how we try all IDP certs in idp_cert_multi[:signing] when verifiying the IDP signature.)

  9. Extract out OneLogin::RubySaml::Utils.build_cert_object and build_private_key_object.

  10. Deprecate the certificate_new parameter since sp_cert_multi fulfills the same role better. It still works but it is removed from the docs.

  11. When there are multiple SP certs, the ordering of SP KeyDescriptor node in the SP metadata XML will now be all signing keys first, and then all encryption keys. (Previously it would be signing, encryption, signing, encryption.) This does not affect XML integrity in any way.

  12. This PR contains unit tests and integration tests for all major SP signing flows (both Redirect and POST). Decryption is covered as well.

johnnyshields commented 7 months ago

@pitbulk this is ready for final review. Let me know what I can do to help get this merged.

johnnyshields commented 6 months ago

FYI I am using this in production now without issues.

oliverguenther commented 2 months ago

I was just looking into how to build multi_cert support for SP signing myself and great to see you already did it @johnnyshields . We're interested in getting this upstream, anything we can support in to move this forward?

johnnyshields commented 2 months ago

@pitbulk what do you think?

pitbulk commented 1 month ago

I will be adding this on next ruby-saml release. Hopefully soon.