fix: add subkey count cap in AddKeyToVerificationQueueCommandHandler

[bmarwell]

Problem

UIDs are capped at MAX_EMAIL_UIDS = 20, but subkeys are not capped. A key with tens of thousands of subkeys:

1. Is parsed in full by Bouncy Castle during POST /pks/add.
2. Triggers one stripToVerifiedUids() call per verified UID during publishVerifiedUid — each of which rebuilds the entire key ring including all subkeys.

This is a DoS vector independent of UID count.

Fix

After obtaining masterKey, count the subkeys and reject the submission if it exceeds a limit:

static final int MAX_SUBKEYS = 50;

int subkeyCount = 0;
for (Iterator<PGPPublicKey> it = keyRing.getPublicKeys(); it.hasNext(); it.next()) {
    if (++subkeyCount > MAX_SUBKEYS) {
        throw new KeyValidationException("Key has too many subkeys (limit: " + MAX_SUBKEYS + ")");
    }
}

Legitimate keys carry 1–5 subkeys. 50 is generous headroom.

Note

The existing MAX_EMAIL_UIDS pattern is the template for this.

[bmarwell]

Priority: 2 of 10 🔴

Fix immediately after #135. Together these two close the DoS guard trio (UIDs are already capped at 20).

The subkey threat is less obvious than payload size but actually more dangerous at scale: a key with 50 000 subkeys passes the UID cap, gets stored in the verification queue, and then causes one full stripToVerifiedUids() ring-rebuild per verified UID — potentially minutes of CPU per verification click.

Effort: ~10 lines. Risk of fix: near zero.

[bmarwell]

I dug through the current command-handler structure before touching the fix.

Today the execution path is very small:

• TransactionalCommandDispatcher chooses a handler
• AbstractKeyServerCommandHandler.execute(...) casts and directly calls doExecute(...)
• each concrete handler is responsible for doing all of its own validation inline

That keeps things simple, but it also means a command can forget a check unless the handler author remembers to add it.

Common patterns we could use

1. Template method in AbstractKeyServerCommandHandler

Add a mandatory pre-execution hook in the base class, e.g. execute() calls verify(...) before doExecute(...), and subclasses implement doVerify(...).

Pros:

• very small change to the current design
• hard to forget because every handler already goes through the base class
• keeps validation close to the command handler

Cons:

• if doVerify(...) parses heavy data (like an armored key), we need a way to pass the parsed result into doExecute(...) or we will parse twice
• can become a monolithic base-class API if different commands need very different preparation data

A stronger variant of this is: execute() calls prepare(...), and prepare(...) returns a typed verification/preparation result that doExecute(...) consumes.

2. Validator chain / strategy registry

Introduce something like CommandVerifier<T> beans and run them from the base handler or dispatcher before execution.

Pros:

• most reusable/composable option
• easy to share rules like size limit, subkey limit, token shape, etc.
• makes ordering explicit and keeps handlers smaller

Cons:

• more plumbing now: typed registry, ordering, passing shared prepared state
• probably overkill while we only have two command handlers

This is the best fit if we expect several commands to share reusable validation rules.

3. Dispatcher-level validation registry

Run verification in TransactionalCommandDispatcher before calling the handler.

Pros:

• strongest “cannot forget it” guarantee because execution is centrally gated
• keeps handlers focused on business actions

Cons:

• more indirection than we have today
• type matching becomes more complex
• shared parsed artifacts still need a transport object or request-scoped context

4. CDI interceptor / annotation-based validation

Use an interceptor or Bean Validation-style annotations.

Pros:

• good for generic structural rules
• very standard for null/format/basic constraints

Cons:

• poor fit for heavy PGP-specific checks that need Bouncy Castle parsing and shared prepared results
• likely too magical/indirect for the current size of the codebase

Recommended direction

For #136 itself, I would keep the fix small but move toward reuse:

1. Extract a focused key-submission verifier/preparer in application-core (for example KeySubmissionVerifier or KeyRingVerificationSupport).
2. Let it parse once and enforce the add-path checks in one place:
   • non-blank input
   • max UTF-8 byte size
   • master key not revoked
   • at least one verifiable email UID
   • max verifiable UID count
   • new: max subkey count
3. Return a prepared result object that AddKeyToVerificationQueueCommandHandler can consume without reparsing.

That gives us an immediately useful reusable verification utility and keeps #136 small.

For the broader “commands should not forget verification” goal, the most realistic next step would be either:

• a template-method hook in AbstractKeyServerCommandHandler, if we want to stay simple, or
• a validator chain/registry, if we expect several commands to accumulate reusable checks.

My current preference is:

• now: reusable key-submission verifier/preparer + subkey cap for fix: add subkey count cap in AddKeyToVerificationQueueCommandHandler #136
• later, if more commands need it: lift verification into a template-method hook or validator registry

That avoids overengineering the current codebase while still giving us a clean path to central enforcement.