Add RSA-PSS signing support (TLS 1.3 client mutual authentication with RSA cert)

[EdouardMALOT]

Summary

This PR completes the RSA-PSS support introduced in #377 by adding the signing side: a TLS 1.3 client can now perform mutual authentication (client certificate) with an RSA certificate.

#377 added RSA-PSS verification, which lets a NetX Duo client verify the CertificateVerify sent by an RSA server. However, when the server requests a client certificate, the client must also sign its own CertificateVerify with RSA-PSS — RFC 8446 §4.4.3 forbids RSASSA-PKCS1-v1_5 in CertificateVerify. Before this PR, _nx_secure_tls_process_certificate_request() returned NX_SECURE_TLS_UNSUPPORTED_CERT_SIGN_TYPE for any RSA local certificate in TLS 1.3, making client-cert authentication impossible with RSA keys.

This implements rsa_pss_rsae_sha256 (0x0804), which is the mandatory-to-implement signature algorithm for CertificateVerify per RFC 8446 §9.1. Related: #161.

Changes

• crypto_libraries/inc/nx_crypto_rsa.h — declare _nx_crypto_rsa_pss_sign()
• crypto_libraries/src/nx_crypto_rsa.c — implement _nx_crypto_rsa_pss_sign(): EMSA-PSS-ENCODE (RFC 8017 §9.1.1), salt length == hash length as required by RFC 8446 §4.2.3, salt generated via NX_CRYPTO_RAND() (same entropy source as the ECDSA nonce path). The RSA private-key operation is then applied to the encoded message by the existing signing flow.
• nx_secure/src/nx_secure_tls_process_certificate_request.c — TLS 1.3: instead of rejecting RSA local certificates, select rsa_pss_rsae_sha256 and proceed with the client Certificate/CertificateVerify flow
• nx_secure/src/nx_secure_tls_send_certificate_verify.c — TLS 1.3: build the CertificateVerify message with the SignatureScheme wire code (rsa_pss_rsae_sha256/384/512 handled) and the PSS-encoded message, instead of PKCS#1 v1.5 padding (which remains used for TLS 1.2)

Notes / limitations

• When answering a CertificateRequest, the client currently selects rsa_pss_rsae_sha256 unconditionally (the RFC 8446 §9.1 MUST algorithm, supported by any compliant server). The CertificateVerify encoding path itself also supports rsa_pss_rsae_sha384/512 should the selection be extended later.
• rsa_pss_pss_* variants (RSASSA-PSS certificates, 0x0809–0x080b) are out of scope; this covers the common case of classic RSA certificates (rsae).

Test plan

• TLS 1.3 mutual authentication handshake (server requests a client certificate) completes with an RSA client certificate, rsa_pss_rsae_sha256 CertificateVerify accepted by the server (tested against a Mosquitto/OpenSSL broker requiring client certificates)
• TLS 1.3 server-only handshake (no CertificateRequest) unaffected
• TLS 1.2 client certificate path still uses RSASSA-PKCS1-v1_5 (unchanged behavior)

[EdouardMALOT]

@fdesbiens : the remaining Secure_Interoperability / run_tests failure is unrelated to this change — the job fails during the "Install softwares" step.

[fdesbiens]

Thank you for this PR, @EdouardMALOT, and for all the others you submitted.

I am currently on vacation; I will review them once I am back, the week of July 20.

[fdesbiens]

@fdesbiens : the remaining Secure_Interoperability / run_tests failure is unrelated to this change — the job fails during the "Install softwares" step.

This is expected. The script relies on ifconfig to tweak networking configuration, and this should be modernised. I will spend some time fixing this along with the reviews.

[fdesbiens]

Thank you for this — it is careful work, and the parts that are hardest to get right are right. Before the findings, what I verified as correct, because it is worth recording:

The PSS encoding is byte-exact. I extracted _nx_crypto_rsa_pss_sign() into a harness, made the salt deterministic by compiling with -DNX_CRYPTO_RAND=test_rand, and compared the resulting EM against an independent EMSA-PSS-ENCODE implementation:

SHA-256 / RSA-2048         modulus= 256  emLen= 256  MATCH
SHA-256 / RSA-3072         modulus= 384  emLen= 384  MATCH
SHA-256 / RSA-4096         modulus= 512  emLen= 512  MATCH
SHA-256 / 66-byte mod      modulus=  66  emLen=  66  MATCH   <- emLen == hLen + sLen + 2, empty PS
EMSA-PSS-ENCODE (RFC 8017 §9.1.1) byte-exact agreement: ALL MATCH

Every step is correct: emLen = ceil(emBits/8), the step 3 rejection, DB = PS || 0x01 || salt with the right PS length, MGF1 over H, the XOR, clearing the top 8*emLen - emBits bits, and the 0xBC trailer. The boundary case where PS is empty works, a 65-byte modulus is correctly refused, and an undersized scratch is correctly refused. Each EM round-trips through _nx_crypto_rsa_pss_verify(), and flipping one bit of mHash makes the verifier reject it. No ASan or UBSan findings on any of those runs.

