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:19:02 UTC
(tooling-agents) branch main updated: ASVS L3 audit: 11.2.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 34fbb15 ASVS L3 audit: 11.2.1 (vote_encryption_and_crypto)
34fbb15 is described below
commit 34fbb157dd77379d75d525e1f01053f58c378b73
Author: ASF Tooling Team <ro...@apache.org>
AuthorDate: Wed Apr 22 02:18:58 2026 +0200
ASVS L3 audit: 11.2.1 (vote_encryption_and_crypto)
---
.../d0aa7e9/vote_encryption_and_crypto/11.2.1.md | 356 +++++++++++++++++++++
1 file changed, 356 insertions(+)
diff --git a/repos/apache/ASVS/reports/steve/v3/d0aa7e9/vote_encryption_and_crypto/11.2.1.md b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/vote_encryption_and_crypto/11.2.1.md
new file mode 100644
index 0000000..ee1f8bf
--- /dev/null
+++ b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/vote_encryption_and_crypto/11.2.1.md
@@ -0,0 +1,356 @@
+# ASVS 11.2.1 Security Audit Report
+
+## Executive Summary
+
+- **Repository:** apache/steve
+- **Audit Date:** Apr 22, 2026
+- **Auditor:** Tooling Agents
+- **ASVS Requirement:** 11.2.1 — Verify that industry-validated implementations (including libraries and hardware-accelerated implementations) are used for cryptographic operations.
+ - **Level:** 2
+ - **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 Summary
+
+| Severity | Count | Status |
+|----------|-------|--------|
+| Critical | 0 | ✅ None Found |
+| High | 0 | ✅ None Found |
+| Medium | 3 | ⚠️ Requires Attention |
+| Low | 0 | ✅ None Found |
+| **Total** | **3** | |
+
+---
+
+## Security Controls Inventory
+
+| Control | Library/Implementation | Industry Validated? | Location | Coverage |
+|---------|------------------------|---------------------|----------|----------|
+| CSPRNG (salt/ID generation) | `secrets` (Python stdlib, OS entropy) | ✅ Yes | `crypto.py:gen_salt()`, `create_id()` | All salt/ID paths |
+| Password/Key Hashing | `argon2-cffi` (reference C impl wrapper) | ✅ Yes (library) | `crypto.py:_hash()` | `gen_opened_key`, `gen_vote_token` |
+| Pre-hashing for Argon2 | `hashlib.blake2b` (OpenSSL-backed) | ✅ Yes | `crypto.py:gen_opened_key()` | Election data hashing |
+| Key Derivation (HKDF) | `cryptography.hazmat.primitives.kdf.hkdf` | ✅ Yes | `crypto.py:_b64_vote_key()` | Vote encryption key |
+| Symmetric Encryption | `cryptography.fernet.Fernet` (AES-128-CBC+HMAC) | ✅ Yes | `crypto.py:create_vote()`, `decrypt_votestring()` | Vote encrypt/decrypt |
+| Cryptographic Shuffle | Custom Fisher-Yates with `secrets.randbelow()` | ⚠️ Standard algo, proper CSPRNG | `crypto.py:shuffle()` | Vote order anonymization |
+| Tamper Detection Comparison | Python `!=` operator | ❌ Not constant-time | `election.py:is_tampered()` | Tamper check before tally |
+
+---
+
+## Critical File Review
+
+### `v3/steve/crypto.py` — Core Cryptographic Module
+
+| Function | Industry-Validated Library? | Algorithm Choice Correct? | Notes |
+|----------|----------------------------|---------------------------|-------|
+| `gen_salt()` | ✅ `secrets.token_bytes` | ✅ 16 bytes | Proper CSPRNG |
+| `gen_opened_key()` | ✅ `hashlib.blake2b` + `argon2` | ⚠️ Argon2d | SEV-001 |
+| `gen_vote_token()` | ✅ `argon2` | ⚠️ Argon2d | SEV-001 |
+| `_b64_vote_key()` | ✅ `cryptography.hkdf.HKDF` | ⚠️ Wrong info param | SEV-003 |
+| `create_vote()` | ✅ `cryptography.fernet.Fernet` | ✅ AES-128-CBC+HMAC | Industry standard |
+| `decrypt_votestring()` | ✅ `cryptography.fernet.Fernet` | ✅ | Mirrors create_vote |
+| `_hash()` | ✅ `argon2.low_level` | ⚠️ Type.D not Type.ID | SEV-001 (root) |
+| `shuffle()` | ✅ `secrets.randbelow` | ✅ Fisher-Yates | Correct implementation |
+| `create_id()` | ✅ `secrets.token_hex` | ✅ 5 bytes = 10 hex | Proper entropy |
+
+### `v3/steve/election.py` — Election Logic
+
+| Function | Crypto Delegation | Issue? |
+|----------|-------------------|--------|
+| `open()` | `crypto.gen_salt()`, `crypto.gen_opened_key()` | Uses SEV-001 affected functions |
+| `add_salts()` | `crypto.gen_salt()` | ✅ Proper per-voter salts |
+| `add_vote()` | `crypto.gen_vote_token()`, `crypto.create_vote()` | ✅ Proper delegation |
+| `tally_issue()` | `crypto.gen_vote_token()`, `crypto.decrypt_votestring()`, `crypto.shuffle()` | ✅ |
+| `is_tampered()` | `crypto.gen_opened_key()` | ⚠️ SEV-002 non-constant-time compare |
+
+---
+
+## Findings
+
+### Medium Severity
+
+#### ASVS-1121-MED-001: Argon2d Used Instead of RFC 9106-Recommended Argon2id
+
+**Severity:** MEDIUM
+**Gap Type:** Type B — Correct variant (Argon2id) is present in the benchmark function but NOT used in the production `_hash()` function.
+
+**Location:**
+- **File:** `v3/steve/crypto.py`
+- **Function:** `_hash()` (line ~97) vs `benchmark_argon2()` (line ~146)
+- **Lines:** ~97, ~146
+
+**Related Functions Checked:**
+- `gen_opened_key()` — calls `_hash()` for election master key
+- `gen_vote_token()` — calls `_hash()` for per-voter-per-issue token
+
+**Description:**
+
+The production `_hash()` function uses `argon2.low_level.Type.D` (Argon2d), while the benchmark function correctly uses `argon2.low_level.Type.ID` (Argon2id). Argon2d uses data-dependent memory access patterns, making it vulnerable to side-channel attacks (cache-timing, memory bus snooping) that could leak information about the secret input. RFC 9106 Section 4 explicitly recommends Argon2id for general-purpose use because it combines Argon2i's side-channel resistance with Argon2d's GPU r [...]
+
+**Vulnerable Code:**
+```python
+# Production code (_hash, line ~97):
+def _hash(data: bytes, salt: bytes) -> bytes:
+ 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 (data-dependent)
+ )
+
+# Benchmark code (line ~146):
+ argon2.low_level.hash_secret_raw(
+ ...
+ type=argon2.low_level.Type.ID, # <-- Argon2id (correct)
+ )
+```
+
+**Data Flow:**
+1. `_hash()` is called by → `gen_opened_key()` (election master key)
+2. `_hash()` is called by → `gen_vote_token()` (per-voter-per-issue token)
+3. These are critical for ballot encryption and tamper detection
+4. The correct variant (Argon2id) appears in `benchmark_argon2()` but was not propagated to production
+
+**Proof of Concept:**
+
+N/A — This is a configuration issue rather than an exploitable vulnerability requiring PoC.
+
+**Impact:**
+
+While the `argon2-cffi` library itself is industry-validated, the selected variant does not align with current standards. Argon2d's data-dependent memory access patterns could leak information about secret inputs through side-channel attacks in environments where attackers have access to cache-timing or memory bus monitoring capabilities. This affects both the election master key and per-voter tokens, potentially compromising ballot encryption and vote anonymity.
+
+**Remediation:**
+
+```python
+def _hash(data: bytes, salt: bytes) -> bytes:
+ 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, # RFC 9106 recommended
+ )
+```
+
+> **Note:** Changing the Argon2 variant will change all derived keys. This requires a coordinated migration: existing open/closed elections will fail tamper checks and vote decryption. Plan a migration path or apply only to new elections.
+
+---
+
+#### ASVS-1121-MED-002: Non-Constant-Time Comparison of Cryptographic Material in Tamper Detection
+
+**Severity:** MEDIUM
+**Gap Type:** Type A — No constant-time comparison control exists anywhere in the codebase for cryptographic value comparison.
+
+**Location:**
+- **File:** `v3/steve/election.py`
+- **Function:** `is_tampered()` (line ~381)
+- **Lines:** ~381
+
+**Related Functions Checked:**
+- `tally.py:main()` (line ~155) — calls `is_tampered()` before tallying
+- `crypto.gen_opened_key()` — generates the key being compared
+
+**Description:**
+
+The `is_tampered()` function uses Python's standard `!=` operator to compare cryptographic material (`opened_key`). This performs a byte-by-byte comparison that short-circuits on the first difference, creating a timing side-channel that could leak information about the stored key value.
+
+**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 # <-- Standard Python != comparison
+```
+
+**Data Flow:**
+1. `md.opened_key` → stored 32-byte Argon2 hash (database)
+2. `opened_key` → freshly computed 32-byte Argon2 hash
+3. Python `!=` performs byte-by-byte comparison that short-circuits on first difference
+4. Comparison result returned to `tally.py:main()` (line ~155) and potentially web handlers
+
+**Proof of Concept:**
+
+An attacker who can measure response timing of the tamper check (e.g., through a web endpoint that calls `is_tampered()` before serving election data) could use statistical timing analysis to determine matching byte prefixes of `opened_key`. While the Argon2 computation dominates runtime (~100ms), the comparison timing leak of ~nanoseconds per byte could be measurable with sufficient samples in a high-precision environment.
+
+**Impact:**
+
+Partial leakage of the `opened_key` value through timing side-channel. The `opened_key` is critical — it's the root from which all vote tokens are derived. Any leakage could help an attacker forge vote tokens or correlate votes to voters. Python's standard `hmac.compare_digest()` is the industry-validated approach for comparing cryptographic values.
+
+**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)
+
+ # Constant-time comparison prevents timing side-channel
+ return not hmac.compare_digest(opened_key, md.opened_key)
+```
+
+---
+
+#### ASVS-1121-MED-003: HKDF Domain Separation Label Does Not Match Actual Cryptographic Usage
+
+**Severity:** MEDIUM
+**Gap Type:** Type C-adjacent — Cryptographic control (HKDF domain separation) is called, but with an incorrect `info` parameter that undermines its purpose.
+
+**Location:**
+- **File:** `v3/steve/crypto.py`
+- **Function:** `_b64_vote_key()` (lines ~57–68)
+- **Lines:** ~57–68
+
+**Related Functions Checked:**
+- `create_vote()` — calls `_b64_vote_key()` for encryption
+- `decrypt_votestring()` — calls `_b64_vote_key()` for decryption
+
+**Description:**
+
+The `_b64_vote_key()` function uses HKDF with `info=b'xchacha20_key'` to derive encryption keys, but the actual encryption algorithm is Fernet (AES-128-CBC+HMAC-SHA256), not XChaCha20-Poly1305. The HKDF `info` parameter provides cryptographic domain separation per NIST SP 800-56C / RFC 5869, ensuring keys derived for different purposes are cryptographically independent. This mismatch violates that principle.
+
+**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 <-- wrong comment
+ salt=salt,
+ info=b'xchacha20_key', # <-- domain label for XChaCha20, but used with Fernet/AES
+ )
+ vote_key = keymaker.derive(vote_token)
+ return base64.urlsafe_b64encode(vote_key)
+```
+
+**Data Flow:**
+1. `_b64_vote_key()` is called by → `create_vote()` and `decrypt_votestring()`
+2. HKDF derives a 32-byte key with `info=b'xchacha20_key'`
+3. Key is base64-encoded and used as a **Fernet** key (AES-128-CBC + HMAC-SHA256)
+4. The `info` parameter provides cryptographic domain separation per NIST SP 800-56C / RFC 5869
+
+**Proof of Concept:**
+
+N/A — This is a configuration issue that creates migration risk rather than an immediately exploitable vulnerability.
+
+**Impact:**
+
+The HKDF `info` parameter exists specifically for domain separation — ensuring keys derived for different purposes are cryptographically independent. Using `b'xchacha20_key'` when the actual algorithm is Fernet (AES-128-CBC+HMAC-SHA256) violates this principle:
+
+- If the planned XChaCha20-Poly1305 migration occurs and reuses `info=b'xchacha20_key'`, old Fernet keys and new XChaCha20 keys could be derived from the same HKDF context, violating key separation.
+- The comment "still using Fernet now, but will switch soon" indicates active plans to change, making this a concrete migration risk.
+- Mismatched domain labels make cryptographic audit and inventory verification unreliable — the code claims XChaCha20 but uses AES.
+
+**Remediation:**
+
+```python
+def _b64_vote_key(vote_token: bytes, salt: bytes) -> bytes:
+ "Key-stretch the vote_token into a Fernet encryption key."
+ keymaker = hkdf.HKDF(
+ algorithm=hashes.SHA256(),
+ length=32, # Fernet key: 16 bytes AES-128 + 16 bytes HMAC-SHA256
+ salt=salt,
+ info=b'steve_fernet_vote_key_v1', # Accurate domain label with version
+ )
+ vote_key = keymaker.derive(vote_token)
+ return base64.urlsafe_b64encode(vote_key)
+```
+
+> **Note:** Changing the `info` parameter changes all derived keys. Existing encrypted votes will become undecryptable. Coordinate with migration planning, or version the derivation to support both old and new keys during transition.
+
+---
+
+## Positive Security Patterns
+
+1. **All cryptographic operations centralized in `crypto.py`**: Clean separation of concerns. `election.py` delegates all crypto to the dedicated module, making audit tractable and algorithm changes localized.
+
+2. **Industry-standard libraries throughout**: Every cryptographic primitive uses a recognized library (`cryptography`, `argon2-cffi`, Python `secrets`/`hashlib`). No hand-rolled AES, no custom hash functions, no weak PRNGs.
+
+3. **Multi-stage key derivation**: The key hierarchy (BLAKE2b → Argon2 → HKDF → Fernet) follows defense-in-depth principles with proper key stretching at each stage.
+
+4. **Per-voter-per-issue salts**: Each `mayvote` entry gets a unique 16-byte salt from `secrets.token_bytes()`, preventing vote correlation even with database access.
+
+5. **Cryptographically secure shuffle**: Vote order anonymization uses `secrets.randbelow()` with correct Fisher-Yates implementation, preventing database-order leakage during tally.
+
+6. **Random non-sequential IDs**: `create_id()` uses `secrets.token_hex(5)` with collision retry, preventing URL enumeration of elections and issues.
+
+7. **Sensitive value exclusion from public APIs**: `get_metadata()` explicitly omits `salt` and `opened_key`. `get_issue()` documents "NEVER return issue.salt."
+
+8. **Tamper detection before tallying**: `tally.py` calls `is_tampered()` before processing any votes, with hard exit on failure.
+
+---
+
+## Architecture Observations
+
+1. **Crypto Inventory Consistency**: The actual cryptographic usage in `crypto.py` and `election.py` matches the documented inventory (BLAKE2b, Argon2, HKDF-SHA256, Fernet/AES-128-CBC+HMAC). No undocumented crypto operations found.
+
+2. **Migration Planning**: The `_b64_vote_key` comment about switching to XChaCha20-Poly1305 suggests a planned migration. This should be tracked in the crypto inventory and coordinated with HKDF info parameter correction (ASVS-1121-MED-003) to avoid key reuse during transition.
+
+3. **Argon2 Parameters**: `time_cost=2`, `memory_cost=65536` (64 MB), `parallelism=4` align with the Passlib defaults and are reasonable for server-side key derivation. The `benchmark_argon2()` function shows deliberate parameter tuning was considered.
+
+4. **State Machine Integrity**: The election state machine (editable → open → closed) properly gates cryptographic operations through assertions. Salt generation only occurs during the editable→open transition, and tampering checks verify the entire state chain.
+
+---
+
+## Recommendations Summary
+
+### Immediate Actions (0-30 days)
+
+| Priority | Finding | Effort | Risk if Unaddressed |
+|----------|---------|--------|---------------------|
+| 1 | **ASVS-1121-MED-002**: Replace `!=` with `hmac.compare_digest()` | Very Low (1 line, no migration) | Timing oracle on opened_key |
+
+**Rationale:** This fix can be applied immediately with zero side effects and eliminates a timing side-channel vulnerability in critical tamper detection logic.
+
+### Short-term Actions (1-3 months)
+
+| Priority | Finding | Effort | Risk if Unaddressed |
+|----------|---------|--------|---------------------|
+| 2 | **ASVS-1121-MED-001**: Change `argon2.Type.D` → `argon2.Type.ID` | Low (1 line) but requires migration plan | Side-channel attack on key derivation |
+| 3 | **ASVS-1121-MED-003**: Correct HKDF `info` parameter | Low (1 line) but requires migration plan | Key reuse during XChaCha20 migration |
+
+**Implementation Note for ASVS-1121-MED-001 and ASVS-1121-MED-003:** Both changes alter derived key material. Existing elections with encrypted votes will break unless a versioned migration strategy is implemented (e.g., store a `crypto_version` column in the election table and branch derivation logic accordingly).
+
+### Long-term Actions (3-6 months)
+
+1. **Implement Cryptographic Versioning System**: Add a `crypto_version` field to the election metadata to support future algorithm migrations without breaking existing elections.
+
+2. **Complete XChaCha20-Poly1305 Migration**: If the planned migration from Fernet to XChaCha20-Poly1305 proceeds, ensure proper domain separation with corrected HKDF `info` parameters and versioned key derivation.
+
+3. **Establish Cryptographic Inventory Review Process**: Create a quarterly review process to verify that all cryptographic implementations continue to align with current NIST/ISO standards and ASVS requirements.
+
+---
+
+## Appendix: Files Analyzed
+
+<details>
+<summary>Click to expand file list (4 files)</summary>
+
+1. **`v3/schema.sql`** (relevance: 10)
+ - Database schema defining election, issue, and vote storage structures
+ - Reviewed for cryptographic material storage patterns
+
+2. **`v3/server/bin/tally.py`** (relevance: 10)
+ - Vote tallying script that calls tamper detection
+ - Reviewed for cryptographic operation invocation patterns
+
+3. **`v3/steve/crypto.py`** (relevance: 10)
+ - Core cryptographic module containing all cryptographic primitives
+ - Primary focus of audit for library selection and implementation correctness
+
+4. **`v3/steve/election.py`** (relevance: 10)
+ - Election state management and vote handling logic
+ - Reviewed for proper delegation to crypto module and tamper detection implementation
+
+</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