SAML2: EncryptedID Edge Case

[slaughter550]

Hello,

I'm currently working on a shibboleth project for several public universities and my team has encountered an issue where the line below is being triggered even though if its commented out, everything works correctly.

We were wondering what the utility of this line is and what case its attempting to prevent? I'd be happy to submit a PR if there is a cleanup opportunity here.

Sorry in advanced if this is a stupid question. Still getting our feet wet with SAML.

if ($this->encrypted) {
    $encryptedIDNodes = OneLogin_Saml2_Utils::query($this->decryptedDocument, '/samlp:Response/saml:Assertion/saml:Subject/saml:EncryptedID');
    if ($encryptedIDNodes->length > 0) {
        throw new OneLogin_Saml2_ValidationError(
            'Unsigned SAML Response that contains a signed and encrypted Assertion with encrypted nameId is not supported.',
            OneLogin_Saml2_ValidationError::NOT_SUPPORTED
        );
    }
}

From: https://github.com/onelogin/php-saml/blob/master/lib/Saml2/Response.php#L353

[pitbulk]

That piece of code tries to detect if an Assertion of a SAMLResponse which had the NameID value encrypted (saml:EncryptedID) was later encrypted.

The toolkit is not able to process that scenario properly.

Take in mind makes no much sense to encrypt something already encrypted. If there are plans to encrypt the assertion that contains a NameID, then leave the NameID clear and don't encrypt it.

[slaughter550]

Ahh I see. Unfortunately by default, the Shibboleth IdP will encrypt NameIDs if there is an encryption certificate present in the relying party metadata regardless if the assertion is already encrypted.

I agree it adds no value although a lot of institutions use Shibboleth IdP & SAML. I was playing around with it a little bit and I saw that the reference validation broke if the response was unencrypted twice. Is this what you're referring to that the toolkit won't handle it?

Would it be desirable to have the toolkit handle this case?

[jkmoe]

Hi! The exact same issue bugs my team right now. The IdP does not seem to be able to change the fact, that the decrypted XML contains an EncryptedID node. We as a service provider might need to drop the toolkit because of this. Any advice?

[pitbulk]

@jkmoe. Implementing this isn't trivial; that is why I have not implemented this yet.

I remember that I experienced signature invalidations while trying to cover this scenario of having encrypted elements with encrypted elements inside.

[jkmoe]

@pitbulk Okay, thanks for the info. Since in our case we will probably not need the name id anyways, we might be good with the OneLogin_Saml2_ValidationError not being thrown. From what I understand the exception makes sense as a default behavior. Being able to influence the exception being thrown or not would help us out here.

[joonlabs]

Hey together, we are also facing the issue of the NameID being encrypted by the IdP (But we need the NameID attribute). The IdP is the Elster-Provider of the German Government (https://www.elster.de), which is widely used in Germany. The IdP always encrypts the NameID.

Is a pull request wanted to support it in this toolkit @pitbulk ?

[pitbulk]

@joonlabs If you are able to contribute that, happy to review and merge. Please include test coverage.

[joonlabs]

@pitbulk found time now. I will fork now and create the PR later. Thanks for being open to this!

[joonlabs]

@pitbulk Created the PR #594 into branch 4.x-dev to support encrypted name ids in encrypted assertions including test coverage.