Conversation
|
I'm preparing yet another in the series of fixes for #10797 issues, which depends on this cleanup of |
crypto/ec/ec_ameth.c
Outdated
There was a problem hiding this comment.
This should probably backported to 1.1.1 as well!
There was a problem hiding this comment.
Hmmm... There was some construction to establish link between a signature and hash algorithm somewhere, and it was used for GOST algorithms.
3b46bcc to
ffa0c7a
Compare
|
I'd really like it if @InfoHunter commented on this PR |
…rdingly This means that when loaded or created, EC EVP_PKEYs with the SM2 curve will be regarded as EVP_PKEY_SM2 type keys by default. Applications are no longer forced to check and fix this. It's still possible, for those who want this, to set the key type to EVP_PKEY_EC and thereby run the normal EC computations with the SM2 curve. This has to be done explicitly.
This makes it possible to generate SM2 parameters and keys like this:
EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_SM2);
EVP_PKEY *pkey = EVP_PKEY_new();
EVP_PKEY_keygen_init(pctx);
EVP_PKEY_keygen(pctx, pkey);
…ining The solution to incorporate the SM2 identity processing was an off the side hack that more or less duplicated the ASN1_item_verify() code with just a few lines being different. We replace this with a new function ASN1_item_verify_ctx(), which takes an EVP_MD_CTX pointer instead of an EVP_PKEY pointer, just like its sibling ASN1_item_sign_ctx(). This allows us to refactor X509_verify() and X509_REQ_verify() to simply create a local EVP_MD_CTX and an attached EVP_PKEY_CTX, which gets to hold the SM2 identity, if there is one, and then let ASN1_item_verify_ctx() to its job. This will also make it easier to adapt ASN1_item_verify_ctx() for provider based keys.
Since the SM2 curve now implies SM2 computation by default, test/ecdsatest.c is faulty for not passing an SM2 id. We could handle it by creating the appropriate EVP_PKEY_CTX and giving it the SM2 id, but we do want a test of switching a key with the SM2 curve to become a "normal" EC key with EVP_PKEY_set_alias_type(), so that's what we do here. test/evp_test.c, on the other hand, doesn't need to explicitly set the EVP_PKEY_SM2 alias type, as that now happens automatically.
There's no longer any need to make an EVP_PKEY type change for SM2 keys, so we trim away that code.
New narrative for ecdsatest: We test all the curves once for each EC key type we have, i.e. one round trip with EVP_PKEY_EC and one with EVP_PKEY_SM2. This shows that we can use "normal" EC computations on keys with the SM2 curve (which have the type EVP_PKEY_SM2 by default) and SM2 computations with any other curve (which have the type EVP_PKEY_EC by default)
|
No, it was not, I wasn't sure if it is a problem of the PR or master. |
|
I've fixed all the things I could catch on my laptop, let's hope the CIs get happier |
|
Looks good apart from the documentation issue - which I do think needs resolving. |
…rdingly This means that when loaded or created, EC EVP_PKEYs with the SM2 curve will be regarded as EVP_PKEY_SM2 type keys by default. Applications are no longer forced to check and fix this. It's still possible, for those who want this, to set the key type to EVP_PKEY_EC and thereby run the normal EC computations with the SM2 curve. This has to be done explicitly. Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Dmitry Belyavskiy <[email protected]> (Merged from #10942)
This makes it possible to generate SM2 parameters and keys like this:
EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_SM2);
EVP_PKEY *pkey = EVP_PKEY_new();
EVP_PKEY_keygen_init(pctx);
EVP_PKEY_keygen(pctx, pkey);
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Dmitry Belyavskiy <[email protected]>
(Merged from #10942)
…ining The solution to incorporate the SM2 identity processing was an off the side hack that more or less duplicated the ASN1_item_verify() code with just a few lines being different. We replace this with a new function ASN1_item_verify_ctx(), which takes an EVP_MD_CTX pointer instead of an EVP_PKEY pointer, just like its sibling ASN1_item_sign_ctx(). This allows us to refactor X509_verify() and X509_REQ_verify() to simply create a local EVP_MD_CTX and an attached EVP_PKEY_CTX, which gets to hold the SM2 identity, if there is one, and then let ASN1_item_verify_ctx() to its job. This will also make it easier to adapt ASN1_item_verify_ctx() for provider based keys. Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Dmitry Belyavskiy <[email protected]> (Merged from #10942)
Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Dmitry Belyavskiy <[email protected]> (Merged from #10942)
With test/ecdsatest.c, we test all the curves once for each EC key type we have, i.e. one round trip with EVP_PKEY_EC and one with EVP_PKEY_SM2. This shows that we can use "normal" EC computations on keys with the SM2 curve (which have the type EVP_PKEY_SM2 by default) and SM2 computations with any other curve (which have the type EVP_PKEY_EC by default) test/evp_test.c, on the other hand, doesn't need to explicitly set the EVP_PKEY_SM2 alias type, as that now happens automatically. Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Dmitry Belyavskiy <[email protected]> (Merged from #10942)
There's no longer any need to make an EVP_PKEY type change for SM2 keys, so we trim away that code. Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Dmitry Belyavskiy <[email protected]> (Merged from #10942)
Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Dmitry Belyavskiy <[email protected]> (Merged from #10942)
|
Merged f4e4382 EVP_PKEY_assign_EC_KEY(): detect SM2 curve and set EVP_PKEY type accordingly |
| * as an error, so it's better to accept the control, check the | ||
| * value and return a corresponding value. | ||
| */ | ||
| if (p1 != NID_sm2) { |
There was a problem hiding this comment.
I am sorry for the very late response - Just got back from the Chinese lunar new year vacation.
I would like to make a note here explaining that why we just use NID_sm2 and ignore other curves. Though the current NID_sm2 is the only recommended curve but SM2 algorithms (signature/encryption) can actually take other curves as well - if I recall correctly there exist test vectors for SM2 using other curves in some SM2 specification documents.
For OpenSSL I don't think we need to provide the ability to set other curves other than NID_sm2 because the NID_sm2 curve is the only one being used in practice. And when we talk about SM2 in different standard organization (ISO/IETF/...), there is also one curve NID_sm2.
But I still think it's better to leave a comment here (and the paramgen function below as well) that OpenSSL implements only the NID_sm2 curve and provides no other capability of setting other customized curves so that OpenSSL's implementation can only be valid with the SM2 test vectors that use NID_sm2.
@levitte ,what do you think?
There was a problem hiding this comment.
I didn't limit it like that. What I did was to change the default behaviour, but you can still change the EVP_PKEY type with EVP_PKEY_set_alias_type
| It's possible to switch back and forth between the types B<EVP_PKEY_EC> | ||
| and B<EVP_PKEY_SM2> with a call to EVP_PKEY_set_alias_type() on keys | ||
| assigned with this macro if it's desirable to do a normal EC | ||
| computations with the SM2 curve instead of the special SM2 |
There was a problem hiding this comment.
I find this paragraph is misleading that it sounds we provide a way to encourage users to use the the SM2 curve to do a normal EC computation, which I think it should be discouraged to do so. Or should we make it mandatory to use SM2 curve with SM2 special computation only?
PR openssl#10942 introduced the new function ASN1_item_verify_ctx(), but did not document it with the promise that documentation would follow soon. We temporarily add this function to missingcrypto.txt until it has been done.
PR #10942 introduced the new function ASN1_item_verify_ctx(), but did not document it with the promise that documentation would follow soon. We temporarily add this function to missingcrypto.txt until it has been done. Reviewed-by: Paul Dale <[email protected]> (Merged from #10980)
Up until now, applications had to set the type of SM2
EVP_PKEYs explicitly for SM2 processing to take place, with a call like this:The goal of this refactoring is to eliminate the need for such extra application side processing, and instead having that happen automatically and an
EC_KEYis assigned to anEVP_PKEY.Furthermore, the functions
X509_REQ_verify()andX509_verify()were doing some heavy lifting with SM2 keys, via a function that was almost a straight copy ofASN1_item_verify(). This is replaced with a new functionASN1_item_verify_ctx()that takes anEVP_MD_CTXpointer instead of aEVP_PKEYone, and through which all sorts of parameters can be passed, such as the SM2 identity.Doing this also prepares for a provider side key adaptation.