Consider adding support for `RS256` algorithm identifier

hslatman commented 1 year ago

What is the areas you would like to add the new feature to?

Go-COSE Library

Is your feature request related to a problem?

The RS256 algorithm is one of the algorithms that can be used with ACME device-attest-01 (which can be backed by a TPM with an AK certificate) to enroll with an ACME server. Currently we rely on basic checks for valid values of the COSE algorithm identifier in alg in smallstep/certificates, but I figured it might be nice if we could rely on a standard set, hence the need for the AlgorithmRS256 identifier.

What solution do you propose?

Adding AlgorithmRS256 (and potentially related ones) to the list of known algorithms.

A PR for this is available here: https://github.com/veraison/go-cose/pull/140. The PR does more than just exposing the AlgorithmRS256 identifier; it also includes signing and verification using RSASSA PKCS#1 v1.5 backed by the Go stdlib.

What alternatives have you considered?

One alternative is to rely on a constant or type for the RS256 algorithm in a different package or in the project that needs it directly (the current solution). My thinking was go-cose would be the right place to put this identifier, as it's a registered COSE algorithm identifier. It's not an recommended algorithm, but from the README it's not clear that go-cose will only support the recommended algorithms. If go-cose will only support the recommended algorithms, then maybe the README needs a slight tweak to mention that.

I can also foresee removing the signing and verification support for AlgorithmRS256, and to only expose the constant. Then if one wants to actually use RSASSA PKCS#1 v1.5, they can do so by implementing the Signer and Verifier themselves, or import it from a different public package. The go-cose package could return an error when AlgorithmRS256 were tried to be used with the package directly when instantiating an rsaSigner or rsaVerifier.

Any additional context?

No response

shizhMSFT commented 1 year ago

The use of RS256 is defined in RFC 8812 Section 2 and is marked as Not Recommended.

Questions to other maintainers: Should we support the implementation of algorithms which is marked as Not Recommended in the go-cose library?

shizhMSFT commented 1 year ago

If we do support RS256, I guess that we would support RS384 and RS512 as they are all SHA-2.

roywill commented 1 year ago

None of these seem to be recommended to create. The other question is should we handle them on decode?

hslatman commented 1 year ago

So far most votes seem to go to not adding support for RS256 (nor RS384 and RS512) signing and verification in this library.

Because I think people consider this package to be following the RFCs, I think it would be good to at least add the appropriate constants, but to make signing and verification happen in a user-provided package instead, like I described in the last paragraph. Within this library, when trying to sign or verify using one of those unsupported algorithms, an error could be returned.

I can adapt the current PR to work like that.

Does this sound reasonable?