Encryption of secrets in json wallet use AES-CTR with a constant IV for the counter

[jonZlotnik]

source of bug:

smapp/desktop/fileEncryptionService.ts

Lines 34 to 46 in a8bfea7

	static encryptData = ({ data, key }: { data: string; key: Buffer }) => {
	if (!data || !data.length) {
	throw new Error('missing data to encrypt');
	}
	if (!key || !key.length) {
	throw new Error('missing encryption key');
	}
	const textBytes = aes.utils.utf8.toBytes(data);
	const aesCtr = new aes.ModeOfOperation.ctr(key, new aes.Counter(5)); // eslint-disable-line new-cap
	const encryptedBytes = aesCtr.encrypt(textBytes);
	const encryptedHex = aes.utils.hex.fromBytes(encryptedBytes);
	return encryptedHex;
	};

Using the same IV for every encryption is dangerous:

• The IV must be unique for every encrypted message
• The IV must be unpredictable to an attacker prior to encryption of the message.

Additionally, AES-CTR does not provide authentication, thus some authentication step is needed to check if data was altered while encrypted. Malleability is trivial with AES-CTR and can be catastrophic if the wallet contents are trusted verbatim.

Solution path:

1. Replace the aes-js library with the native WebCrypto Library in Node.js
2. Use the subtle.encrypt() function with the AesGcmParams so that we get authentication for free. (AES-GCM instead of just AES-CTR)
3. Add AES-GCM and key generation parameters to the wallet file.

Parameter derivation as follows:

• I will implement something in smcli, and update here.
  See comment below with nodejs reference and link to test in smcli

Note: the WebCrypto API for node.js version 16.18.0 seems to have a stability index of 1, but it doesn't seem that any changes regarding the security of the features we are using have been implemented between 16.18 and 19.0. As of node.js 19.0, the api is considered stable.

• I will double check we're not missing security updates (but I think security updates are included in LTS versions, just the api might change in future versions)
  Seems security updates are being included so should be safe to use:
  https://github.com/nodejs/node/blob/main/doc/changelogs/CHANGELOG_V16.md#16.18.0

[jonZlotnik]

Node JS Reference Code

Here is a reference implementation.
This first section is copy/paste-able (parameters are safe and match smcli)

const { subtle } = require('node:crypto').webcrypto;
const crypto = require("crypto");
const ec = new TextEncoder();

// salt should come from `crypto.randomBytes(16)`
async function pbkdf2Key(pass, salt) {
    const ec = new TextEncoder();
    const keyMaterial = await subtle.importKey(
        'raw',
        ec.encode(pass),
        'PBKDF2',
        false,
        ['deriveKey']);
    const key = await subtle.deriveKey(
        // Recommended PBKDF2 parameters from OWASP
        // https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pbkdf2
        {
            name: 'PBKDF2',
            hash: 'SHA-512',
            salt: salt,
            iterations: 120000
        },
        keyMaterial,
        { name: 'AES-GCM', length: 256 },
        true,
        ['encrypt', 'decrypt']
    );
    return key;
}

async function constructAesGcmIv(key, input) {
    if (key.algorithm.name !== 'AES-GCM') {
        throw new Error('Key is not an AES-GCM key');
    }
    const rawKey = await subtle.exportKey('raw', key);
    const hmacKey = await subtle.importKey(
        'raw', rawKey,
        { name: 'HMAC', hash: 'SHA-512'},
        true,
        ['sign']
    )
    const iv = await subtle.sign(
        {name: 'HMAC'},
        hmacKey,
        input
    )
    return iv.slice(0, 12); // IV is 12 bytes
}

async function encrypt(key, iv, plaintext) {
    const ciphertext = await subtle.encrypt(
        {
            name: 'AES-GCM',
            iv: iv,
            tagLength: 128,
        },
        key,
        plaintext
    );
    return ciphertext;
}
async function decrypt(key, iv, ciphertext) {
    const plaintext = await subtle.decrypt(
        {
            name: 'AES-GCM',
            iv: iv,
            tagLength: 128,
        },
        key,
        ciphertext
    );
    return plaintext;
}

----- NOTHING BELOW HERE SHOULD RUN IN THE APP -----

But it at least gives you an idea of how the functions work together.

Test vectors against this test in smcli

(async () => {
    // Input
    const password = 'password';
    const passwordSalt = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]);
    const plaintext = ec.encode('super secret secret that needs to be secret');
    console.log(' ---- INPUT ---- ');
    console.log('password:   ', Buffer.from(password).toString('hex'));
    console.log('salt:       ', Buffer.from(passwordSalt).toString('hex'));
    console.log('plaintext:  ', Buffer.from(plaintext).toString('hex'));

    // Process
    const key = await pbkdf2Key(password, passwordSalt);
    const iv = await constructAesGcmIv(key, plaintext);
    const ciphertext = await encrypt(key, iv, plaintext);

    // Output
    console.log('\n ---- OUTPUT ---- ');
    console.log('key:        ', Buffer.from(await subtle.exportKey('raw', key)).toString('hex'));
    console.log('iv:         ', Buffer.from(iv).toString('hex'));
    console.log('ciphertext: ', Buffer.from(ciphertext).toString('hex'));

    // Verify
    console.log('\n ---- VERIFY ---- ');
    const decrypted = await decrypt(key, iv, ciphertext);
    console.log('decrypted:  ', Buffer.from(decrypted).toString('hex'));
    console.log('plaintext:  ', Buffer.from(plaintext).toString('hex'));
    console.log('plaintext === decrypted: ', Buffer.from(plaintext).equals(Buffer.from(decrypted)));
})();