From e46f2ebe40551599ff12a22bde90e7b50677ac15 Mon Sep 17 00:00:00 2001 From: Holger Koser Date: Thu, 26 Jan 2017 19:05:40 +0100 Subject: [PATCH] Improve SAML Signature and Response Validation * Improve Order of Namespace Declarations and Attributes in Canonical XML. This is related to an issue in goxmldsig for which I created an [pull request](https://github.com/russellhaering/goxmldsig/pull/17). * Do not compress the AuthnRequest if `HTTP-POST` binding is used. * SAML Response is valid if the Message and/or the Assertion is signed. * Add `AssertionConsumerServiceURL` to `AuthnRequest` * Validate Status on the Response * Validate Conditions on the Assertion * Validation SubjectConfirmation on the Subject --- connector/saml/saml.go | 181 +++++++++++-- connector/saml/saml_test.go | 241 +++++++++++++++++- connector/saml/testdata/idp-cert.pem | 26 ++ .../testdata/idp-resp-signed-assertion.xml | 29 +++ .../idp-resp-signed-message-and-assertion.xml | 34 +++ .../saml/testdata/idp-resp-signed-message.xml | 30 +++ connector/saml/testdata/idp-resp.xml | 34 +++ connector/saml/types.go | 63 ++++- glide.yaml | 2 +- 9 files changed, 611 insertions(+), 29 deletions(-) create mode 100644 connector/saml/testdata/idp-cert.pem create mode 100644 connector/saml/testdata/idp-resp-signed-assertion.xml create mode 100644 connector/saml/testdata/idp-resp-signed-message-and-assertion.xml create mode 100644 connector/saml/testdata/idp-resp-signed-message.xml create mode 100644 connector/saml/testdata/idp-resp.xml diff --git a/connector/saml/saml.go b/connector/saml/saml.go index 0c5b806b..6e984ab6 100644 --- a/connector/saml/saml.go +++ b/connector/saml/saml.go @@ -2,8 +2,6 @@ package saml import ( - "bytes" - "compress/flate" "crypto/rand" "crypto/x509" "encoding/base64" @@ -36,6 +34,15 @@ const ( nameIDFormatKerberos = "urn:oasis:names:tc:SAML:2.0:nameid-format:kerberos" nameIDFormatPersistent = "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent" nameIDformatTransient = "urn:oasis:names:tc:SAML:2.0:nameid-format:transient" + + // top level status codes + statusCodeSuccess = "urn:oasis:names:tc:SAML:2.0:status:Success" + + // subject confirmation methods + subjectConfirmationMethodBearer = "urn:oasis:names:tc:SAML:2.0:cm:bearer" + + // allowed clock drift for timestamp validation + allowedClockDrift = time.Duration(30) * time.Second ) var ( @@ -253,6 +260,7 @@ func (p *provider) POSTData(s connector.Scopes) (action, value string, err error AllowCreate: true, Format: p.nameIDPolicyFormat, }, + AssertionConsumerServiceURL: p.redirectURI, } data, err := xml.MarshalIndent(r, "", " ") @@ -260,19 +268,7 @@ func (p *provider) POSTData(s connector.Scopes) (action, value string, err error return "", "", fmt.Errorf("marshal authn request: %v", err) } - buff := new(bytes.Buffer) - fw, err := flate.NewWriter(buff, flate.DefaultCompression) - if err != nil { - return "", "", fmt.Errorf("new flate writer: %v", err) - } - if _, err := fw.Write(data); err != nil { - return "", "", fmt.Errorf("compress message: %v", err) - } - if err := fw.Close(); err != nil { - return "", "", fmt.Errorf("flush message: %v", err) - } - - return p.ssoURL, base64.StdEncoding.EncodeToString(buff.Bytes()), nil + return p.ssoURL, base64.StdEncoding.EncodeToString(data), nil } func (p *provider) HandlePOST(s connector.Scopes, samlResponse string) (ident connector.Identity, err error) { @@ -296,6 +292,10 @@ func (p *provider) HandlePOST(s connector.Scopes, samlResponse string) (ident co } + if err = p.validateStatus(&resp); err != nil { + return ident, err + } + assertion := resp.Assertion if assertion == nil { return ident, fmt.Errorf("response did not contain an assertion") @@ -305,6 +305,13 @@ func (p *provider) HandlePOST(s connector.Scopes, samlResponse string) (ident co return ident, fmt.Errorf("response did not contain a subject") } + if err = p.validateConditions(assertion); err != nil { + return ident, err + } + if err = p.validateSubjectConfirmation(subject); err != nil { + return ident, err + } + switch { case subject.NameID != nil: if ident.UserID = subject.NameID.Value; ident.UserID == "" { @@ -348,19 +355,151 @@ func (p *provider) HandlePOST(s connector.Scopes, samlResponse string) (ident co return ident, nil } +// Validate that the StatusCode of the Response is success. +// Otherwise return a human readable message to the end user +func (p *provider) validateStatus(resp *response) error { + // Status is mandatory in the Response type + status := resp.Status + if status == nil { + return fmt.Errorf("response did not contain a Status") + } + // StatusCode is mandatory in the Status type + statusCode := status.StatusCode + if statusCode == nil { + return fmt.Errorf("response did not contain a StatusCode") + } + if statusCode.Value != statusCodeSuccess { + parts := strings.Split(statusCode.Value, ":") + lastPart := parts[len(parts)-1] + errorMessage := fmt.Sprintf("status code of the Response was not Success, was %q", lastPart) + statusMessage := status.StatusMessage + if statusMessage != nil && statusMessage.Value != "" { + errorMessage += " -> " + statusMessage.Value + } + return fmt.Errorf(errorMessage) + } + return nil +} + +// Multiple subject SubjectConfirmation can be in the assertion +// and at least one SubjectConfirmation must be valid. +// This is described in the spec "Profiles for the OASIS Security +// Assertion Markup Language" in section 3.3 Bearer. +// see https://www.oasis-open.org/committees/download.php/35389/sstc-saml-profiles-errata-2.0-wd-06-diff.pdf +func (p *provider) validateSubjectConfirmation(subject *subject) error { + validSubjectConfirmation := false + subjectConfirmations := subject.SubjectConfirmations + if subjectConfirmations != nil && len(subjectConfirmations) > 0 { + for _, subjectConfirmation := range subjectConfirmations { + // skip if method is wrong + method := subjectConfirmation.Method + if method != "" && method != subjectConfirmationMethodBearer { + continue + } + subjectConfirmationData := subjectConfirmation.SubjectConfirmationData + if subjectConfirmationData == nil { + continue + } + inResponseTo := subjectConfirmationData.InResponseTo + if inResponseTo != "" { + // TODO also validate InResponseTo if present + } + // only validate that subjectConfirmationData is not expired + now := p.now() + notOnOrAfter := time.Time(subjectConfirmationData.NotOnOrAfter) + if !notOnOrAfter.IsZero() { + if now.After(notOnOrAfter) { + continue + } + } + // validate recipient if present + recipient := subjectConfirmationData.Recipient + if recipient != "" && recipient != p.redirectURI { + continue + } + validSubjectConfirmation = true + } + } + if !validSubjectConfirmation { + return fmt.Errorf("no valid SubjectConfirmation was found on this Response") + } + return nil +} + +// Validates the Conditions element and all of it's content +func (p *provider) validateConditions(assertion *assertion) error { + // Checks if a Conditions element exists + conditions := assertion.Conditions + if conditions == nil { + return nil + } + // Validates Assertion timestamps + now := p.now() + notBefore := time.Time(conditions.NotBefore) + if !notBefore.IsZero() { + if now.Add(allowedClockDrift).Before(notBefore) { + return fmt.Errorf("at %s got response that cannot be processed before %s", now, notBefore) + } + } + notOnOrAfter := time.Time(conditions.NotOnOrAfter) + if !notOnOrAfter.IsZero() { + if now.After(notOnOrAfter.Add(allowedClockDrift)) { + return fmt.Errorf("at %s got response that cannot be processed because it expired at %s", now, notOnOrAfter) + } + } + // Validates audience + audienceRestriction := conditions.AudienceRestriction + if audienceRestriction != nil { + audiences := audienceRestriction.Audiences + if audiences != nil && len(audiences) > 0 { + issuerInAudiences := false + for _, audience := range audiences { + if audience.Value == p.issuer { + issuerInAudiences = true + break + } + } + if !issuerInAudiences { + return fmt.Errorf("required audience %s was not in Response audiences %s", p.issuer, audiences) + } + } + } + return nil +} + // verify checks the signature info of a XML document and returns // the signed elements. +// The Validate function of the goxmldsig library only looks for +// signatures on the root element level. But a saml Response is valid +// if the complete message is signed, or only the Assertion is signed, +// or but elements are signed. Therefore we first check a possible +// signature of the Response than of the Assertion. If one of these +// is successful the Response is considered as valid. func verify(validator *dsig.ValidationContext, data []byte) (signed []byte, err error) { doc := etree.NewDocument() - if err := doc.ReadFromBytes(data); err != nil { + if err = doc.ReadFromBytes(data); err != nil { return nil, fmt.Errorf("parse document: %v", err) } - - result, err := validator.Validate(doc.Root()) - if err != nil { - return nil, err + verified := false + response := doc.Root() + transformedResponse, err := validator.Validate(response) + if err == nil { + verified = true + doc.SetRoot(transformedResponse) + } + assertion := response.SelectElement("Assertion") + if assertion == nil { + return nil, fmt.Errorf("response does not contain an Assertion element") + } + transformedAssertion, err := validator.Validate(assertion) + if err == nil { + verified = true + response.RemoveChild(assertion) + response.AddChild(transformedAssertion) + } + if verified != true { + return nil, fmt.Errorf("response does not contain a valid Signature element") } - doc.SetRoot(result) return doc.WriteToBytes() } diff --git a/connector/saml/saml_test.go b/connector/saml/saml_test.go index 4e455688..ec073450 100644 --- a/connector/saml/saml_test.go +++ b/connector/saml/saml_test.go @@ -2,12 +2,24 @@ package saml import ( "crypto/x509" + "encoding/base64" "encoding/pem" "errors" "io/ioutil" + "strings" "testing" + "time" - sdig "github.com/russellhaering/goxmldsig" + "github.com/Sirupsen/logrus" + + dsig "github.com/russellhaering/goxmldsig" + + "github.com/coreos/dex/connector" +) + +const ( + defaultIssuer = "http://localhost:5556/dex/callback" + defaultRedirectURI = "http://localhost:5556/dex/callback" ) func loadCert(ca string) (*x509.Certificate, error) { @@ -22,21 +34,238 @@ func loadCert(ca string) (*x509.Certificate, error) { return x509.ParseCertificate(block.Bytes) } -func TestVerify(t *testing.T) { - cert, err := loadCert("testdata/okta-ca.pem") +func runVerify(t *testing.T, ca string, resp string, shouldSucceed bool) { + cert, err := loadCert(ca) if err != nil { t.Fatal(err) } s := certStore{[]*x509.Certificate{cert}} - validator := sdig.NewDefaultValidationContext(s) + validator := dsig.NewDefaultValidationContext(s) - data, err := ioutil.ReadFile("testdata/okta-resp.xml") + data, err := ioutil.ReadFile(resp) if err != nil { t.Fatal(err) } if _, err := verify(validator, data); err != nil { - t.Fatal(err) + if shouldSucceed { + t.Fatal(err) + } + } else { + if !shouldSucceed { + t.Fatalf("expected an invalid signatrue but verification has been successful") + } + } +} + +func newProvider(issuer string, redirectURI string) *provider { + if issuer == "" { + issuer = defaultIssuer + } + if redirectURI == "" { + redirectURI = defaultRedirectURI + } + now, _ := time.Parse(time.RFC3339, "2017-01-24T20:48:41Z") + timeFunc := func() time.Time { return now } + return &provider{ + issuer: issuer, + ssoURL: "http://idp.org/saml/sso", + now: timeFunc, + usernameAttr: "user", + emailAttr: "email", + redirectURI: redirectURI, + logger: logrus.New(), + } +} + +func TestVerify(t *testing.T) { + runVerify(t, "testdata/okta-ca.pem", "testdata/okta-resp.xml", true) +} + +func TestVerifySignedMessageAndUnsignedAssertion(t *testing.T) { + runVerify(t, "testdata/idp-cert.pem", "testdata/idp-resp-signed-message.xml", true) +} + +func TestVerifyUnsignedMessageAndSignedAssertion(t *testing.T) { + runVerify(t, "testdata/idp-cert.pem", "testdata/idp-resp-signed-assertion.xml", true) +} + +func TestVerifySignedMessageAndSignedAssertion(t *testing.T) { + runVerify(t, "testdata/idp-cert.pem", "testdata/idp-resp-signed-message-and-assertion.xml", true) +} + +func TestVerifyUnsignedMessageAndUnsignedAssertion(t *testing.T) { + runVerify(t, "testdata/idp-cert.pem", "testdata/idp-resp.xml", false) +} + +func TestHandlePOST(t *testing.T) { + p := newProvider("", "") + scopes := connector.Scopes{ + OfflineAccess: false, + Groups: true, + } + data, err := ioutil.ReadFile("testdata/idp-resp.xml") + if err != nil { + t.Fatal(err) + } + ident, err := p.HandlePOST(scopes, base64.StdEncoding.EncodeToString(data)) + if err != nil { + t.Fatal(err) + } + if ident.UserID != "eric.chiang+okta@coreos.com" { + t.Fatalf("unexpected UserID %q", ident.UserID) + } + if ident.Username != "admin" { + t.Fatalf("unexpected Username: %q", ident.UserID) + } +} + +func TestValidateStatus(t *testing.T) { + p := newProvider("", "") + var err error + resp := response{} + // Test missing Status element + err = p.validateStatus(&resp) + if err == nil || !strings.HasSuffix(err.Error(), `Status`) { + t.Fatalf("validation should fail with missing Status") + } + // Test missing StatusCode element + resp.Status = &status{} + err = p.validateStatus(&resp) + if err == nil || !strings.HasSuffix(err.Error(), `StatusCode`) { + t.Fatalf("validation should fail with missing StatusCode") + } + // Test failed request without StatusMessage + resp.Status.StatusCode = &statusCode{ + Value: ":Requester", + } + err = p.validateStatus(&resp) + if err == nil || !strings.HasSuffix(err.Error(), `"Requester"`) { + t.Fatalf("validation should fail with code %q", "Requester") + } + // Test failed request with StatusMessage + resp.Status.StatusMessage = &statusMessage{ + Value: "Failed", + } + err = p.validateStatus(&resp) + if err == nil || !strings.HasSuffix(err.Error(), `"Requester" -> Failed`) { + t.Fatalf("validation should fail with code %q and message %q", "Requester", "Failed") + } +} + +func TestValidateSubjectConfirmation(t *testing.T) { + p := newProvider("", "") + var err error + var notAfter time.Time + subj := &subject{} + // Subject without any SubjectConfirmation + err = p.validateSubjectConfirmation(subj) + if err == nil { + t.Fatalf("validation of %q should fail", "Subject without any SubjectConfirmation") + } + // SubjectConfirmation without Method and SubjectConfirmationData + subj.SubjectConfirmations = []subjectConfirmation{subjectConfirmation{}} + err = p.validateSubjectConfirmation(subj) + if err == nil { + t.Fatalf("validation of %q should fail", "SubjectConfirmation without Method and SubjectConfirmationData") + } + // SubjectConfirmation with invalid Method and no SubjectConfirmationData + subj.SubjectConfirmations = []subjectConfirmation{subjectConfirmation{ + Method: "invalid", + }} + err = p.validateSubjectConfirmation(subj) + if err == nil { + t.Fatalf("validation of %q should fail", "SubjectConfirmation with invalid Method and no SubjectConfirmationData") + } + // SubjectConfirmation with valid Method and empty SubjectConfirmationData + subjConfirmationData := subjectConfirmationData{} + subj.SubjectConfirmations = []subjectConfirmation{subjectConfirmation{ + Method: "urn:oasis:names:tc:SAML:2.0:cm:bearer", + SubjectConfirmationData: &subjConfirmationData, + }} + err = p.validateSubjectConfirmation(subj) + if err != nil { + t.Fatalf("validation of %q should succeed", "SubjectConfirmation with valid Method and empty SubjectConfirmationData") + } + // SubjectConfirmationData with invalid Recipient + subjConfirmationData.Recipient = "invalid" + err = p.validateSubjectConfirmation(subj) + if err == nil { + t.Fatalf("validation of %q should fail", "SubjectConfirmationData with invalid Recipient") + } + // expired SubjectConfirmationData + notAfter = p.now().Add(-time.Duration(60) * time.Second) + subjConfirmationData.NotOnOrAfter = xmlTime(notAfter) + subjConfirmationData.Recipient = defaultRedirectURI + err = p.validateSubjectConfirmation(subj) + if err == nil { + t.Fatalf("validation of %q should fail", " expired SubjectConfirmationData") + } + // valid SubjectConfirmationData + notAfter = p.now().Add(+time.Duration(60) * time.Second) + subjConfirmationData.NotOnOrAfter = xmlTime(notAfter) + subjConfirmationData.Recipient = defaultRedirectURI + err = p.validateSubjectConfirmation(subj) + if err != nil { + t.Fatalf("validation of %q should succed", "valid SubjectConfirmationData") + } +} + +func TestValidateConditions(t *testing.T) { + p := newProvider("", "") + var err error + var notAfter, notBefore time.Time + cond := conditions{ + AudienceRestriction: &audienceRestriction{}, + } + assert := &assertion{} + // Assertion without Conditions + err = p.validateConditions(assert) + if err != nil { + t.Fatalf("validation of %q should succeed", "Assertion without Conditions") + } + // Assertion with empty Conditions + assert.Conditions = &cond + err = p.validateConditions(assert) + if err != nil { + t.Fatalf("validation of %q should succeed", "Assertion with empty Conditions") + } + // Conditions with valid timestamps + notBefore = p.now().Add(-time.Duration(60) * time.Second) + notAfter = p.now().Add(+time.Duration(60) * time.Second) + cond.NotBefore = xmlTime(notBefore) + cond.NotOnOrAfter = xmlTime(notAfter) + err = p.validateConditions(assert) + if err != nil { + t.Fatalf("validation of %q should succeed", "Conditions with valid timestamps") + } + // Conditions where notBefore is 45 seconds after now + notBefore = p.now().Add(+time.Duration(45) * time.Second) + cond.NotBefore = xmlTime(notBefore) + err = p.validateConditions(assert) + if err == nil { + t.Fatalf("validation of %q should fail", "Conditions where notBefore is 45 seconds after now") + } + // Conditions where notBefore is 15 seconds after now + notBefore = p.now().Add(+time.Duration(15) * time.Second) + cond.NotBefore = xmlTime(notBefore) + err = p.validateConditions(assert) + if err != nil { + t.Fatalf("validation of %q should succeed", "Conditions where notBefore is 15 seconds after now") + } + // Audiences contains the issuer + validAudience := audience{Value: p.issuer} + cond.AudienceRestriction.Audiences = []audience{validAudience} + err = p.validateConditions(assert) + if err != nil { + t.Fatalf("validation of %q should succeed", "Audiences contains the issuer") + } + // Audiences is not empty and not contains the issuer + invalidAudience := audience{Value: "invalid"} + cond.AudienceRestriction.Audiences = []audience{invalidAudience} + err = p.validateConditions(assert) + if err == nil { + t.Fatalf("validation of %q should succeed", "Audiences is not empty and not contains the issuer") } } diff --git a/connector/saml/testdata/idp-cert.pem b/connector/saml/testdata/idp-cert.pem new file mode 100644 index 00000000..0c55213e --- /dev/null +++ b/connector/saml/testdata/idp-cert.pem @@ -0,0 +1,26 @@ +-----BEGIN CERTIFICATE----- +MIIEUTCCAzmgAwIBAgIJAJdmunb39nFKMA0GCSqGSIb3DQEBCwUAMHgxCzAJBgNV +BAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMQwwCgYDVQQKEwNJRFAxFDASBgNV +BAsTC1NTT1Byb3ZpZGVyMRMwEQYDVQQDEwpkZXYtOTY5MjQ0MRswGQYJKoZIhvcN +AQkBFgxpbmZvQGlkcC5vcmcwHhcNMTcwMTI0MTczMTI3WhcNMjcwMTIyMTczMTI3 +WjB4MQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEMMAoGA1UEChMD +SURQMRQwEgYDVQQLEwtTU09Qcm92aWRlcjETMBEGA1UEAxMKZGV2LTk2OTI0NDEb +MBkGCSqGSIb3DQEJARYMaW5mb0BpZHAub3JnMIIBIjANBgkqhkiG9w0BAQEFAAOC +AQ8AMIIBCgKCAQEA0X/AE1tmDmhGRROAWaJ82XSORivRfgNt9Fb4rLrf6nIJsQN3 +vNb1Nk4DSUEDdQuvHNaEemSVkSPgfq5qnhh37bJaghr0728J8dOyYzV5eArPvsby +CRcnhXQzpCK2zvHwjgxNJMsNJLbnYpG/U+dCdCtcOOn9JEhKO8wKn06y2tcrvC1u +uVs7bodukPUNq82KJTyvCQP8jh1hEZXeR2siJFDeJj1n2FNTMeCKIqOb42J/i+sB +TlyK3mV5Ni++hI/ssIYVbPwrMIBd6sKLVAgInshBHOj/7XcXW/rMf468YtBKs4Xn +XsE3hLoU02aWCRDlVHa4hm3jfIAqEADOUumklQIDAQABo4HdMIHaMB0GA1UdDgQW +BBRjN/dQSvhZxIsHTXmDKQJkPrjp0TCBqgYDVR0jBIGiMIGfgBRjN/dQSvhZxIsH +TXmDKQJkPrjp0aF8pHoweDELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3Ju +aWExDDAKBgNVBAoTA0lEUDEUMBIGA1UECxMLU1NPUHJvdmlkZXIxEzARBgNVBAMT +CmRldi05NjkyNDQxGzAZBgkqhkiG9w0BCQEWDGluZm9AaWRwLm9yZ4IJAJdmunb3 +9nFKMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBAIqHUglIUAA+BKMW +6B0Q+cqIgDr9fWlsvDwIVK7/cvUeGIH3icSsje9AVZ4nQOJpxmC/E06HfuDXmbT1 +wG16jNo01mPW9qaOGRJuQqlZdegCSF385o/OHcbaEKBRwyYuvLfu80EREj8wcMUK +FpExoaxK7K8DS7hh3w7exLB80jyhIaDEYc1hdyAl+206XpOXSYBetsg7I622R2+a +jSL7ygUxQjmKQ5DyInPdXzCFCL6Ew/BN0dwzfnBEEK223ruOWBLpj13zMC077dor +/NgYyHZU6iqiDS2eYO5jhVMve/mP9734+6N34seQRmekfmsf2dJcEQhPVYr/j0De +Jc3men4= +-----END CERTIFICATE----- diff --git a/connector/saml/testdata/idp-resp-signed-assertion.xml b/connector/saml/testdata/idp-resp-signed-assertion.xml new file mode 100644 index 00000000..26e7200d --- /dev/null +++ b/connector/saml/testdata/idp-resp-signed-assertion.xml @@ -0,0 +1,29 @@ + + http://www.okta.com/exk91cb99lKkKSYoy0h7 + + + + + http://www.okta.com/exk91cb99lKkKSYoy0h7 + + + HFNooGfpAONF7T96W3bFsXkH51k=dI0QBihhNT5rtRYE9iB0lEKXkE7Yr4+QueOItRH2RcKwAXJ6DA/m3D/S7qwXk00Hn8ZpHu48ZO+HJpyweEEh2UuUWJCCTwwggagKybbSoRx3UTnSuNAFTdoDWTGt89z8j4+gRMC0sepYwppF3u87vJKRVBh8HjFfrHmWsZKwNtfoeXOOFCeatwxcI1sKCoBs2fTn78683ThoAJe3pygipSHY5WPt4dfT/yAY5Ars+OPY/N02M80OfIygZXdJwND0tVPJIF3M9DaehSkvCBHs7QA7DARsRXcuXdsYY7R8wHzqDVJZ4OvcsprONamm5AgUIpql1CjT94rFwWOFyxF2tg== +MIIEUTCCAzmgAwIBAgIJAJdmunb39nFKMA0GCSqGSIb3DQEBCwUAMHgxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMQwwCgYDVQQKEwNJRFAxFDASBgNVBAsTC1NTT1Byb3ZpZGVyMRMwEQYDVQQDEwpkZXYtOTY5MjQ0MRswGQYJKoZIhvcNAQkBFgxpbmZvQGlkcC5vcmcwHhcNMTcwMTI0MTczMTI3WhcNMjcwMTIyMTczMTI3WjB4MQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEMMAoGA1UEChMDSURQMRQwEgYDVQQLEwtTU09Qcm92aWRlcjETMBEGA1UEAxMKZGV2LTk2OTI0NDEbMBkGCSqGSIb3DQEJARYMaW5mb0BpZHAub3JnMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0X/AE1tmDmhGRROAWaJ82XSORivRfgNt9Fb4rLrf6nIJsQN3vNb1Nk4DSUEDdQuvHNaEemSVkSPgfq5qnhh37bJaghr0728J8dOyYzV5eArPvsbyCRcnhXQzpCK2zvHwjgxNJMsNJLbnYpG/U+dCdCtcOOn9JEhKO8wKn06y2tcrvC1uuVs7bodukPUNq82KJTyvCQP8jh1hEZXeR2siJFDeJj1n2FNTMeCKIqOb42J/i+sBTlyK3mV5Ni++hI/ssIYVbPwrMIBd6sKLVAgInshBHOj/7XcXW/rMf468YtBKs4XnXsE3hLoU02aWCRDlVHa4hm3jfIAqEADOUumklQIDAQABo4HdMIHaMB0GA1UdDgQWBBRjN/dQSvhZxIsHTXmDKQJkPrjp0TCBqgYDVR0jBIGiMIGfgBRjN/dQSvhZxIsHTXmDKQJkPrjp0aF8pHoweDELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExDDAKBgNVBAoTA0lEUDEUMBIGA1UECxMLU1NPUHJvdmlkZXIxEzARBgNVBAMTCmRldi05NjkyNDQxGzAZBgkqhkiG9w0BCQEWDGluZm9AaWRwLm9yZ4IJAJdmunb39nFKMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBAIqHUglIUAA+BKMW6B0Q+cqIgDr9fWlsvDwIVK7/cvUeGIH3icSsje9AVZ4nQOJpxmC/E06HfuDXmbT1wG16jNo01mPW9qaOGRJuQqlZdegCSF385o/OHcbaEKBRwyYuvLfu80EREj8wcMUKFpExoaxK7K8DS7hh3w7exLB80jyhIaDEYc1hdyAl+206XpOXSYBetsg7I622R2+ajSL7ygUxQjmKQ5DyInPdXzCFCL6Ew/BN0dwzfnBEEK223ruOWBLpj13zMC077dor/NgYyHZU6iqiDS2eYO5jhVMve/mP9734+6N34seQRmekfmsf2dJcEQhPVYr/j0DeJc3men4= + + eric.chiang+okta@coreos.com + + + + + + + http://localhost:5556/dex/callback + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport + + + + diff --git a/connector/saml/testdata/idp-resp-signed-message-and-assertion.xml b/connector/saml/testdata/idp-resp-signed-message-and-assertion.xml new file mode 100644 index 00000000..87cf41e7 --- /dev/null +++ b/connector/saml/testdata/idp-resp-signed-message-and-assertion.xml @@ -0,0 +1,34 @@ + + + + + P2k0nQ19ZcowlcaOz6do6Tyu8WI=ytdy1qRPdMeIGnlkkaeLdblzTPtIFc0EJNm8WktsNU1Mn6G/6AmNaXEUik2BkTpk8zKabHdSf6+le8hwRiyfNWPTF84lzVdMjQ/+I8pnX/srpG534zoSAsP6ZFQvHp46AHPx31KP75H/ymqx2DNppqxh8JjUeMKQkPUEqduWUZ4kFjcsrz9H3MNVsHfxntnswibiknU/wAthtBuY2I6yOIF55RprUgYb5j2TqDd3IArF6LkxWRvHvhaw66MdhY1iiit7AFOcuHJVyPe8Attra94jwM+O1Ch+HQgoI43nX91d/jkP0vyWzWD8Xkcwb+KuRPsQflxjV22UU0+JbwrBYA== +MIIEUTCCAzmgAwIBAgIJAJdmunb39nFKMA0GCSqGSIb3DQEBCwUAMHgxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMQwwCgYDVQQKEwNJRFAxFDASBgNVBAsTC1NTT1Byb3ZpZGVyMRMwEQYDVQQDEwpkZXYtOTY5MjQ0MRswGQYJKoZIhvcNAQkBFgxpbmZvQGlkcC5vcmcwHhcNMTcwMTI0MTczMTI3WhcNMjcwMTIyMTczMTI3WjB4MQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEMMAoGA1UEChMDSURQMRQwEgYDVQQLEwtTU09Qcm92aWRlcjETMBEGA1UEAxMKZGV2LTk2OTI0NDEbMBkGCSqGSIb3DQEJARYMaW5mb0BpZHAub3JnMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0X/AE1tmDmhGRROAWaJ82XSORivRfgNt9Fb4rLrf6nIJsQN3vNb1Nk4DSUEDdQuvHNaEemSVkSPgfq5qnhh37bJaghr0728J8dOyYzV5eArPvsbyCRcnhXQzpCK2zvHwjgxNJMsNJLbnYpG/U+dCdCtcOOn9JEhKO8wKn06y2tcrvC1uuVs7bodukPUNq82KJTyvCQP8jh1hEZXeR2siJFDeJj1n2FNTMeCKIqOb42J/i+sBTlyK3mV5Ni++hI/ssIYVbPwrMIBd6sKLVAgInshBHOj/7XcXW/rMf468YtBKs4XnXsE3hLoU02aWCRDlVHa4hm3jfIAqEADOUumklQIDAQABo4HdMIHaMB0GA1UdDgQWBBRjN/dQSvhZxIsHTXmDKQJkPrjp0TCBqgYDVR0jBIGiMIGfgBRjN/dQSvhZxIsHTXmDKQJkPrjp0aF8pHoweDELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExDDAKBgNVBAoTA0lEUDEUMBIGA1UECxMLU1NPUHJvdmlkZXIxEzARBgNVBAMTCmRldi05NjkyNDQxGzAZBgkqhkiG9w0BCQEWDGluZm9AaWRwLm9yZ4IJAJdmunb39nFKMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBAIqHUglIUAA+BKMW6B0Q+cqIgDr9fWlsvDwIVK7/cvUeGIH3icSsje9AVZ4nQOJpxmC/E06HfuDXmbT1wG16jNo01mPW9qaOGRJuQqlZdegCSF385o/OHcbaEKBRwyYuvLfu80EREj8wcMUKFpExoaxK7K8DS7hh3w7exLB80jyhIaDEYc1hdyAl+206XpOXSYBetsg7I622R2+ajSL7ygUxQjmKQ5DyInPdXzCFCL6Ew/BN0dwzfnBEEK223ruOWBLpj13zMC077dor/NgYyHZU6iqiDS2eYO5jhVMve/mP9734+6N34seQRmekfmsf2dJcEQhPVYr/j0DeJc3men4= + http://www.okta.com/exk91cb99lKkKSYoy0h7 + + + + + http://www.okta.com/exk91cb99lKkKSYoy0h7 + + + 4jNCSI3tTnbpozZ8qT4FZe+EWV8=xtTnl92ArZyxskD3b34cIjo5LIpeE+3RjW+jgtXMXhUIZp3uGJ2RC6n1CbJ6IWuo4KmezpnVUnSWNz/fgOTZCN/1VlqsfLDpoTf790GrP+q6rKyw8CW7nd0uVS5FRYe05HTO6C5RqnaE9PmZ/YYbiWtLIDx0+kqvu/jFr+D144G/mukaVG4ydnDQ/tl21N6hWIOpi1tWaNPv50OEEgY//9VPql9Us3YuhfrxNggVugauArwY9RL4nVFVjALP1wpkZn1JzpgNMFgvXfY3MxnI1OnWg6ypJESugIKroKqj5RyqMIaLICsUOBwIKk8R4zAATrB+D+kuFV9Ec837duW/Eg== +MIIEUTCCAzmgAwIBAgIJAJdmunb39nFKMA0GCSqGSIb3DQEBCwUAMHgxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMQwwCgYDVQQKEwNJRFAxFDASBgNVBAsTC1NTT1Byb3ZpZGVyMRMwEQYDVQQDEwpkZXYtOTY5MjQ0MRswGQYJKoZIhvcNAQkBFgxpbmZvQGlkcC5vcmcwHhcNMTcwMTI0MTczMTI3WhcNMjcwMTIyMTczMTI3WjB4MQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEMMAoGA1UEChMDSURQMRQwEgYDVQQLEwtTU09Qcm92aWRlcjETMBEGA1UEAxMKZGV2LTk2OTI0NDEbMBkGCSqGSIb3DQEJARYMaW5mb0BpZHAub3JnMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0X/AE1tmDmhGRROAWaJ82XSORivRfgNt9Fb4rLrf6nIJsQN3vNb1Nk4DSUEDdQuvHNaEemSVkSPgfq5qnhh37bJaghr0728J8dOyYzV5eArPvsbyCRcnhXQzpCK2zvHwjgxNJMsNJLbnYpG/U+dCdCtcOOn9JEhKO8wKn06y2tcrvC1uuVs7bodukPUNq82KJTyvCQP8jh1hEZXeR2siJFDeJj1n2FNTMeCKIqOb42J/i+sBTlyK3mV5Ni++hI/ssIYVbPwrMIBd6sKLVAgInshBHOj/7XcXW/rMf468YtBKs4XnXsE3hLoU02aWCRDlVHa4hm3jfIAqEADOUumklQIDAQABo4HdMIHaMB0GA1UdDgQWBBRjN/dQSvhZxIsHTXmDKQJkPrjp0TCBqgYDVR0jBIGiMIGfgBRjN/dQSvhZxIsHTXmDKQJkPrjp0aF8pHoweDELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExDDAKBgNVBAoTA0lEUDEUMBIGA1UECxMLU1NPUHJvdmlkZXIxEzARBgNVBAMTCmRldi05NjkyNDQxGzAZBgkqhkiG9w0BCQEWDGluZm9AaWRwLm9yZ4IJAJdmunb39nFKMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBAIqHUglIUAA+BKMW6B0Q+cqIgDr9fWlsvDwIVK7/cvUeGIH3icSsje9AVZ4nQOJpxmC/E06HfuDXmbT1wG16jNo01mPW9qaOGRJuQqlZdegCSF385o/OHcbaEKBRwyYuvLfu80EREj8wcMUKFpExoaxK7K8DS7hh3w7exLB80jyhIaDEYc1hdyAl+206XpOXSYBetsg7I622R2+ajSL7ygUxQjmKQ5DyInPdXzCFCL6Ew/BN0dwzfnBEEK223ruOWBLpj13zMC077dor/NgYyHZU6iqiDS2eYO5jhVMve/mP9734+6N34seQRmekfmsf2dJcEQhPVYr/j0DeJc3men4= + + eric.chiang+okta@coreos.com + + + + + + + http://localhost:5556/dex/callback + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport + + + + diff --git a/connector/saml/testdata/idp-resp-signed-message.xml b/connector/saml/testdata/idp-resp-signed-message.xml new file mode 100644 index 00000000..4898259e --- /dev/null +++ b/connector/saml/testdata/idp-resp-signed-message.xml @@ -0,0 +1,30 @@ + + + + + Oy9CTB/hzFWyr6QF99EZ4ymIEfY=COKw1DUpDzNVulVNFlcrPwaalCwr8BfTye92GN5snTmx3IDKLudr1PT+WPt5i2N4xaoFcq/X1p/yEVmWtC1O+YYXNNIouFwps9Gyw/iDEMs1TtVKfikbloKWkDdYgfqgcon+mOq/lHagLVAcgz5QRBHVTIrFWcFYbnemsj1hy8q7ToIeoyHX9f5TAZBfZEEbdZcsD581xKPafNGowgfWxkgEwLBzFsJVYg/QfeoNnORTsKlsQBuswiXrsWatZNOOjWpdF9qqYn/3f9axqx2CJPD2HB38Vl0g4dTFnpmMAe45ndJq0IpXr9YJNCDJjUIvR7srdV1AW7qe2Mp6LxBBNw== +MIIEUTCCAzmgAwIBAgIJAJdmunb39nFKMA0GCSqGSIb3DQEBCwUAMHgxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMQwwCgYDVQQKEwNJRFAxFDASBgNVBAsTC1NTT1Byb3ZpZGVyMRMwEQYDVQQDEwpkZXYtOTY5MjQ0MRswGQYJKoZIhvcNAQkBFgxpbmZvQGlkcC5vcmcwHhcNMTcwMTI0MTczMTI3WhcNMjcwMTIyMTczMTI3WjB4MQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEMMAoGA1UEChMDSURQMRQwEgYDVQQLEwtTU09Qcm92aWRlcjETMBEGA1UEAxMKZGV2LTk2OTI0NDEbMBkGCSqGSIb3DQEJARYMaW5mb0BpZHAub3JnMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0X/AE1tmDmhGRROAWaJ82XSORivRfgNt9Fb4rLrf6nIJsQN3vNb1Nk4DSUEDdQuvHNaEemSVkSPgfq5qnhh37bJaghr0728J8dOyYzV5eArPvsbyCRcnhXQzpCK2zvHwjgxNJMsNJLbnYpG/U+dCdCtcOOn9JEhKO8wKn06y2tcrvC1uuVs7bodukPUNq82KJTyvCQP8jh1hEZXeR2siJFDeJj1n2FNTMeCKIqOb42J/i+sBTlyK3mV5Ni++hI/ssIYVbPwrMIBd6sKLVAgInshBHOj/7XcXW/rMf468YtBKs4XnXsE3hLoU02aWCRDlVHa4hm3jfIAqEADOUumklQIDAQABo4HdMIHaMB0GA1UdDgQWBBRjN/dQSvhZxIsHTXmDKQJkPrjp0TCBqgYDVR0jBIGiMIGfgBRjN/dQSvhZxIsHTXmDKQJkPrjp0aF8pHoweDELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExDDAKBgNVBAoTA0lEUDEUMBIGA1UECxMLU1NPUHJvdmlkZXIxEzARBgNVBAMTCmRldi05NjkyNDQxGzAZBgkqhkiG9w0BCQEWDGluZm9AaWRwLm9yZ4IJAJdmunb39nFKMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBAIqHUglIUAA+BKMW6B0Q+cqIgDr9fWlsvDwIVK7/cvUeGIH3icSsje9AVZ4nQOJpxmC/E06HfuDXmbT1wG16jNo01mPW9qaOGRJuQqlZdegCSF385o/OHcbaEKBRwyYuvLfu80EREj8wcMUKFpExoaxK7K8DS7hh3w7exLB80jyhIaDEYc1hdyAl+206XpOXSYBetsg7I622R2+ajSL7ygUxQjmKQ5DyInPdXzCFCL6Ew/BN0dwzfnBEEK223ruOWBLpj13zMC077dor/NgYyHZU6iqiDS2eYO5jhVMve/mP9734+6N34seQRmekfmsf2dJcEQhPVYr/j0DeJc3men4= + http://www.okta.com/exk91cb99lKkKSYoy0h7 + + + + + http://www.okta.com/exk91cb99lKkKSYoy0h7 + + eric.chiang+okta@coreos.com + + + + + + + http://localhost:5556/dex/callback + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport + + + + diff --git a/connector/saml/testdata/idp-resp.xml b/connector/saml/testdata/idp-resp.xml new file mode 100644 index 00000000..44ba20af --- /dev/null +++ b/connector/saml/testdata/idp-resp.xml @@ -0,0 +1,34 @@ + + + http://www.okta.com/exk91cb99lKkKSYoy0h7 + + + + + http://www.okta.com/exk91cb99lKkKSYoy0h7 + + eric.chiang+okta@coreos.com + + + + + + + http://localhost:5556/dex/callback + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport + + + + + admin + + + eric.chiang+okta@coreos.com + + + + diff --git a/connector/saml/types.go b/connector/saml/types.go index 7c1d89be..54d1b46a 100644 --- a/connector/saml/types.go +++ b/connector/saml/types.go @@ -57,6 +57,8 @@ type authnRequest struct { IsPassive bool `xml:"IsPassive,attr,omitempty"` ProtocolBinding string `xml:"ProtocolBinding,attr,omitempty"` + AssertionConsumerServiceURL string `xml:"AssertionConsumerServiceURL,attr,omitempty"` + Subject *subject `xml:"Subject,omitempty"` Issuer *issuer `xml:"Issuer,omitempty"` NameIDPolicy *nameIDPolicy `xml:"NameIDPolicy,omitempty"` @@ -68,7 +70,8 @@ type authnRequest struct { type subject struct { XMLName xml.Name `xml:"urn:oasis:names:tc:SAML:2.0:assertion Subject"` - NameID *nameID `xml:"NameID,omitempty"` + NameID *nameID `xml:"NameID,omitempty"` + SubjectConfirmations []subjectConfirmation `xml:"SubjectConfirmation"` // TODO(ericchiang): Do we need to deal with baseID? } @@ -80,6 +83,60 @@ type nameID struct { Value string `xml:",chardata"` } +type subjectConfirmationData struct { + XMLName xml.Name `xml:"urn:oasis:names:tc:SAML:2.0:assertion SubjectConfirmationData"` + + NotOnOrAfter xmlTime `xml:"NotOnOrAfter,attr,omitempty"` + Recipient string `xml:"Recipient,attr,omitempty"` + InResponseTo string `xml:"InResponseTo,attr,omitempty"` +} + +type subjectConfirmation struct { + XMLName xml.Name `xml:"urn:oasis:names:tc:SAML:2.0:assertion SubjectConfirmation"` + + Method string `xml:"Method,attr,omitempty"` + SubjectConfirmationData *subjectConfirmationData `xml:"SubjectConfirmationData,omitempty"` +} + +type audience struct { + XMLName xml.Name `xml:"urn:oasis:names:tc:SAML:2.0:assertion Audience"` + Value string `xml:",chardata"` +} + +type audienceRestriction struct { + XMLName xml.Name `xml:"urn:oasis:names:tc:SAML:2.0:assertion AudienceRestriction"` + + Audiences []audience `xml:"Audience"` +} + +type conditions struct { + XMLName xml.Name `xml:"urn:oasis:names:tc:SAML:2.0:assertion Conditions"` + + NotBefore xmlTime `xml:"NotBefore,attr,omitempty"` + NotOnOrAfter xmlTime `xml:"NotOnOrAfter,attr,omitempty"` + + AudienceRestriction *audienceRestriction `xml:"AudienceRestriction,omitempty"` +} + +type statusCode struct { + XMLName xml.Name `xml:"urn:oasis:names:tc:SAML:2.0:protocol StatusCode"` + + Value string `xml:"Value,attr,omitempty"` +} + +type statusMessage struct { + XMLName xml.Name `xml:"urn:oasis:names:tc:SAML:2.0:protocol StatusMessage"` + + Value string `xml:",chardata"` +} + +type status struct { + XMLName xml.Name `xml:"urn:oasis:names:tc:SAML:2.0:protocol Status"` + + StatusCode *statusCode `xml:"StatusCode"` + StatusMessage *statusMessage `xml:"StatusMessage,omitempty"` +} + type issuer struct { XMLName xml.Name `xml:"urn:oasis:names:tc:SAML:2.0:assertion Issuer"` Issuer string `xml:",chardata"` @@ -112,6 +169,8 @@ type response struct { Issuer *issuer `xml:"Issuer,omitempty"` + Status *status `xml:"Status"` + // TODO(ericchiang): How do deal with multiple assertions? Assertion *assertion `xml:"Assertion,omitempty"` } @@ -127,6 +186,8 @@ type assertion struct { Subject *subject `xml:"Subject,omitempty"` + Conditions *conditions `xml:"Conditions"` + AttributeStatement *attributeStatement `xml:"AttributeStatement,omitempty"` } diff --git a/glide.yaml b/glide.yaml index 3e7d13e6..c6d1991f 100644 --- a/glide.yaml +++ b/glide.yaml @@ -134,7 +134,7 @@ import: # XML signature validation for SAML connector - package: github.com/russellhaering/goxmldsig - version: d9f653eb27ee8b145f7d5a45172e81a93def0860 + version: e2990269f42f6ddfea940870a0800a14acdb8c21 - package: github.com/beevik/etree version: 4cd0dd976db869f817248477718071a28e978df0 - package: github.com/jonboulle/clockwork