reviewing-security-architecture
Installation
SKILL.md
Authentication Architecture
Token Handling
Review these aspects of token-based authentication:
| Aspect | Secure Pattern | Anti-Pattern |
|---|---|---|
| Issuance | Short-lived tokens with refresh mechanism | Long-lived tokens that never expire |
| Validation | Validate signature, issuer, audience, and expiry on every request | Validate only the signature, or skip validation for "internal" calls |
| Storage (server) | Stateless JWT or server-side session store | Token stored in querystring or URL |
| Storage (client) | HttpOnly Secure cookies or secure platform storage | localStorage, sessionStorage, or cookies without HttpOnly/Secure flags |
| Refresh | Refresh token rotation (old refresh token invalidated on use) | Reusable refresh tokens with no rotation |
| Revocation | Token blocklist or short expiry + refresh rotation | No revocation mechanism for compromised tokens |
Session Management
- Server-side sessions should have absolute timeouts (maximum session duration) and idle timeouts
- Session identifiers must be cryptographically random and sufficiently long (128+ bits of entropy)
- Regenerate session ID after authentication state changes (login, privilege escalation)
- Bind sessions to client properties where possible (IP range, user agent) for anomaly detection
Credential Storage
- Passwords must be hashed with a modern KDF: Argon2id (preferred), bcrypt, or PBKDF2 with high iteration count
- Never use MD5, SHA-1, or SHA-256 alone for password hashing (too fast, no salt by default)
- Salt must be unique per credential, cryptographically random, at least 128 bits
- Consider pepper (application-level secret added to hash input) for defense in depth
Authorization Patterns
Role-Based Access Control (RBAC)
// CORRECT — explicit role check at the API layer
[Authorize(Roles = "Admin")]
public async Task<IActionResult> DeleteUser(Guid userId)
// WRONG — checking role in business logic with string comparison
if (currentUser.Role == "admin") // Fragile, case-sensitive, easy to bypass
Object-Level Authorization
// WRONG — trusts the userId from the route, no ownership check
public async Task<Cipher> GetCipher(Guid cipherId) {
return await _cipherRepository.GetByIdAsync(cipherId);
}
// CORRECT — verify the requesting user owns the resource
public async Task<Cipher> GetCipher(Guid cipherId) {
var cipher = await _cipherRepository.GetByIdAsync(cipherId);
if (cipher.UserId != _currentContext.UserId)
throw new NotFoundException();
return cipher;
}
Authorization Principles
- Check at every layer. API controller, service layer, and data access should all enforce authorization. Don't rely on a single checkpoint.
- Least privilege. Grant the minimum permissions needed. Default to deny.
- Fail closed. If an authorization check fails or throws an exception, deny access. Never fail open.
- Don't trust client-side authorization. UI visibility controls are UX, not security. Always enforce server-side.
Data Protection
Encryption at Rest
- All sensitive data must be encrypted at rest using AES-256 or equivalent
- Encryption keys must be stored separately from encrypted data (never in the same database)
- Use envelope encryption: data encrypted with a data encryption key (DEK), DEK encrypted with a key encryption key (KEK) in a key management system
- Bitwarden's end-to-end encryption ensures vault data is encrypted before leaving the client
Encryption in Transit
- TLS 1.2 minimum, TLS 1.3 preferred
- Disable older protocols (SSL 3.0, TLS 1.0, TLS 1.1)
- Use strong cipher suites (ECDHE for key exchange, AES-GCM for encryption)
- Certificate pinning for mobile apps where appropriate
- Internal service-to-service communication should also use TLS
Data Classification
When reviewing architecture, identify data by classification:
| Classification | Examples | Required Protection |
|---|---|---|
| Critical | Encryption keys, master passwords, vault data | End-to-end encryption, HSM key storage |
| Confidential | PII, email addresses, billing info | Encryption at rest + in transit, access logging |
| Internal | Organizational settings, feature flags | Encryption in transit, role-based access |
| Public | Marketing content, public API docs | Integrity protection |
Trust Boundaries
A trust boundary exists wherever data crosses between components with different levels of trust. Every crossing must be validated.
Common Trust Boundaries
Client ←→ API Gateway (user-controlled → server-controlled)
API Gateway ←→ Backend Service (internet-facing → internal)
Backend Service ←→ Database (application → data store)
Service ←→ External API (internal → third-party)
Browser ←→ Browser Extension (page context → extension context)
Main Thread ←→ Web Worker (different execution contexts)
Validation at Trust Boundaries
At each boundary crossing:
- Validate all input — type, format, range, length. Don't trust upstream validation.
- Authenticate the caller — verify identity before processing requests.
- Authorize the action — verify the caller has permission for this specific operation.
- Sanitize output — encode/escape data appropriate to the destination context.
- Log the crossing — security-relevant boundary crossings should be auditable.
Zero-Trust Principles
- Don't trust internal network location as a proxy for authentication
- Every service-to-service call should be authenticated and authorized
- Assume the network is compromised — encrypt all internal communication
- Validate data from internal services just as rigorously as external input
Reference Material
For detailed lookup tables and code examples, consult:
references/crypto-algorithms.md— Algorithm selection table (recommended vs. deprecated) and common crypto anti-pattern code examplesreferences/architectural-anti-patterns.md— Common security architecture anti-patterns (implicit trust, single points of failure, insecure defaults, monolithic auth) with fixes
Connection to Threat Modeling
Architecture security review directly feeds into the threat modeling process:
- Trust boundary identification informs where to draw boundaries in data flow diagrams
- Architectural weaknesses become threats in the threat catalog
- Security properties (auth, encryption, access control) map to security goals in security definitions
- Anti-patterns found become candidates for Bitwarden's engagement model Phase 1 initial security assessment
When conducting architecture review, consider whether the findings warrant engaging the AppSec team (#team-eng-appsec) for a full threat modeling session.