-
Notifications
You must be signed in to change notification settings - Fork 13
Define how to validate Entity Statements #228
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
Conversation
…nd trust_mark rules
zachmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is some inconsistency with regards to claims that can only occur in entity configurations / subordinate statements. For some claims it is listed as something to check, for others not.
This should be made consistent, either by adding the required check to all claims or by adding a general step to verify that no "wrong" claims are present.
Co-authored-by: Giuseppe De Marco <demarcog83@gmail.com>
Co-authored-by: Gabriel Zachmann <gabriel.zachmann@kit.edu>
| MUST exactly match the <spanx style="verb">kid</spanx> value | ||
| for a key in the <spanx style="verb">jwks</spanx> (JWK Set) Claim | ||
| of the Entity Configuration of the issuing Entity. | ||
| </t> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To better support key rollover, replace 837 to 843 with:
The Entity Statement's kid (Key ID) header parameter value
MUST be a non-zero length string and
MUST exactly match the kid value
for a key in the jwks (JWK Set) Claim
of the Entity Configuration of the issuing Entity.
The jwks Claim MAY contain multiple active signing keys
(for example, during key rotation). A validator MUST accept any signature
that verifies successfully with a currently published key in that set.
Publishers SHOULD retain the retiring key alongside the new key
for an overlap period sufficient to allow dependent Entity Statements
to refresh before the old key is withdrawn.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation section isn't the place to add statements about how the "jwks" claim is used. If you want to file an issue suggesting text to add in the claim definition, I'd welcome that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it and accept the recommendation. Will work through the way to get these items organized for discussion.
| its signature validates with a public key of the Trust Anchor. | ||
| </t> | ||
| </list> | ||
| </t> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is another comment that probably warrants its own issue. Please file one with suggested wording updates. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it and accept the recommendation as in previous comments. Will work through the way to get these items organized for discussion.
| used for the Explicit Registration. | ||
| This Claim MUST NOT be present in Entity Statements that are not | ||
| Explicit Registration responses. | ||
| </t> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update 932-939 with this to support possible multiple trust anchors
If the trust_anchor Claim is present,
validate that its value is an Entity Identifier corresponding to one
of the configured Trust Anchors in the validator’s Anchor Set.
Implementations MAY retrieve the Entity Configuration for the
Entity Identifier and validate that it is a Trust Anchor,
and they MAY also validate that it is the Trust Anchor
used for the Explicit Registration.
Validators MAY also accept Explicit Registration responses that omit
the trust_anchor Claim if the associated Trust Chain
terminates at any Trust Anchor in the Anchor Set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
| A Trust Anchor MUST publish an Entity Configuration about itself. | ||
| The expiration time (exp) set on this Entity Configuration should be chosen | ||
| such that it ensures that federation participants re-fetch it at reasonable intervals. | ||
| When a Trust Anchor rolls over its signing keys, it needs to: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
..at reasonable intervals that take into consideration the key roll over grace window where the new Trust Anchor key is valid and the old one is valid to allow for adequate key material circulation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
| After a reasonable time period, remove the old keys. What is | ||
| regarded as a reasonable time is dependent on the security profile | ||
| and risk assessment of the Trust Anchor. | ||
| </t> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto. This deserves an issue of its own right for discussion by the working group.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it and accept the recommendation and add it to the items for discussion like the others.
teamktown
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Augmented some existing comments alongside others and the flow of trustmark verification.
- added where necessary the recognition of multiple keys for CA rollover into the Entity Statement verification process so that it allowed for a set of CAs
- add some operational aspects to key rollover section
No new fields, no behaviour removals. Only action item is to ensure the validator picks the correct key by kid from a set and to optionally allow multiple anchors. No durations mandated and included guidance to model grace window to reflect refresh expectations to not be in conflict thus avoiding to short a refresh window for slow refreshes thus breaking trust due to rollover anchor aged out but not yet fetched.
While back compatible with deployed OpenIDFed it will reveal where libraries are overly rigid to a single key/single TA and not a set. There are clear, testable paths forward with this that in turn will mean more seamless keyrollover experience.
teamktown
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to proceed.
Will be working through feedback from @selfissued for items I commented on.
This PR explicitly defines how to validate entity statements. It begins by defining how to validate all entity statements.
While currently incomplete, I will add rules for validating specific kinds of entity statements, such as those used for Explicit Registration.
Fixes #84