reviewing-security-architecture

What this skill does

## 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 work factor and a unique salt
- Never use raw cryptographic hash functions alone for password hashing (too fast, no salt by default)
- Salts should be unique per credential to prevent rainbow-tables from accelerating brute-force attacks

## Authorization Patterns

### Role-Based Access Control (RBAC)

```csharp
// 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

```csharp
// 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
- Cryptographic keys MUST NEVER be stored directly accessible in a database, without being wrapped by another key
- 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:

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.