feat(kms): demo script for dinamo interface#117
Conversation
cpatino-intive
force-pushed
the
WP5535/demo-script-dinamo
branch
from
96ccd55 to
f63cd60
Compare
pranavjain97
left a comment
pranavjain97
left a comment
There was a problem hiding this comment.
Pretty solid documentation! Just some minor changes needed to clarify that this is just a suggested reference implementation and not set in stone.
| **Key Features:** | ||
| - Environment-based configuration | ||
| - Automatic connection cleanup using try/finally | ||
| - Error handling for disconnect failures | ||
| - Generic typing for return values | ||
| - Centralized connection logic for all HSM operations |
There was a problem hiding this comment.
this is too specific, can remove
|
|
||
| ```typescript | ||
| async generateDataKey(rootKey: string, keySpec: DataKeyTypeType): Promise<GenerateDataKeyKmsRes> { | ||
| return await this.withClient(async (client) => { |
There was a problem hiding this comment.
worth mentioning why this is needed? (to not have dangling connections to the HSM)
| @@ -0,0 +1,266 @@ | |||
| # Dinamo HSM KMS Implementation Documentation | |||
|
|
|||
| This document provides comprehensive documentation for the KMS API's integration with Dinamo HSM, covering the complete request-response flow from API handlers to HSM operations. | |||
There was a problem hiding this comment.
| This document provides comprehensive documentation for the KMS API's integration with Dinamo HSM, covering the complete request-response flow from API handlers to HSM operations. | |
| This document provides a reference implementation for integrating the 4 KMS API's with Dinamo HSM |
| | `POST /generateDataKey` | `generateDataKey.ts` | `generateDataKey()` | Create/export AES key | | ||
| | `POST /decryptDataKey` | `decryptDataKey.ts` | `decryptDataKey()` | Local SJCL decryption | | ||
|
|
||
| ## Envelope Encryption Pattern |
There was a problem hiding this comment.
| ## Envelope Encryption Pattern | |
| ## Envelope Encryption Pattern (Recommended) |
pranavjain97
left a comment
pranavjain97
left a comment
There was a problem hiding this comment.
We should note that the code references are in JavaScript, but this would ideally be implemented in C++ or Rust to allow explicit memory management to wipe out the plaintext data keys after use
|
|
||
| // 2. Export plaintext key material | ||
| const exportedKey = await client.key.exportSymmetric(dataKeyName); | ||
| const plaintextKey = exportedKey.toString('base64'); |
There was a problem hiding this comment.
need to add a comment here in bold saying the plaintext key should be wiped out of memory after use.
cpatino-intive
force-pushed
the
WP5535/demo-script-dinamo
branch
from
8c25d95 to
30ea0a0
Compare
| let plaintextKey: string | null = null; | ||
|
|
||
| try { | ||
| const result = await this.generateDataKey(rootKey, 'AES-256'); | ||
| plaintextKey = result.plaintextKey; | ||
|
|
||
| // Use the plaintext key immediately | ||
| const encryptedData = encrypt(plaintextKey, sensitiveData); | ||
|
|
||
| return { | ||
| encryptedKey: result.encryptedKey, | ||
| encryptedData: encryptedData | ||
| }; | ||
| } finally { | ||
| // **CRITICAL**: Explicitly wipe plaintext key from memory | ||
| if (plaintextKey) { | ||
| // Overwrite with random data multiple times | ||
| for (let i = 0; i < 3; i++) { | ||
| plaintextKey = crypto.randomBytes(plaintextKey.length).toString('base64'); | ||
| } | ||
| plaintextKey = null; | ||
| } | ||
|
|
||
| // Force garbage collection (if available) | ||
| if (global.gc) { | ||
| global.gc(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This would not clean the plaintext key from memory. JS strings are immutable.
That's why we need to suggest they either implement the KMS-API in a c++ like language, or use typed arrays like a Uint8Array for all sensitive data (which would still not work if the provider returned the plaintext in string)
2 participants
TASKS: WP-5535