Skip to content

REF-26: Validate the SAML signature profile before trusting an assertion signature #93

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.

Sign up for GitHub

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

Merged
thomasnymand merged 1 commit into from
Aug 26, 2026
Merged

REF-26: Validate the SAML signature profile before trusting an assertion signature #93

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions oiosaml/src/main/java/dk/gov/oio/saml/service/validation/AssertionValidationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@
import org.opensaml.saml.saml2.core.Subject;
import org.opensaml.saml.saml2.core.SubjectConfirmation;
import org.opensaml.saml.saml2.core.SubjectConfirmationData;
import org.opensaml.saml.security.impl.SAMLSignatureProfileValidator;
import org.opensaml.security.credential.UsageType;
import org.opensaml.security.x509.BasicX509Credential;
import org.opensaml.xmlsec.signature.Signature;
import org.opensaml.xmlsec.signature.support.SignatureException;
import org.opensaml.xmlsec.signature.support.SignatureValidator;

Expand Down Expand Up @@ -188,11 +190,6 @@ private void validateAssertion(Assertion assertion, AuthnRequestWrapper authnReq
String nameIDValue = assertion.getSubject().getNameID().getValue();
validateAttributeStatement(attributeValues, nameIDValue.startsWith("https://data.gov.dk/model/core/eid/professional"));
validateAssurance(attributeValues, authnRequest);

// The Assertion within the response MUST be directly signed
if (!assertion.isSigned()) {
throw new AssertionValidationException("The Assertion within the response MUST be directly signed");
}
}

