You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tooling.apache.org by as...@apache.org on 2026/04/22 00:41:37 UTC
(tooling-agents) branch main updated: ASVS L3 audit: 11.2.5 (vote_encryption_and_crypto)
This is an automated email from the ASF dual-hosted git repository.
asf-tooling pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tooling-agents.git
The following commit(s) were added to refs/heads/main by this push:
new a55bbd2 ASVS L3 audit: 11.2.5 (vote_encryption_and_crypto)
a55bbd2 is described below
commit a55bbd2817168c32cdc7fff40b81cc7f1bfcf322
Author: ASF Tooling Team <ro...@apache.org>
AuthorDate: Wed Apr 22 02:41:33 2026 +0200
ASVS L3 audit: 11.2.5 (vote_encryption_and_crypto)
---
.../d0aa7e9/vote_encryption_and_crypto/11.2.5.md | 346 +++++++++++++++++++++
1 file changed, 346 insertions(+)
diff --git a/repos/apache/ASVS/reports/steve/v3/d0aa7e9/vote_encryption_and_crypto/11.2.5.md b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/vote_encryption_and_crypto/11.2.5.md
new file mode 100644
index 0000000..110c914
--- /dev/null
+++ b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/vote_encryption_and_crypto/11.2.5.md
@@ -0,0 +1,346 @@
+# ASVS 11.2.5 Security Audit Report
+
+## Executive Summary
+
+- **Repository:** apache/steve
+- **Audit Date:** Apr 22, 2026
+- **Auditor:** Tooling Agents
+- **ASVS Requirement:** 11.2.5 — Verify that all cryptographic modules fail securely, and errors are handled in a way that does not enable vulnerabilities, such as Padding Oracle attacks.
+- **Level:** 3
+- **Section:** Secure Cryptography Implementation
+- **Section Description:** This section defines the requirements for the selection, implementation, and ongoing management of core cryptographic algorithms for an application. The objective is to ensure that only robust, industry-accepted cryptographic primitives are deployed, in alignment with current standards (e.g., NIST, ISO/IEC) and best practices.
+- **Files Analyzed:** 4 relevant out of 4 total, 0 skipped
+
+### Finding Counts
+
+| Severity | Count |
+|----------|-------|
+| Critical | 0 |
+| High | 0 |
+| Medium | 2 |
+| Low | 0 |
+| **Total** | **2** |
+
+---
+
+## Security Controls Inventory
+
+| Control | Defined In | Applied At | Coverage | Status |
+|---------|-----------|------------|----------|--------|
+| Fernet encrypt-then-MAC (prevents padding oracle) | `crypto.py:create_vote`, `decrypt_votestring` | Vote encryption/decryption | All vote encrypt/decrypt paths | ✅ GOOD |
+| Argon2 key derivation | `crypto.py:_hash` | `gen_opened_key`, `gen_vote_token` | All KDF paths | ✅ GOOD |
+| HKDF key stretching | `crypto.py:_b64_vote_key` | Vote key derivation | All vote key paths | ✅ GOOD |
+| CSPRNG salt generation | `crypto.py:gen_salt` | Election open, mayvote salts | All salt generation paths | ✅ GOOD |
+| Constant-time comparison | **NOT DEFINED** | `election.py:is_tampered` | Missing | ❌ GAP |
+| Crypto exception handling/wrapping | **NOT DEFINED** | `crypto.py` all functions, `election.py` callers | Missing | ❌ GAP |
+
+---
+
+## Critical File Review
+
+### `v3/steve/crypto.py`
+
+| Function | Error Handling | Fails Securely? | Notes |
+|----------|---------------|-----------------|-------|
+| `gen_salt()` | None | N/A (generation only) | Uses `secrets` - very unlikely to fail |
+| `gen_opened_key()` | None (propagates) | ⚠️ No | Argon2/BLAKE2b exceptions propagate raw |
+| `gen_vote_token()` | None (propagates) | ⚠️ No | Argon2 exceptions propagate raw |
+| `_b64_vote_key()` | None (propagates) | ⚠️ No | HKDF exceptions propagate raw |
+| `create_vote()` | None (propagates) | ⚠️ No | Fernet exceptions propagate raw |
+| `decrypt_votestring()` | None (propagates) | ⚠️ No | `InvalidToken` propagates raw |
+| `_hash()` | None (propagates) | ⚠️ No | Argon2 exceptions propagate raw |
+
+### `v3/steve/election.py`
+
+| Method | Calls Crypto | Handles Crypto Errors? | Notes |
+|--------|-------------|----------------------|-------|
+| `open()` | `gen_salt`, `gen_opened_key` | No | Propagates to caller |
+| `add_vote()` | `gen_vote_token`, `create_vote` | No | Propagates to web layer |
+| `tally_issue()` | `gen_vote_token`, `decrypt_votestring` | No | Propagates to tally caller |
+| `has_voted_upon()` | `gen_vote_token` | No | Propagates to web layer |
+| `is_tampered()` | `gen_opened_key` | No | Uses non-constant-time `!=` |
+
+---
+
+## Findings
+
+### Medium Severity
+
+#### ASVS-1125-MED-001: Non-Constant-Time Comparison of Cryptographic Hash in Tamper Detection
+
+**Severity:** MEDIUM
+**Gap Type:** Type A — Entry point with no control (constant-time comparison never used)
+**File:** `v3/steve/election.py`
+**Function:** `is_tampered()` (approximately line 329-342)
+**ASVS:** 11.2.5 — Cryptographic modules fail securely, errors handled without enabling vulnerabilities
+
+##### Vulnerable Code
+
+```python
+def is_tampered(self, pdb):
+ md = self._all_metadata() # no required state
+ edata = self.gather_election_data(pdb)
+ opened_key = crypto.gen_opened_key(edata, md.salt)
+
+ # The computed key should be unchanged.
+ return opened_key != md.opened_key # <-- Non-constant-time comparison
+```
+
+##### Data Flow
+
+1. **Source:** `md.opened_key` retrieved from database (stored 32-byte Argon2 hash)
+2. **Computed:** `opened_key` freshly derived via `gen_opened_key(edata, salt)` → BLAKE2b → Argon2d
+3. **Comparison:** Python `!=` on two `bytes` objects — CPython short-circuits on first differing byte
+4. **Missing control:** `hmac.compare_digest()` for constant-time comparison
+
+##### Related Functions Checked
+
+- `_all_metadata()` — Retrieves stored `opened_key` from database
+- `gather_election_data()` — Collects election data for hash computation
+- `crypto.gen_opened_key()` — Derives fresh key from election data
+
+##### Proof of Concept
+
+An attacker with database write access and network-level timing observation could:
+1. Modify election data (e.g., change an issue title by one character)
+2. Trigger tamper check (if exposed via a web endpoint or observe CLI timing)
+3. Measure response time — faster rejection means the first byte of the recomputed hash already differs
+4. Iteratively modify election data to match successive bytes of `opened_key`, eventually forging tamper-undetectable modifications
+
+While the Argon2 computation (~50ms) adds noise, the comparison timing difference (first byte mismatch ~10ns vs. full 32-byte comparison ~100ns) is detectable with statistical aggregation over repeated observations.
+
+##### Impact
+
+Timing side-channel in the anti-tamper mechanism. An attacker who can modify database records and observe response timing could craft election data modifications that pass tamper detection. This undermines the integrity guarantee of the `opened_key` scheme.
+
+##### Remediation
+
+```python
+import hmac
+
+def is_tampered(self, pdb):
+ md = self._all_metadata()
+ edata = self.gather_election_data(pdb)
+ opened_key = crypto.gen_opened_key(edata, md.salt)
+
+ # Use constant-time comparison to prevent timing side-channels
+ return not hmac.compare_digest(opened_key, md.opened_key)
+```
+
+Consider adding `hmac.compare_digest` as a utility in `crypto.py` for reuse:
+```python
+import hmac
+
+def secure_compare(a: bytes, b: bytes) -> bool:
+ """Constant-time comparison of two byte strings."""
+ return hmac.compare_digest(a, b)
+```
+
+---
+
+#### ASVS-1125-MED-002: Cryptographic Decryption Errors Propagate Without Secure Handling
+
+**Severity:** MEDIUM
+**Gap Type:** Type A — No error handling control exists for cryptographic failures
+**File:** `v3/steve/crypto.py` (line ~75), `v3/steve/election.py` (line ~290, ~250)
+**Functions:** `decrypt_votestring()`, `tally_issue()`, `add_vote()`
+**ASVS:** 11.2.5 — Cryptographic modules fail securely
+
+##### Vulnerable Code
+
+**crypto.py — No exception handling:**
+```python
+def decrypt_votestring(vote_token: bytes, salt: bytes, ciphertext: bytes) -> str:
+ "Decrypt CIPHERTEXT into a VOTESTRING."
+ b64key = _b64_vote_key(vote_token, salt)
+ f = cryptography.fernet.Fernet(b64key)
+ return f.decrypt(ciphertext).decode() # <-- InvalidToken propagates raw
+```
+
+**election.py — tally_issue, no handling around decryption:**
+```python
+# Inside tally_issue loop (~line 290):
+votestring = crypto.decrypt_votestring(
+ vote_token,
+ mayvote.salt,
+ row.ciphertext,
+)
+votes.append(votestring)
+```
+
+**election.py — add_vote, no handling around encryption:**
+```python
+def add_vote(self, pid: str, iid: str, votestring: str):
+ md = self._all_metadata(self.S_OPEN)
+ mayvote = self.q_get_mayvote.first_row(pid, iid)
+ vote_token = crypto.gen_vote_token(md.opened_key, pid, iid, mayvote.salt)
+ ciphertext = crypto.create_vote(vote_token, mayvote.salt, votestring)
+ # If create_vote or gen_vote_token fails, raw crypto exception reaches web layer
+```
+
+**tally.py — prints error details before failing:**
+```python
+except Exception as e:
+ print(f'Error tallying issue {issue.iid}: {e}')
+ raise # Fail hard
+```
+
+##### Data Flow
+
+1. **Source:** Ciphertext from database `vote.ciphertext` or vote_token computation
+2. **Sink:** `Fernet.decrypt()` / `Fernet.encrypt()` / `argon2.low_level.hash_secret_raw()`
+3. **Missing Control:** No try/except wrapping of cryptographic operations; no application-level crypto error class
+4. **Propagation:** Raw `cryptography.fernet.InvalidToken`, `argon2.exceptions.*`, or `ValueError` propagate through `election.py` to the transport layer (CLI stdout or web response)
+
+##### Related Functions Checked
+
+- `crypto.create_vote()` — Encryption function without error handling
+- `crypto._b64_vote_key()` — HKDF key derivation without error handling
+- `crypto.gen_vote_token()` — Argon2 hashing without error handling
+- `election.py:tally_issue()` — Decryption caller without error handling
+- `election.py:add_vote()` — Encryption caller without error handling
+
+##### Proof of Concept
+
+**Scenario 1 — Information leakage via stack trace:**
+If the web framework returns unhandled exceptions as 500 responses with tracebacks in development/misconfigured mode:
+```
+cryptography.fernet.InvalidToken
+ File "steve/crypto.py", line 77, in decrypt_votestring
+ return f.decrypt(ciphertext).decode()
+ File "steve/election.py", line 292, in tally_issue
+ votestring = crypto.decrypt_votestring(vote_token, mayvote.salt, row.ciphertext)
+```
+This confirms the encryption library, algorithm (Fernet), and internal function structure.
+
+**Scenario 2 — Single corrupted vote prevents all tallying:**
+If an attacker with database write access corrupts one `vote.ciphertext` value, the entire `tally_issue` call fails, and `tally.py` exits with code 1 after printing the error. No other issues are tallied.
+
+##### Impact
+
+- **Information disclosure:** Raw exception types and stack traces from cryptographic operations can confirm encryption library, algorithm choices, and internal architecture to an attacker
+- **Availability:** A single corrupted ciphertext prevents tallying of an entire election — no graceful degradation or skip-and-log capability
+- **Note:** Fernet's encrypt-then-MAC design inherently prevents padding oracle attacks (HMAC verified before decryption), so the classical padding oracle is not exploitable here. The finding is about the broader fail-secure principle.
+
+##### Remediation
+
+**1. Add a dedicated crypto error class in `crypto.py`:**
+```python
+class CryptoError(Exception):
+ """Raised when a cryptographic operation fails.
+
+ This wraps all internal crypto exceptions to prevent
+ leaking implementation details to callers.
+ """
+ pass
+
+def decrypt_votestring(vote_token: bytes, salt: bytes, ciphertext: bytes) -> str:
+ """Decrypt CIPHERTEXT into a VOTESTRING."""
+ try:
+ b64key = _b64_vote_key(vote_token, salt)
+ f = cryptography.fernet.Fernet(b64key)
+ return f.decrypt(ciphertext).decode()
+ except (cryptography.fernet.InvalidToken, Exception) as e:
+ # Log internally at DEBUG level, raise sanitized error
+ _LOGGER.debug('Decryption failed: %s', type(e).__name__)
+ raise CryptoError('Vote decryption failed') from None
+
+def create_vote(vote_token: bytes, salt: bytes, votestring: str) -> bytes:
+ """Encrypt VOTESTRING using the VOTE_TOKEN and SALT."""
+ try:
+ b64key = _b64_vote_key(vote_token, salt)
+ f = cryptography.fernet.Fernet(b64key)
+ return f.encrypt(votestring.encode())
+ except Exception as e:
+ _LOGGER.debug('Encryption failed: %s', type(e).__name__)
+ raise CryptoError('Vote encryption failed') from None
+```
+
+**2. Handle decryption failures gracefully in `tally_issue`:**
+```python
+try:
+ votestring = crypto.decrypt_votestring(
+ vote_token, mayvote.salt, row.ciphertext,
+ )
+ votes.append(votestring)
+except crypto.CryptoError:
+ _LOGGER.error(
+ 'Failed to decrypt vote for issue %s (vote_token hash: %s). '
+ 'Skipping corrupted vote.',
+ iid, hashlib.sha256(vote_token).hexdigest()[:8]
+ )
+ # Continue tallying other votes; flag issue for audit
+ continue
+```
+
+---
+
+## Positive Security Patterns
+
+| Pattern | Location | Assessment |
+|---------|----------|------------|
+| **Fernet encrypt-then-MAC** | `crypto.py:create_vote`, `decrypt_votestring` | ✅ HMAC verified before decryption — inherently prevents padding oracle attacks |
+| **Cryptographically secure random generation** | `crypto.py:gen_salt`, `create_id`, `shuffle` | ✅ Uses `secrets` module throughout — no use of `random` |
+| **Fisher-Yates shuffle with CSPRNG** | `crypto.py:shuffle` | ✅ Prevents vote ordering correlation after decryption |
+| **Per-voter unique salts** | `election.py:add_salts`, `schema.sql:mayvote.salt` | ✅ Each (person, issue) pair gets independent 16-byte salt |
+| **Argon2 with appropriate parameters** | `crypto.py:_hash` | ✅ time_cost=2, memory_cost=64MB, parallelism=4 — reasonable for server-side |
+| **HKDF for key stretching** | `crypto.py:_b64_vote_key` | ✅ Proper use of HKDF-SHA256 for deriving encryption keys from tokens |
+| **State machine enforcement** | `election.py` asserts | ✅ Crypto operations gated by election state (editable/open/closed) |
+| **Sensitive fields excluded from public API** | `election.py:get_metadata` | ✅ SALT and OPENED_KEY never returned in public methods |
+| **Schema-level validation** | `schema.sql` CHECK constraints | ✅ Salt lengths (16 bytes), key lengths (32 bytes), ID formats enforced at DB level |
+| **BLAKE2b pre-hashing** | `crypto.py:gen_opened_key` | ✅ Properly reduces variable-length election data before Argon2 |
+
+---
+
+## Architecture Observations
+
+### 1. HKDF Info Parameter Mismatch
+The `_b64_vote_key` function uses `info=b'xchacha20_key'` in HKDF, but the actual encryption algorithm is Fernet (AES-128-CBC + HMAC-SHA256). The comment confirms this is transitional: `"### still using Fernet now, but will switch soon."` During migration to XChaCha20-Poly1305, if the same HKDF parameters are retained, old Fernet keys and new XChaCha20 keys will be derived identically. **Recommendation:** When migrating, change the `info` parameter (e.g., `b'fernet_key'` now, `b'xchacha2 [...]
+
+### 2. Vote Token Input Concatenation Without Domain Separation
+`gen_vote_token` concatenates `opened_key + pid.encode() + iid.encode()` without delimiters or length-prefixing. While the fixed 10-character hex format of IID (enforced by schema CHECK constraint) prevents practical collisions, this deviates from cryptographic best practice. **Recommendation:** Use length-prefixed encoding or a structured serialization:
+```python
+data = opened_key + len(pid).to_bytes(2, 'big') + pid.encode() + iid.encode()
+```
+
+### 3. Argon2d vs. Argon2id
+The production `_hash` uses `Type.D` (Argon2d) while the benchmark function uses `Type.ID` (Argon2id). Argon2d has data-dependent memory access patterns, making it theoretically vulnerable to side-channel attacks in shared-hosting environments. Argon2id (the recommended variant per RFC 9106) provides hybrid protection. For server-side key derivation, this is low risk but worth aligning with current recommendations.
+
+---
+
+## Recommendations Summary
+
+### Immediate Actions (1-2 weeks)
+
+| Priority | Finding | Effort | Impact |
+|----------|---------|--------|--------|
+| **1** | Use `hmac.compare_digest()` for all cryptographic value comparisons (ASVS-1125-MED-001) | Low (1 line change + import) | Eliminates timing side-channel in tamper detection |
+| **2** | Add `CryptoError` wrapper class and try/except in `crypto.py` functions (ASVS-1125-MED-002) | Medium (new class + wrapping 4-5 functions) | Prevents implementation detail leakage; enables graceful degradation |
+
+### Short-term Actions (1-3 months)
+
+| Priority | Recommendation | Effort | Impact |
+|----------|---------------|--------|--------|
+| **3** | Handle individual vote decryption failures in `tally_issue` with logging and continue | Medium (error handling + audit logging) | Prevents single corrupted vote from blocking entire tally |
+| **4** | Update HKDF `info` parameter to match current algorithm before migration | Low (1 constant change) | Ensures proper domain separation during algorithm transition |
+
+### Long-term Actions (3-6 months)
+
+| Priority | Recommendation | Effort | Impact |
+|----------|---------------|--------|--------|
+| **5** | Consider migrating from Argon2d to Argon2id for alignment with RFC 9106 | Low (1 parameter change) | Reduces theoretical side-channel risk in shared environments |
+| **6** | Implement length-prefixed encoding in `gen_vote_token` input concatenation | Low (encoding function update) | Aligns with cryptographic best practices for domain separation |
+
+---
+
+## Appendix: Files Analyzed
+
+<details>
+<summary>Click to expand file list</summary>
+
+- `v3/schema.sql` (relevance: 10)
+- `v3/server/bin/tally.py` (relevance: 10)
+- `v3/steve/crypto.py` (relevance: 10)
+- `v3/steve/election.py` (relevance: 10)
+
+</details>
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org