Skip to content

Commit d7b4944

Browse files
committed
feat: implement secure storage unlock with password verification
- Implement SecureStorageManager.unlock(password) that returns boolean - Add master salt management (16-byte persistent salt) - Implement PBKDF2 key derivation (100k iterations, SHA-256) - Add password verification using verification payload - Encryption key stored only in memory, never persisted - Returns true for valid passwords, false for invalid passwords - Add comprehensive security tests (108 tests total, 100% coverage) - Maintain backward compatibility with existing encrypted data - Fix ESLint configuration for Jest globals and Web Crypto API types - Replace 'any' types with 'unknown' for better type safety Closes #116
1 parent 95e0066 commit d7b4944

File tree

11 files changed

+3624
-1725
lines changed

11 files changed

+3624
-1725
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"specId": "9a008c19-7adb-4bf7-89f3-f671527e11df", "workflowType": "requirements-first", "specType": "feature"}
Lines changed: 382 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,382 @@
1+
# Design Document: Secure Storage Unlock
2+
3+
## Overview
4+
5+
This design enhances the SecureStorageManager's unlock mechanism to provide secure password verification without exposing plaintext data on incorrect passwords. The current implementation stores a base PBKDF2 key in memory and uses per-payload salts for encryption, but throws errors when wrong passwords are used during decryption. This design introduces:
6+
7+
1. A persistent master salt for consistent key derivation
8+
2. A verification payload mechanism for password checking before data access
9+
3. Boolean return values from unlock() to indicate password correctness
10+
4. Enhanced security guarantees preventing plaintext exposure
11+
12+
The design maintains backward compatibility with existing encrypted data while adding robust password verification capabilities.
13+
14+
## Architecture
15+
16+
### Current Architecture
17+
18+
The existing SecureStorageManager uses a two-tier key derivation approach:
19+
- **Base Key**: Derived directly from the password using PBKDF2, stored in memory
20+
- **Per-Payload Keys**: Derived from the base key using random salts stored with each encrypted payload
21+
22+
```
23+
Password → PBKDF2 → Base Key (in memory)
24+
25+
Base Key + Random Salt → PBKDF2 → AES-GCM Key → Encrypt/Decrypt Payload
26+
```
27+
28+
### Enhanced Architecture
29+
30+
The new architecture introduces a master salt and verification payload:
31+
32+
```
33+
First Run:
34+
Password + Master Salt → PBKDF2 → Encryption Key (in memory)
35+
36+
Create Verification Payload
37+
38+
Store to Storage Adapter
39+
40+
Subsequent Runs:
41+
Password + Master Salt → PBKDF2 → Encryption Key (in memory)
42+
43+
Decrypt Verification Payload
44+
45+
Success → return true
46+
Failure → clear key, return false
47+
```
48+
49+
### Key Changes
50+
51+
1. **Master Salt Storage**: A single persistent salt (16 bytes) stored under key "master_salt"
52+
2. **Verification Payload**: A lightweight encrypted payload under key "verification_payload" containing known plaintext
53+
3. **Encryption Key**: Derived from password + master salt (replaces the base key concept)
54+
4. **Per-Payload Salts**: Maintained for backward compatibility with existing EncryptedPayload structure
55+
56+
## Components and Interfaces
57+
58+
### Modified SecureStorageManager Class
59+
60+
```typescript
61+
export class SecureStorageManager {
62+
private encryptionKey: CryptoKey | null = null;
63+
private storage: StorageAdapter;
64+
65+
constructor(storage: StorageAdapter);
66+
67+
// Modified method signature
68+
public async unlock(password: string): Promise<boolean>;
69+
70+
// Existing methods (unchanged)
71+
public lock(): void;
72+
public get isUnlocked(): boolean;
73+
public async saveAccount(account: AccountData): Promise<void>;
74+
public async getAccount(): Promise<AccountData | null>;
75+
public async saveSessionKeys(sessionKeys: SessionKeysData): Promise<void>;
76+
public async getSessionKeys(): Promise<SessionKeysData | null>;
77+
78+
// New private methods
79+
private async initializeMasterSalt(): Promise<Uint8Array>;
80+
private async loadMasterSalt(): Promise<Uint8Array | null>;
81+
private async deriveEncryptionKey(password: string, masterSalt: Uint8Array): Promise<CryptoKey>;
82+
private async createVerificationPayload(): Promise<void>;
83+
private async verifyPassword(): Promise<boolean>;
84+
}
85+
```
86+
87+
### Storage Keys
88+
89+
The Storage Adapter will contain these keys:
90+
91+
- `"master_salt"`: Base64-encoded 16-byte master salt
92+
- `"verification_payload"`: EncryptedPayload structure containing known plaintext
93+
- `"account"`: EncryptedPayload structure containing account data (existing)
94+
- `"sessionKeys"`: EncryptedPayload structure containing session keys (existing)
95+
96+
### Data Structures
97+
98+
#### Master Salt Storage Format
99+
```typescript
100+
// Stored as base64 string in Storage Adapter
101+
type MasterSaltStorage = string; // base64-encoded 16 bytes
102+
```
103+
104+
#### Verification Payload Content
105+
```typescript
106+
interface VerificationContent {
107+
marker: "KIRO_VERIFICATION_V1";
108+
timestamp: number; // Creation timestamp
109+
}
110+
```
111+
112+
The verification payload uses the same EncryptedPayload structure as other encrypted data.
113+
114+
## Data Models
115+
116+
### EncryptedPayload (Unchanged)
117+
118+
```typescript
119+
export interface EncryptedPayload {
120+
iv: string; // Base64-encoded Initialization Vector (12 bytes)
121+
salt: string; // Base64-encoded per-payload salt (16 bytes)
122+
data: string; // Base64-encoded ciphertext + auth tag
123+
}
124+
```
125+
126+
This structure remains unchanged for backward compatibility. The per-payload salt is still used for deriving the final AES-GCM key from the encryption key.
127+
128+
### Key Derivation Flow
129+
130+
```
131+
Step 1: Derive Encryption Key (once per unlock)
132+
password (string) + master_salt (16 bytes)
133+
→ PBKDF2(100k iterations, SHA-256)
134+
→ encryption_key (AES-256-GCM capable)
135+
136+
Step 2: Derive Per-Payload Key (per encrypt/decrypt operation)
137+
encryption_key + payload_salt (16 bytes from EncryptedPayload)
138+
→ PBKDF2(100k iterations, SHA-256)
139+
→ aes_key (AES-256-GCM)
140+
→ encrypt/decrypt with IV
141+
```
142+
143+
### Unlock Flow State Machine
144+
145+
```
146+
State: LOCKED
147+
↓ unlock(password)
148+
149+
Check if master_salt exists
150+
151+
NO → Generate master_salt → Store master_salt
152+
YES → Load master_salt
153+
154+
Derive encryption_key from password + master_salt
155+
156+
Check if verification_payload exists
157+
158+
NO → Create verification_payload → Store verification_payload → State: UNLOCKED → return true
159+
YES → Attempt decrypt verification_payload
160+
161+
SUCCESS → State: UNLOCKED → return true
162+
FAILURE → Clear encryption_key → State: LOCKED → return false
163+
```
164+
165+
166+
## Correctness Properties
167+
168+
*A property is a characteristic or behavior that should hold true across all valid executions of a system—essentially, a formal statement about what the system should do. Properties serve as the bridge between human-readable specifications and machine-verifiable correctness guarantees.*
169+
170+
### Property Reflection
171+
172+
After analyzing all acceptance criteria, I identified the following redundancies:
173+
- Properties 3.2, 3.3, 4.1, and 4.2 all test unlock return values and can be consolidated into comprehensive properties
174+
- Property 5.3 is redundant with 3.3 (both test key clearing on wrong password)
175+
- Properties 2.5 and 5.4 both test that storage contains no plaintext and can be combined
176+
- Properties 3.1, 3.2, and 3.3 can be combined into a single verification property
177+
178+
### Property 1: Master salt format consistency
179+
180+
*For any* master salt stored in the Storage Adapter, it should be a valid base64-encoded string representing exactly 16 bytes of data.
181+
182+
**Validates: Requirements 1.4**
183+
184+
### Property 2: Encryption key derivation enables decryption
185+
186+
*For any* password and master salt, if an encryption key is derived and used to encrypt data, then deriving a key with the same password and master salt should successfully decrypt that data.
187+
188+
**Validates: Requirements 2.1**
189+
190+
### Property 3: Encryption keys never persisted
191+
192+
*For any* state of the Storage Adapter after any sequence of operations, inspecting the storage should reveal no plaintext encryption keys or sensitive secrets.
193+
194+
**Validates: Requirements 2.5, 5.4**
195+
196+
### Property 4: Password verification through verification payload
197+
198+
*For any* password, when a verification payload exists in storage, unlock should attempt to decrypt it and return true if decryption succeeds or return false and clear the encryption key if decryption fails.
199+
200+
**Validates: Requirements 3.1, 3.2, 3.3**
201+
202+
### Property 5: Verification payload structure consistency
203+
204+
*For any* verification payload created by the system, it should have the same EncryptedPayload structure (iv, salt, data fields) as other encrypted data.
205+
206+
**Validates: Requirements 3.6**
207+
208+
### Property 6: Correct password returns true
209+
210+
*For any* correct password, calling unlock should return true and leave the SecureStorageManager in an unlocked state.
211+
212+
**Validates: Requirements 4.1**
213+
214+
### Property 7: Incorrect password returns false
215+
216+
*For any* incorrect password, calling unlock should return false and leave the SecureStorageManager in a locked state with no encryption key in memory.
217+
218+
**Validates: Requirements 4.2, 4.4**
219+
220+
### Property 8: Unlock idempotence
221+
222+
*For any* correct password, calling unlock multiple times should always return true without re-deriving keys after the first successful unlock.
223+
224+
**Validates: Requirements 4.3**
225+
226+
### Property 9: No plaintext exposure on wrong password
227+
228+
*For any* incorrect password, calling unlock or attempting to decrypt data should not expose any plaintext secrets through return values, error messages, or accessible state.
229+
230+
**Validates: Requirements 5.1, 5.2**
231+
232+
### Property 10: EncryptedPayload structure preservation
233+
234+
*For any* data encrypted by the SecureStorageManager, the resulting EncryptedPayload should contain exactly three fields: iv, salt, and data, all as base64-encoded strings.
235+
236+
**Validates: Requirements 6.2**
237+
238+
### Property 11: Per-payload salt uniqueness
239+
240+
*For any* two distinct encryption operations, the resulting EncryptedPayloads should have different salt values, demonstrating that each payload uses its own random salt.
241+
242+
**Validates: Requirements 6.3**
243+
244+
### Property 12: Master salt independence
245+
246+
*For any* encrypted payload and the master salt, the payload's salt field should be different from the master salt, demonstrating independence between master salt and per-payload salts.
247+
248+
**Validates: Requirements 6.4**
249+
250+
## Error Handling
251+
252+
### Error Scenarios
253+
254+
1. **Storage Adapter Failures**
255+
- If Storage Adapter operations fail (get/set/remove), propagate the error to the caller
256+
- Do not catch or suppress storage errors
257+
- Maintain locked state if unlock fails due to storage errors
258+
259+
2. **Decryption Failures**
260+
- When decryption fails (wrong password or corrupted data), throw error: "Invalid password or corrupted data"
261+
- Clear encryption key from memory before throwing
262+
- Ensure error messages contain no sensitive information
263+
264+
3. **Locked State Access**
265+
- When encrypt/decrypt operations are attempted while locked, throw error: "Storage manager is locked"
266+
- This prevents accidental access attempts without proper authentication
267+
268+
4. **Crypto API Failures**
269+
- If Web Crypto API operations fail, propagate the native error
270+
- These are typically programming errors (invalid parameters) rather than user errors
271+
272+
### Error Recovery
273+
274+
- After any error during unlock, the manager should remain in a locked state
275+
- Users can retry unlock with a different password
276+
- Storage corruption should be detectable through decryption failures
277+
- No automatic retry or recovery mechanisms (caller decides retry strategy)
278+
279+
## Testing Strategy
280+
281+
### Dual Testing Approach
282+
283+
This feature requires both unit tests and property-based tests for comprehensive coverage:
284+
285+
- **Unit tests**: Verify specific examples, edge cases, and error conditions
286+
- **Property tests**: Verify universal properties across all inputs
287+
288+
Both approaches are complementary and necessary. Unit tests catch concrete bugs and verify specific scenarios, while property tests verify general correctness across a wide range of inputs.
289+
290+
### Unit Testing Focus
291+
292+
Unit tests should cover:
293+
294+
1. **First-run initialization**
295+
- Master salt generation and storage
296+
- Verification payload creation
297+
- Successful unlock on first run
298+
299+
2. **Subsequent unlock scenarios**
300+
- Correct password returns true
301+
- Incorrect password returns false
302+
- Already unlocked returns true immediately
303+
304+
3. **State transitions**
305+
- Lock/unlock cycles
306+
- Multiple manager instances sharing storage
307+
308+
4. **Error conditions**
309+
- Accessing encrypted data while locked
310+
- Storage adapter failures
311+
- Corrupted data handling
312+
313+
5. **Backward compatibility**
314+
- Loading and decrypting data encrypted with previous implementation
315+
- Migrating from old to new unlock mechanism
316+
317+
### Property-Based Testing Configuration
318+
319+
**Library Selection**: Use `fast-check` for TypeScript/JavaScript property-based testing
320+
321+
**Configuration**:
322+
- Minimum 100 iterations per property test (due to randomization)
323+
- Each property test must reference its design document property
324+
- Tag format: `// Feature: secure-storage-unlock, Property {number}: {property_text}`
325+
326+
**Property Test Coverage**:
327+
328+
1. **Property 1**: Master salt format consistency
329+
- Generate random byte arrays, encode as base64, verify decoding produces 16 bytes
330+
331+
2. **Property 2**: Encryption key derivation enables decryption
332+
- Generate random passwords and data, verify encrypt/decrypt round trip
333+
334+
3. **Property 3**: Encryption keys never persisted
335+
- Generate random operations, inspect storage, verify no plaintext keys
336+
337+
4. **Property 4**: Password verification through verification payload
338+
- Generate random correct/incorrect passwords, verify unlock behavior
339+
340+
5. **Property 5**: Verification payload structure consistency
341+
- Generate random verification payloads, verify structure matches EncryptedPayload
342+
343+
6. **Property 6**: Correct password returns true
344+
- Generate random passwords, verify unlock returns true for correct password
345+
346+
7. **Property 7**: Incorrect password returns false
347+
- Generate random incorrect passwords, verify unlock returns false and clears state
348+
349+
8. **Property 8**: Unlock idempotence
350+
- Generate random passwords, call unlock multiple times, verify consistent behavior
351+
352+
9. **Property 9**: No plaintext exposure on wrong password
353+
- Generate random incorrect passwords, verify no plaintext in errors or state
354+
355+
10. **Property 10**: EncryptedPayload structure preservation
356+
- Generate random data, encrypt, verify payload structure
357+
358+
11. **Property 11**: Per-payload salt uniqueness
359+
- Generate random data, encrypt multiple times, verify different salts
360+
361+
12. **Property 12**: Master salt independence
362+
- Generate random data, verify master salt differs from payload salts
363+
364+
### Test File Organization
365+
366+
```
367+
packages/core-sdk/src/storage/__tests__/
368+
├── manager.test.ts (existing unit tests - update for new unlock signature)
369+
├── unlock.test.ts (new comprehensive unlock tests)
370+
└── unlock.properties.test.ts (new property-based tests)
371+
```
372+
373+
### Backward Compatibility Testing
374+
375+
Create a test that:
376+
1. Uses the old implementation to encrypt data
377+
2. Stores the encrypted data
378+
3. Uses the new implementation to unlock and decrypt
379+
4. Verifies data integrity
380+
381+
This ensures existing users can upgrade without losing access to their encrypted data.
382+

0 commit comments

Comments
 (0)