Skip to content

Add configurable keytext byte-limit guard for /pks/add and document proxy limits - #143

Merged
bmarwell merged 7 commits into
mainfrom
copilot/fix-add-keytext-size-limit
May 25, 2026
Merged

Add configurable keytext byte-limit guard for /pks/add and document proxy limits#143
bmarwell merged 7 commits into
mainfrom
copilot/fix-add-keytext-size-limit

Conversation

Copilot AI commented May 25, 2026

Copy link
Copy Markdown
Contributor

POST /pks/add accepted unbounded keytext, allowing oversized payloads to be read and parsed fully in memory. This change adds an early byte-size guard before OpenPGP parsing and documents HTTP-layer limits for defense in depth.

  • Application-layer DoS guard in add command handler

    • Added a UTF-8 byte-length check in AddKeyToVerificationQueueCommandHandler#doExecute.
    • Rejects oversized submissions before Bouncy Castle parsing with:
      Key submission exceeds maximum allowed size.
    • Introduced reusable config key:
      • keyserver.pks.max-key-bytes
      • default: 131072 (128 KiB)
    • Added fallback to default when configured value is non-positive.
  • Config wiring

    • Added microprofile-config-api as provided dependency where needed.
    • Injected max-size via @ConfigProperty into the handler.
  • Focused tests for size-limit behavior

    • Added tests that verify:
      • oversized payloads are rejected by size guard,
      • payloads exactly at the limit are not rejected by the guard (and proceed to normal parse path).
  • Operational documentation (defense in depth)

    • Updated README.adoc with:
      • app-level limit property + env var mapping,
      • reverse-proxy examples for nginx and Caddy body-size limits.
int keyTextBytes = command.keyText().getBytes(StandardCharsets.UTF_8).length;
if (keyTextBytes > effectiveMaxKeyBytes()) {
    throw new KeyParsingException("Key submission exceeds maximum allowed size");
}

Copilot AI changed the title [WIP] Fix keytext size limit to guard against oversized upload DoS Add configurable keytext byte-limit guard for /pks/add and document proxy limits May 25, 2026
Copilot AI requested a review from bmarwell May 25, 2026 18:21
@codecov

codecov Bot commented May 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.15%. Comparing base (33cba77) to head (8382bb3).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
...ndler/AddKeyToVerificationQueueCommandHandler.java 71.42% 2 Missing and 4 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #143      +/-   ##
============================================
- Coverage     59.80%   52.15%   -7.65%     
- Complexity       58       69      +11     
============================================
  Files            16       21       +5     
  Lines           204      278      +74     
  Branches         22       32      +10     
============================================
+ Hits            122      145      +23     
- Misses           68      115      +47     
- Partials         14       18       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an application-layer byte-size guard for POST /pks/add key submissions (to reduce oversized-upload DoS risk) and documents complementary reverse-proxy request body limits.

Changes:

  • Enforces a configurable UTF-8 byte limit on keytext in AddKeyToVerificationQueueCommandHandler (default 128 KiB).
  • Wires MicroProfile Config (keyserver.pks.max-key-bytes) into the application-core module.
  • Adds unit tests for “over limit” and “exactly at limit” behavior; updates README with app + proxy limits.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
README.adoc Documents the new application-level limit and recommends proxy body-size limits (nginx/Caddy examples).
pom.xml Adds MicroProfile Config API to dependency management (provided scope).
application/application-core/pom.xml Declares MicroProfile Config API dependency for application-core.
application/application-core/src/main/java/.../AddKeyToVerificationQueueCommandHandler.java Implements the byte-size guard and config plumbing (with default + fallback).
application/application-core/src/test/java/.../AddKeyToVerificationQueueCommandHandlerTest.java Adds focused tests covering over-limit rejection and equal-to-limit behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 119 to +125
if (command.keyText() == null || command.keyText().isBlank()) {
throw new KeyParsingException("keytext must not be null or blank");
}
int keyTextBytes = command.keyText().getBytes(StandardCharsets.UTF_8).length;
if (keyTextBytes > effectiveMaxKeyBytes()) {
throw new KeyParsingException("Key submission exceeds maximum allowed size");
}
Comment on lines +109 to +110
@ConfigProperty(name = MAX_KEY_BYTES_CONFIG_KEY, defaultValue = "131072")
int maxKeyBytes = DEFAULT_MAX_KEY_BYTES;
Comment on lines +252 to +258
void setMaxKeyBytes(int maxKeyBytes) {
this.maxKeyBytes = maxKeyBytes;
}

private int effectiveMaxKeyBytes() {
return maxKeyBytes > 0 ? maxKeyBytes : DEFAULT_MAX_KEY_BYTES;
}
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comment on lines +123 to +128
int maxKeyBytes = effectiveMaxKeyBytes();
if (keyText.length() > maxKeyBytes) {
throw new KeyParsingException("Key submission exceeds maximum allowed size");
}
byte[] keyTextBytes = keyText.getBytes(StandardCharsets.UTF_8);
if (keyTextBytes.length > maxKeyBytes) {
Comment thread README.adoc
Comment on lines +94 to +112
For defense in depth, also enforce request/body limits at your reverse proxy.
Example snippets:

.nginx
[source,nginx]
----
server {
client_max_body_size 128k;
}
----

.Caddy
[source,caddyfile]
----
example.com {
request_body {
max_size 128KB
}
}
bmarwell and others added 2 commits May 25, 2026 22:26
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bmarwell
bmarwell marked this pull request as ready for review May 25, 2026 20:31
@bmarwell
bmarwell merged commit db30827 into main May 25, 2026
5 of 6 checks passed
@bmarwell
bmarwell deleted the copilot/fix-add-keytext-size-limit branch May 25, 2026 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: add keytext size limit to guard against oversized upload DoS

3 participants