Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PSS pack: Add provider support for PSS parameters #11710

Closed
wants to merge 32 commits into from

Conversation

@levitte
Copy link
Member

@levitte levitte commented May 2, 2020

PSS parameter support is sorely lacking in our providers. This PR seeks to remedy that.
Please read the separate commits for better understanding.

@levitte levitte requested review from mattcaswell, paulidale and slontis May 2, 2020
@levitte
Copy link
Member Author

@levitte levitte commented May 2, 2020

This is currently incomplete, the EVP_SIGNATURE implementation still needs to be reworked, and serialization doesn't quite kick in as I expected.

In spite of that, early comments welcome!

@levitte
Copy link
Member Author

@levitte levitte commented May 2, 2020

The serializer issue was a non-issue, apart from a missing dependency...

crypto/rsa/rsa_ameth.c Outdated Show resolved Hide resolved
crypto/rsa/rsa_backend.c Outdated Show resolved Hide resolved
crypto/rsa/rsa_lib.c Outdated Show resolved Hide resolved
crypto/rsa/rsa_lib.c Outdated Show resolved Hide resolved
crypto/rsa/rsa_local.h Outdated Show resolved Hide resolved
@levitte
Copy link
Member Author

@levitte levitte commented May 3, 2020

@slontis, I'm confused, I seem to have imagined that RSASSA-PSS shouldn't be part of the fips module. Thanks for pointing it out.

@levitte levitte force-pushed the levitte:provider-rsa-pss branch May 4, 2020
crypto/rsa/rsa_backend.c Outdated Show resolved Hide resolved
crypto/rsa/rsa_pss.c Outdated Show resolved Hide resolved
return NID_undef;
}

static const char *nid2name(int meth, const OSSL_ITEM *items, size_t items_n)

This comment has been minimized.

@slontis

slontis May 5, 2020
Contributor

meth is probably not the best name here..

This comment has been minimized.

@slontis

slontis May 13, 2020
Contributor

?

providers/implementations/signature/rsa.c Outdated Show resolved Hide resolved
providers/implementations/serializers/serializer_rsa.c Outdated Show resolved Hide resolved
providers/implementations/keymgmt/rsa_kmgmt.c Outdated Show resolved Hide resolved
crypto/rsa/rsa_ameth.c Outdated Show resolved Hide resolved
@levitte levitte force-pushed the levitte:provider-rsa-pss branch 2 times, most recently May 7, 2020
@levitte levitte changed the title [WIP] PSS pack: Add provider support for PSS parameters [WIP, pending on #11750] PSS pack: Add provider support for PSS parameters May 7, 2020
@levitte levitte marked this pull request as ready for review May 7, 2020
@levitte
Copy link
Member Author

@levitte levitte commented May 7, 2020

Note that this depends on #11750. With that merged in, all tests succeed in my setup, except test_ssl_new...

openssl-machine pushed a commit that referenced this pull request May 14, 2020
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from #11710)
openssl-machine pushed a commit that referenced this pull request May 14, 2020
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from #11710)
openssl-machine pushed a commit that referenced this pull request May 14, 2020
…meters

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from #11710)
openssl-machine pushed a commit that referenced this pull request May 14, 2020
To make it easier to check the generated key manually, display it
before comparing diverse other serializations.

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from #11710)
openssl-machine pushed a commit that referenced this pull request May 14, 2020
There were a few RSA-PSS related tests that were disabled for non-default
library contexts.  We now re-enable them.

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from #11710)
openssl-machine pushed a commit that referenced this pull request May 14, 2020
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from #11710)
openssl-machine pushed a commit that referenced this pull request May 14, 2020
There are a few things in the OpenSSL code that are known to give
warnings that we know are harmless.  We test our builds accordingly.

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from #11710)
openssl-machine pushed a commit that referenced this pull request May 14, 2020
The problem encountered is that some arrays were deemed unnecessary by
clang, for example:

    providers/common/der/der_rsa.c:424:28: error: variable 'der_aid_sha224Identifier' is not needed and will not be emitted [-Werror,-Wunneeded-internal-declaration]
    static const unsigned char der_aid_sha224Identifier[] = {
                               ^

However, these arrays are used in sizeof() expressions in other parts
of the code that's actually used, making that warning-turned-error a
practical problem.  We solve this by making the array non-static,
which guarantees that the arrays will be emitted, even though
unnecessarily.  Fortunately, they are very small.

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from #11710)
@levitte
Copy link
Member Author

@levitte levitte commented May 14, 2020

Merged

484d1a7 RSA: Add RSA key types
645a541 RSA: Extract much of the rsa_pkey_export_to() code to a separate function
e9d6186 RSA: Add rsa_schemes.c, to store scheme data and translator functions
1567109 RSA: Add a less loaded PSS-parameter structure
967cc3f RSA: Add PSS-parameter processing in EVP_PKEY_ASN1_METHOD functions
2275ff6 DER writer: Add the possibility to abandon empty SEQUENCEs
36a2a55 PROV: Refactor the RSA DER support
0ec36bf PROV: Refactor the RSA SIGNATURE implementation for better param control
8a758e9 PROV & KEYMGMT: Add PSS-parameter support in the RSA KEYMGMT implementation
ea297dc PROV & SERIALIZER: Adapt the RSA serializers for PSS-parameters
2c6094b EVP: For SIGNATURE operations, pass the propquery early
2d55366 PROV & SIGNATURE: Adapt the RSA signature code for PSS-parameters
e25761b EVP: Refactor the RSA-PSS key generation controls for providers
106ec30 PROV & ASYM_CIPHER: Adapt the RSA asymmetric cipher code for PSS-parameters
d59b7a5 test/evp_pkey_provided_test.c: Display first, compare after
f63f3b7 test/ssl-tests/20-cert-select.cnf.in: Re-enable RSA-PSS related tests
d49be01 test/recipes/15-test_rsapss.t: Add test with unrestricted signature
16e3588 .travis.yml: never use -Werror, use --strict-warnings instead
90ad284 PROV: make some DER AID arrays non-static, to avoid clang complaints

@levitte levitte closed this May 14, 2020
@mattcaswell mattcaswell mentioned this pull request May 19, 2020
0 of 2 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants