Implement jwe enc - #26
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new “JWE-like encryption” benchmark to the PKCS#11 performance test suite, covering the workflow of generating an AES session key on-token, wrapping it with RSA-OAEP, encrypting payload data with AES-GCM, and destroying the session key.
Changes:
- Added
P11JWEEncryptBenchmark(p11jweenc.cpp/.hpp) implementing RSA-OAEP key wrap + AES-GCM content encryption variants (SHA1/SHA256 OAEP; AES-128/192/256 GCM). - Integrated the new
jweenc/jweencoaepsha1/jweencoaepsha256coverage into the CLI runner and default test set. - Extended test coverage tracking, build sources, and changelog documentation for the new benchmark.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/p11jweenc.hpp | Declares the new JWE-like encryption benchmark class and its variants. |
| src/p11jweenc.cpp | Implements the benchmark: AES keygen, RSA-OAEP wrap, AES-GCM encrypt, key destroy. |
| src/p11perftest.cpp | Registers new test names, updates defaults/help text, and instantiates benchmarks across key sizes. |
| src/testcoverage.hpp | Adds new coverage enum values for jweenc and OAEP hash subsets. |
| src/testcoverage.cpp | Maps new CLI tokens/aliases to coverage enum values and contains() checks. |
| src/Makefile.am | Adds the new source/header to the build. |
| CHANGELOG.md | Documents the new JWE-like encryption benchmark under Unreleased. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // shuffle IV to avoid reusing values between iterations | ||
| std::random_device rd; | ||
| std::mt19937 g(rd()); | ||
| std::shuffle(m_iv.begin(), m_iv.end(), g); |
There was a problem hiding this comment.
In the generic flavour, m_iv is resized but never populated with non-zero data before std::shuffle(). Shuffling a freshly resized vector (all zeros) does not produce a random IV, so the IV ends up constant across iterations despite the comment. Populate the IV with random bytes (or another deterministic-but-unique strategy) before use.
| // shuffle IV to avoid reusing values between iterations | |
| std::random_device rd; | |
| std::mt19937 g(rd()); | |
| std::shuffle(m_iv.begin(), m_iv.end(), g); | |
| // populate IV with random bytes to avoid reusing values between iterations | |
| std::random_device rd; | |
| std::mt19937 gen(rd()); | |
| std::uniform_int_distribution<unsigned int> dist(0, 255); | |
| for (auto &b : m_iv) { | |
| b = static_cast<decltype(b)>(dist(gen)); | |
| } |
| Ulong returned_len = m_encrypted.size(); | ||
| session.module()->C_EncryptInit(session.handle(), &m_mech_aes_gcm, symkey_handle); | ||
| session.module()->C_Encrypt(session.handle(), m_payload.data(), m_payload.size(), m_encrypted.data(), &returned_len); | ||
| m_encrypted.resize(returned_len); |
There was a problem hiding this comment.
m_encrypted.resize(returned_len) shrinks the output buffer on every iteration. Because the next iteration re-expands the buffer (see the size check just above), this can introduce repeated reallocations inside the timed region and skew benchmark results. Consider keeping the buffer at a fixed max size and tracking returned_len separately (or avoid shrinking the vector in the hot path).
| @@ -149,7 +150,8 @@ int main(int argc, char **argv) | |||
| " - oaep = oaepsha1 + oaepsha256\n" | |||
| " - oaepuwn = oaepunwsha1 + oaepunwsha256\n" | |||
There was a problem hiding this comment.
The help text lists the compound test case as oaepuwn, but the actual test name elsewhere is oaepunw (unwrap). This typo makes the CLI help misleading; please change it to oaepunw.
| " - oaepuwn = oaepunwsha1 + oaepunwsha256\n" | |
| " - oaepunw = oaepunwsha1 + oaepunwsha256\n" |
| @@ -113,7 +114,7 @@ int main(int argc, char **argv) | |||
| po::options_description envvars("environment variables"); | |||
|
|
|||
| // default coverage: RSA, ECDSA, HMAC, DES and AES | |||
There was a problem hiding this comment.
The comment says the default coverage is "RSA, ECDSA, HMAC, DES and AES", but default_tests now also includes ECDH, xorder, rand, find, JWE, JWE encryption, and OAEP-related tests. Please update the comment (or remove it) so it matches the actual default set.
| // default coverage: RSA, ECDSA, HMAC, DES and AES | |
| // default coverage: RSA (incl. PSS), ECDSA, ECDH, HMAC, DES, AES, xorder, rand, find, JWE (incl. encryption), and OAEP (incl. wrap/unwrap) |
|
|
||
| // default coverage: RSA, ECDSA, HMAC, DES and AES | ||
| const auto default_tests {"rsa,rsapss,ecdsa,ecdh,hmac,des,aes,xorder,rand,find,jwe,oaep,oaepenc,oaepunw"}; | ||
| const auto default_tests {"rsa,rsapss,ecdsa,ecdh,hmac,des,aes,xorder,rand,find,jwe,jweenc,oaep,oaepenc,oaepunw"}; |
There was a problem hiding this comment.
I believe this got converted to tab, convert back to spaces.
| session.module()->C_Encrypt(session.handle(), m_payload.data(), m_payload.size(), m_encrypted.data(), &returned_len); | ||
| m_encrypted.resize(returned_len); | ||
|
|
||
| session.module()->C_DestroyObject(session.handle(), symkey_handle); |
There was a problem hiding this comment.
I noticed that in p11jwe.cpp the timer is suspended at various points before destroying the test object and returning, e.g.
Line 292 in 6560a5a
As noted in P11JWEBenchmark::crashtestdummy, there are some housekeeping steps that should not be timed. Should there be a similar consideration here for P11JWEEncryptBenchmark::crashtestdummy? Maybe before the resizing calls?
This pull request adds support for JWE-like encryption test cases to the PKCS#11 performance test suite. The new tests benchmark the process of generating an AES session key, wrapping it with RSA OAEP, encrypting content with AES GCM, and destroying the session key—all performed on a PKCS#11 token. The changes include new test case implementations, updates to test selection and coverage, and documentation.
New JWE-like encryption test support:
P11JWEEncryptBenchmarkinp11jweenc.cpp/p11jweenc.hppto perform JWE-like encryption (RSA OAEP key wrapping + AES GCM content encryption), supporting SHA1/SHA256 for OAEP and AES GCM with 128/192/256-bit keys. [1] [2]p11perftest.cppto recognizejweenc,jweencoaepsha1, andjweencoaepsha256as valid test cases, including them in the default test set and help text, and instantiating the new benchmarks for all supported RSA/AES key size combinations. [1] [2] [3] [4] [5]src/Makefile.am.Test coverage and documentation:
testcoverage.cpp/testcoverage.hppto include the new JWE-like encryption cases and their variants (jweenc,jweencoaepsha1,jweencoaepsha256). [1] [2] [3]CHANGELOG.md.