Repository navigation
[ENHANCEMENT]: Signature compliant to http://www.w3.org/2007/05/xmldsig-more#sha256-rsa-MGF1 #328
Description
Is your feature request related to a problem? Please describe...
I am trying to connect to a SAML IDP which expects a http://www.w3.org/2007/05/xmldsig-more#sha256-rsa-MGF1 Signature as described in rfc6931 RSASSA-PSS without Parameters.
Unfortunately, I am a bit stuck on how to implement it.
Describe teh solution you'd like...
I would like to have a new option to use http://www.w3.org/2007/05/xmldsig-more#sha256-rsa-MGF1 signatures.
Describe the alternatives you've considered...
I tried to use https://github.com/digitalbazaar/forge to calculate the signature, but did not have any success.
Activity
cjbarth commentedon Jul 6, 2023
The first step would be to see if NodeJS supports this. If so, then you can easily put up a PR that mimics the existing methods for doing this. If not, you'll have to figure that out yourself and create a function to do it by hand.
rahxam commentedon Jul 10, 2023
I was able to implement it with https://www.npmjs.com/package/node-forge#rsa, but I guess the dependency is a bit overkill for xml-crypto.
cjbarth commentedon Jul 11, 2023
In that case you might just use the plugable nature of this library to add such support yourself. There should be information in the README on that.
rahxam commentedon Jul 11, 2023
Hey,
Yes, I actually did, but I am using passport-saml and I needed to fork node-saml and xml-encryption as well to use the algorithm in xml-crypto and get everything running, which is lot's of forks for 10 lines of code.
cjbarth commentedon Jul 11, 2023
If you'd like to add support for custom signing methods to
node-saml, I'd be happy to look at that. This way you could just pass your function throughnode-samltoxml-crypto.cornzz commentedon Dec 10, 2024
Hi @cjbarth, any info on whether this is planned?
I have posted on the discussions board about this, our IDP (BundID) is switching to sha256-rsa-MGF1 next year and we would optimally like to keep using node-saml for our auth process. Maybe I can help?
cjbarth commentedon Dec 10, 2024
@cornzz , I've been a little busy lately, but I always try to do a big release of all the updates and new features to coincide with the NodeJS release schedule. So, in the coming months we'll be doing that. I'm not writing much code for this anymore. The only bit of code that I'm thinking I might get to is #464. Other than that, I'll review code and consult on development, but I depend on others to write it. If you and @rahxam would like to put up a PR, I'll gladly look at it and get your changes merged in so that you don't have to use a fork.
cornzz commentedon Feb 13, 2025
Hi @cjbarth, I'm looking into integrating support for sha256-rsa-MGF1 in node-saml and I am confused as to what needs to be changed. For one, I understand that https://github.com/node-saml/xml-crypto/blob/master/src/signature-algorithms.ts and https://github.com/node-saml/xml-crypto/blob/master/src/types.ts need to be updated as these are used for the actual creation of the SAML payloads. But what about https://github.com/node-saml/node-saml/blob/master/src/algorithms.ts? It seems these are used in
getAuthorizeUrlAsync(),getLogoutUrlAsync()andgetLogoutResponseUrlAsync(), however I don't see where these functions are used? Are they obsolete? Or should both libraries be updated?Add support for sha256-rsa-MGF1 signing algorithm (node-saml#328)
cornzz commentedon Feb 13, 2025
@cjbarth I assume both libraries have to be updated, I opened PRs in both repos #488 node-saml/node-saml#387
The
signeritself does not change, merely the additional parameterspaddingandsaltLengthhave to be provided tocrypto.sign(): https://nodejs.org/docs/latest-v18.x/api/crypto.html#signsignprivatekey-outputencodingLet me know if this seems about right to you.
Add support for sha256-rsa-MGF1 signing algorithm (node-saml#328)
Add support for sha256-rsa-MGF1 signing algorithm (node-saml#328)
Add support for sha256-rsa-MGF1 signing algorithm (#328) (#488)
cjbarth commentedon Oct 17, 2025
This has been resolved by #488 .