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

Regardiing exposing the options of<md:EncryptionMethod in metadata #404

Open
Kalpa2019 opened this issue Apr 15, 2024 · 1 comment
Open

Comments

@Kalpa2019
Copy link

Hi All,
Recently, i got to work wiht Shibboleth idp, i was trying out with basic metadata configuration.
however, at the shibboleth idp, they had some relay/nameid configurations to encrypt always.
so, if authnrequest has been made, it was rejecting saying encryption parameters are not right.
but, encryption data was there, finally, adding the encryptionmethod along with the certificate worked.
it makes sense, because, they want to know what is the encryptionmethod(algorithm) supported. having this along with "use"=encryption options, have worked.
so, i wanted to inform that, it is very simple change, can I check-in the code. the code list all the encryption lsited in constatnts.py and updates to the meatadata in metadata.py.
if i update, what is the criteria. and mechanism. lke creating a branch, review and merget?

thanks,
Kalpa.

@pitbulk
Copy link
Contributor

pitbulk commented Jun 26, 2024

HI @Kalpa2019, sorry for the delayed reply

is not very common the need of encrypting in the SP side, at the end, no confidential data is sent on AuthNRequest, LogoutRequest and LogoutResponses, with the exception of the NameId, but you could use persistent NameID to anonymize the user.

That said, if I'm not wrong you want to expose the md:EncryptionMethod in addition to the ds:KeyInfo when md:KeyDescriptor has use="encryption".

For that I believe we need to extend the Metadata builder to support an additional parameter encryption_method=None
and then extend as well the methods _add_x509_key_descriptors and add_x509_key_descriptors and inside it, have something like:

cls._add_x509_key_descriptors(sp_sso_descriptor, cert, False,encryption_method)

and:

@staticmethod
def _add_x509_key_descriptors(root, cert, signing, encryption_method):
    key_descriptor = OneLogin_Saml2_XML.make_child(root, "{%s}KeyDescriptor" % OneLogin_Saml2_Constants.NS_MD)
    root.remove(key_descriptor)
    root.insert(0, key_descriptor)
    key_info = OneLogin_Saml2_XML.make_child(key_descriptor, "{%s}KeyInfo" % OneLogin_Saml2_Constants.NS_DS)
    key_data = OneLogin_Saml2_XML.make_child(key_info, "{%s}X509Data" % OneLogin_Saml2_Constants.NS_DS)

    x509_certificate = OneLogin_Saml2_XML.make_child(key_data, "{%s}X509Certificate" % OneLogin_Saml2_Constants.NS_DS)
    x509_certificate.text = OneLogin_Saml2_Utils.format_cert(cert, False)
    key_descriptor.set("use", ("encryption", "signing")[signing])
    if encryption_method:
        encryption_method_data = OneLogin_Saml2_XML.make_child(key_descriptor, "{%s}EncryptionMethod" % OneLogin_Saml2_Constants.NS_MD)
        encryption_method_data.set("Algorithm", encryption_method)

Do you want to contribute with a PR, if so please add as well test coverage.

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

2 participants