private void validateAttributeStatement(Map<String, String> attributes, boolean isProfessional) throws AssertionValidationException {
Expand Down Expand Up @@ -278,13 +275,27 @@ private void validateInResponseTo(Response response, AuthnRequestWrapper authnRe
}

private void validateSignature(Assertion assertion) throws ExternalException, InternalException, AssertionValidationException {
// The assertion MUST be directly signed, an unsigned assertion has no signature to validate
Signature signature = assertion.getSignature();
if (!assertion.isSigned() || signature == null) {
throw new AssertionValidationException("The Assertion within the response MUST be directly signed");
}

// Establishes that the signature is bound to this assertion. Validating it cryptographically only
// proves that some element in the document was signed with the IdP key.
try {
new SAMLSignatureProfileValidator().validate(signature);
} catch (SignatureException e) {
throw new AssertionValidationException("Assertion signature does not follow the SAML signature profile", e);
}

// Get Signing credential
X509Certificate x509Certificate = IdPMetadataService.getInstance().getIdPMetadata().getValidX509Certificate(UsageType.SIGNING);
BasicX509Credential credential = new BasicX509Credential(x509Certificate);

// Validate Signature
try {
SignatureValidator.validate(assertion.getSignature(), credential);
SignatureValidator.validate(signature, credential);
} catch (SignatureException e) {
throw new AssertionValidationException("Could not validate assertion signature", e);
}
Expand Down
109 changes: 109 additions & 0 deletions oiosaml/src/test/java/dk/gov/oio/saml/service/validation/AssertionValidationServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,27 @@
import dk.gov.oio.saml.service.AssertionService;
import dk.gov.oio.saml.service.AuthnRequestService;
import dk.gov.oio.saml.service.BaseServiceTest;
import dk.gov.oio.saml.service.IdPMetadataService;
import dk.gov.oio.saml.session.AuthnRequestWrapper;
import dk.gov.oio.saml.util.ExternalException;
import dk.gov.oio.saml.util.IdpUtil;
import dk.gov.oio.saml.util.SamlHelper;
import dk.gov.oio.saml.util.TestConstants;
import java.security.cert.X509Certificate;
import java.util.List;
import java.util.UUID;
import javax.servlet.http.HttpServletRequest;
import org.joda.time.DateTime;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import org.opensaml.core.config.InitializationException;
import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport;
import org.opensaml.messaging.context.MessageContext;
import org.opensaml.saml.common.SAMLObject;
import org.opensaml.saml.common.assertion.AssertionValidationException;
import org.opensaml.saml.common.xml.SAMLConstants;
import org.opensaml.saml.saml2.core.Assertion;
import org.opensaml.saml.saml2.core.Audience;
import org.opensaml.saml.saml2.core.AudienceRestriction;
Expand All @@ -29,6 +34,12 @@
import org.opensaml.saml.saml2.core.Response;
import org.opensaml.saml.saml2.core.impl.EncryptedAssertionMarshaller;
import org.opensaml.saml.saml2.core.impl.EncryptedAssertionUnmarshaller;
import org.opensaml.security.credential.UsageType;
import org.opensaml.security.x509.BasicX509Credential;
import org.opensaml.xmlsec.signature.support.SignatureConstants;
import org.opensaml.xmlsec.signature.support.SignatureValidator;
import org.w3c.dom.Document;
import org.w3c.dom.Element;

public class AssertionValidationServiceTest extends BaseServiceTest {

Expand Down Expand Up @@ -326,6 +337,104 @@ public void testFailWrongSubject() throws Exception {
});
}

@DisplayName("Test that validator will fail an assertion whose signature reference resolves to another element")
@Test
public void testFailSignatureNotBoundToAssertion() throws Exception {
AssertionValidationService validationService = new AssertionValidationService();

// Mock HttpServletRequest
HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
Mockito.when(request.getRequestURL()).thenReturn(new StringBuffer(TestConstants.SP_ASSERTION_CONSUMER_URL));

// Create AuthnRequest
AuthnRequestService authnRequestService = AuthnRequestService.getInstance();
AuthnRequest authnRequest = getAuthnRequest(authnRequestService);
String inResponseToId = authnRequest.getID();

// Create MessageContext, Response and a correctly signed Assertion
String nameID = "https://data.gov.dk/model/core/eid/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7";
MessageContext<SAMLObject> messageContext = IdpUtil.createMessageWithAssertion(true, true, true, nameID, TestConstants.SP_ENTITY_ID, TestConstants.SP_ASSERTION_CONSUMER_URL, inResponseToId);
Response response = (Response) messageContext.getMessage();

Assertion signedAssertion = new AssertionService().getAssertion(response);

// Assertion with a different ID and subject than the one the signature reference resolves to
String substituteNameID = "https://data.gov.dk/model/core/eid/person/uuid/11111111-2222-3333-4444-555555555555";
Assertion unboundAssertion = buildAssertionWithUnboundSignature(signedAssertion, "_copy" + UUID.randomUUID().toString().replace("-", ""), substituteNameID);

// Only interesting while the signature itself still verifies, otherwise the test would pass for the
// wrong reason
X509Certificate idpCertificate = IdPMetadataService.getInstance().getIdPMetadata().getValidX509Certificate(UsageType.SIGNING);
SignatureValidator.validate(unboundAssertion.getSignature(), new BasicX509Credential(idpCertificate));

// Validate, should fail because the signature is not bound to the assertion being consumed
AssertionValidationException exception = Assertions.assertThrows(AssertionValidationException.class, () -> {
validationService.validate(request, messageContext, response, unboundAssertion, new AuthnRequestWrapper(authnRequest, NSISLevel.SUBSTANTIAL, ""));
});
Assertions.assertTrue(exception.getMessage().toLowerCase().contains("signature"), "Expected the signature validation to reject the assertion, but failed with: " + exception.getMessage());
}

@DisplayName("Test that validator will fail an assertion whose signature reference is ambiguous")
@Test
public void testFailSignatureNotBoundToAssertionWithReusedId() throws Exception {
AssertionValidationService validationService = new AssertionValidationService();

// Mock HttpServletRequest
HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
Mockito.when(request.getRequestURL()).thenReturn(new StringBuffer(TestConstants.SP_ASSERTION_CONSUMER_URL));

// Create AuthnRequest
AuthnRequestService authnRequestService = AuthnRequestService.getInstance();
AuthnRequest authnRequest = getAuthnRequest(authnRequestService);
String inResponseToId = authnRequest.getID();

// Create MessageContext, Response and a correctly signed Assertion
String nameID = "https://data.gov.dk/model/core/eid/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7";
MessageContext<SAMLObject> messageContext = IdpUtil.createMessageWithAssertion(true, true, true, nameID, TestConstants.SP_ENTITY_ID, TestConstants.SP_ASSERTION_CONSUMER_URL, inResponseToId);
Response response = (Response) messageContext.getMessage();

Assertion signedAssertion = new AssertionService().getAssertion(response);

// Same document shape, but the consumed assertion keeps the ID of the element the reference resolves
// to, so comparing the reference URI to the ID of its parent element is not enough to tell them apart
String substituteNameID = "https://data.gov.dk/model/core/eid/person/uuid/11111111-2222-3333-4444-555555555555";
Assertion unboundAssertion = buildAssertionWithUnboundSignature(signedAssertion, signedAssertion.getID(), substituteNameID);

// Validate, should fail because the reference does not resolve to the assertion being consumed
AssertionValidationException exception = Assertions.assertThrows(AssertionValidationException.class, () -> {
validationService.validate(request, messageContext, response, unboundAssertion, new AuthnRequestWrapper(authnRequest, NSISLevel.SUBSTANTIAL, ""));
});
Assertions.assertTrue(exception.getMessage().toLowerCase().contains("signature"), "Expected the signature validation to reject the assertion, but failed with: " + exception.getMessage());
}

/**
* Build an assertion carrying a signature that verifies but is not bound to it: a copy of the signed
* assertion with the given ID and subject NameID, holding the signature, while the element the signature
* reference resolves to sits further down the same document.
*/
private static Assertion buildAssertionWithUnboundSignature(Assertion signedAssertion, String id, String nameID) throws Exception {
Element signedElement = signedAssertion.getDOM();
Document document = signedElement.getOwnerDocument();

Element copy = (Element) signedElement.cloneNode(true);
copy.setAttributeNS(null, "ID", id);
((Element) copy.getElementsByTagNameNS(SAMLConstants.SAML20_NS, "NameID").item(0)).setTextContent(nameID);

// Only the copy keeps the signature, so the referenced element still digests to the signed value
Element signature = (Element) signedElement.getElementsByTagNameNS(SignatureConstants.XMLSIG_NS, "Signature").item(0);
signedElement.removeChild(signature);

// The copy becomes the document element and the referenced element is nested inside it
document.removeChild(signedElement);
document.appendChild(copy);

Element advice = document.createElementNS(SAMLConstants.SAML20_NS, "saml2:Advice");
copy.appendChild(advice);
advice.appendChild(signedElement);

return (Assertion) XMLObjectProviderRegistrySupport.getUnmarshallerFactory().getUnmarshaller(copy).unmarshall(copy);
}

private static AuthnRequest getAuthnRequest(AuthnRequestService authnRequestService) throws InitializationException {
AuthnRequest authnRequest = authnRequestService.createAuthnRequest( TestConstants.SP_ASSERTION_CONSUMER_URL, false, false, NSISLevel.SUBSTANTIAL, null);
return authnRequest;
Expand Down