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 01:29:49 UTC

(tooling-agents) branch main updated: ASVS L3 audit: 11.6.1 (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 a581301  ASVS L3 audit: 11.6.1 (vote_encryption_and_crypto)
a581301 is described below

commit a581301ecba0c23edfca75a2265aedd7f432bd18
Author: ASF Tooling Team <ro...@apache.org>
AuthorDate: Wed Apr 22 03:29:44 2026 +0200

    ASVS L3 audit: 11.6.1 (vote_encryption_and_crypto)
---
 .../d0aa7e9/vote_encryption_and_crypto/11.6.1.md   | 341 +++++++++++++++++++++
 1 file changed, 341 insertions(+)

diff --git a/repos/apache/ASVS/reports/steve/v3/d0aa7e9/vote_encryption_and_crypto/11.6.1.md b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/vote_encryption_and_crypto/11.6.1.md
new file mode 100644
index 0000000..6adf631
--- /dev/null
+++ b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/vote_encryption_and_crypto/11.6.1.md
@@ -0,0 +1,341 @@
+# ASVS 11.6.1 Security Audit Report
+
+## Executive Summary
+
+**Repository:** apache/steve  
+**Audit Date:** Apr 22, 2026  
+**Auditor:** Tooling Agents  
+**ASVS Requirement:** 11.6.1 — Verify that only approved cryptographic algorithms and modes of operation are used for key generation and seeding, and digital signature generation and verification. Key generation algorithms must not generate insecure keys vulnerable to known attacks, for example, RSA keys which are vulnerable to Fermat factorization.
+
+**Files Analyzed:** 4 relevant out of 4 total, 0 skipped
+
+**Finding Summary:**
+
+| Severity | Count |
+|----------|-------|
+| Critical | 0 |
+| High | 0 |
+| Medium | 3 |
+| Low | 0 |
+| **Total** | **3** |
+
+The audit evaluated the cryptographic subsystem of the Apache STeVe election system against ASVS 11.6.1, focusing on approved cryptographic algorithms and modes for key generation, seeding, and verification. The system demonstrates strong cryptographic design overall, but three findings of MEDIUM severity were identified relating to algorithm variant selection, key derivation domain separation, and cryptographic comparison implementation.
+
+---
+
+## Security Controls Inventory
+
+| Control | Defined In | Coverage | Status |
+|---------|------------|----------|--------|
+| Salt Generation (CSPRNG) | `crypto.py:gen_salt()` | `election.py:open()`, `election.py:add_salts()` | ✅ Applied at all entry points |
+| Argon2 Key Derivation | `crypto.py:_hash()` | `gen_opened_key()`, `gen_vote_token()` | ⚠️ Wrong variant (Type.D) |
+| BLAKE2b Pre-hashing | `crypto.py:gen_opened_key()` | Election data fingerprinting | ✅ Correctly applied |
+| HKDF Key Stretching | `crypto.py:_b64_vote_key()` | `create_vote()`, `decrypt_votestring()` | ⚠️ Mismatched domain label |
+| Fernet Encryption (AES-128-CBC+HMAC) | `crypto.py:create_vote()` | `election.py:add_vote()` | ✅ Correctly applied |
+| Fernet Decryption | `crypto.py:decrypt_votestring()` | `election.py:tally_issue()` | ✅ Correctly applied |
+| CSPRNG ID Generation | `crypto.py:create_id()` | `election.py:create()`, `election.py:add_issue()` | ✅ Applied consistently |
+| Cryptographic Shuffle | `crypto.py:shuffle()` | `election.py:tally_issue()` | ✅ Applied before results |
+| Tamper Detection | `election.py:is_tampered()` | `tally.py:main()` | ⚠️ Non-constant-time comparison |
+
+---
+
+## Critical File Review
+
+### `v3/steve/crypto.py` — Central Cryptographic Module
+
+| Function | Algorithm | Approved? | Notes |
+|----------|-----------|-----------|-------|
+| `gen_salt()` | `secrets.token_bytes(16)` | ✅ Yes | CSPRNG, 128-bit salt |
+| `gen_opened_key()` | BLAKE2b → Argon2d | ⚠️ Argon2d | Should be Argon2id |
+| `gen_vote_token()` | Argon2d | ⚠️ Argon2d | Should be Argon2id |
+| `_b64_vote_key()` | HKDF-SHA256 | ✅ Yes | Domain label incorrect |
+| `create_vote()` | Fernet (AES-128-CBC + HMAC-SHA256) | ✅ Yes | Authenticated encryption |
+| `decrypt_votestring()` | Fernet (AES-128-CBC + HMAC-SHA256) | ✅ Yes | HMAC verified before decrypt |
+| `_hash()` | Argon2d | ⚠️ Argon2d | RFC 9106 recommends Argon2id |
+| `shuffle()` | Fisher-Yates + `secrets.randbelow()` | ✅ Yes | Cryptographically secure |
+| `create_id()` | `secrets.token_hex(5)` | ✅ Yes | 40-bit entropy, adequate for IDs |
+
+### `v3/steve/election.py` — Cryptographic Integration
+
+| Operation | Crypto Control Called? | Result Checked? | Timing |
+|-----------|------------------------|-----------------|--------|
+| `open()` | ✅ `gen_salt()`, `gen_opened_key()` | ✅ Stored in DB | ✅ Before state transition |
+| `add_salts()` | ✅ `gen_salt()` per voter | ✅ Written to DB | ✅ Before election opens |
+| `add_vote()` | ✅ `gen_vote_token()`, `create_vote()` | ✅ Ciphertext stored | ✅ After state check |
+| `tally_issue()` | ✅ `gen_vote_token()`, `decrypt_votestring()`, `shuffle()` | ✅ Results returned | ✅ After state check |
+| `is_tampered()` | ✅ `gen_opened_key()` | ⚠️ Non-constant comparison | ✅ Before tally |
+
+---
+
+## Findings
+
+### Medium Severity
+
+#### ASVS-1161-MED-001: Argon2d Variant Used Instead of Recommended Argon2id for Key Derivation
+
+**Severity:** MEDIUM  
+**Finding ID:** ASVS-1161-MED-001  
+**Gap Type:** Type B — Correct variant is known (used in benchmark) but NOT applied in production code
+
+**Location:**
+- **File:** `v3/steve/crypto.py`
+- **Function:** `_hash()` (lines 88–98)
+- **Related Functions Checked:** `gen_opened_key()`, `gen_vote_token()`
+
+**Description:**
+
+The system uses Argon2d (data-dependent variant) for all key derivation operations instead of the recommended Argon2id (hybrid variant). Argon2d uses data-dependent memory access patterns, making it vulnerable to side-channel attacks such as cache-timing and memory-access pattern analysis. RFC 9106 Section 4 explicitly recommends Argon2id for general-purpose use: *"If you do not know the difference between them or you consider side-channel attacks as a viable threat, choose Argon2id."*
+
+**Vulnerable Code:**
+```python
+def _hash(data: bytes, salt: bytes) -> bytes:
+    "Use Argon2 to hash the data, with default tuning parameters."
+
+    return argon2.low_level.hash_secret_raw(
+        secret=data,
+        salt=salt,
+        time_cost=2,
+        memory_cost=65536,
+        parallelism=4,
+        hash_len=32,
+        type=argon2.low_level.Type.D,  # <-- Argon2d: side-channel vulnerable
+    )
+```
+
+**Benchmark code uses the correct variant (line ~148):**
+```python
+type=argon2.low_level.Type.ID,  # <-- Argon2id: recommended variant
+```
+
+**Data Flow:**
+- `election.py:open()` → `crypto.gen_opened_key()` → `_hash(digest, salt)` → Argon2**d** → `opened_key` stored in DB
+- `election.py:add_vote()` → `crypto.gen_vote_token()` → `_hash(opened_key + pid + iid, salt)` → Argon2**d** → `vote_token` used to derive encryption key
+
+**Proof of Concept:**
+
+In a shared hosting or VM environment, an attacker on the same physical host could use cache-timing analysis tools (e.g., Flush+Reload) to observe Argon2d's data-dependent memory access patterns during `gen_vote_token()` execution, potentially recovering the vote token and thus the vote encryption key.
+
+**Impact:**
+
+In multi-tenant server environments, a co-located attacker could potentially extract key material through memory access timing. This directly impacts both the election integrity key (`opened_key`) and per-voter encryption key derivation (`vote_token`), which protect ballot secrecy.
+
+**Remediation:**
+```python
+def _hash(data: bytes, salt: bytes) -> bytes:
+    "Use Argon2id to hash the data, with default tuning parameters."
+
+    return argon2.low_level.hash_secret_raw(
+        secret=data,
+        salt=salt,
+        time_cost=2,
+        memory_cost=65536,
+        parallelism=4,
+        hash_len=32,
+        type=argon2.low_level.Type.ID,  # Argon2id: hybrid, side-channel resistant
+    )
+```
+
+> **Note:** Changing the variant will produce different output for the same input. Any existing elections that have been opened will have their `opened_key` invalidated, triggering tamper detection. This change requires a migration strategy for any active elections.
+
+---
+
+#### ASVS-1161-MED-002: HKDF Domain Separation Label Misrepresents Actual Encryption Algorithm
+
+**Severity:** MEDIUM  
+**Finding ID:** ASVS-1161-MED-002  
+**Gap Type:** Type A — No correct domain separation for the actual algorithm in use
+
+**Location:**
+- **File:** `v3/steve/crypto.py`
+- **Function:** `_b64_vote_key()` (lines 59–69)
+- **Related Functions Checked:** `create_vote()`, `decrypt_votestring()`
+
+**Description:**
+
+The HKDF key derivation function uses `info=b'xchacha20_key'` as the domain separation label, but the derived key is actually used with Fernet (AES-128-CBC + HMAC-SHA256), not XChaCha20-Poly1305. The HKDF `info` parameter provides cryptographic domain separation per NIST SP 800-56C. This mismatch violates proper domain separation and creates risk during algorithm transitions.
+
+**Vulnerable Code:**
+```python
+def _b64_vote_key(vote_token: bytes, salt: bytes) -> bytes:
+    "Key-stretch the vote_token. (ref: PBKDF)"
+
+    ### still using Fernet now, but will switch soon. Leaving comments.
+    keymaker = hkdf.HKDF(
+        algorithm=hashes.SHA256(),
+        length=32,  # 32-byte key for XChaCha20-Poly1305
+        salt=salt,
+        info=b'xchacha20_key',  # <-- Label for algorithm NOT in use
+    )
+    vote_key = keymaker.derive(vote_token)
+    return base64.urlsafe_b64encode(vote_key)
+```
+
+**Actual usage (lines 72–77, 80–85):**
+```python
+f = cryptography.fernet.Fernet(b64key)  # AES-128-CBC + HMAC-SHA256
+```
+
+**Data Flow:**
+- `vote_token` + `salt` → HKDF(SHA-256, info=`xchacha20_key`) → 32-byte key → base64 → **Fernet** (AES-128-CBC + HMAC-SHA256)
+
+**Proof of Concept:**
+
+During algorithm transition, if a developer adds XChaCha20-Poly1305 encryption using the existing `_b64_vote_key()` function (since its label already says `xchacha20_key`), the same key material would encrypt votes under both AES-128-CBC (old votes) and XChaCha20-Poly1305 (new votes). An attacker with access to ciphertexts under both algorithms using the same key could exploit algorithmic differences to recover plaintext.
+
+**Impact:**
+
+When XChaCha20-Poly1305 is added (as indicated by the comment "will switch soon"), if the same HKDF parameters are used, identical key material would be derived for both algorithms. This creates a **key reuse vulnerability across different ciphers** — a well-documented cryptographic failure mode that can leak plaintext. The incorrect label also makes cryptographic inventory auditing unreliable, as the documented algorithm doesn't match the actual implementation.
+
+**Remediation:**
+```python
+def _b64_vote_key(vote_token: bytes, salt: bytes) -> bytes:
+    "Key-stretch the vote_token into a Fernet-compatible key."
+
+    keymaker = hkdf.HKDF(
+        algorithm=hashes.SHA256(),
+        length=32,  # Fernet: 16-byte signing key + 16-byte AES-128 key
+        salt=salt,
+        info=b'fernet_vote_key_v1',  # Correct domain label for actual algorithm
+    )
+    vote_key = keymaker.derive(vote_token)
+    return base64.urlsafe_b64encode(vote_key)
+```
+
+> **Note:** As with Finding 001, changing the `info` parameter changes the derived key. This requires coordinated migration of any existing encrypted votes.
+
+---
+
+#### ASVS-1161-MED-003: Non-Constant-Time Comparison of Cryptographic Key Material in Tamper Detection
+
+**Severity:** MEDIUM  
+**Finding ID:** ASVS-1161-MED-003  
+**Gap Type:** Type B — `hmac.compare_digest()` is available in stdlib but NOT used for key comparison
+
+**Location:**
+- **File:** `v3/steve/election.py`
+- **Function:** `is_tampered()` (lines 359–377)
+- **Related Functions Checked:** Entry points in `tally.py:main()` (line ~158)
+
+**Description:**
+
+The tamper detection function uses Python's `!=` operator to compare cryptographic key material (`opened_key`). This operator performs byte-by-byte comparison and returns early on the first mismatch, creating a timing side-channel vulnerability. For a 32-byte key, this creates a measurable timing differential depending on how many leading bytes match.
+
+**Vulnerable Code:**
+```python
+def is_tampered(self, pdb):
+    md = self._all_metadata()
+    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
+```
+
+**Entry Points Where Called:**
+- `tally.py:main()` (line ~158): Called before every tally operation
+- Potentially called from web endpoints during election state transitions
+
+**Data Flow:**
+- `gather_election_data(pdb)` → `crypto.gen_opened_key()` → 32-byte `opened_key` → Python `!=` operator (short-circuits on first byte difference) → boolean result
+
+**Proof of Concept:**
+
+An authenticated administrator making repeated modifications to election data (e.g., voter list) while measuring the time `is_tampered()` takes to respond could observe timing variations correlating with how many bytes of the recomputed key match the stored key. While direct key recovery is impractical due to Argon2's avalanche properties, the principle of defense in depth requires constant-time comparison for all cryptographic material.
+
+**Impact:**
+
+While the Argon2 output is pseudorandom (making targeted byte-by-byte recovery impractical through input manipulation alone), this violates cryptographic implementation best practices. In scenarios where an attacker can repeatedly trigger tamper checks with controlled election data modifications and measure server response times (e.g., through a web-facing endpoint), information about the stored `opened_key` could leak.
+
+**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)
+```
+
+---
+
+## Positive Security Patterns
+
+| Pattern | Location | Assessment |
+|---------|----------|------------|
+| **CSPRNG for all random values** | `crypto.py:gen_salt()`, `create_id()`, `shuffle()` — all use `secrets` module | ✅ Excellent — no use of `random` module |
+| **Fernet authenticated encryption** | `crypto.py:create_vote()`, `decrypt_votestring()` | ✅ Provides AES-128-CBC + HMAC-SHA256 (encrypt-then-MAC) |
+| **Per-voter unique salts** | `election.py:add_salts()` — individual `crypto.gen_salt()` per mayvote row | ✅ Prevents vote correlation across voters |
+| **Per-election salts** | `election.py:open()` — `crypto.gen_salt()` for election-level key | ✅ Election isolation |
+| **BLAKE2b pre-hashing** | `crypto.py:gen_opened_key()` — reduces variable-length input to fixed size | ✅ Correct use of fast, secure hash before KDF |
+| **HKDF for key stretching** | `crypto.py:_b64_vote_key()` — proper KDF between Argon2 token and encryption key | ✅ Follows NIST SP 800-56C |
+| **Fisher-Yates with CSPRNG** | `crypto.py:shuffle()` — uses `secrets.randbelow()` | ✅ Cryptographically unpredictable vote ordering |
+| **Salt length validation** | `schema.sql` — `CHECK (length(salt) = 16)` on both election and mayvote | ✅ Database-level enforcement |
+| **Key length validation** | `schema.sql` — `CHECK (length(opened_key) = 32)` | ✅ Database-level enforcement |
+| **Sensitive field filtering** | `election.py:get_metadata()` — excludes salt and opened_key from public API | ✅ Data minimization |
+| **State machine enforcement** | `election.py` — assert checks + `_all_metadata(required_state)` before crypto ops | ✅ Defense in depth |
+| **Tamper detection before tally** | `tally.py:main()` — calls `is_tampered()` before `tally_election()` | ✅ Integrity verified before decryption |
+| **Transaction atomicity for salt generation** | `election.py:add_salts()` — `BEGIN TRANSACTION` / `COMMIT` | ✅ Prevents partial salt assignment |
+| **Argon2 parameters** | `time_cost=2, memory_cost=65536, parallelism=4` | ✅ Meets OWASP minimum recommendations |
+
+---
+
+## Architecture Observations
+
+1. **Layered Key Derivation:** The system uses a well-designed multi-layer key hierarchy: election salt → opened_key (Argon2) → vote_token (Argon2 with per-voter salt) → vote_key (HKDF) → Fernet encryption. Each layer provides different security properties (election isolation, voter anonymity, key stretching).
+
+2. **Crypto Centralization:** All cryptographic operations are properly centralized in `crypto.py`, making audit tractable. No raw cryptographic primitives are used in `election.py` — only the `crypto` module's API.
+
+3. **Algorithm Transition Risk:** The codebase shows clear signs of a planned transition from Fernet to XChaCha20-Poly1305 (comments, HKDF info label). This transition needs careful management to avoid key reuse and ensure backward compatibility for existing encrypted votes.
+
+4. **No Digital Signatures:** The system uses Argon2 hashing rather than digital signatures for tamper detection. This is acknowledged as intentional in the false positive patterns, and provides adequate integrity checking for the state machine model.
+
+---
+
+## Recommendations Summary
+
+### Immediate (Critical Path)
+
+| Priority | Finding | Effort | Impact |
+|----------|---------|--------|--------|
+| 1 | **ASVS-1161-MED-001:** Change `Type.D` to `Type.ID` in `_hash()` | Low (one-line change + migration) | Eliminates side-channel vulnerability in all key derivation |
+| 2 | **ASVS-1161-MED-003:** Replace `!=` with `hmac.compare_digest()` in `is_tampered()` | Low (one-line change) | Eliminates timing side channel, no migration needed |
+
+### Short-term (Next Sprint)
+
+| Priority | Finding | Effort | Impact |
+|----------|---------|--------|--------|
+| 3 | **ASVS-1161-MED-002:** Correct HKDF `info` parameter to match actual algorithm | Low (one-line change + migration) | Prevents key reuse during algorithm transition |
+
+### Long-term (Architectural)
+
+| Priority | Recommendation | Effort | Impact |
+|----------|---------------|--------|--------|
+| 4 | **Pre-transition planning:** Before switching to XChaCha20-Poly1305, define versioned key derivation with distinct `info` labels per algorithm | Medium | Prevents cryptographic key reuse across algorithm boundary |
+
+---
+
+## Appendix: Files Analyzed
+
+<details>
+<summary>Click to expand file list (4 files)</summary>
+
+1. **`v3/schema.sql`** (relevance: 10)
+   - Database schema with cryptographic field constraints
+   - Salt and key length validation
+   
+2. **`v3/server/bin/tally.py`** (relevance: 10)
+   - Election tallying entry point
+   - Calls tamper detection before decryption
+
+3. **`v3/steve/crypto.py`** (relevance: 10)
+   - Central cryptographic module
+   - All key derivation and encryption functions
+
+4. **`v3/steve/election.py`** (relevance: 10)
+   - Election state machine
+   - Cryptographic operation integration
+
+</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