-
Notifications
You must be signed in to change notification settings - Fork 76
Description
Hi,
TL;DR; Allow Public Key usage through cert option.
Background
I'm currently maintaining suomifi-passport-saml Fork and we are trying to get back to upstream usage, because node-saml currently supports SAML Authn Request Extensions.
But node-saml is lacking support for plain public key usage, it only supports certs through cert configuration option. suomifi-passport-saml currently has this kind-of "hax" https://github.com/vrk-kpa/suomifi-passport-saml/blob/master/lib/passport-saml/saml.js#L546-L570 to allow public key usage, pretty trivial.
SAML Metadata can contain KeyInfo in two formats; ds:RSAKeyValue and/or ds:X509Data both must represent the same key. See http://docs.oasis-open.org/security/saml/Post2.0/sstc-metadata-iop.html
2.5.1 Key Representation
In the case of the latter, only a single certificate is permitted. If both forms are used, then they MUST represent the same key.
Also in same document, section 2.6.1 Key Processing
Each key expressed by a md:KeyDescriptor element within a particular role MUST be treated as valid when processing messages or assertions in the context of that role. Specifically, any signatures or transport communications (e.g., TLS/SSL sessions) verifiable with a signing key MUST be treated as valid, and any encryption keys found MAY be used to encrypt messages or assertions (or encryption keys) intended for the containing entity.
... a metadata consumer MUST extract the public key found in the certificate and MUST NOT honor, interpret, or make use of any of the information found in the certificate other than as an aid in identifying the key used...
This means that even if certificate is expired or it is found in revocation list, public key cannot be ignored, it must be treated as valid. x509 certificate can be seen as a container for the public key.
SAML idP's / SP's may be able to provide both key presentations or only ds:RSAKeyValue or only ds:X509Data. Usually there is some technical limitation which is not in "our hands" if only one of the key presentations is available.
What should be done?
In short term: allow both x509 certificates and plain public keys through cert configuration parameter. certToPem() function https://github.com/node-saml/node-saml/blob/master/src/crypto.ts#L27-L36 should be re-named and re-factored to handle both -----BEGIN CERTIFICATE----- and -----BEGIN PUBLIC KEY----- cases, or in more generally if cert contains reasonable header information it should not be modified. Why?, because verifier.verify() returned by crypto.createVerify() has pretty wide range of formats which it accepts.
This would allow plain public key usage as well.
In long term: allow both x509 certificates and plain public keys through keyInfos (or such) configuration parameter. Key infos could be more wider set of presentatios which verifier.verify() accepts. This could be breaking change or cert could be left in the configuration interface, but internally cert would be treaded as x509 certificate as it is now done. And internallly add it to keyInfos array. And deprecate certusage.
This change would take node-saml closer to handle SAML idp metadata.
I'll try to add PR in few hours into the short term case.