reviewing-security-architecture

name: reviewing-security-architecture description: This skill should be used when the user asks to "review the security architecture", "check authentication patterns", "evaluate trust boundaries", "review encryption implementation", "assess authorization design", or needs to evaluate system designs for authentication, authorization, data protection, or cryptographic correctness.

Authentication Architecture

Token Handling

Review these aspects of token-based authentication:

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:

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:

1. Validate all input — type, format, range, length. Don't trust upstream validation.
2. Authenticate the caller — verify identity before processing requests.
3. Authorize the action — verify the caller has permission for this specific operation.
4. Sanitize output — encode/escape data appropriate to the destination context.
5. 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 examples
• references/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.