Fix the encoding of SM2 keys

[levitte]

OpenSSL's encoding of SM2 keys used the SM2 OID for the algorithm OID
where an AlgorithmIdentifier is encoded (for encoding into the structures
PrivateKeyInfo and SubjectPublicKeyInfo).

Such keys should be encoded as ECC keys.

Fixes #22184

[levitte]

This may have unforeseen consequences for our decoders. I haven't
investigated that yet, and this PR will remain a draft until I have.

[mattcaswell]

I was under the impression that the SM2 OID is the preferred way to do this now - but maybe we're the only tool actually using it. It might be we need some option for this for interoperability purposes.

OTC Question: Which OID should we be using for the SM2 AlgorithmIdentifier?

[levitte]

A question is also, how does 1.1.1 encode SM2 keys? (I haven't looked at that for a few years and can't remember for the life of me)

So the question is, have we caused a regression of some sort? Compared to our own older versions? Compared to the rest of the world (#22184 at least indicates that we don't agree with what others expect)? Compared to actual standards?

[mattcaswell]

Compared to actual standards?

I don't think there is a standard for this AFAIK - which is part of the problem (or at least there wasn't - maybe there is now somewhere).

[levitte]

Compared to actual standards?

I don't think there is a standard for this AFAIK - which is part of the problem (or at least there wasn't - maybe there is now somewhere).

Well, yes and no. SM2 is piggy backed on ECC, and the key itself is really an ECC key, just with a very specific curve, and with a slightly different algorithm to sign and verify.

So I'm leaning on this being a confusion, at least in terms of AlgorithmIndentifier. "there is no standard" could very well be "there is an ECC encoding standard, use that".

@romen, could I ask for a comment from you?

[levitte]

Seeing further discussions in #22184, and digging into what standards we have available, this PR may turn out to be quite wrong.

[slontis]

Any thoughts on this @dghgit?

[romen]

@levitte maybe this can also help?

https://gist.github.com/romen/d752b8d1897bc1a0009017511770de06

As far as I recall, the encoding of SM2 keys, for compatibility with 1.1.1 and the other software stacks that use it, SHOULD use the EC OID even though they are not ECDSA keys: the only way to distinguish that one should use the SM2DSA algorithm rather than proper ECDSA is the combination of ECC OID and then the SM2 OID for the named curve inside the envelope.

I have complained about the mess that the poor outcome of the SM2 standardization reached, and One could argue that it is actually violating the ECC standards, by abusing the OID reserved for ECC for an algorithm other than ECDSA/DH.
This is clear when thinking that one could use either the ECDSA or SM2DSA algorithm on the SM2 curve, leading to different results, and there isn't really a way to distinguish between the 2 things, given SM2DSA key encodings abuse the ECDSA OID.

[dghgit]

So we've always done it as an EC key with the regular EC OID, years ago there was an attempt to produce RFCs for these algorithms and that's what was in used at the time. SM2 signing is also carried out using P-256 not just the specific curve, so it makes sense that it the SM2 curve was designed as an EC curve first. Since we first provided SM2 we have had feedback about some unexpected surprises in the algorithm (such as the default ID string) in order to make our generated certificates compliant with the Chinese based CAs, but nothing related to the manner we are encoding public keys.

[levitte]

I did a quick bit of reading up, and based on RFC 5915 "Elliptic Curve Private Key Structure" - 1. Introduction and RFC 5480 "ECC SubjectPublicKeyInfo Format" - 2.1. Elliptic Curve Cryptography Public Key Algorithm Identifiers, one might draw the conclusion that either encoding could be seen as legitimate:

• With the AlgorithmIdentifier.algorithm OID id-ecPublicKey (1.2.840.10045.2.1), it would be an unrestricted ECC key that just happens to be in the SM2 curve.

• With the AlgorithmIdentifier.algorithm OID SM2 (1.2.156.10197.1.301), it would be an ECC key restricted to be used with the SM2 signature scheme, the same way that id-ecDH restricts the use to the ECDH key exchange scheme, or id-ecMQV restricts the use to the ECMQV key agreement scheme.

Am I reading this right? It does seem to leave things up to some level of interpretation, and makes it harder to come with a categorically correct answer...

[levitte]

In the discussion comment #22184 (reply in thread), a few appendices with example was pointed at. The example in appendix D.2 of GM/T 0015-2012 is the clearest to me, very clearly showing a public key that has the AlgorithmIdentifier.algorithm set to ecPublicnumber, not SM2.

[t8m]

OTC: This is a regression. We need to test the backwards compatibility with keys encoded both ways.

[InfoHunter]

Deliberately moved away from using the ECC OID for SM2. The ECC OID is for ECDSA which SM2 is not. We used to use the ECC OID for this but I believe we deliberately changed it to use the SM2 OID.

This is also what I recall of move

[levitte]

The ECC OID is for ECDSA

Actually, this is what RFC 5480 "ECC SubjectPublicKeyInfo Format" - 2.1. Elliptic Curve Cryptography Public Key Algorithm Identifiers has to say on that specific topic:

• id-ecPublicKey indicates that the algorithms that can be used with the subject public key are unrestricted. ...

It does go on to say that this is the OID used for ECDSA keys, but this introduction indicates that id-ecPublicKey isn't exclusively reserved for ECDSA.

And thank you @romen for your comment.

[mattcaswell]

This is also what I recall of move

I think there is confusion between the OID used for signatures and the OID used for keys. So I now think my earlier recollection of this was about the OID for signatures. The commit which made the change for keys (f2db052) seems to make no mention of this change of behaviour and I now believe it was an accidental side effect.

[t8m]

This IMO also needs a CHANGES.md entry.

[t8m]

We also need #21037 and #20488

[levitte]

I disagree re #21037 (see comment I just made there)

[Eno-CN]

I want to know this. #20973

[levitte]

@slontis, I've added a CHANGES.md change

[mattcaswell]

Needs reconfirmation from @t8m

[t8m]

Assuming @slontis approval holds.

[openssl-machine]

This pull request is ready to merge

[t8m]

Merged to master, 3.2, 3.1 and 3.0 branches. Thank you.