search

IdentityPython / pysaml2

Python implementation of SAML2
Apache License 2.0
555 stars 422 forks source link

Add xenc-schema-11.xsd #964

Open johanlundberg opened 4 months ago

johanlundberg commented 4 months ago

Description

This adds XML Encryption 1.1 Schema to the schema validator.

The feature or problem addressed by this PR

Sweden Connect has recently added an MGF element with an algorithm in xmlenc11 to their responses. This PR starts addressing that change.

What your changes do and why you chose this solution

This change let's the schema validator handle an MGF element like this.

<ns3:EncryptionMethod Algorithm="http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p">
    <ns2:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1" />
    <ns4:MGF Algorithm="http://www.w3.org/2009/xmlenc11#mgf1sha1" />
</ns3:EncryptionMethod>

Checklist

johanlundberg commented 4 months ago

The trouble does not end there sadly as xmlsec1 version 1.2.37 that I get with Debian Stable does not seem to handle the response.

error=func=xmlSecTransformRsaOaepReadParams:file=transforms.c:line=2703:obj=unknown:subj=unknown:error=27:unexpected node:node=MGF
func=xmlSecOpenSSLRsaOaepNodeRead:file=kt_rsa.c:line=1002:obj=rsa-oaep-mgf1p:subj=xmlSecTransformRsaOaepReadParams:error=1:xmlsec library function failed: 
func=xmlSecTransformNodeRead:file=transforms.c:line=1351:obj=rsa-oaep-mgf1p:subj=readNode:error=1:xmlsec library function failed: 
func=xmlSecTransformCtxNodeRead:file=transforms.c:line=600:obj=EncryptionMethod:subj=xmlSecTransformNodeRead:error=1:xmlsec library function failed: 
func=xmlSecEncCtxEncDataNodeRead:file=xmlenc.c:line=751:obj=EncryptionMethod:subj=xmlSecTransformCtxNodeRead:error=1:xmlsec library function failed: 
func=xmlSecEncCtxDecryptToBuffer:file=xmlenc.c:line=610:obj=unknown:subj=xmlSecEncCtxEncDataNodeRead:error=1:xmlsec library function failed: 
func=xmlSecKeysMngrGetKey:file=keys.c:line=1256:obj=unknown:subj=xmlSecKeysMngrFindKey:error=1:xmlsec library function failed: 
func=xmlSecEncCtxEncDataNodeRead:file=xmlenc.c:line=788:obj=unknown:subj=unknown:error=45:key is not found:encMethod=aes256-cbc
func=xmlSecEncCtxDecryptToBuffer:file=xmlenc.c:line=610:obj=unknown:subj=xmlSecEncCtxEncDataNodeRead:error=1:xmlsec library function failed: 
func=xmlSecEncCtxDecrypt:file=xmlenc.c:line=536:obj=unknown:subj=xmlSecEncCtxDecryptToBuffer:error=1:xmlsec library function failed: 
Error: failed to decrypt file

It is working with latest xmlsec1 (1.3.4) but to run that pysaml2 needs some more modifications. See https://github.com/SUNET/pysaml2/commit/855f1067dba7f51b228f13489ba8f1020c988015.

Maybe this is something that https://github.com/IdentityPython/pysaml2/pull/961 will fix?

johanlundberg commented 3 months ago

As a follow up the EncryptionMethod stanza seems incorrect as the spec says the following:

The http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p identifier defines the mask generation function as the fixed value of MGF1 with SHA1. In this case the optional xenc11:MGF element of the xenc:EncryptionMethod element MUST NOT be provided.

The http://www.w3.org/2009/xmlenc11#rsa-oaep identifier defines the mask generation function using the optional xenc11:MGF element of the xenc:EncryptionMethod element. If not present, the default of MGF1 with SHA1 is to be used.

But that doesn't really matter as I guess there will be viable use cases of the MGF element in the future.

c00kiemon5ter commented 3 months ago

Regarding adding the newer xenc-schema-11.xsd schema and supporting it, I thin we should do it. This is what this PR is introducing and I think we can merge it.

Within IdentityPython we have agreed to support running on the latest stable Debian release. For now xmlsec1 v1.2 is distributed with Debian. But we should be cautious and be ready to support newer versions.

Support for xmlsec1 v1.3 was added with https://github.com/IdentityPython/pysaml2/pull/902 that should be part of PySAML2 v7.5.0.

Having said this, improvements are welcome! It is entirely possible that the output from xmlsec1 will change again.

Maybe this is something that https://github.com/IdentityPython/pysaml2/pull/961 will fix?

Supporting the xmlsec module (bindings to the xmlsec1) is a possible option. I support this and we should definitely have a further look. We need to ensure the options (and restrictions) that are set using the cli binary are supported by the module. As a bonus, we will get away with creating and managing certain temporary files.