Fix serialization pub key #491
Conversation
esabedo
force-pushed
the
fix-serialization-pub-key
branch
2 times, most recently
from
ce1a190 to
20e91a4
Compare
|
Do you have example outputs and reproducer that could be used in the tests to make sure we do not regress? |
esabedo
force-pushed
the
fix-serialization-pub-key
branch
from
20e91a4 to
8e6ce77
Compare
PKCS11 RSA Public Key (1024 bits)
Modulus:
00:d0:84:e1:59:fa:69:3d:38:54:92:7c:77:a9:74:
5e:9f:4d:e9:f7:12:9a:ab:89:77:bf:c5:d7:63:e5:
9b:16:89:a6:cb:c6:cd:71:16:ed:da:3c:93:a4:76:
63:6d:65:99:e8:02:40:80:86:0e:92:3f:3a:5e:51:
fe:ba:86:aa:c7:45:cc:30:c1:61:c0:7f:7b:fa:af:
78:54:8e:4b:09:03:70:fc:e1:61:ae:3c:88:ba:f3:
dc:a0:8c:6d:b4:be:da:af:74:26:a9:ca:b1:0e:c3:
7e:9c:c2:84:36:86:4e:e5:37:31:b3:6b:2a:c4:b2:
21:a8:e6:e0:64:8d:03:dd:53
Exponent: 65537 (0x10001)
URI pkcs11:model=Abracadabra;manufacturer=Abracadabra%20Co.;serial=44444444;token=Test%20custom;id=%67%6C%6F%62%61%6C%52%53%41%31%30%32%34%4B%65%79;type=privateIn this output, we are specifically interested in the
A commit with test has also been added |
Currently, EVP_PKEY_print_public specifies an incorrect key class for the generated EVP_PKEY. Now selection is also used to get the key class. Signed-off-by: Eduard Sabirov <sabirov@aktiv-company.ru>
… allocated using openssl Signed-off-by: Eduard Sabirov <sabirov@aktiv-company.ru>
Signed-off-by: Eduard Sabirov <sabirov@aktiv-company.ru>
esabedo
force-pushed
the
fix-serialization-pub-key
branch
from
8e6ce77 to
70f5d83
Compare
There was a problem hiding this comment.
I do not think this change is correct.
I think rather that if the public key is requested, but we are passed a private key, that you should search and find the associated public key (handling the case where it may not be available) and then print that key.
We already have functions that handles this in src/objects.c for various other cases.
| @@ -140,7 +140,7 @@ static int p11prov_rsa_encoder_encode_text(void *inctx, OSSL_CORE_BIO *cbio, | |||
| } | |||
| } | |||
|
|
|||
| uri = p11prov_key_to_uri(ctx->provctx, key); | |||
| uri = p11prov_key_to_uri(ctx->provctx, key, selection); | |||
There was a problem hiding this comment.
In this place selection may contain both flags, which means you will always print a URI that strictly refers to the public key.
You need to pass the key class not the selection to get the correct type.
| @@ -670,7 +670,7 @@ static char *uri_component(const char *name, const char *val, size_t vlen, | |||
| return c; | |||
| } | |||
|
|
|||
| char *p11prov_key_to_uri(P11PROV_CTX *ctx, P11PROV_OBJ *key) | |||
| char *p11prov_key_to_uri(P11PROV_CTX *ctx, P11PROV_OBJ *key, int selection) | |||
There was a problem hiding this comment.
Using selection here makes little sense and will lead to incorrect results.
In PKCS#11 objects are either private or public keys, never both.
I do not see how using selection here improves anything, all you get is a lie.
If you need to "force" the type for some reason, then you should straight on pass the requested class as an attribute and let the caller make the call.
An alternative is to pass a boolean called use_class, and when set to false the URI will not include the keytype. This may be better than trying to lie about the object type.
There was a problem hiding this comment.
Note that this comment makes sense only if we determine that this change is needed at all, which I do not think it is, see the main review comment about this change.
There was a problem hiding this comment.
In this case, we pointed out an error when requesting information about the public key. It's acceptable if you fix it in any way you can. As users of the pkcs11 library, it would be important for us to fix this error.
Also important in this pr is the correction of UB when calling free from an object for which memory is allocated using openssl.
There was a problem hiding this comment.
@esabedo Can you please isolate the the commit fixing use of free() into a separate PR?
|
This should probably be fixed together with the code in #479 |
Currently, EVP_PKEY_print_public specifies an incorrect key class for the generated EVP_PKEY.
Now selection is also used to get the key class.