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

SAML authentication requires signature for either SAML Response or its Assertion, not both #199

Open
fairclothjm opened this issue Sep 9, 2024 · 1 comment

Comments

@fairclothjm
Copy link

Hello! Thanks for the library!

In my testing, it appears as though this library requires a signature for either the SAML Response or its Assertion, but not both.

gosaml2/decode_response.go

Lines 272 to 304 in 8690358

var responseSignatureValidated bool
if !sp.SkipSignatureValidation {
el, err = sp.validateElementSignature(el)
if err == dsig.ErrMissingSignature {
// Unfortunately we just blew away our Response
el = doc.Root()
} else if err != nil {
return nil, err
} else if el == nil {
return nil, fmt.Errorf("missing transformed response")
} else {
responseSignatureValidated = true
}
}
err = sp.decryptAssertions(el)
if err != nil {
return nil, err
}
var assertionSignaturesValidated bool
if !sp.SkipSignatureValidation {
err = sp.validateAssertionSignatures(el)
if err == dsig.ErrMissingSignature {
if !responseSignatureValidated {
return nil, fmt.Errorf("response and/or assertions must be signed")
}
} else if err != nil {
return nil, err
} else {
assertionSignaturesValidated = true
}
}

Would you accept a PR to enable requiring both Response and assertion to be signed? Thanks

@landron
Copy link

landron commented Nov 4, 2024

You can check them outside of the library by using ResponseSignatureValidated and SignatureValidated on each assertion, respectively.

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