0
\$\begingroup\$

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.

\$\endgroup\$
1
  • 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
    Commented May 7, 2023 at 20:45

1 Answer 1

2
\$\begingroup\$

"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.

\$\endgroup\$
1
  • \$\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\$ Commented Jan 20 at 20:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.