The TLS 1.3 wiring is correct. Two things I specifically checked because they are the usual sources of interop failure:

• The value passed as mHash really is the right thing. By the time the PSS call is reached, handshake_hash holds the digest of 0x20 × 64 || context || 0x00 || transcript_hash (built at nx_secure_tls_send_certificate_verify.c:231-257, hashed immediately after, with handshake_hash_length reset to the digest length). That matches RFC 8446 §4.4.3.
• The 0x0804 selection is not blindly trusted. _nx_secure_tls_process_certificate_request() still matches expected_sign_alg against the server's advertised signature_algorithms list and returns NX_SECURE_TLS_UNSUPPORTED_CERT_SIGN_ALG if the server does not offer it, and nx_secure_tls_signature_algorithm is only assigned on a match. So a server that does not advertise rsa_pss_rsae_sha256 fails cleanly rather than receiving a signature it will reject. Good.

The tls_1_3_enable_build_coverage configuration builds clean under -Werror -Wall -Wextra, and all 150 nx_secure tests pass in that configuration, so nothing existing regresses.

Now the findings. One is blocking:

_nx_crypto_rsa_pss_sign() writes emLen bytes into a buffer whose size it is never told. The only caller passes the 600-byte _nx_secure_padded_signature with em_bits computed from the certificate's modulus length, which nx_secure bounds nowhere. With a 6144-bit RSA client certificate this walks off the end. Reproduced under ASan; details on nx_crypto_rsa.c:789.

The salt does not go through NX_CRYPTO_RBG, and the comment justifying that is incorrect — ECDSA does not use NX_CRYPTO_RAND() directly. Details on nx_crypto_rsa.c:837.

There is no test coverage. gcov on the coverage build reports 0 of 47 executable lines in _nx_crypto_rsa_pss_sign, and the same for the two functions #377 added. AGENTS.md asks for matching regression tests and 100% coverage. Details on nx_crypto_rsa.c:760, including a concrete way to make the function KAT-testable — I already have a harness that does it and am happy to hand it over.

Everything else is minor. Thank you also for basing this on dev.

[fdesbiens]

We need matching regression tests and 100% coverage for new features. There are none here, and gcov confirms nothing reaches the code. From tls_1_3_enable_build_coverage after the full 150-test run:

_nx_crypto_rsa_pss_mgf1      executable= 33  executed=  0  never= 33  (0%)
_nx_crypto_rsa_pss_verify    executable= 53  executed=  0  never= 53  (0%)
_nx_crypto_rsa_pss_sign      executable= 47  executed=  0  never= 47  (0%)

So the two functions from #377 are also untested — that is not this PR's doing, but it means the whole PSS feature currently has no coverage, and this PR is the natural point to fix that.

There is a structural obstacle worth solving deliberately: because the salt is generated inside the function, the output is not reproducible, so it cannot be compared against the RFC 8017 test vectors. Two ways out, and I would suggest the second:

1. Compile-time override. NX_CRYPTO_RAND is overridable, so a test can build with -DNX_CRYPTO_RAND=test_rand and get a deterministic salt. This is exactly how I produced the byte-exact comparison in the summary, so I know it works — happy to hand over the harness, it is about 120 lines and already covers RSA-2048/3072/4096, the empty-PS boundary, the step 3 rejection and the scratch-size rejection.
2. Accept an optional salt. Add a const UCHAR *salt, UINT salt_length pair where NX_CRYPTO_NULL means "generate internally". That makes the function directly KAT-testable without build tricks, matches what most PSS APIs do, and would let a test use the published RFC 8017 vectors as-is.

Either way, please also add a sign-then-verify round-trip test and a negative test (tampered EM must be rejected), and ideally one end-to-end TLS 1.3 client-certificate test with an RSA cert so the nx_secure side is covered too. Note that a test built on option 1 would also give #377's verify path its first coverage.

If you do this part, I could look into providing the rest of the test coverage for #377 myself.

[EdouardMALOT]

Yes please, I would gladly take the harness. The deterministic-salt mechanism is exactly the part I did not want to invent twice, and starting from yours means the tests will match what you expect.

I have a local sign/verify round-trip harness covering the boundaries (worst-case scratch, empty PS, undersized modulus, tampered mHash), but it is a standalone program rather than a regression test, and a round-trip alone does not pin the encoding the way a KAT does.

My plan, unless you would rather structure it differently: fold your KAT vectors plus those boundary cases into test/regression/crypto_test, covering _nx_crypto_rsa_pss_sign, _nx_crypto_rsa_pss_verify and _nx_crypto_rsa_pss_mgf1, so the two functions from #377 get covered at the same time.

[fdesbiens]

This brings the file to roughly 1.4 KB of static RAM — handshake_hash[162], _nx_secure_padded_signature[600] and now this — which is a noticeable amount on the class of device ThreadX targets.

The sizing comment says "db[<=511 B] + salt[<=64 B] for RSA-4096+SHA-512", which checks out: RSA-4096 with SHA-512 needs 447 + 64 = 511 bytes, so 600 is comfortable. But the buffer is only ever needed for the duration of one call, and the function already fails cleanly with NX_CRYPTO_INVALID_BUFFER_SIZE when the scratch is too small, so there is room to be tighter — 512 would do for everything up to RSA-4096, and deriving it from a documented maximum modulus constant would be clearer than a bare 600.

Also worth stating in a comment: because this is static, two TLS sessions signing concurrently will corrupt each other. That is the existing pattern in this file rather than something you introduced, but this adds one more instance of it, and it is not obvious to a reader.

[EdouardMALOT]

Done in 5826b5c. The buffer is now sized from NX_SECURE_TLS_PSS_MAX_MODULUS_SIZE (512, RSA-4096), saving 88 bytes, and the comment states the static-buffer hazard you pointed out.

One consequence worth flagging: 600 bytes of scratch would have carried a modulus up to roughly 4800 bits, whereas 512 stops at RSA-4096. Anything larger now fails with NX_CRYPTO_INVALID_BUFFER_SIZE instead of being signed. That looks like the right trade given the RAM, but say the word if you would rather keep the headroom.

I checked the worst case rather than trusting the arithmetic. A sign/verify round-trip harness gives:

SHA-256 / RSA-2048           mod=256  sign+verify -> MATCH
SHA-256 / RSA-4096           mod=512  sign+verify -> MATCH
SHA-512 / RSA-4096 (worst)   mod=512  sign+verify -> MATCH     <- 447 + 64 = 511 of 512 used
SHA-512 / 130-byte mod       mod=130  sign+verify -> MATCH     <- emLen == hLen + sLen + 2, empty PS
SHA-512 / 129-byte mod       mod=129  refused, NX_CRYPTO_NOT_SUCCESSFUL
SHA-256 / 576-byte mod       mod=576  refused, NX_CRYPTO_INVALID_BUFFER_SIZE

A tampered mHash is rejected in every MATCH case.

[fdesbiens]

Every other externally visible function in this file is declared NX_CRYPTO_KEEP — _nx_crypto_rsa_operation at :82, _nx_crypto_method_rsa_init at :188, _nx_crypto_method_rsa_cleanup at :263, _nx_crypto_method_rsa_operation at :327. Neither _nx_crypto_rsa_pss_sign nor _nx_crypto_rsa_pss_verify has it.

That macro controls linker section placement and keep-out on several ports, so the omission can change what a port emits. #377 has the same gap, so fixing both here would be tidy.

[EdouardMALOT]

Fixed in 69f4722, on _nx_crypto_rsa_pss_sign and _nx_crypto_rsa_pss_verify.

I also added it to the static _nx_crypto_rsa_pss_mgf1: nx_crypto_ccm.c uses NX_CRYPTO_KEEP static for its file-local routines, so that appeared to be the convention. Happy to drop that third one if you would rather keep it to externally visible functions.

[fdesbiens]

Thank you for your meaningful contribution, @EdouardMALOT, and for your patience. I just submitted my review.

[EdouardMALOT]

Thanks for the review. Pushed four commits: the blocking finding, the two other substantive ones, and the header blocks. The remaining minor items and the test coverage are still to come.

[EdouardMALOT]

Pushed 69f4722 and 5826b5c, covering the four remaining minor items: the named PSS scheme constants, the NX_CRYPTO_KEEP attributes, the scratch buffer sizing, and the indentation. I resolved the threads that are now purely done and left open the two where I asked you a question (the RSA-4096 ceiling that comes with the smaller scratch, and whether NX_CRYPTO_KEEP belongs on the static _nx_crypto_rsa_pss_mgf1).

The four files build clean under -Werror -Wall -Wextra -Wconversion -Wsign-conversion in four configurations, including NX_CRYPTO_SELF_TEST.

That leaves test coverage as the one substantive item outstanding, and I have taken you up on the harness offer in that thread. I will follow up with the regression tests.

[EdouardMALOT]

Rebased onto current dev (force-push, 5826b5c1 -> 6d7ed660). No conflicts, and the seven commits are unchanged.

Worth doing rather than cosmetic: nx_secure_tls_send_certificate_verify.c has moved under this branch since #417, so the merge result had never actually been compiled anywhere. It builds clean now under -Werror -Wall -Wextra -Wconversion -Wsign-conversion in four configurations, and the PSS sign/verify round-trip still passes on the rebased tree, including the RSA-4096 + SHA-512 worst case.

Apologies for any review anchors this moves. Doing it now rather than later keeps it to a single force-push before the coverage work lands.

[EdouardMALOT]

rebased on "dev"