Skip to content

Commit ba6cddd

Browse files
Athroniaethclaude
andcommitted
fix(security): replace bcrypt pepper concatenation with HMAC-SHA256 pre-hashing
Bcrypt silently truncates input at 72 bytes. Concatenating key_secret (64 chars) with a pepper could push the combined string past that limit, causing the tail characters to be ignored — two distinct secrets could produce the same hash. Fix: _apply_pepper() now computes HMAC-SHA256(pepper, secret), producing a fixed 32-byte digest that is always well within bcrypt's input limit. The pepper remains cryptographically bound to the secret as the HMAC key. This is a breaking change: hashes stored under the old concatenation scheme will not verify with the new implementation; affected keys must be re-issued. Also updates README, docs/index.md, and docs/schema.mermaid to document the NIST SP 800-132 compliance entry, the OWASP API2:2023 bullet (missing from docs/index.md), the STATE_CHECK node (missing from the docs/index.md diagram), and the NOTE_ARGON annotation to cover both Argon2 and Bcrypt strategies. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent 39b0804 commit ba6cddd

File tree

6 files changed

+70
-38
lines changed

6 files changed

+70
-38
lines changed

README.md

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,13 @@ This library try to follow best practices and relevant RFCs for API key manageme
3737
- **[OWASP API2:2023](https://owasp.org/API-Security/editions/2023/en/0xa2-broken-authentication/)**: Hash
3838
verification is performed before status/scope checks to prevent key-state enumeration — a caller with
3939
a wrong secret always receives `401 Invalid`, regardless of whether the key is inactive or expired.
40+
- **[NIST SP 800-132](https://csrc.nist.gov/publications/detail/sp/800-132/final)**: The Bcrypt hasher
41+
pre-hashes the secret via `HMAC-SHA256(pepper, secret)` before passing it to bcrypt, producing a fixed
42+
32-byte digest that eliminates bcrypt's silent 72-byte input truncation.
43+
- **Input validation (defense-in-depth)**: Scope strings are validated against a strict allowlist
44+
pattern (`^[a-z][a-z0-9:_\-]*$`) at the schema level. Note: [RFC 6749 §3.3](https://datatracker.ietf.org/doc/html/rfc6749#section-3.3)
45+
permits a broader character set; this restriction is a deliberate keyshield design choice to
46+
prevent injection of arbitrary characters (HTML, SQL fragments, etc.) in scope values.
4047

4148
## Installation
4249

@@ -257,9 +264,10 @@ flowchart LR
257264
• invalid if api key updated or deleted
258265
• invalid after 3600s`"]:::noteStyle
259266
260-
NOTE_ARGON["`**ArgonApiKeyHasher**
261-
• line salt apikey
262-
• global pepper api key`"]:::noteStyle
267+
NOTE_ARGON["`**Hashing strategy**
268+
Argon2: concat(secret, pepper)
269+
Bcrypt: HMAC-SHA256(pepper, secret)
270+
→ fixed 32 bytes, no 72-byte truncation`"]:::noteStyle
263271
264272
NOTE_SLEEP["`**sleep random (0.1s – 0.5s)**
265273
makes brute force less effective;

docs/index.md

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ This library try to follow best practices and relevant RFCs for API key manageme
3434
valid but inactive/expired keys
3535
- **[RFC 6750](https://datatracker.ietf.org/doc/html/rfc6750)**: Supports `Authorization: Bearer <api_key>` header for
3636
key transmission (also supports deprecated `X-API-Key` header and `api_key` query param)
37+
- **[OWASP API2:2023](https://owasp.org/API-Security/editions/2023/en/0xa2-broken-authentication/)**: Hash
38+
verification is performed before status/scope checks to prevent key-state enumeration — a caller with
39+
a wrong secret always receives `401 Invalid`, regardless of whether the key is inactive or expired.
40+
- **[NIST SP 800-132](https://csrc.nist.gov/publications/detail/sp/800-132/final)**: The Bcrypt hasher
41+
pre-hashes the secret via `HMAC-SHA256(pepper, secret)` before passing it to bcrypt, producing a fixed
42+
32-byte digest that eliminates bcrypt's silent 72-byte input truncation.
3743

3844
## How API Keys Work
3945

@@ -103,6 +109,10 @@ flowchart LR
103109
COMPARE{"`**Compare db api key hash
104110
to received api key hash**`"}:::processNode
105111
112+
STATE_CHECK{"`**Check state & scopes**
113+
_(active? not expired?
114+
required scopes?)_`"}:::processNode
115+
106116
%% ── Outcomes ────────────────────────────────────────────
107117
REJECT(["`🔴 **Reject API Key**`"]):::rejectNode
108118
ACCEPT(["`🟢 **Accept API Key**`"]):::acceptNode
@@ -119,7 +129,8 @@ flowchart LR
119129
120130
%% ── Accept path ─────────────────────────────────────────
121131
CACHED -- "yes" --> ACCEPT
122-
COMPARE -- "equals" --> ACCEPT
132+
COMPARE -- "equals" --> STATE_CHECK
133+
STATE_CHECK -- "valid" --> ACCEPT
123134
ACCEPT -- "`**APIKey.touch()**
124135
_(update last_used_at)_`" --> CACHE_STORE
125136
@@ -130,6 +141,7 @@ flowchart LR
130141
PREFIX_CHECK -- "no" --> REJECT
131142
QUERY_DB -- "not found" --> REJECT
132143
COMPARE -- "not equals" --> REJECT
144+
STATE_CHECK -- "invalid (403)" --> REJECT
133145
134146
%% ── Notes (annotations) ─────────────────────────────────
135147
NOTE_FORMAT["`**Format API Key**
@@ -147,9 +159,10 @@ flowchart LR
147159
• invalid if api key updated or deleted
148160
• invalid after 3600s`"]:::noteStyle
149161
150-
NOTE_ARGON["`**ArgonApiKeyHasher**
151-
• line salt apikey
152-
• global pepper api key`"]:::noteStyle
162+
NOTE_ARGON["`**Hashing strategy**
163+
Argon2: concat(secret, pepper)
164+
Bcrypt: HMAC-SHA256(pepper, secret)
165+
→ fixed 32 bytes, no 72-byte truncation`"]:::noteStyle
153166
154167
NOTE_SLEEP["`**sleep random (0.1s – 0.5s)**
155168
makes brute force less effective;

docs/schema.mermaid

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,10 @@ flowchart LR
8787
• invalid if api key updated or deleted
8888
• invalid after 3600s`"]:::noteStyle
8989

90-
NOTE_ARGON["`**ArgonApiKeyHasher**
91-
• line salt apikey
92-
• global pepper api key`"]:::noteStyle
90+
NOTE_ARGON["`**Hashing strategy**
91+
Argon2: concat(secret, pepper)
92+
Bcrypt: HMAC-SHA256(pepper, secret)
93+
→ fixed 32 bytes, no 72-byte truncation`"]:::noteStyle
9394

9495
NOTE_SLEEP["`**sleep random (0.1s – 0.5s)**
9596
makes brute force less effective;

src/keyshield/_schemas.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,16 @@
1414
) from e
1515

1616
from datetime import datetime, timedelta
17-
from typing import List, Optional
17+
from typing import Annotated, List, Optional
1818

1919
from keyshield.domain.entities import ApiKey
2020
from keyshield.repositories.base import ApiKeyFilter, SortableColumn
2121
from keyshield.utils import datetime_factory
2222

23+
# Scope strings must follow the format: lowercase letter, then lowercase
24+
# alphanumerics, colons, underscores, or hyphens (e.g. "read", "api:write").
25+
_ScopeStr = Annotated[str, Field(pattern=r"^[a-z][a-z0-9:_\-]*$")]
26+
2327

2428
class ApiKeyCreateIn(BaseModel):
2529
"""Payload to create a new API key.
@@ -35,7 +39,7 @@ class ApiKeyCreateIn(BaseModel):
3539
name: str = Field(..., min_length=1, max_length=128)
3640
description: Optional[str] = Field(None, max_length=1024)
3741
is_active: bool = Field(default=True)
38-
scopes: List[str] = Field(default_factory=list)
42+
scopes: List[_ScopeStr] = Field(default_factory=list)
3943
expires_at: Optional[datetime] = Field(
4044
default=None,
4145
examples=[(datetime_factory() + timedelta(days=30)).isoformat()],
@@ -58,7 +62,7 @@ class ApiKeyUpdateIn(BaseModel):
5862
name: Optional[str] = Field(None, min_length=1, max_length=128)
5963
description: Optional[str] = Field(None, max_length=1024)
6064
is_active: Optional[bool] = None
61-
scopes: Optional[List[str]] = None
65+
scopes: Optional[List[_ScopeStr]] = None
6266
expires_at: Optional[datetime] = Field(None, description="New expiration datetime (ISO 8601)")
6367
clear_expires: bool = Field(False, description="Remove expiration date")
6468

@@ -120,8 +124,8 @@ class ApiKeySearchIn(BaseModel):
120124
last_used_before: Optional[datetime] = Field(None, description="Keys last used before this date")
121125
last_used_after: Optional[datetime] = Field(None, description="Keys last used after this date")
122126
never_used: Optional[bool] = Field(None, description="True = never used keys, False = used keys")
123-
scopes_contain_all: Optional[List[str]] = Field(None, description="Keys must have ALL these scopes")
124-
scopes_contain_any: Optional[List[str]] = Field(None, description="Keys must have at least ONE of these scopes")
127+
scopes_contain_all: Optional[List[_ScopeStr]] = Field(None, description="Keys must have ALL these scopes")
128+
scopes_contain_any: Optional[List[_ScopeStr]] = Field(None, description="Keys must have at least ONE of these scopes")
125129
name_contains: Optional[str] = Field(None, description="Name contains this substring (case-insensitive)")
126130
name_exact: Optional[str] = Field(None, description="Exact name match")
127131
order_by: SortableColumn = Field(SortableColumn.CREATED_AT, description="Field to sort by")

src/keyshield/hasher/bcrypt.py

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import hashlib
2+
import hmac
13
from typing import Optional
24

35
try:
@@ -25,16 +27,15 @@ def __init__(
2527
super().__init__(pepper=pepper)
2628
self._rounds = rounds
2729

28-
def _apply_pepper(self, api_key: str) -> str:
29-
return f"{api_key}{self._pepper}"
30+
def _apply_pepper(self, api_key: str) -> bytes:
31+
# HMAC-SHA256 with the pepper as key produces a fixed 32-byte digest,
32+
# well within bcrypt's 72-byte input limit. This avoids the silent
33+
# truncation that occurs when key_secret + pepper exceeds 72 bytes.
34+
return hmac.digest(self._pepper.encode("utf-8"), api_key.encode("utf-8"), hashlib.sha256)
3035

3136
def hash(self, key_secret: str) -> str:
32-
salted_key = self._apply_pepper(key_secret).encode("utf-8")
33-
# Avoid exception : ValueError: password cannot be longer than 72 bytes, truncate manually if necessary (e.g. my_password[:72])
34-
hashed = bcrypt.hashpw(salted_key[:72], bcrypt.gensalt(self._rounds))
37+
hashed = bcrypt.hashpw(self._apply_pepper(key_secret), bcrypt.gensalt(self._rounds))
3538
return hashed.decode("utf-8")
3639

3740
def verify(self, key_hash: str, key_secret: str) -> bool:
38-
# Ensure that verify truncates the supplied key to 72 bytes like hash()
39-
salted_key = self._apply_pepper(key_secret).encode("utf-8")[:72]
40-
return bcrypt.checkpw(salted_key, key_hash.encode("utf-8"))
41+
return bcrypt.checkpw(self._apply_pepper(key_secret), key_hash.encode("utf-8"))

tests/unit/test_hasher.py

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -145,57 +145,62 @@ class TestBcryptHasher:
145145

146146
@patch("keyshield.hasher.bcrypt.bcrypt")
147147
def test_hash_applies_pepper(self, mock_bcrypt):
148-
"""hash() passes peppered key to bcrypt.hashpw."""
148+
"""hash() passes HMAC-SHA256(pepper, key) digest to bcrypt.hashpw."""
149+
import hashlib
150+
import hmac
151+
149152
mock_bcrypt.gensalt.return_value = b"$2b$04$salt"
150153
mock_bcrypt.hashpw.return_value = b"hashed-value"
151154

152155
hasher = BcryptApiKeyHasher(pepper="my-pepper", rounds=4)
153156
result = hasher.hash("api-key")
154157

155-
# Check pepper was applied
158+
expected_digest = hmac.digest(b"my-pepper", b"api-key", hashlib.sha256)
156159
call_args = mock_bcrypt.hashpw.call_args[0]
157-
assert call_args[0] == b"api-keymy-pepper"
160+
assert call_args[0] == expected_digest
158161
assert result == "hashed-value"
159162

160163
@patch("keyshield.hasher.bcrypt.bcrypt")
161-
def test_hash_truncates_long_keys(self, mock_bcrypt):
162-
"""hash() truncates keys longer than 72 bytes."""
164+
def test_hash_digest_fixed_size(self, mock_bcrypt):
165+
"""hash() always passes a fixed 32-byte digest to bcrypt regardless of key length."""
163166
mock_bcrypt.gensalt.return_value = b"$2b$04$salt"
164167
mock_bcrypt.hashpw.return_value = b"hashed"
165168

166169
hasher = BcryptApiKeyHasher(pepper="pepper", rounds=4)
167-
long_key = "a" * 100
168170

169-
hasher.hash(long_key)
171+
hasher.hash("a" * 100)
170172

171173
call_args = mock_bcrypt.hashpw.call_args[0]
172-
assert len(call_args[0]) == 72
174+
assert len(call_args[0]) == 32
173175

174176
@patch("keyshield.hasher.bcrypt.bcrypt")
175177
def test_verify_applies_pepper(self, mock_bcrypt):
176-
"""verify() passes peppered key to bcrypt.checkpw."""
178+
"""verify() passes HMAC-SHA256(pepper, key) digest to bcrypt.checkpw."""
179+
import hashlib
180+
import hmac
181+
177182
mock_bcrypt.checkpw.return_value = True
178183

179184
hasher = BcryptApiKeyHasher(pepper="my-pepper", rounds=4)
180185
result = hasher.verify("stored-hash", "api-key")
181186

187+
expected_digest = hmac.digest(b"my-pepper", b"api-key", hashlib.sha256)
182188
call_args = mock_bcrypt.checkpw.call_args[0]
183-
assert call_args[0] == b"api-keymy-pepper"
189+
assert call_args[0] == expected_digest
184190
assert call_args[1] == b"stored-hash"
185191
assert result is True
186192

187193
@patch("keyshield.hasher.bcrypt.bcrypt")
188-
def test_verify_truncates_long_keys(self, mock_bcrypt):
189-
"""verify() truncates keys longer than 72 bytes."""
194+
def test_verify_digest_fixed_size(self, mock_bcrypt):
195+
"""verify() always passes a fixed 32-byte digest to bcrypt regardless of key length."""
190196
mock_bcrypt.checkpw.return_value = True
191197

192198
hasher = BcryptApiKeyHasher(pepper="pepper", rounds=4)
193-
long_key = "a" * 100
194199

195-
hasher.verify("hash", long_key)
200+
hasher.verify("hash", "a" * 100)
196201

197202
call_args = mock_bcrypt.checkpw.call_args[0]
198-
assert len(call_args[0]) == 72
203+
assert len(call_args[0]) == 32
199204

200205
def test_rounds_too_low_raises(self):
201206
"""Rounds below 4 raises ValueError."""

0 commit comments

Comments
 (0)