asf-tooling opened a new issue, #953: URL: https://github.com/apache/tooling-trusted-releases/issues/953
**ASVS Level(s):** [L1, L2] **Description:** ### Summary The key details update endpoint allows authenticated users to modify which committees their OpenPGP key is associated with. While the endpoint verifies key ownership, it writes key-committee associations directly to the database, bypassing the storage layer's `as_committee_participant()` authorization check. The function performs committee membership authorization checks AFTER committing changes to the database. This allows any authenticated committer who owns a key to associate it with committees they do not belong to, potentially causing their key to appear in unauthorized committee KEYS files. ### Details **Affected Files and Lines:** - `atr/post/keys.py:89-119` - Key details update with direct DB writes - `atr/post/keys.py:101-108` - Committee association without authorization - `atr/post/keys.py:68-105` - Authorization check after commit - `atr/post/keys.py:75` - Direct database manipulation - `atr/post/keys.py:76-115` - Committee loop without validation - `atr/post/keys.py:87-114` - Post-commit authorization (ineffective) The authorization check that occurs post-commit silently ignores failures without rolling back the transaction, allowing unauthorized associations to persist. ### Recommended Remediation Validate committee membership BEFORE any database modification. Replace direct database writes with storage layer authorization checks: ```python # For each committee disassociation write.as_committee_participant(committee_key).keys.disassociate_fingerprint(fingerprint) # For each committee association write.as_committee_participant(committee_key).keys.associate_fingerprint(fingerprint) ``` Handle `AccessError` exceptions and display appropriate error messages to users. Add integration test attempting unauthorized committee association. Audit existing key-committee associations for unauthorized entries. Add audit log entry for all key-committee association changes. ### Acceptance Criteria - [ ] Committee membership validated before DB changes - [ ] Storage layer authorization used for associations - [ ] AccessError exceptions handled properly - [ ] Integration test verifies authorization enforcement - [ ] Audit of existing associations completed - [ ] Audit logging added - [ ] Unit test verifying the fix ### References - Source reports: L1:8.1.1.md, L1:8.2.1.md, L1:8.2.2.md, L1:8.3.1.md, L2:8.2.3.md - Related findings: FINDING-008 - ASVS sections: 8.1.1, 8.2.1, 8.2.2, 8.3.1, 8.2.3 ### Priority Critical --- --- **Triage notes:** overall key management, historical keys, etc. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
