Add support for RSA-PSS Keys #514
Conversation
|
(sorry for huge one-commit, but it was pile of different interleaving changes I did not manage to separate to sensible smaller chunks yet) The failed test on macos times out in uri test as it is likely much slower with the new keys -- I can probably increase the timeout if we will agree that this is the right direction. |
There was a problem hiding this comment.
I wish the ASN.1 code was more straightforward and weren't obfuscated by all these OpenSSL Opaque structures.
For example all of the MD and MFG1 EVP_md stuff is ultimately only need to translate from the Hash name to the corresponding OID used in X509 ... I wonder if we shouldn't just store ASN1_OBJECT * pointers instead of the EVP_md ones in our structures to avoid all the churn. Even better if there was a way to just store the serialized ASN.1 once instead of reconstructing it from scratch at every invocation ...
After all there is nothing really dynamic in this information from our POV, the same fixed algorithm will always generate a very defined ASN.1 output.
Maybe we should just generate it once via some ASN.1 utility and store the pre-generated ASN.1 in the rsa-pss_map ?
Is there any need for runtime generation ?
src/signature.c
Outdated
| @@ -335,6 +351,7 @@ struct p11prov_mech { | |||
| int der_ecdsa_algorithm_id_len; | |||
| const unsigned char *der_digestinfo; | |||
| int der_digestinfo_len; | |||
| const EVP_MD *(*hash_md)(void); | |||
There was a problem hiding this comment.
Why do we need this? We already have p11prov_digest_get_name() to get the name to be used in EVP_MD_fetch() like w already do in mech_fallback_init() or p11prov_digest_util() ...
There was a problem hiding this comment.
I probably misunderstood the OpenSSL API. Now reading that the EVP_sha1() has performance impact, while I though it will just return const reference to the existing instance.
I wanted to avoid doing the EVP_MD_fetch() as it involves searching for the actual implementation with context, propq ...
Given that we really want just the algorithm name, I think this would be good to simplify. Will have a look what can be done.
There was a problem hiding this comment.
Oh, thats the reason:
Prior to OpenSSL 3.0, functions such as EVP_sha256() which return a "const" object were used directly to indicate the algorithm to use in various function calls.
Looking through the API, we could store the ASN1 DER format, convert it to ASN1_OBJECT using d2i_ASN1_OBJECT function and then pass the object to the algorithm with X509_ALGOR_set0(). Its not straightforward either, but avoids fetching the MD that many times. What do you think?
I would really really like to avoid constructing the Algorithm Identifier structure with PSS_PARAMS by hand. Its hell of nested sequences and I do not completely understand each of the encoding there.
There was a problem hiding this comment.
Or we could go through the NID with nid having stored in the table, then calling OBJ_nid2obj() and X509_ALGOR_set0(), similarly as done in ossl_X509_ALGOR_from_nid(). That sounds quite reasonable from my point of view.
There was a problem hiding this comment.
I got rid of the EVP_MD now.
The problem is that really all of the fields have default value so they are all optional and we need them in two contexts -- the key restrictions (where we might want to omit some) + signature parameters (we need to have all of them and we might need to support whole matrix of each SHA for digest x SHA for MGF1 (even though it does not make sense as PKCS#11 support this -- not sure if there is way to configure this in openssl though).
https://datatracker.ietf.org/doc/html/rfc8017#appendix-A.2.3
There was a problem hiding this comment.
So the only problem to pre-computing is deciding if want to restrinc md == mfg1_md instead of allowing free choices, because tht would cause matrix explosion?
I think I might be ok with that. In general it does no make sense anyway to mix hash strengths ...
There was a problem hiding this comment.
After all that RFC says:
it is RECOMMENDED
that the underlying hash function be the same as the one
identified by hashAlgorithm
There was a problem hiding this comment.
Then I can see if I can get them pregenerated somehow reasonably. It should simplify things a bit ...
Jakuje
force-pushed
the
rsa-pss
branch
2 times, most recently
from
1eb982a to
55cbfdf
Compare
When we force all operation in pkcs11 provider, key comparison and matching of RSA-PSS did not work. Signed-off-by: Jakub Jelen <jjelen@redhat.com>
... instead of the removed autotools Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
This is not as straightforward as it looked like initially and consists of
several changes that surface on different occasions:
* The private RSA keys in PKCS#11 can have `CKA_ALLOWED_MECHANISMS` which can
be used to determine if a key is generic key (can be used for any operation)
or RSA-PSS key, so that it could be used only for any or specified
operations.
* These keys get different identifier on OpenSSL level. They also get different
OIDs on the ASN.1 in various places of the X.509 certificates:
* The signatures in X.509 has a OID + parameters describing the hashes, mgf
and salt length used to create the signature. This is mandatory.
* The public key encoding can contain the RSA-PSS restrictions. These are not
mandatory so it allows us to indicate the key is restricted to any PSS
operations without the need to stick to specific combination of parameters.
But it allows to specify only one of the parameters (hash, MGF1-hash,
salt size). But there is no way to express this in PKCS#11. We can limit
ourselves to specific hash.
Given that the certtool we use for signing certificates during setup can not
distinguish RSA-PSS restricted keys and therefore generates unrestricted
certificates, we need to generate the certificate later using openssl, making
the tests a bit more ugly.
Fixes: latchset#489
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
At this moment, this was already copied into two places. This also generates second restricted RSA-PSS key during setup to avoid the need to generate key as part of rsapssam test, which makes it reentrant and should make the uri test a bit faster as we will have less keys. Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Description
This is not as straightforward as it looked like initially and consists of several changes that surface on different occasions:
The private RSA keys can have CKA_ALLOWED_MECHANISMS which can be used to determine if a key is generic key (can be used for any operation) or RSA-PSS key, so that it could be used only for any or specific RSA-PSS operations.
These keys get different identifier on OpenSSL level. They also get different OIDs on the ASN.1 in various places of the X.509 certificates:
The signatures in X.509 has a OID + parameters describing the hashes, mgf and salt length used. This is mandatory.
The public key encoding can contain the RSA-PSS restrictions. These are not mandatory so it allows us to indicate the key is restricted to PSS operations without the need to stick to specific combination of parameters.
When we force all operation in pkcs11 provider, the rsapss table was missing the match() callback, making key comparison fail when using RSA-PSS keys.
Given that the certtool we use for signing certificates during setup can not distinguish RSA-PSS restricted keys and therefore generates unrestricted certificates, we need to generate the certificate later using openssl, making the tests a bit more ugly.
Fixes: #489
Checklist
Reviewer's checklist: