-
Notifications
You must be signed in to change notification settings - Fork 203
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
Investigate SAML Request signing #46092
Comments
|
The signature would look like: <saml2p:AuthnRequest xmlns:saml2p="urn:oasis:names:tc:SAML:2.0:protocol" AssertionConsumerServiceURL="https://tpm.dev.iam.va.gov:443/ssoe-internal-sample-sp/saml/SSO" Destination="https://int.eauth.va.gov/isam/sps/saml20idp/saml20/login" ForceAuthn="false" ID="a364bhe34i4ebb094fd430ac3fh9ah4" IsPassive="false" IssueInstant="2022-08-18T17:47:47.561Z" ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Version="2.0"> |
From reading the OneLogin documentation in order to sign SAML messages we must first designate a I can confirm that Update: I found in |
The issue appears to be a mismatch between the default After manually setting the request service binding to |
IAM stated this should be safe to test in Dev and staging if we want to give the change a go. |
Related devops PR: https://github.com/department-of-veterans-affairs/devops/pull/11894 I'm also researching the difference betwen the "Artifact" service binding that we have now and the "POST" binding that this is changing our requests to in order to enable SAML signing. Is this something that we need to take into account? |
Yes, we likely cannot just switch from artifact to post. In reading the document you posted there are some mentions of signing requests, and also of combining bindings to allow for both artifact and post. From the doc, it appears as though signing is used/required because they dont encrypt the contents. Whereas Artifact doesnt require signing because they expect encryption. Nothing in there states we cant do signing and encrypted responses with POST. For not, review the documentation, if you dont find any concerns with using POST then lets give it a try. |
VSP Infra Application Manifests PR |
Further inquiry has revealed that the SAML message is unable to be signed due to a bug in the OneLogin RubySAML gem that vets-api leverages for its SAML communications, the
The The long-term solution for this is for an update to the RubySAML, however the project is no longer under active maintenance as of approximately a month ago, which makes this seem unlikely. In lieu of that, our best course of action might be to fork our own version of the gem to make the bugfix ourselves (while also submitting a ticket to the project requesting it be merged into the original version). |
This is rough draft for an issue ticket on the RubySAML gem repository, any feedback @bosawt @joeniquette? We are updating our SAML authentication requests to our service provider by including SAML signing, however the resulting
We are using What is the purpose of using |
Yeah, seems good to me |
After further effort I was unable to introduce a change to our SAML metadata that would induce a whitespace in the string that is passed into the |
SAML signing has been enabled in dev and staging, with a planned prod release @ 9 PM ET on 10/20 The decision was made to not enable SAML signing for localhost as it would have necessitated either including the signing private key into version control or requiring all local vets-api builds to download/generate a key to make SAML authentication work. ISAM will continue to accept signed or unsigned SAML requests from localhost builds. |
VSP Identity & the IAM team have confirmed that vets-api production is now signing SAML requests - the ISAM production switch to only accepting signed SAML requests is still due to occur at 9 EST tonight. |
SAML request signing is confirmed present & working VA.gov production. |
From IAM:
The text was updated successfully, but these errors were encountered: