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

Reference id not matching with documentElement id #863

Closed
miracode-richard opened this issue Jun 1, 2023 · 8 comments
Closed

Reference id not matching with documentElement id #863

miracode-richard opened this issue Jun 1, 2023 · 8 comments

Comments

@miracode-richard
Copy link

To Reproduce

Receiving this response from Azure AD:

<samlp:Response ID="_f2446b4a-e6f3-4af6-9e9d-b0d424544c07" Version="2.0" IssueInstant="2023-06-01T10:28:37.108Z" Destination="http://localhost:3001/api/v1/auth/signin/saml" InResponseTo="_fa8a6a25f1fdd20d3b803fce3297edf6e430a540" xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol">
	<Issuer xmlns="urn:oasis:names:tc:SAML:2.0:assertion">STRIPPED</Issuer>
	<samlp:Status>
		<samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/>
	</samlp:Status>
	<Assertion ID="_e0126c1e-7655-4998-8a0b-de691b944d00" IssueInstant="2023-06-01T10:28:37.103Z" Version="2.0" xmlns="urn:oasis:names:tc:SAML:2.0:assertion">
		<Issuer>STRIPPED</Issuer>
		<Signature xmlns="http://www.w3.org/2000/09/xmldsig#">
			<SignedInfo>
				<CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
				<SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/>
				<Reference URI="#_e0126c1e-7655-4998-8a0b-de691b944d00">
					<Transforms>
						<Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/>
						<Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
					</Transforms>
					<DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/>
					<DigestValue>STRIPPED</DigestValue>
				</Reference>
			</SignedInfo>
			<SignatureValue>STRIPPED</SignatureValue>
			<KeyInfo>
				<X509Data>
					<X509Certificate>STRIPPED</X509Certificate>
				</X509Data>
			</KeyInfo>
		</Signature>
		<Subject>
			<NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">STRIPPED</NameID>
			<SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer">
				<SubjectConfirmationData InResponseTo="_fa8a6a25f1fdd20d3b803fce3297edf6e430a540" NotOnOrAfter="2023-06-01T11:28:36.918Z" Recipient="http://localhost:3001/api/v1/auth/signin/saml"/>
			</SubjectConfirmation>
		</Subject>
		<Conditions NotBefore="2023-06-01T10:23:36.918Z" NotOnOrAfter="2023-06-01T11:28:36.918Z">
			<AudienceRestriction>
				<Audience>STRIPPED</Audience>
			</AudienceRestriction>
		</Conditions>
		<AttributeStatement>
			<Attribute Name="http://schemas.microsoft.com/identity/claims/tenantid">
				<AttributeValue>STRIPPED</AttributeValue>
			</Attribute>
			<Attribute Name="http://schemas.microsoft.com/identity/claims/objectidentifier">
				<AttributeValue>STRIPPED</AttributeValue>
			</Attribute>
			<Attribute Name="http://schemas.microsoft.com/identity/claims/displayname">
				<AttributeValue>STRIPPED</AttributeValue>
			</Attribute>
			<Attribute Name="http://schemas.microsoft.com/identity/claims/identityprovider">
				<AttributeValue>STRIPPED</AttributeValue>
			</Attribute>
			<Attribute Name="http://schemas.microsoft.com/claims/authnmethodsreferences">
				<AttributeValue>urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport</AttributeValue>
				<AttributeValue>http://schemas.microsoft.com/claims/multipleauthn</AttributeValue>
			</Attribute>
			<Attribute Name="http://schemas.microsoft.com/ws/2008/06/identity/claims/role">
				<AttributeValue>STRIPPED</AttributeValue>
				<AttributeValue>STRIPPED</AttributeValue>
				<AttributeValue>STRIPPED</AttributeValue>
			</Attribute>
			<Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/givenname">
				<AttributeValue>STRIPPED</AttributeValue>
			</Attribute>
			<Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/surname">
				<AttributeValue>STRIPPED</AttributeValue>
			</Attribute>
			<Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress">
				<AttributeValue>STRIPPED</AttributeValue>
			</Attribute>
			<Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name">
				<AttributeValue>STRIPPED</AttributeValue>
			</Attribute>
		</AttributeStatement>
		<AuthnStatement AuthnInstant="2022-10-10T09:47:48.145Z" SessionIndex="_e0126c1e-7655-4998-8a0b-de691b944d00">
			<AuthnContext>
				<AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport</AuthnContextClassRef>
			</AuthnContext>
		</AuthnStatement>
	</Assertion>
</samlp:Response>

When validating the signature, the documentElement of the SAML response is used and the reference id tries to match with the id of it. But they don't match, instead the reference id matches with the <Assertion>-Element id, thus successfully validating the signature is not possible.

Expected behavior
Signature should validate successfully.

Environment

  • Node.js version: 16.14.0
  • passport-saml version: 4.0.4
@miracode-richard
Copy link
Author

Setting wantAuthnResponseSigned to false "solved" the issue so far. But I would really like to keep it active, to grant the highest security possible.

@cjbarth
Copy link
Collaborator

cjbarth commented Jun 1, 2023

What did you get when testing over at https://www.samltool.com/online_tools.php? If you got it to work, what options did you select on their web page?

@cjbarth cjbarth added need-more-info and removed bug labels Jun 1, 2023
@srd90
Copy link

srd90 commented Jun 1, 2023

Setting wantAuthnResponseSigned to false "solved" the issue so far. But I would really like to keep it active, to grant the highest security possible.

@Kittenhunter If you ran into this problem due passport-saml upgrade from something to >= 4.0.0 then you have had throughout the history of your deployment just assertion level signing enabled (this is not a Bug from passport-saml point-of-view).

FWIW: passport-saml has been changed since >= 4.0.0 to require by default response and assertion level signatures.

Here is comment / quote how to configure Azure AD side:

Got it, so it would seem azure active directory does not sign top level document by default. For anyone coming here from google or whatever, the option to sign response and assertion is under saml signing certificate options in the enterprise application config.

source of aforementioned quote: #816 (comment)

If you work also with ADFS see ADFS configuration tips from here: #840 (comment)


Long list of similar issues:

@miracode-richard
Copy link
Author

What did you get when testing over at https://www.samltool.com/online_tools.php? If you got it to work, what options did you select on their web page?

Both Request and Response are valid. Do you have any other specific test in mind?

@miracode-richard
Copy link
Author

Setting wantAuthnResponseSigned to false "solved" the issue so far. But I would really like to keep it active, to grant the highest security possible.

@Kittenhunter If you ran into this problem due passport-saml upgrade from something to >= 4.0.0 then you have had throughout the history of your deployment just assertion level signing enabled (this is not a Bug from passport-saml point-of-view).

FWIW: passport-saml has been changed since >= 4.0.0 to require by default response and assertion level signatures.

Here is comment / quote how to configure Azure AD side:

Got it, so it would seem azure active directory does not sign top level document by default. For anyone coming here from google or whatever, the option to sign response and assertion is under saml signing certificate options in the enterprise application config.

source of aforementioned quote: #816 (comment)

If you work also with ADFS see ADFS configuration tips from here: #840 (comment)

Long list of similar issues:

The error comes not from upgrading of a previous version.
Thank you for your hint about the behavior of Azure AD. I will take a deeper look at the configuration of Azure AD to enable response signing.
This indeed looks like it is not a bug in the first place but rather a configuration error now.
So we likely can close this issue, but please wait until I looked the config tomorrow.

@cjbarth
Copy link
Collaborator

cjbarth commented Jun 1, 2023

@srd90 I've created a Wiki page that summarized this. Hopefully that will save you some time. Feel free to edit it, or let me know in an issue reply if you'd like some text added to it and I'll do it.

https://github.com/node-saml/passport-saml/wiki/Common-Issues

@srd90
Copy link

srd90 commented Jun 1, 2023

@cjbarth I was just logging in back to github in order to comment that maybe this "invalid signature thing" could / should be added to issue template 😄

Something like

Are you going to report invalid signature issue especially from >= 4.0.0? Have you searched for existing issues and discussions?

or maybe a link from issue template to that wiki page that you created.


Adding one more link to this issue's thread (just in case someone lands here with search engine or with github search):
See comment from this discussion comment (#671/discussioncomment-5261103 and scroll down a bit) to see how to spot from authnresponse what sort of signatures it has and possibilities to configure @node-saml/passport-saml / @node-saml/node-saml if IdP side configuration management is not an option for you or if you rather modify SP side configuration.

@cjbarth cjbarth changed the title [BUG] Reference id not matching with documentElement id Reference id not matching with documentElement id Jun 2, 2023
@miracode-richard
Copy link
Author

Thank you very much with your help.
I've looked at the SAML configuration and found the option to enable the response and assertion signing option.
It can be found, when editing the SAML certificate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants