Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

/metadata endpoint should expose all the available keys #1223

Open
tvdijen opened this issue Feb 20, 2023 · 2 comments
Open

/metadata endpoint should expose all the available keys #1223

tvdijen opened this issue Feb 20, 2023 · 2 comments

Comments

@tvdijen
Copy link
Contributor

tvdijen commented Feb 20, 2023

The IDP- and SP-metadata endpoints should add KeyDescriptor-elements for all (or all non-expired) available keys to accommodate for a key-rollover.
This has been discussed multiple times, but every time I have to do key rollover I run into this issue, esp. because we sign our AuthnRequests towards IDPs.
See this old thread; https://groups.google.com/g/openconext/c/1oexBnx5KHA/m/Z9V9vYWIBwAJ

@thijskh
Copy link
Member

thijskh commented May 23, 2023

The status quo is that engineblock metadata generation does not support this classic SAML keyrollover. Engineblock will accept them, so if you generate xml that includes both keys you can perform this key rollover - you can just not use the Engineblock built in generated metadata currently.

Engineblock does support a gradual key rollover for SP's (IdP endpoint) via generated metadata that lists a key and an SSO location with that key ID in the same metadata. This means that any SP consuming that metadata has a working combination. This has the advantage that the rollover is gradual (SPs that consume the new metadata will be using the new key immediately) and more importantly that one can monitor progress of phase out of the old key and which SPs still use it.

Adding all configured keys to the generated metadata endpoints should be possible. We just need to ensure not to break the gradual rollover scenario. Endpoints with a specific keyID should therefore only contain the respective key.

@tvdijen
Copy link
Contributor Author

tvdijen commented May 23, 2023

I don't see how classic key-rollover can 'bite' the Dutch way.?
Anyway, reading your response I get the feeling I'm being misunderstood; SP's are not my concern and the Dutch way is usually fine.

The problem is that we sign our AuthnRequests towards IdP's and during a rollover we need the IdP's to be aware of both the old and the new cert. The Dutch way doesn't apply to IdP's, and so I think the key-slugs don't make any sense on the SP-metadata and we actually need a metadata file containing both old+new keys (EB SP metadata, but I see no harm in providing one for EB IdP metadata as well).

@ArnoutvdKnaap looked into this and the Twig-templates seem to be aware of the fact that the md:KeyDescriptor can be an array, but the array never contains more than one key.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants