Adding support of Pkcs11 v3.2 ml kem and migration to Eclipse - #83
Draft
Nicolas-Peiffer wants to merge 133 commits into
Draft
Adding support of Pkcs11 v3.2 ml kem and migration to Eclipse#83Nicolas-Peiffer wants to merge 133 commits into
Nicolas-Peiffer wants to merge 133 commits into
Conversation
…y "CLI flags > environment variables > configuration files > default". Improve logging. Update to go1.23.9 Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com> go mod tidy Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
… go1.24.5. Update and Improve Documentation. Add sample kubernetes KMS v2 manifest. Implement key rotation #52 Remove KMS v1 #43 add sequence diagram for KMS v2 add import kms/apis/v2 and start update to v2 add kek key id to NewP11 rename dek label reorganize switch case to put EncryptRequest after StatusResponse refactor StatusResponse refactor unaryinterceptor unaryinterceptor use StatusResponse unaryinterceptor use EncryptResponse refactor unaryinterceptor & Status refactor unaryinterceptor & Status refactor Status add key id in encryptresponse refactor UML & update SVG improve logrus for Status refactor UnaryInterceptor add return to status encrypt retrun CKA_ID update decrypt uniformise keyId type accross StatusResponse and EncryptResponse remove references to KMS v1 ; identify with comments istio related methods ; add usefull TODOs that need to be checked ; improve logging ; rename some objects improve logs of Encrypt remove return in UnaryInterceptor add logrus error to decrypt need to handle the KEK ID (CKA_ID) and key label (CKA_LABEL) better viper add support for cobra MarkFlagsMutuallyExclusive and MarkFlagsOneRequired modify title NewP11 handle retrieving the KEK ID by label or label by ID remove defaultKekId from the default value of the cobra flag for KEK ID fix eval for empty byte array initialize p11 context before FindKey remove annotations from EncryptResponse remove annotations from EncryptResponse remove default values for labels improve logging improve converting KeyId from byte array to string refactor keyId and conversion from string to byte array fix NewP11 label for RSA use crypto11 patch return error in findkey when key is nil eclipse-keypont/crypto11#122 user can use CKA_ID for the HMAC NewP11 better handle HMAC ID or Label version does not exist in KMSv2 separate istio related function in a dedicated file clean to converge toward KMS v2 testing BUT tests are not fully compatible with KMSv2 upgrade dependencies update doc with KMSv2 updates re-order the atttribute of the struct that caches values of the CLI create a branch for key rotation serve command: re-order the attribute of the struct that caches values of the CLI implement FindCkaAttrByIdOrLabel to simplify NewP11 and update crypto11 fix typo hmac add support for key rotation during decrypt for now key rotation is a sub command of serve fix viper init of subcommand fix grpc fir p11 default add rotation param do NewP11 support key roation for AES CBC hmac and improve Decrypt method for key rotation add timestamps to logrus update documentation for KMS v2 support rotation subcommand MarkFlagsMutuallyExclusive for labels and ids GoLint don't use Yoda conditions (ST1017) catch exception generateDEK catch exception for empty byte arrays separate unit tests from integration tests simplify string related content harmonize names add tests for NewP11 update ViperFlagsServe fix rebase error isKeyRotation for HMAC update main README add archlinux add TLDR and fix typo Improve docs Improve docs with svg figures add a README move user cli auto generated docs add a script to mimic k8s KMS APIserver add references to gose and crypto11 and github repo add docs for YubiHSM and Thales eToken fusion fetch the KMS v2 protobuf file test if file api.proto is already there update documentation remove KMS v1 config file update go version of goreleaser custom image fix an env var example in config file update to go 1.24.5 fix spelling mistake add verbose mode doc add a k3s doc DEPRECATED: nfpms.builds should not be used anymore check https://goreleaser.com/deprecations#nfpmsbuilds for more info update golang.org/x/sys & protobuf update gose and crypto11 with latest go 1.23.6 versions (#66) go mod tidy add full default path for k3s sqlite db rename and fix link to KMS manifest sample file update figure Update k8s-kms-plugin-deployment-scenario-examples.svg fix embeded images add Drawio source file Add emoji to show the hidden figure add figures for k8s x3 server nodes HA cluster add ref to figure for HA k8s cluster update puml before spliting it start spliting plantUML diagrams update indentation update puml rename puml add patch for key rotation remove old SVG add new SVG updage with JWE add key rotation figure examples Update k8s-kms-plugin-Key_Rot_1.svg Update k8s-kms-plugin-Key_Rot_2.svg Update k8s-kms-plugin-Key_Rot_3.svg add section about key rotation update package installation section update dependencies & update to kms v0.34.1 and be compatible with github.com/protocolbuffers/protobuf-go UnimplementedKeyManagementServiceServer Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
use corresponding go1.25.1 release tags for gose and crypto11 Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
…AESCBC, AlgRSAOAEP, AlgMLKEM — all named after the user-facing slugs Every routing case and if p.algorithm == now uses the sentinel, not a size-specific jose constant JWE internals (gose.NewAesCbcCryptor(..., jose.AlgA256CBC), pubJwk.SetAlg(jose.AlgRSAOAEP)) are untouched
…e with constants, pflag.Value methods, and validateAlgorithmFamily algFromString removed — initProvider just casts: jose.Alg(vprFlgsServe.AlgorithmFamily) --algorithm flag → --algorithm-family registered with Var (cobra validates at parse time) sanitizeViperFlagsServe validates config file / env var values; called from PersistentPreRunE gose import dropped
…family (now explicitly required) sanitizeViperFlagsRotation added; called from rotation's PersistentPreRunE Both algFromString calls replaced with direct casts
…thmFamily — renamed throughout p11.go (struct fields, struct literal in NewP11, and all 15 call sites) gose.NewAesCbcCryptor(..., p.algorithmFamily) fixed to jose.AlgA256CBC with a comment explaining why — the JWE spec only standardizes AES-CBC at 256-bit (no 128/192 variants), whereas AES-GCM has all three sizes auto-detected from CKA_VALUE_LEN
Adds section 3.3 covering the no-clone install path, and renumbers the build sections below it (3.3.x -> 3.4.x, plus the goreleaser and container-image sections) along with every cross-reference. Two traps are called out because both produce a silently wrong result rather than an error: `@latest` skips pre-releases and so installs the two-year-old v0.6.0 until v1.0.0 is tagged, and `go install` cannot pass the Makefile LDFLAGS, so the binary reports an empty snapshot version while being fully functional. Also notes the goenv GOROOT mismatch that makes the compiler refuse to build.
Documents, per --algorithm-family, what the plugin does to the 32-byte DEK seed the apiserver sends it: which HSM keys it needs, which primitives it composes, what lands in EncryptResponse.ciphertext, and which operations run inside the HSM. The ML-KEM section covers the part that is not obvious from the code: a KEM takes no plaintext, so the scheme is necessarily hybrid, and the 1568-byte FIPS 203 ciphertext cannot fit the KMS v2 1 kB ciphertext cap and therefore travels in an annotation — including why that is safe to publish in etcd and why substituting it fails cleanly. Each family section carries a collapsible EncryptResponse captured with scripts/grpcurl/collect-jwe-samples.sh, and the rsa-oaep section adds measured sizes: RSA-4096 sits at 92% of the 1 kB cap. Also indexes the page in docs/README.md as a new "Cryptographic reference" section, links it from the main README's family list, and drops the stale TPM background PDF row from the HSM guide table. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The three sequence diagrams predated ML-KEM and described a JWE-only plugin. Encrypt and Decrypt now branch on --algorithm-family: the classical JWE path, and the ML-KEM path (Encaps -> KMAC KDF -> AES-GCM seal, with the KEM ciphertext in an annotation and the 60-byte binary envelope in ciphertext). Fixes a wrong payload label: the key-rotation diagram described EncryptRequest.Plaintext as a "base64 encoded JWE" — it is the raw 32-byte DEK seed in every family. Retargets the PKCS #11 reference from v3.0 to v3.2, states that KeyId is the hex-encoded CKA_ID, and adds notes on annotation-independent dispatch and on which flags apply to the old KEK during rotation. Adds a Mermaid envelope overview to the README architecture section — short enough that an inline diagram GitHub renders beats a committed SVG — and records in docs/README.md which format is used for what, plus how to regenerate the PlantUML SVGs. Footers also pick up the Eclipse rebrand, since they are baked into the regenerated SVGs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
projects.eclipse.org/projects/technology.keysealer spells the display name with a capital S. Aligns the prose in README.md and CHANGELOG.md and the remaining PlantUML footer, which also moves from "Thales Group for Thales Open Source" to the Eclipse Foundation and from 2025 to 2026. Identifiers are deliberately untouched: the eclipse-keysealer org and Go module path, the technology.keysealer project URL, and the *.keysealer.eclipse.org annotation keys, which KMS v2 requires to be RFC 1123 lowercase. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Also documents why decryptWithContext passes an explicit crypto.SHA256 rather than crypto.Hash(0), which would derive the digest from the "alg" header. This plugin has only ever wrapped the CEK with SHA-256, so SHA-256 is correct for every object it can encounter — including those written before gose v1.0.0-rc2, whose headers say "RSA-OAEP" (SHA-1 per RFC 7518) while the CEK is SHA-256-wrapped. Deriving from the header would fail on exactly those, and a KMS plugin must never lose the ability to read its own data at rest.
Moves the PKCS#11 stack forward as one: pkcs11-go v1.0.0 -> v1.1.0-rc1,
crypto11/v2 v2.0.0-rc3 -> rc4, gose v1.0.0-rc2 -> rc3. The three form a
chain — crypto11 rc4 requires pkcs11-go v1.1.0-rc1, gose rc3 requires
crypto11 rc4 — so they cannot move independently. gose rc3 is a
dependency-only release: nothing but go.mod and go.sum differs from rc2.
No plugin change is needed. Verified with build, vet, the unit suite and
the full integration suite against SoftHSMv3 (pqctoday-hsm), where every
case ran rather than skipping.
Two upstream changes were checked for impact and have none:
- gose v1.0.0 removed its ML-KEM support, draft-ietf-jose-pqc-kem-06
having dropped JOSE from its scope and been retitled "PQ KEMs for
COSE". The ml-kem family reaches crypto11 directly
(FindMLKEMKeyPair, Encapsulate, MLKEMDeriveKey) and never used the
gose path.
- crypto11 renamed FindKeyRSAPairsWithAttributes to
FindRSAKeyPairsWithAttributes; the plugin does not call it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The KEM ciphertext travels in the kem-ciphertext annotation, which KMS v2 stores in plaintext in etcd and explicitly does not protect from tampering. It is now bound into the AES-GCM tag as additional authenticated data — "k8s-kms-plugin/ml-kem/v1" || c — so an envelope opens only under the KEM ciphertext it was sealed with. This is defence in depth, not a fix. Substituting c already failed closed: ML-KEM.Decaps returns a different shared secret for a different ciphertext (FIPS 203 uses implicit rejection rather than signalling an error), which derives a different key and fails the tag check. But that binding holds only because the KEM ciphertext happens to be the one annotation feeding the KDF. Any annotation added later that influences decryption — a KDF version, a second KEM for a hybrid mode — would not be covered unless it were mixed into the derivation too. The AAD makes the binding a property of the format instead, and gives those future fields somewhere to go. key_id is deliberately left out. It is already bound the same way, since a wrong key_id resolves a different decapsulation key and so yields a different secret, and including it would make decryption depend on the apiserver returning the string byte-identically for no gain. The version suffix in the context string domain-separates the format: bump it on any change to the binary layout, the KDF or the AAD's field set, and a mismatched reader gets a clean authentication failure rather than a silent misinterpretation. TestMlkemAAD_BindsEnvelopeToKemCiphertext isolates the binding against a fixed AES-GCM key, which is the only way to vary c while holding the derived key constant — no real KEM allows that. TestMLKEM_TamperedKemCiphertextAnnotation covers the end-to-end behaviour, flipping one bit of the annotation and asserting the decrypt fails while the untouched response still succeeds; it cannot attribute the failure to the AAD specifically, which is why both exist. BREAKING CHANGE: ml-kem envelopes sealed before this change carry no AAD and will not decrypt afterwards. Only release candidates are affected — ml-kem has never shipped in a stable release — so no supported data at rest needs re-encrypting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
collect-jwe-samples.sh had two reporting bugs, both visible in the generated summary.md. The alg column showed the PKCS#11 key label for ml-kem cases — "dev-ml-kem-1024" where every other row holds a JWE alg value — because ALG was reused to carry the key label into messages.md. ml-kem produces no JWE and so has no alg header; the column now shows an em dash, and the messages.md line that printed the label a second time is gone, the line above it already naming the key. Annotation sizes were measured on grpcurl's base64 rendering rather than the protobuf bytes the KMS v2 32 kB limit applies to, inflating every value by ~4/3: ML-KEM-1024 read 2204 B (6% of the limit) instead of 1678 B (5%). The decoded length is now recovered arithmetically, three bytes per four base64 characters less one per '=' of padding. The obvious alternative, @base64d | length, is also wrong: jq measures the result in codepoints, so a non-UTF-8 value like the raw ML-KEM ciphertext is mangled and under-reported at 1604 B. The size bug over-reported, so no published figure was ever understated. Re-running against SoftHSMv3 leaves every ciphertext measurement byte-identical; only the two corrected fields move. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Refreshes the Cryptographic Schemes page against a full
collect-jwe-samples.sh run on SoftHSMv3, and documents the AAD the
ml-kem envelope now carries.
The RSA-OAEP size table was captured before the RSA-OAEP-256 header
landed and carried a provisional note estimating the shift. It is now
measured — 607 / 777 / 948 B at 59 / 76 / 93% of the 1 kB limit — with
wrapped-CEK and on-the-wire columns added, the data showing that all
three rows share exactly 265 B of JWE framing and only the modulus grows.
RSA-4096 headroom is ~76 B, not ~80 B.
New §7.6 covers what binds the annotation to the envelope: the two
independent mechanisms, why the AAD is defence in depth rather than a
fix, and which of the two tests can attribute a failure to it.
§7.4 gains the encoding-cost comparison, the strongest argument for the
split after the 1 kB limit itself: annotations carry protobuf bytes, so
the KEM ciphertext travels raw at 1.000x, while a cryptogram in the JWE
Encrypted Key segment is base64url-inflated to ~1.333x. It also records
that there is no standards-track way to express ML-KEM in JOSE at all,
draft-ietf-jose-pqc-kem-06 having dropped JOSE from scope — a more
fundamental reason for the binary envelope than size.
The annotation footprint is corrected to the figures the apiserver
validates: 878 / 1198 / 1678 B raw, where a grpcurl capture of the same
annotations reads 2204 B because it renders them as base64.
All four sample captures are refreshed from one run. The ml-kem block
previously paired a ciphertext from one encryption with the
kem-ciphertext annotation from another — two halves of different
responses shown as one, when those are precisely the fields the AAD
binds. The elision label now states what its number is ("N chars total,
middle elided"), three of the samples having used the total where a
fourth used the elided count.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
…d ml-kem cbc-class, gcm-class and rsa-class still described the pre-rotation Service/P11 shape (kid/cid, k8sDefaultDekLabel/k8sHmacKeyLabel, algorithm) and, for RSA, a local AsymmetricDecryptionKeyImpl that gose v1.0.0-rc2 replaced with gose/hsm.AsymmetricDecryptionKey. Bring them in line with the current Provider interface and P11 struct (kekCkaID/kekCkaLabel, hmacCkaID/ hmacCkaLabel, algorithmFamily, KEK rotation fields), and add the ml-kem-class diagram that was missing entirely. Switch these class diagrams from PNG to SVG, matching the convention already used for the kmsv2-*.sqce-diag diagrams, and embed them inline in cryptographic-schemes.md next to each algorithm family instead of leaving them unreferenced in puml-diagrams/.
Add a minimal ANSI theme (help_theme.go) that colorizes only --help/-h section headers (Usage:, Flags:, Available Commands:, ...) via a custom cobra usage template, gated on NO_COLOR/CLICOLOR_FORCE and TTY detection. Command names, flag lines, and Short/Long text are left untouched, so shell completion (reads .Short/.Long verbatim) and `k8s-kms-plugin docs` (renders Long/Short/flags directly via doc.GenMarkdownTree, bypassing the usage template) are both unaffected.
…ference Replace the orphaned inputs_cka_id_cka_label.puml activity diagram — never linked from any doc, and misleading in treating --p11-key-id/--p11-key-label as symmetric alternatives — with docs/cli-user-interface/cka-id-vs-cka-label.md: a hand-written page (not touched by `make doc`) with a Mermaid flowchart that makes the actual asymmetry explicit: CKA_ID is the canonical identifier KMS v2 requires, CKA_LABEL is a human-friendly lookup that always resolves to one. Link it from serve/rotation --help text, docs/README.md, and every HSM quick-start guide next to their --p11-key-id/--p11-key-label examples. Regenerate docs/cli-user-interface/ via `make doc`, which also drops a handful of flags (--allow-any, --host, --port, --tls-*, --enable-server, --disable-socket) already removed from the code but still listed in the committed docs from a stale prior generation.
The nonce/sealed split in decryptMLKEMWithContext was interleaved with HSM calls, so the envelope layout could not be exercised without a token even though the parsing itself touches no key material. Pull it out as parseMLKEMEnvelope, with formatMLKEMEnvelope as its inverse on the encrypt side, and use both at the existing call sites. parseMLKEMEnvelope is the first thing the ML-KEM path does with bytes an attacker can reach, before anything has authenticated them, which is exactly the code worth being able to test in isolation. Behaviour is unchanged: same minimum-length check, same error text. Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
--old-p11-key-label and --old-p11-hmac-label went straight to FindCkaAttrByIDOrLabel with no length check, unlike their active-KEK counterparts which route through GetKeyIDAndLabel and so pick up validateCkaLabel. An over-long label was handed to the token as-is, leaving it to truncate, reject obscurely, or accept a CKA_LABEL the plugin considers invalid everywhere else. Add integration tests driving all eight CKA_ID/CKA_LABEL flags one byte past their PKCS#11 bound against a live token, so the two fixed paths are covered alongside the six that were already correct. The tests assert on the specific length error rather than merely on failure. That distinction matters: NewP11 resolves the active KEK before it looks at the HMAC or rotation parameters, so without a real key provisioned every case fails at KEK lookup and never reaches the validation under test. Each case therefore provisions the keys the earlier lookups need, and the at-maximum cases assert the error is *not* a length complaint, which is what keeps the bound from silently drifting off by one. Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
The API server rejects any KeyId over KeyIDMaxSize (1 kB), so a CKA_ID that cannot fit takes encryption at rest down entirely rather than degrading it — and it does so silently from the plugin's side: the plugin starts, reports healthy, and the only diagnostic is on the API server, which refuses every Status response. Operator-supplied CKA_IDs were already safe, since maxCkaIDHexLen (510 hex characters) is roughly half the KMS v2 bound. The gap was the CKA_ID this plugin reads back from the token when started with --p11-key-label: PKCS#11 places no limit on CKA_ID length, so a key provisioned by other tooling can carry an ID that is perfectly valid for the HSM and still too long for KMS v2. Guard that path in GetKeyIDAndLabel, and SetKekKeyIDFromBytes, which took raw bytes with only a nil check. Restate the bound as maxKMSv2KeyIDSize rather than importing KeyIDMaxSize. It is exported, but it lives in k8s.io/apiserver — the API server's internal storage-encryption machinery, not the plugin-facing contract; k8s.io/kms, the module this plugin depends on, does not define it. Importing it would invert the dependency, add ~60 modules and ~350 packages to a vendored tree for one integer, and pin the plugin to one API server version when it must serve whatever version the cluster runs. Add a compile-time assertion that the PKCS#11 bound stays the stricter of the two, so widening maxCkaIDHexLen past the KMS v2 limit fails the build instead of producing rejected Status responses at runtime. Note in both the assertion and the validator that the comparison is only sound because the KeyId string is ASCII hex: 1024 bytes of KeyId is 1024 hex characters, encoding a 512-byte CKA_ID, and halving the wrong side doubles the bound. Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
maxCiphertextSize was 64 kB, built on the premise that this plugin encrypts the Kubernetes object. It does not. KMS v2 does envelope encryption on the API server side: the API server encrypts the object with a local DEK and sends this plugin only that key, a 32-byte seed (aestransformer.MinSeedSizeExtendedNonceGCM). Its own error message names the field for what it is — "encrypted DEK source". api.proto caps EncryptResponse.ciphertext at 1 kB, enforced by the API server's ValidateEncryptedObject, and DecryptRequest.Ciphertext is that same value handed back, so nothing larger could ever have reached etcd. Measured JWEs for a 32-byte seed are 147 bytes at a typical 8-char kid and 816 bytes at the longest legal one, so the old guard was 64x looser than the protocol allows. Rename it maxKMSv2CiphertextSize now that the value actually is the KMS v2 limit. maxPlaintextSize was 8 kB — above the ciphertext cap, which is incoherent: encryption only grows the payload. The plugin would have accepted an 8 kB plaintext, encrypted it into an ~11 kB JWE, had the API server reject it, and then refused that same ciphertext on Decrypt via its own interceptor, having produced it itself. It is now 4x the real DEK seed (128 bytes), expressed against a named kmsv2DEKSeedSize so the relationship is visible, leaving slack should a future API server send a longer seed. api.proto sets no limit on this field, so it keeps a plain name rather than a maxKMSv2 prefix. Capping the plaintext is necessary but not sufficient. The JWE protected header carries a kid — the CKA_LABEL on the AES-GCM path, the hex CKA_ID on AES-CBC — so ciphertext size grows with the operator's key naming, not just with the payload: 689 bytes of plaintext fit under an 8-char kid but only 187 under a 510-char one. Encrypt therefore also checks the finished ciphertext, on both the JWE and ML-KEM paths, which covers every configuration without modelling that relationship in a constant and names the key identifier length as the cause. Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
api.proto caps EncryptResponse.annotations at 32 kB, and the API server's validateAnnotations enforces it with two properties that are easy to miss: the budget is shared across all annotations rather than applying per annotation, and it counts keys as well as values (totalSize += len(k) + len(v)). Keys matter here because the API server requires a fully qualified domain name, so this plugin's are ~50 bytes each. Add maxKMSv2AnnotationsSize with annotationsTotalSize and validateEncryptResponseAnnotations mirroring that rule, and check it on both encrypt paths before returning. Nothing is close to the limit today: the ML-KEM path is the only one with substantial annotation content, and at ML-KEM-1024 its KEM ciphertext brings the total to 1678 of 32768 bytes. The check exists so that stays true — annotations are the natural place to add per-object metadata, and the next one added spends the same shared budget. Annotation key format is left to the API server: the keys are package-level constants that already satisfy the FQDN requirement, and nothing an operator or a request can influence, so a check here would only ever assert against a compile-time value. Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
The limits had accumulated three different conventions — Size, Len, and a
mix of protocol-derived and self-imposed bounds sharing the same shape — so
reading one told you neither where it came from nor what it counted. Settle
on a single rule, documented above the const block:
maxKMSv2<Field>Size a limit KMS v2 imposes; always BYTES on the wire
max<Attr>Size a PKCS#11 attribute limit in BYTES
max<Attr>HexLen a PKCS#11 attribute limit in HEX CHARACTERS
maxPlaintextSize this plugin's own guard; no maxKMSv2 prefix, because
api.proto sets no such limit
Rename maxCkaLabelLen to maxCkaLabelSize accordingly, and state the unit
explicitly on every constant, validator and error message.
The hex/raw distinction is the one that bites. A CKA_ID is raw bytes on the
token but travels as a hex string through the CLI and through every KMS v2
KeyId field, where it occupies twice the space. maxKMSv2KeyIDSize is
therefore 1024 BYTES of KeyId string — 1024 hex characters, encoding only a
512-byte CKA_ID. Treating it as raw bytes would admit a KeyId twice the legal
size; treating maxCkaIDHexLen as raw bytes would reject valid CKA_IDs at half
the bound. The compile-time assertion now carries a warning not to "helpfully"
restate either side in raw bytes, since that silently doubles what it
enforces.
Add TestKeyIDHexEncodingDoublesSize, which pins the 2x relationship against
the real accessor, and TestLimitUnitsAreSelfConsistent, which checks the
bounds stay even and that PKCS#11 remains the stricter one both as KeyId
string sizes and after conversion to raw bytes — the comparison that
silently inverts when only one side is converted.
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Add 10 fuzz targets across pkg/providers and cmd, covering the JWE compact parser, the CKA_ID and CKA_LABEL validators, the ML-KEM AAD encoding and envelope split, the --algorithm-family flag parser, and the viper config merge. These state properties rather than examples, which is where they earn their place next to the existing table tests. mlkemAAD injectivity is the clearest case: the doc comment warns that the encoding is unambiguous only because the KEM ciphertext is the last unprefixed field, and a fuzz target can assert that as "equal AADs iff equal inputs" where no table of examples can. Bounding only a 32-byte prefix of the KEM ciphertext — a plausible "keep the AAD small" change, and a real vulnerability, since it would let a substituted annotation open the envelope — passes the whole existing suite and is caught in under a second. Config parsing is the other payoff. UnmarshalSubMergedE hand-rolls viper's priority chain, so it meets shapes viper.Sub() never produces: a section that is a scalar, deeply nested maps, type mismatches. It reached 227 coverage-interesting inputs in 60 seconds without panicking. The CLI flags are not a security boundary — whoever passes them already has root on the node — but they cross into cgo via the PKCS#11 stack, where a value slipping past validation fails as a segfault in the vendor library rather than a Go panic. FuzzAlgorithmFamilySet asserts Set is a gate, not a filter: it either rejects the input or stores it verbatim, never coercing a near-miss. No CI change is needed — go test ./... already replays every seed corpus as an ordinary unit test, so the targets are exercised on each push. make fuzz additionally runs the mutation engine, looping because go test fuzzes one target in one package per invocation. A failing input lands in <pkg>/testdata/fuzz/<target>/ and, once committed, becomes a permanent regression seed. Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
p11.go had grown to 1600 lines covering three separable concerns. Move
two of them out, leaving the provider itself in p11.go:
kmsv2_limits.go the KMS v2 / PKCS#11 size constants, the compile-time
CKA_ID-vs-KeyId assertion, and the validate* helpers.
Pure functions with no dependency on P11 state; the
test layout already had them as kmsv2_limits_test.go.
p11_mlkem.go the ML-KEM vertical slice: envelope format, the KEM
ciphertext annotation channel, the shared secret
template, encryptMLKEM and decryptMLKEMWithContext.
Pure relocation: no logic, naming, or signature changes. Only the import
blocks differ, plus AlgorithmFamilyAnnotationKey staying behind in its
own const block since it is not ML-KEM specific.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
golangci-lint had not been run over the test files added by the two
previous commits. Four findings, all mechanical except the last:
revive/unused-parameter rename t to _ in the three closures that never
report through it. The later cases in the
input-limits table do use t, so they are
unchanged.
unused drop newRealHMACKey. Its doc claimed the
AES-CBC cases needed a provisioned HMAC key,
but every HMAC length validator in NewP11 runs
before the matching HMAC lookup, so an
over-long HMAC identifier is rejected without
the key being resolved. No case ever called it.
Record why no HMAC counterpart is needed on newRealKEK, so the apparent
gap is not filled back in later.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Nicolas-Peiffer <102670102+Nicolas-Peiffer@users.noreply.github.com>
Nicolas-Peiffer
force-pushed
the
pkcs11-v3.2-ml-kem
branch
from
August 7, 2026 10:15
a84bb75 to
7c19927
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Target v1