PKCS#11 RSA Padding Offload

[space88man]

Description

This PR# allows tokens to perform CKM_RSA_PKCS (sign/encrypt), CKM_RSA_PKCS_PSS (sign), CKM_RSA_PKCS_OAEP (encrypt). These mechanims are the post-hash versions: i.e. wolfCrypt has already created the message digest.

It is for use cases where tokens do not expose CKM_RSA_X_509, eg., LUNA in FIPS-mode

The changes to callback and pkcs#11 can be gated by -DWOLF_CRYPTO_CB_NOPAD -DNO_PKCS11_RSA_PAD.

Breaks ABI? Might do so as it changes the size of some structs.

Testing

There is a test set here: https://github.com/user-attachments/files/16215326/testing-v2.zip
It includes:

• pure software RSA test (regression checking)
• token RSA test (raw / PKCS / PKCS-PSS / PKCS-OAEP)

Without PR - token should only see CKM_RSA_X_509.
With PR - token should see CKM_RSA_PKCS / CKM_RSA_PKCS_PSS / CKM_RSA_PKCS_OAEP

Checklist

• added tests
• updated/added doxygen
• updated appropriate READMEs
• Updated manual and documentation

Updates

• the other two crypto callbacks are wc_AriaCryptoCb and wc_CAAM_router:

  • Aria doesn't seem to do RSA — so is probably unaffected
  • CAAM does RSA — not clear whether it has padding capability

• make check now passes

• make check also passes when PKCS-offload is disabled (CFLAGS=-DWOLF_CRYPTO_CB_NOPAD …)

