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

Implement SAML2 assertion access control #113

Closed
johakoch opened this issue Feb 5, 2021 · 7 comments · Fixed by #128
Closed

Implement SAML2 assertion access control #113

johakoch opened this issue Feb 5, 2021 · 7 comments · Fixed by #128

Comments

@johakoch
Copy link
Collaborator

johakoch commented Feb 5, 2021

An access control to be used in a SAML2 assertion consumer service endpoint.

https://cheatsheetseries.owasp.org/cheatsheets/SAML_Security_Cheat_Sheet.html mentions some security issues and how to address them.

https://tools.ietf.org/html/rfc7522#section-3 (which describes a similar case) has some requirements for assertion format and processing requirements.

The following is a mix of requirements:

  • check RelayState param (= RelayState from AuthN request query param)
  • always perform schema validation on the XML document prior to using it for any security-­related purposes.
  • validate SAML2 response:
    • valid signature (from IdP metadata file (and samlp:Response/ds:Signature))
    • samlp:Response/@Destination (= ACS endpoint URL of SP)
    • samlp:Response/@InResponseTo (= samlp:AuthnRequest/@ID of the AuthN request)
    • samlp:Response/saml:Issuer (= entity_id of IdP, from IdP metadata file)
    • check samlp:Response/samlp:Status/samlp:StatusCode/@Value
  • validate incoming SAML2 assertion:
    • valid signature (from IdP metadata file (and saml:Assertion/ds:Signature))
    • saml:Assertion/saml:Issuer (= entity_id of IdP, from IdP metadata file)
    • saml:Assertion/saml:Conditions/saml:AudienceRestriction/saml:Audience (= entity_id of SP, from Couper config)
    • has saml:Assertion/saml:Subject/saml:Subject
    • saml:Assertion/saml:Conditions/@NotOnOrAfter or saml:Assertion/saml:Subject/saml:SubjectConfirmation/saml:SubjectConfirmationData/@NotOnOrAfter
      • saml:Assertion/saml:Conditions/@NotOnOrAfter: if time passed, reject Assertion
      • saml:Assertion/saml:Subject/saml:SubjectConfirmation/saml:SubjectConfirmationData/@NotOnOrAfter: if time passed, reject SubjectConfirmation
    • saml:Assertion/saml:Subject/saml:SubjectConfirmation/@Method (= 'urn:oasis:names:tc:SAML:2.0:cm:bearer')
    • saml:Assertion/saml:Subject/saml:SubjectConfirmation/saml:SubjectConfirmationData/@Recipient (= ACS endpoint URL of SP)
    • saml:Assertion/saml:Subject/saml:SubjectConfirmation/saml:SubjectConfirmationData/@InResponseTo (= samlp:AuthnRequest/@ID of the AuthN request)
    • check saml:Assertion/saml:AuthnStatement/saml:AuthnContext/saml:AuthnContextClassRef
    • if invalid: create HTML error response, because the request is a front-channel request
  • transform attributes about the authenticated user into claims, which may be used elsewhere in JWT creation
  saml2 "SAML_AC" {
    # the XML metadata about the identity provider
    idp_metadata_file = "idp-metadata.xml"
    # the entity id of the service provider (couper)
    entity_id = "my-couper-api"

    # single sign on service (idp)
    sso_binding = "HTTP-Redirect" # irrelevant for access control?
    # assertion consumer service (couper)
    acs_binding = "HTTP-Post"
    # alternative: binding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect"

    # whether to generate JSON claims
    generate_json_claims = true
    # which `saml:Attribute` elements are expected to possibly have multiple `saml:AttributeValue` element children
    array_attributes = ["memberOf"]
    
    # generates:
    #claims = {
    #  iss = "https://the-id-provider.com…" # saml:Assertion/saml:Issuer
    #  sub = "username" # saml:Assertion/saml:Subject/saml:NameID
    #  exp = 1612371650 # saml:Assertion/saml:Conditions/@NotOnOrAfter -> unixtime
    #  nbf = 1612300000 # saml:Assertion/saml:Conditions/@NotBefore -> unixtime
    #  iat = 1612300010 # saml:Assertion/@IssueInstant -> unixtime
    #  aud = "my-couper-api" # saml:Assertion/saml:Conditions/saml:AudienceRestriction/saml:Audience
    #  # loop saml:Assertion/saml:AttributeStatement/saml:Attribute -> @Name = ./saml:AttributeValue
    #  displayName = "Doe, John (External)"
    #  objectSid = "S-1-5-21-123456789-0123456789-123456789-000001"
    #  mail = "john@doe.name"
    #  # Attribute with multiple AttributeValue's become arrays?
    #  memberOf = [
    #    "admin",
    #    "users",
    #    "CN=Special AD Group,OU=foo,DC=example,DC=com"
    #  ]
    #}
  }
