Am 2018-06-07 um 10:20 schrieb Ivan Kanakarakis <ivan.kanak at gmail.com>: Hello everyone, I have been looking into PR #495 https://github.com/IdentityPython/pysaml2/pull/495 What the user needs is an option to configure the signing algorithm that will be used by the SP to sign an authentication request. This comes in hand with the digest algorithm and in extension what is entity that should be signed. What the proposed one line change does, is allows this value to exist in pysaml2 Config object. By itself, it does not affect anything in the code - it does not set the signing algorithm. It is only a placeholder for a value. Something else is supposed to look at that value, at the right time, and pass it as an argument to the appropriate function/method. This doesn't seem right. If it is in the configuration, then it should actually do something - it should affect the way the library behaves. Looking into this I stumbled upon some commits, made about a year ago, that implement some of this functionality for the IdP part: * 2aedfa0 - make both sign response and assertion configurable adds sign_assertion and sign_response options * bd4303a - Signing signature and digest algorithm configuration adds sign_alg and digest_alg options These are implemented in the SATOSA repository (see satosa/frontends/saml2.py the _handle_authn_response method). However, their configuration lies between the lines of the pysaml2 configuration (under service/idp/policy). This is wrong - each project should be responsible for its own configuration. The code that decides what should be signed and how, should live in pysaml2. If it is handled by SATOSA then it should be part of the SATOSA configuration and an override of the pysaml2 configuration. Moreover, it seems that this functionality was partially already there in pysaml2 in the first place. See the Policy class in saml2/assertion.py and its get_sign method. An option named 'sign' can be defined under the service/idp/policy part of the configuration, that defines an array of values that represent what should be signed, for example: service: idp: policy: sign: - response - assertion So now we have both the above 'sign' option, plus 'sign_assertion' and 'sign_response', which should do the same thing. What I would like to do is move the code introduced by the commits above into pysaml2: this will allow a consistent behaviour whether pysaml2 is used by SATOSA or some other project. Then the same options can be used by the backend too, which would satisfy the user request. Once that is done we can look into making the configuration work in one way. This is bigger than it looks. What happens now is that we define for example, that we want to use SHA512 as a sign_alg. This will be used when a authentication response is formed, but it is ignored when for example a logout request is to be created. This happens because the configuration of what signing algorithm will be used is only implemented for the authentication response. There are two ways to fix this: - we either assume that a configuration option like 'sign_alg' is global, and as such, it affects the signing algorithm of anything that is to be signed - or we assume that it relates to the authentication req/response only (and in that case it shold probably be called authn_sign_alg or alike) and require new options for other kinds of signatures (logout_sign_alg, metadata_sign_alg, etc). The first solution requires that we find all places in the code use signatures and make sure they respect the configuration. I have already noted (a lot of) entry points to pysaml2 that should be looking into the configuration to derive the signing and digest algorithm values. The second option is "easier" to work with, as it allows for an incremental implementation of this request. The second approach is also more flexible for the end user, but at the same time more complex as it requires more configuration values to be set. Ofcourse we can have both, use an option like sign_alg to defined the signing algorithm, and use "suboptions" like authn_sign_alg to override the sign_alg setting where needed. I hope this makes sense (even though it mixes at least four configuration options together). If you have any comments, I'd like to hear. Cheers, -- Ivan c00kiemon5ter Kanakarakis >:3 _______________________________________________ Satosa-dev mailing list Satosa-dev at lists.sunet.se https://lists.sunet.se/listinfo/satosa-dev