Repository navigation
fix(security): enforce production HMAC signing key readiness #313
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
seonghobae
wants to merge
8
commits into
main
Choose a base branch
from
fix/production-tenant-claims-key-strength
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
2e1dc91
fix(security): rebuild production HMAC readiness on protected main
seonghobae 610f081
test(security): cover null tenant secret normalization
seonghobae ad306bb
Merge branch 'main' into fix/production-tenant-claims-key-strength
opencode-agent[bot] ff25234
test(security): cover effective HMAC secret boundaries
seonghobae 3c99f23
chore: reconcile production auth readiness with protected main
seonghobae ab088a5
test(security): require RFC 7518 HMAC-SHA-256 key size
seonghobae 93d07e4
fix(security): require 256-bit HMAC-SHA-256 keys
seonghobae b422e05
docs(doctoring): record HMAC-SHA-256 key-size authority
seonghobae File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| # Production HMAC-SHA-256 key readiness | ||
|
|
||
| ## Decision | ||
|
|
||
| Clearfolio production startup requires each effective tenant-claims and artifact-token HMAC-SHA-256 signing key to contain at least **32 UTF-8 bytes**. The two purposes must use byte-distinct keys. | ||
|
|
||
| The readiness check evaluates the same effective tenant-claims key that the verifier consumes after removing NUL characters and stripping surrounding Unicode whitespace. Production rejects a configured value that would change under that normalization, so operators cannot mistake the literal configured bytes for a different runtime key. | ||
|
|
||
| ## Standards basis | ||
|
|
||
| RFC 7518 section 3.2 specifies that an HMAC key used with SHA-256 must be at least the size of the hash output: 256 bits. Because Clearfolio uses `HmacSHA256` for tenant claims and artifact-token signatures, a 16-byte minimum is insufficient for the protocol contract even though it may provide a nominal 128-bit input length. | ||
|
|
||
| NIST SP 800-57 Part 1 Revision 5 provides the general key-management principle that a key should ordinarily be used for one purpose. Clearfolio therefore rejects purpose reuse between tenant-claim and artifact-token signing. Distinct configured values do not by themselves prove independent generation, entropy, KMS custody, rotation, or compromise recovery; those remain part of issue #319's credential-registry architecture. | ||
|
|
||
| NIST SP 800-224 remains an Initial Public Draft at this decision point and is not treated as the sole production authority. FIPS 198-1 remains relevant historical HMAC guidance while NIST progresses its announced migration of HMAC specifications. | ||
|
|
||
| ## Executable acceptance | ||
|
|
||
| The exact-head regression requires: | ||
|
|
||
| - former 16-byte tenant and artifact secrets to fail closed; | ||
| - effective UTF-8 byte length, rather than character count, to determine acceptance; | ||
| - exact 32-byte multibyte and ASCII keys to be accepted; | ||
| - null, blank, undersized, NUL-modified, or surrounding-whitespace-modified tenant authority to fail; | ||
| - missing, blank, or undersized artifact-token authority to fail; | ||
| - byte-identical purpose reuse to fail; | ||
| - development fallback behavior to remain outside the production readiness contract. | ||
|
|
||
| ## Scope boundary | ||
|
|
||
| This decision is a startup-strength and purpose-separation guard. It does not replace direct configuration injection with a provider-neutral credential registry, implement active/previous rotation windows, prove key entropy, or establish KMS-backed custody. Those controls remain owned by issue #319 and the ordered credential-registry PR stack. | ||
|
|
||
| ## References | ||
|
|
||
| Barker, E. (2020). *Recommendation for key management: Part 1—General* (NIST Special Publication 800-57 Part 1 Revision 5). National Institute of Standards and Technology. https://doi.org/10.6028/NIST.SP.800-57pt1r5 | ||
|
|
||
| Jones, M. (2015). *JSON Web Algorithms (JWA)* (RFC 7518). RFC Editor. https://doi.org/10.17487/RFC7518 | ||
|
|
||
| National Institute of Standards and Technology. (2008). *The Keyed-Hash Message Authentication Code (HMAC)* (FIPS PUB 198-1). https://doi.org/10.6028/NIST.FIPS.198-1 |
77 changes: 73 additions & 4 deletions
77
src/main/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfig.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,28 +1,97 @@ | ||
| package com.clearfolio.viewer.config; | ||
|
|
||
| import java.nio.charset.StandardCharsets; | ||
| import java.security.MessageDigest; | ||
|
|
||
| import org.springframework.beans.factory.annotation.Value; | ||
| import org.springframework.context.annotation.Configuration; | ||
| import org.springframework.context.annotation.Profile; | ||
| import org.springframework.util.StringUtils; | ||
|
|
||
| /** | ||
| * Fails closed when production profile is started without signed tenant claims. | ||
| * Fails closed when production starts without stable, adequately sized, purpose-separated HMAC | ||
| * signing material. | ||
| */ | ||
| @Configuration | ||
| @Profile("production") | ||
| public class ProductionAuthReadinessConfig { | ||
|
|
||
| private static final int MINIMUM_HMAC_KEY_BYTES = 32; | ||
|
|
||
| /** | ||
| * Verifies that production cannot start with unsigned tenant headers. | ||
| * Verifies that production cannot start with unsigned tenant headers, undersized HMAC keys, | ||
| * an ephemeral artifact-token signing key, ambiguous tenant-key normalization, or one key reused | ||
| * across the two signing purposes. | ||
| * | ||
| * <p>The tenant-claims runtime removes NUL characters and strips surrounding Unicode whitespace | ||
| * before using the configured HMAC secret. Readiness measures that effective key so padding | ||
| * cannot inflate the minimum length or hide purpose reuse. Production additionally rejects a | ||
| * configured tenant secret that would be changed by that runtime normalization. This prevents an | ||
| * operator or gateway from believing the literal configured secret is the signing key while the | ||
| * verifier silently uses different bytes. | ||
| * | ||
| * <p>RFC 7518 section 3.2 requires a key of the same size as the HMAC-SHA-256 output or larger; | ||
| * Clearfolio therefore requires at least 32 effective UTF-8 bytes for both signing purposes. Key | ||
| * length alone does not prove entropy, independent generation, approved custody, or rotation; | ||
| * production operators remain responsible for generating and protecting both secrets through a | ||
| * reviewed credential-management boundary. | ||
| * | ||
| * <p>NIST SP 800-57 Part 1 Revision 5 states that, in general, one key is used for one purpose. | ||
| * Clearfolio therefore rejects byte-identical tenant-claim and artifact-token signing keys so a | ||
| * compromise in one authority does not automatically expose the other authority's signing key. | ||
| * Distinct values are still not evidence of independent generation, custody, rotation, or KMS | ||
| * provenance; those remain operational controls. | ||
| * | ||
| * <p>{@code ArtifactLinkService} intentionally generates a process-local random key when no | ||
| * artifact-token secret is configured so development can run without external secret plumbing. | ||
| * Production must instead provide stable artifact-token signing material so issued links remain | ||
| * verifiable across process restart and replica changes. | ||
| * | ||
| * @param tenantClaimsSecret shared gateway signing secret | ||
| * @param artifactTokenSecret stable artifact-token signing secret | ||
| */ | ||
| public ProductionAuthReadinessConfig( | ||
| @Value("${clearfolio.tenant-claims.hmac-secret:}") String tenantClaimsSecret) { | ||
| if (!StringUtils.hasText(tenantClaimsSecret)) { | ||
| @Value("${clearfolio.tenant-claims.hmac-secret:}") String tenantClaimsSecret, | ||
| @Value("${clearfolio.artifact-token.secret:}") String artifactTokenSecret) { | ||
| String effectiveTenantClaimsSecret = effectiveTenantClaimsSecret(tenantClaimsSecret); | ||
| if (!StringUtils.hasText(effectiveTenantClaimsSecret)) { | ||
| throw new IllegalStateException( | ||
| "production profile requires clearfolio.tenant-claims.hmac-secret" | ||
| ); | ||
| } | ||
| byte[] tenantClaimsKey = effectiveTenantClaimsSecret.getBytes(StandardCharsets.UTF_8); | ||
| if (tenantClaimsKey.length < MINIMUM_HMAC_KEY_BYTES) { | ||
| throw new IllegalStateException( | ||
| "production profile requires clearfolio.tenant-claims.hmac-secret with at least 32 UTF-8 bytes" | ||
| ); | ||
| } | ||
| if (!StringUtils.hasText(artifactTokenSecret)) { | ||
| throw new IllegalStateException( | ||
| "production profile requires clearfolio.artifact-token.secret" | ||
| ); | ||
| } | ||
| byte[] artifactTokenKey = artifactTokenSecret.getBytes(StandardCharsets.UTF_8); | ||
| if (artifactTokenKey.length < MINIMUM_HMAC_KEY_BYTES) { | ||
| throw new IllegalStateException( | ||
| "production profile requires clearfolio.artifact-token.secret with at least 32 UTF-8 bytes" | ||
| ); | ||
| } | ||
| if (MessageDigest.isEqual(tenantClaimsKey, artifactTokenKey)) { | ||
| throw new IllegalStateException( | ||
| "production profile requires distinct tenant-claims and artifact-token HMAC secrets" | ||
| ); | ||
| } | ||
| if (!tenantClaimsSecret.equals(effectiveTenantClaimsSecret)) { | ||
| throw new IllegalStateException( | ||
| "production profile requires clearfolio.tenant-claims.hmac-secret without NUL or surrounding whitespace" | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| private static String effectiveTenantClaimsSecret(String configuredSecret) { | ||
| if (configuredSecret == null) { | ||
| return null; | ||
| } | ||
| return configuredSecret.replace("\u0000", "").strip(); | ||
| } | ||
| } | ||
149 changes: 147 additions & 2 deletions
149
src/test/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfigTest.java
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
비밀 조회를 Spring 환경 placeholder에서 분리하세요.
Line 53과 Line 54는 HMAC 비밀을
@ValueSpring 환경 placeholder에서 읽습니다. 이는 production 비밀의 허용된 소스가 아닙니다.ProductionAuthReadinessConfig,TenantAccessService,ArtifactLinkService가 동일한 KV 또는 credential registry에서 해석한 비밀을 사용하도록 마이그레이션하세요. 환경 변수는 해당 저장소의 bootstrap에만 사용하세요.As per coding guidelines, 새 비밀과 기존 artifact-token 및 tenant-claims 비밀은 Spring 환경 placeholder 대신 KV 또는 credential registry에서 읽어야 합니다.
🤖 Prompt for AI Agents
Source: Coding guidelines