• scan-build is leaving one warning

    wolfcrypt/src/rsa.c:3179:95: warning: The left operand of '!=' is a garbage value   [core.UndefinedBinaryOperatorResult]
     3179 |         if ((type == RSA_PUBLIC_ENCRYPT || type == RSA_PRIVATE_ENCRYPT) &&  (padding->pad_type != WC_RSA_NO_PAD)) {
          |                                                                             ~~~~~~~~~~~~~~~~~ ^
    1 warning generated.
  

• code formatting (line length <= 80 chars)

[wolfSSL-Bot]

Can one of the admins verify this patch?

[dgarske]

Okay to test. Contributor agreement on file.

[dgarske]

Seeing errors like:

wolfcrypt/test/testwolfcrypt
...
RSA      test failed!
 error L=19776 code=-1792 (unknown error number)
 [fiducial line numbers: 8773 27740 43402 55860]
FAIL testsuite/testsuite.test (exit status: 255)

[space88man]

...
RSA test failed!
error L=19776 code=-1792 (unknown error number)

@dgarske Forced push - this is passing now but I am getting an infinite loop in unit.test — I can reproduce this on master so may not be related to this PR# including unit.test.

[dgarske]

Thanks @space88man , this is great work. I can see some issues with the asynchronous RSA cases (stack variable used). I like the new RsaPadding struct and the new function RsaFunctionPad API, but it's a pretty big change.
I will likely push a few changes to your fork before it could be accepted.

[dgarske]

Maximum 80 characters per line:

overlong lines added:
/tests/api.c:76660         if (info->pk.type == WC_PK_TYPE_RSA || info->pk.type == WC_PK_TYPE_RSA_PSS) {
/tests/api.c:76692                                 info->pk.rsa.type, &key, info->pk.rsa.rng, info->pk.rsa.padding);
/wolfcrypt/src/cryptocb.c:422                     word32* outLen, int type, RsaKey* key, WC_RNG* rng, RsaPadding *padding)
/wolfcrypt/src/rsa.c:3135         ret = wc_CryptoCb_RsaPadding(in, inLen, out, outLen, type, key, rng, padding);
/wolfcrypt/src/rsa.c:3143             ret = wc_RsaPad_ex(in, inLen, out, in2Len, padding->pad_value, rng, padding->pad_type,
/wolfcrypt/src/rsa.c:3144                                padding->hash, padding->mgf, padding->label, padding->labelSz, padding->saltLen,
/wolfcrypt/src/rsa.c:3153         ret = wc_CryptoCb_RsaPadding(in2, in2Len, out, outLen, type, key, rng, padding);
/wolfcrypt/src/rsa.c:3179         if ((type == RSA_PUBLIC_ENCRYPT || type == RSA_PRIVATE_ENCRYPT) && (padding->pad_type != WC_RSA_NO_PAD)) {
/wolfcrypt/src/rsa.c:3181             ret = wc_RsaPad_ex(in, inLen, out, in2Len, padding->pad_value, rng, padding->pad_type,
/wolfcrypt/src/rsa.c:3182                                padding->hash, padding->mgf, padding->label, padding->labelSz, padding->saltLen,
/wolfcrypt/src/rsa.c:3253     return wc_RsaFunction_ex(in, inLen, out, outLen, type, key, rng, 1, &padding);
/wolfcrypt/src/rsa.c:3257                       word32* outLen, int type, RsaKey* key, WC_RNG* rng, RsaPadding *padding)
/wolfcrypt/src/rsa.c:3260     return wc_RsaFunction_ex(in, inLen, out, outLen, type, key, rng, 1, padding);
/wolfcrypt/src/rsa.c:3393             ret = wc_RsaPad_ex(in, inLen, out, (word32)sz, pad_value, rng, pad_type,
/wolfcrypt/src/rsa.c:3416         ret = wc_RsaFunctionPad(in, (word32)inLen, out, &key->dataLen, rsa_type, key,
/wolfcrypt/src/wc_pkcs11.c:2163     rv = session->func->C_GetMechanismInfo(session->slotId, mechanism, &mechInfo);
/wolfssl/wolfcrypt/cryptocb.h:484 WOLFSSL_LOCAL int wc_CryptoCb_RsaPadding(const byte* in, word32 inLen, byte* out,
/wolfssl/wolfcrypt/rsa.h:310                                    word32* outLen, int type, RsaKey* key, WC_RNG* rng, RsaPadding* padding);```

[space88man]

Maximum 80 characters per line:

overlong lines added:
/tests/api.c:76660         if (info->pk.type == WC_PK_TYPE_RSA || info->pk.type == WC_PK_TYPE_RSA_PSS) {

Incoming push - there are some existing lines len > 80 chars but I won't touch those to minimize the diff.

[dgarske]

[check-source-text] [2 of 7] [wolfssl]
overlong lines added:
/wolfcrypt/src/rsa.c:3188                                padding->label, padding->labelSz, padding->saltLen,
/wolfssl/wolfcrypt/cryptocb.h:484 WOLFSSL_LOCAL int wc_CryptoCb_RsaPadding(const byte* in, word32 inLen, byte* out,
/wolfssl/wolfcrypt/rsa.h:310                                    word32* outLen, int type, RsaKey* key, WC_RNG* rng, RsaPadding* padding);

[clang-tidy-defaults] [7 of 7] [wolfssl]
    configure...   real 0m11.163s  user 0m5.349s  sys 0m2.509s
    build.../tmp/workspace/PRB-multi-test-script/wolfssl/wolfcrypt/src/rsa.c:3184:43: warning: The left operand of '!=' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
(padding && padding->pad_type != WC_RSA_NO_PAD)) {
^
/tmp/workspace/PRB-multi-test-script/wolfssl/wolfcrypt/src/rsa.c:4297:12: note: Calling 'wc_RsaPSS_Sign_ex'
return wc_RsaPSS_Sign_ex(in, inLen, out, outLen, hash, mgf,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...
make: *** [Makefile:5159: all] Error 2
   real 0m24.244s  user 0m23.129s  sys 0m0.696s
    clang-tidy-defaults fail_analytic_build
    failed config: '--srcdir' '.' '--disable-jobserver' '--enable-option-checking=fatal' 'CPPFLAGS=-DKEEP_OUR_CERT -DKEEP_PEER_CERT -DWOLFSSL_ALT_NAMES -DNO_WOLFSSL_CIPHER_SUITE_TEST -DWOLFSSL_OLD_PRIME_CHECK -pedantic -DSP_ALLOC -DWOLFSSL_CLANG_TIDY' 'CC=/tmp/workspace/PRB-multi-test-script/testing/git-hooks/clang-tidy-builder.sh' 'CLANG=/usr/lib/llvm-14/bin/clang-14' 'CFLAGS=-Wunreachable-code-aggressive -Wthread-safety -Wloop-analysis -Wenum-compare-conditional -fcolor-diagnostics -fcomplete-member-pointers -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -g -fdebug-types-section -Wunreachable-code-break -Wunreachable-code-return -Wimplicit-fallthrough'
    BUILD_ENV: 'CLANG_TIDY=/usr/lib/llvm-14/bin/clang-tidy' 'CLANG=/usr/lib/llvm-14/bin/clang-14'
final tally: 6 of 7 checks succeeded, 0 skipped, 0 forced, and 1 failed, for wolfssl.
exiting with status 2

[space88man]

[clang-tidy-defaults] [7 of 7] [wolfssl]
configure... real 0m11.163s user 0m5.349s sys 0m2.509s
build.../tmp/workspace/PRB-multi-test-script/wolfssl/wolfcrypt/src/rsa.c:3184:43: warning: The left operand of '!=' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
(padding && padding->pad_type != WC_RSA_NO_PAD)) {

@dgarske Probably some face-palm thing but it is really not clear to me what clang-static-analyzer is complaining about here. I found a similar issue in mbedtls: Mbed-TLS/mbedtls#1002 which was declared as a false positive.

The comparison: padding->pad_type != WC_RSA_NO_PAD looks innocent enough - maybe the static analyzer can't assume the argument padding was initialized by the caller?

[dgarske]

[clang-tidy-defaults] [7 of 7] [wolfssl]
configure... real 0m11.163s user 0m5.349s sys 0m2.509s
build.../tmp/workspace/PRB-multi-test-script/wolfssl/wolfcrypt/src/rsa.c:3184:43: warning: The left operand of '!=' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
(padding && padding->pad_type != WC_RSA_NO_PAD)) {

@dgarske Probably some face-palm thing but it is really not clear to me what clang-static-analyzer is complaining about here. I found a similar issue in mbedtls: Mbed-TLS/mbedtls#1002 which was declared as a false positive.

The comparison: padding->pad_type != WC_RSA_NO_PAD looks innocent enough - maybe the static analyzer can't assume the argument padding was initialized by the caller?

I’ll see if I can locate the cause.

[dgarske]

@space88man , great work on this PR. I think there is some really useful work here, but it is a bit more intrusive then we can accept as-is. I'd like to take bits of this and implement in a slightly different way. I plan to open a new PR and provide a link to it. Thanks, David Garske, wolfSSL

[space88man]

@space88man , great work on this PR. I think there is some really useful work here, but it is a bit more intrusive then we can accept as-is.

Sure - totally understandable; I look forward to testing out the alternative PR. Before I close this - would you like me to PR only the pkcs11.h changes, that has no impact to the rest of the code base?

[dgarske]

@space88man , great work on this PR. I think there is some really useful work here, but it is a bit more intrusive then we can accept as-is.

Sure - totally understandable; I look forward to testing out the alternative PR. Before I close this - would you like me to PR only the pkcs11.h changes, that has no impact to the rest of the code base?

Hi @space88man , yes go ahead and trim this down to just the pkcs11.h changes for now. I hope yo get to this in the next couple of days. Thanks, David Garske, wolfSSL

[space88man]

@space88man , great work on this PR. I think there is some really useful work here, but it is a bit more intrusive then we can accept as-is.

Sure - totally understandable; I look forward to testing out the alternative PR. Before I close this - would you like me to PR only the pkcs11.h changes, that has no impact to the rest of the code base?

The original draft/WIP can be referenced here: https://github.com/space88man/wolfssl/tree/wip2-padding-refactor.

I've reduced this PR to the pkcs11.h header changes

[dgarske]

Retest this please - saw a GitHub origin timeout

[dgarske]

Retest this please - FIPS timeout

[dgarske]

Retest this please. Windows test machine was down...