-
Notifications
You must be signed in to change notification settings - Fork 10
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
Create draft-saml-ipsie-sl1-profile.md #65
base: main
Are you sure you want to change the base?
Conversation
Initial cut from Aaron's OpenID Connect SL1 profiel
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.
Awesome work @topperge getting this draft together for the group to review! I did a 1st pass of the draft and had some feedback.
- shall only support pre-registered Service Providers with established trust relationships; | ||
- shall authenticate Service Providers using X.509 certificates or another strong authentication method; | ||
- shall sign all assertions using XML Signature [XMLDSig]; | ||
- shall encrypt all assertions containing personally identifiable information (PII) or sensitive attributes using XML Encryption [XMLEnc] with AES-GCM with a minimum key length of 128 bits; |
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.
Does FAL2 bring in this requirement?
While I understand that assertions with HTTP-POST/Redirect bindings go through the browser's front channel and could be introspected, requiring encryption would be a large change from what most SaaS apps offer today. A lot of apps would need to invest in retrofitting their SAML implementation just to be SL1 IPSIE compliant. Enterprises are the resource owner for IPSIE deployments and often already have contracts with vendors that establishes how user data is managed/secured/protected.
I think this is good topic for the group to align on as it will have a lot of impact on what apps can be IPSIE level 1.
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.
As I recall, FAL2 requires this today under rev3. Rev4 requires encrypted assertions if they include PII and go through the front channel.
- shall sign all assertions using XML Signature [XMLDSig]; | ||
- shall encrypt all assertions containing personally identifiable information (PII) or sensitive attributes using XML Encryption [XMLEnc] with AES-GCM with a minimum key length of 128 bits; | ||
- shall not expose open redirectors; | ||
- shall only accept entity IDs registered in metadata as audience values; |
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.
Can a SP Entity ID be statically registered and not from metadata?
- shall encrypt all assertions containing personally identifiable information (PII) or sensitive attributes using XML Encryption [XMLEnc] with AES-GCM with a minimum key length of 128 bits; | ||
- shall not expose open redirectors; | ||
- shall only accept entity IDs registered in metadata as audience values; | ||
- shall issue assertions with a maximum lifetime of 5 minutes; |
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.
I think it would be helpful to IPSIE readers and implementers to align a common set of values for both OIDC and SAML profiles for nonces/codes, assertion lifetimes, etc where possible as the goal is similar to prevent replay attacks and handle clock skews.
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.
I agree wholeheartedly. This can also extend to network controls like TLS, DNSSec, etc. We should be able to profile these once to apply to all underlying protocol mechanisms.
- shall only accept entity IDs registered in metadata as audience values; | ||
- shall issue assertions with a maximum lifetime of 5 minutes; | ||
- shall require Service Providers to be preregistered, and shall not support dynamic registration of Service Providers; | ||
- shall support assertion revocation or status verification mechanisms (such as OCSP for certificate checking); |
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.
I don't think this should be needed as we have a small lifetime of the assertion that mitigates risk. This follows where Web PKI is going with moving away from OSCP/revocation for certs and instead issuing short-lived certs (https://letsencrypt.org/2025/01/16/6-day-and-ip-certs/) due to the challenges with fail open/closed and many other well documented issues with revocation.
- shall issue assertions with a maximum lifetime of 5 minutes; | ||
- shall require Service Providers to be preregistered, and shall not support dynamic registration of Service Providers; | ||
- shall support assertion revocation or status verification mechanisms (such as OCSP for certificate checking); | ||
- shall implement key rotation procedures with a maximum key lifetime of 2 years for signing keys. |
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 seems too high IMHO. This is another opportunity to align with OIDC profile and have a common set of requirements for key rollover. If Web PKI is moving to 90 days certs (https://www.chromium.org/Home/chromium-security/root-ca-policy/moving-forward-together/), we should be able to do better than 2 years for signing keys.
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.
From experience in one of the largest, if not the largest, SAML federations in the world, this is practically unobtainable with current lack of support in many SAML implementations without constant rolling service outages due to flag day key rotations and implementations that cannot handle multiple keys in SAML metadata, or for that matter - implementations that can't consume SAML metadata at all.
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.
@dhs-BI, this might be a good topic to align group on. If we don't support automated key rollover with SAML and have long lifetimes of signing certs (e.g 2 years) to enable manual rotation and legacy app support, then there will be a signifiant gap with OIDC when it comes to enterprise security outcomes.
Do we want to push this to levels? I feel pretty strongly having had to deal with very public IdP breaches that this is a core requirement in today's world. Apps that don't support automated rollover need motivation to change. I think this delivers more security per dollar than say focusing on on FAL2's requirement for assertion encryption.
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.
@mcguinness and @sbroddy I agree this needs discussion amongst the group. I have experienced @sbroddy's concerns, and I'm aware of @mcguinness's as well.
@sbroddy In your realm of SAML federations, do you think including a rotation requirement would encourage SAML implementations to enable multiple key in the SAML metadata to support this functionality? Or is the likely outcome for those federations to ignore the guidance and fail to adopt IPSIE?
With my chair hat off: I think this is basic security hygiene that we need to embed in IPSIE at SL1. The specific timing for rotation is certainly up for discussion, but I don't believe there is a future where we don't mandate rotation on a periodic schedule. Although many implementations do not support automating rotation today, this seems to be insufficient justification to avoid adding it to the IPSIE requirements. Since the charter specifically identifies development of "interoperability and security profiles", this feels directionally correct for a secure by design deployment.
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.
SAML is an explicit key trust protocol, unlike TLS. Without discussing the nuances of that and why key rotation does or does not make sense in SAML...
Frequent key rotation in SAML would in many/most cases require architecture changes of many/most SAML implementations. Otherwise, there is an implicit flag-day outage required for any bilateral trust relationships, which most SAML implementations operate on outside of managed federations. This quickly grows to flag-day outages for rotation on the order of one a month, week, or days for medium to large deployments.
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.
My goal was only to limit impact of key compromise & enable crypto agility/reduce exposure to cryptographic weaknesses which is same goal as OIDC key rotation. I understand there are different trusts models between WebPKI, SAML, OIDC, Managed Federations, etc.
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.
Also, I neglected to mention the difficulty in key rotation is nowhere near symmetric in difficulty for an SP rotating a key versus for an IdP rotating a key in a bilateral paradigm.
If I have an SP and I want to rotate a key, I have to coordinate with one IdP (at least in a single tenant SaaS scenario). If I'm an IdP and I want/need to rotate my key, I have to coordinate with 1:N number of SPs who all have to agree to rotate the IdP key at precisely the same instant if they a) can't accommodate multiple keys and b) are not consuming metadata through some other means. If I have 100 SaaS providers, not much chance on getting them all to rotate at the same time on a flag day.
Managed (multilateral) federations solve this problem through curated metadata at least for RPs/IdPs that can support it. In a bilateral paradigm, which I would suspect most "enterprise" (for whatever definition of enterprise) SAML deployments are, don't have this mechanism.
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.
From the linked article:
Although common certificate lifecycle and management practices are important to many types of systems, they do not and should not apply to certificates that are used for SAML signing.
There are many prerequisites to enabling frequent key/cert rollover in SAML that are ancillary to the goal. Those would have to happen first before rollover is feasible without causing periodic flag-day downtime/outages.
- shall not use the HTTP 307 status code when redirecting a request that contains user credentials; | ||
- shall use the HTTP 303 status code when redirecting the user agent using status codes; | ||
- shall support RelayState values up to 80 bytes in length, may reject RelayState values longer than 80 bytes; | ||
- shall require multi-factor authentication for all authentication events and encode this fact in the AuthnContextClassRef element; |
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 requirement of when to require MFA seems higher level. This profile should just define how to serialize the context and what values must be supported for context classes
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.
Agreed - this should be part of the SL1 guidance regardless of the federation protocol used.
- shall use the HTTP 303 status code when redirecting the user agent using status codes; | ||
- shall support RelayState values up to 80 bytes in length, may reject RelayState values longer than 80 bytes; | ||
- shall require multi-factor authentication for all authentication events and encode this fact in the AuthnContextClassRef element; | ||
- shall maintain records of federation events for a minimum of 7 years or as required by applicable regulations; |
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.
Data retention requirements for IPSIE seems like a different topic to debate and not linked to levels.
- shall ensure that the metadata URL used is obtained from an authoritative source and using a secure channel, such that it cannot be modified by an attacker; | ||
- shall ensure that the entity ID used and the entity ID in the obtained metadata match; | ||
- shall implement federation termination procedures that can be executed promptly when required; | ||
- shall maintain session records that establish the link between the local session and the corresponding SAML assertion. |
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 also seems like a higher level IPSIE requirement and not linked to levels.
- shall include attribute statements that use pairwise pseudonymous identifiers for privacy protection when appropriate; | ||
- shall minimize disclosure of personal information by only including required attributes in assertions. | ||
|
||
For the Web Browser SSO Profile, Identity Providers: |
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.
Missing requirements for processing AuthnRequests such as validating signed request if we want to require signed requests which you have below for the SP
- shall use signed authentication requests; | ||
- shall verify the InResponseTo attribute matches the ID of the request that was sent; | ||
- shall check the Issuer element in the SAML response to prevent mix-up attacks; | ||
- shall implement the SAML Enhanced Client or Proxy (ECP) Profile for non-browser clients; |
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.
Do we really want to support this with IPSIE? Feels like a great way to have a MFA downgrade attack especially as we want phishing resistant MFA
|
||
## 3. Profile | ||
|
||
### 3.1. Network Layer Requirements |
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 section 3.1 should be pulled out and put into a common doc to cover both OIDC and SAML.
- When using TLS 1.2, servers shall only use cipher suites allowed in [BCP195]. | ||
- Servers shall not support CORS for the SSO endpoints, as clients must perform an HTTP redirect rather than access these endpoints directly. | ||
|
||
### 3.2. Cryptography and Signatures |
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 section should be pulled out and into a common section for OIDC and SAML. There are some SAML specific items, e.g.XMLDSig, that can remain here.
- Encryption shall use approved algorithms and key sizes as specified in NIST SP 800-131A. | ||
- Assertions shall include mechanisms to detect unauthorized modification or substitution. | ||
|
||
#### 3.4.2. Holder-of-Key Requirements |
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 required at FAL3 in the rev3 and rev4 draft. I suggest this is removed at SL1.
|
||
To meet NIST 800-63-4 FAL2 compliance, Identity Providers: | ||
|
||
- shall require multi-factor authentication (MFA) for all authentication events without exception; |
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 an IPSIE SL1 requirement, should be documented for both federation mechanisms
- shall not expose open redirectors; | ||
- shall only accept entity IDs registered in metadata as audience values; | ||
- shall issue assertions with a maximum lifetime of 5 minutes; | ||
- shall require Service Providers to be preregistered, and shall not support dynamic registration of Service Providers; |
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.
If I'm interpreting this correctly, this is unworkable in federations like InCommon/EduGAIN where the trust anchor is with the FedOp and there can always be service providers which are dynamically registered from the perspective of the IdP.
Initial cut from Aaron's OpenID Connect SL1 profile with a more explicit approach and mapping to the upcoming 800-63 Rev 4 verbiage. Definitely not complete, but at least something to start from.