@filex
Copy link

filex commented Feb 22, 2021

in addition to preparing a ready-to-user claims object in req.ctx.SAML_AC.claims , we could provide some of the assertion values in SAML lingo:

req.ctx.SAML_AC.Issuer
req.ctx.SAML_AC.Subject (or rather .NameID?)
req.ctx.SAML_AC.NotOnOrAfter
…

Otherwise we couldn't use specific values if generate_json_claims = false.

On the other hand, I don't see a reason why we shouldn't always generate the jwt claims. Then, req.ctx.SAML_AC.claims.exp is as good as req.ctx.SAML_AC.NotOnOrAfter.

@johakoch
Copy link
Collaborator Author

req.ctx.SAML_AC provides information from the SAML assertion:

  • ...SAML_AC.iss: issuer from saml:Assertion/saml:Issuer
  • ...SAML_AC.sub: subject ID from saml:Assertion/saml:Subject/saml:NameID
  • ...SAML_AC.exp expiration date (unixtime) from saml:Assertion/saml:Conditions/@NotOnOrAfter
  • ...SAML_AC.nbf not before date (unixtime) from saml:Assertion/saml:Conditions/@NotBefore
  • ...SAML_AC.iat issued at date (unistime) from saml:Assertion/@IssueInstant
  • ...SAML_AC.aud audience from saml:Assertion/saml:Conditions/saml:AudienceRestriction/saml:Audience (array?)
  • ...SAML_AC.attributes object of attributes from saml:Assertion/saml:AttributeStatement/saml:Attribute, e.g.:
    • ...SAML_AC.attributes.displayName
    • ...SAML_AC.attributes.mail
    • ...SAML_AC.attributes.memberOf

generate_json_claims is not necessary.

@filex
Copy link

filex commented Feb 25, 2021

we still need the array_attributes attribute to keep memberOf with a single entry collapsing into a string.

we dropped the idea of ….claims, because it is unlikely to use the whole set of data from the assertion in jwt_sign() as claims. iss, aud, exp will probably be different, nbf and iat, too. we use JWT sprech for the field names, because that is the intended sink for this data.

@johakoch
Copy link
Collaborator Author

I currently use github.com/russellhaering/gosaml2. It requires to set a AssertionConsumerServiceURL, so I added an additional property to the couper.hcl config:

  saml2 "MySAML" {
    idp_metadata_file = "idp-metadata.xml"
    sp_entity_id = "my-sp"
    sp_acs_url = "https://my-sp/saml/acs"
    array_attributes = ["memberOf"]
  }

@johakoch johakoch linked a pull request Feb 26, 2021 that will close this issue
@johakoch
Copy link
Collaborator Author

johakoch commented Mar 5, 2021

Just for the record: github.com/russellhaering/gosaml2 has some issues, that are currently not relevant for us:

  • it uses only the first Assertion within a Response (per spec there may be several Assertions)
  • it uses only the first SubjectConfirmation within a Subject (per spec there may be several SubjectConfirmations)

@johakoch
Copy link
Collaborator Author

johakoch commented Mar 8, 2021

The lib provides a function BuildAuthURL() on a SAMLServiceProvider struct which builds an SSO URL.

The created XML looks like (white-space added):

<samlp:AuthnRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
                    xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"
                    ID="_80314219-e90f-4f36-8071-068b0191dedd"
                    Version="2.0"
                    ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
                    AssertionConsumerServiceURL="https://the-sp.example.com/api/v1/saml/acs"
                    IssueInstant="2021-03-08T15:01:34Z"
                    Destination="https://the-idp.example.org/SSOService">
  <saml:Issuer>the_sp_entity_id</saml:Issuer>
  <samlp:NameIDPolicy AllowCreate="true"
                      Format="urn:oasis:names:tc:SAML:2.0:nameid-format:transient"/>
</samlp:AuthnRequest>

with

  • AuthnRequest/@AssertionConsumerServiceURL can be read from saml block sp_acs_url
  • AuthnRequest/@Destination can be read from idp_metadata: IDPSSODescriptor/SingleSignOnService/@Location
  • AuthnRequest/Issuer can be read from saml block sp_entity_id
  • AuthnRequest/NameIDPolicy/@Format can be read from idp metadata: IDPSSODescriptor/NameIDFormat

The values for AuthnRequest/@Version, AuthnRequest/@ProtocolBinding and AuthnRequest/NameIDPolicy/@AllowCreate are hard-coded in the lib.

@filex
Copy link

filex commented Mar 9, 2021 via email

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

Successfully merging a pull request may close this issue.

2 participants