I'm working on building an encrypt/decrypt feature using AES, and a fixed AES 256-bit key size. I have decided on using AES-GCM, as it seems the most performant and secure.
I would like to know whether the following use of the browser's Crypto.js library is correctly used.
async function generateAESGCMKey() {
const key = await window.crypto.subtle.generateKey(
{
name: 'AES-GCM',
length: 256, // 256-bit key length
},
true, // Extractable
['encrypt', 'decrypt'] // Key usages
);
const encodedKey = btoa(String.fromCharCode(...new Uint8Array(await window.crypto.subtle.exportKey('raw', key))));
// The btoa function encodes the raw key data into base64
return encodedKey;
}
async function encryptText(plainText, base64Key) {
// Convert base64 key to ArrayBuffer
const keyBuffer = Uint8Array.from(atob(base64Key), c => c.charCodeAt(0));
// Import the key
const cryptoKey = await crypto.subtle.importKey(
"raw",
keyBuffer,
{ name: "AES-GCM" },
false,
["encrypt", "decrypt"]
);
// Generate a random initialization vector (IV)
const iv = crypto.getRandomValues(new Uint8Array(12));
// Encrypt the plain text
const plainTextBuffer = new TextEncoder().encode(plainText);
const encryptedBuffer = await crypto.subtle.encrypt(
{ name: "AES-GCM", iv },
cryptoKey,
plainTextBuffer
);
// Combine the IV and encrypted buffer for decryption later
const resultBuffer = new Uint8Array(12 + encryptedBuffer.byteLength);
resultBuffer.set(iv);
resultBuffer.set(new Uint8Array(encryptedBuffer), 12);
// Convert the result to a base64 string
const resultBase64 = btoa(String.fromCharCode(...resultBuffer));
return resultBase64;
}
async function decryptText(base64Combined, base64Key) {
// Convert base64 combined input to ArrayBuffer
const combinedBuffer = Uint8Array.from(atob(base64Combined), c => c.charCodeAt(0));
// Extract IV and encrypted data
const iv = combinedBuffer.slice(0, 12);
const encryptedData = combinedBuffer.slice(12);
// Convert base64 key to ArrayBuffer
const keyBuffer = Uint8Array.from(atob(base64Key), c => c.charCodeAt(0));
// Import the key
const cryptoKey = await crypto.subtle.importKey(
"raw",
keyBuffer,
{ name: "AES-GCM" },
false,
["encrypt", "decrypt"]
);
// Decrypt the encrypted data
const decryptedBuffer = await crypto.subtle.decrypt(
{ name: "AES-GCM", iv },
cryptoKey,
encryptedData
);
// Convert decrypted buffer to a string
const decryptedText = new TextDecoder().decode(decryptedBuffer);
return decryptedText;
}
(async function () {
// key size will be fixed every time
// "oVmzkV09WrEfWGn8fDrp80k2n/aRPHYfu9nPohYneRE=" - generated by SS PQE KEM
// S6l5o2KhMr6E/9G9gmlpO7UD8iAdWGiCreRRzNPVHLk= - generated by generateAESGCMKey
const key = await generateAESGCMKey();
console.log(key)
// plainText is of variable size
const plainText = "This is a super secret message, that should not be shared";
const encryptedText = await encryptText(plainText, key);
console.log("Encrypted text:", encryptedText);
const decryptedText = await decryptText(encryptedText, key);
console.log("Decrypted text:", decryptedText);
})();
Are there any edge-cases I should be wary of, or anything missing?
I usually work with these functions on a higher level, and haven't really worked with code this close to metal.
-
1\$\begingroup\$ As long the code is working and you want to have it reviewed should be the right forum, but the title should be about what the code is doing and not your concerns about it. \$\endgroup\$convert– convert2023年05月07日 20:45:50 +00:00Commented May 7, 2023 at 20:45
1 Answer 1
"I would like to know whether the following use of the browser's Crypto.js library is correctly used."
Yes, the usage of the library seems OK.
generateAESGCMKey
There is no such thing as an AESGCM
key. The key is an AES key and within GCM it will only be used as input to the AES block cipher.
It would also be a good idea to include the size of the key in the name of the function, or - probably better - to parameterize the function.
// The btoa function encodes the raw key data into base64
Yes, but why? When generating a key it is important not to put it into a string as those are hard to clear from memory.
true, // Extractable
It might be an idea to create a constant: const EXTRACTABLE = true
. That's my preference, but I agree that it is debatable.
const iv = crypto.getRandomValues(new Uint8Array(12));
Well done, GCM defaults to a 96 bit / 12 byte IV (or nonce officially, but naming it iv
is definitely OK).
return resultBase64;
Again with the base 64. It's often not necessary to encode the text. Depends what it is for of course, but this seems to be a generic piece of code.
The code does seem to show correct usage of the API to implement GCM. But the main question is: "what is it for?" It seems to hide a lot of detail to the user, but that user may not need the required functionality and may want to know what the security parameters such as the key size are.
Generally cryptography should be used for a specific piece of functionality - usually implementing a specific protocol. It is generally not useful to create such a high level API as shown here as it will just be an inflexible, hard to maintain piece of code that will find it's way anywhere. Any change to it may influence anything that uses GCM - presuming you are using it at all those places of course.
-
\$\begingroup\$ As an aside I've spend weeks removing an old
DES
class from my Java code. I sometimes joke that these kind of wrapping classes wrap knowledge instead of functionality. The joke is that when you have the knowledge then there is no need for the class anymore. \$\endgroup\$Maarten Bodewes– Maarten Bodewes2024年01月20日 20:57:59 +00:00Commented Jan 20, 2024 at 20:57
Explore related questions
See similar questions with these tags.