Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

AES-KW key unwrapping does not check for integrity #175

Open
aguinetqb opened this issue Aug 19, 2021 · 5 comments
Open

AES-KW key unwrapping does not check for integrity #175

aguinetqb opened this issue Aug 19, 2021 · 5 comments
Assignees

Comments

@aguinetqb
Copy link

aguinetqb commented Aug 19, 2021

This Javascript code, when ran in a browser, properly throws an exception when trying to unwrap the modified wrapped key:

"use strict";
const subtle = crypto.subtle;
const AESWrapKeyFormat = "raw";
async function run() {
    const msgKey = await subtle.generateKey({ name: "AES-CTR", length: 128 }, true, ["encrypt"]);
    const wrapKey = await subtle.generateKey({ name: "AES-KW", length: 128 }, true, ["wrapKey", "unwrapKey"]);
    const wrappedKey = await subtle.wrapKey(AESWrapKeyFormat, msgKey, wrapKey, "AES-KW");
    var wrappedKeyAr = new Uint8Array(wrappedKey);
    
    // Inject fault
    wrappedKeyAr[0] ^= 1;
    
    console.log(wrappedKeyAr);
    var unwrappedKey = null;
    try {
        unwrappedKey = await subtle.unwrapKey("raw", wrappedKey, wrapKey, { name: "AES-KW" }, { name: "AES-CTR", length: 128 }, true, ["decrypt"]);
    }
    catch (e) {
        console.log("Unwrapp error: " + e);
        return;
    }
    console.log("unwrapped succeeds");
    var iv = new Uint8Array(16);
    crypto.getRandomValues(iv);
    var data = new Uint8Array(2);
    data[0] = 0x41;
    data[1] = 0x41;
    const encrData = await subtle.encrypt({ name: "AES-CTR", counter: iv, length: 64 }, msgKey, data);
    const decrData = await subtle.decrypt({ name: "AES-CTR", counter: iv, length: 64 }, unwrappedKey, encrData);
    const view = new Uint8Array(decrData)
    console.log(view[0] == 0x41 && view[1] == 0x41)
}
run();

See https://jsfiddle.net/rnf6kdL9/1/

When the same code runs in Node 14.x using node-webcrypto-ossl v2.1.3, no exception is thrown when trying to unwrap the modified wrapped key. The integrity of the unwrapped key should be checked, as defined here: https://datatracker.ietf.org/doc/html/rfc3394#page-6

@rmhrisk
Copy link
Contributor

rmhrisk commented Aug 19, 2021

Thanks for the quality bug. We will investigate.

@microshine
Copy link
Contributor

microshine commented Aug 19, 2021

I've tried to reproduce this issue using @peculiar/webcrypto. Do you know about that module? It doesn't use C++ language and is based on NodeJS CryptoAPI?

it throws

Error: Trying to add data in unsupported state
    at Decipheriv.update (internal/crypto/cipher.js:161:29)
    at Function.decryptAesKW (node_modules/@peculiar/webcrypto/build/webcrypto.js:269:28)
    at Function.decrypt (node_modules/@peculiar/webcrypto/build/webcrypto.js:203:29)
    at AesKwProvider.onDecrypt (node_modules/@peculiar/webcrypto/build/webcrypto.js:511:26)
    at AesKwProvider.decrypt (node_modules/webcrypto-core/build/webcrypto-core.js:176:31)
    at SubtleCrypto.unwrapKey (node_modules/webcrypto-core/build/webcrypto-core.js:892:38)
    at run (index.js:19:37)

ossl doesn't throw the error

I'll take a look at this

@microshine
Copy link
Contributor

node-webcrypto-ossl uses OSSL function AES_unwrap_key (@peculiar/webcrypto does it using encrypt/decrypt functions). Looks like OpenSSL doesn't see any problems in incoming data

@aguinetqb
Copy link
Author

Thanks for looking into it and your prompt answers :)

I think I've tried @peculiar/webcrypto, but I had an issue having it working with the app I'm working on.

AES_unwrap_key seems to check for the integrity of the key here: https://docs.huihoo.com/doxygen/openssl/1.0.1c/aes__wrap_8c_source.html#l00131 . It returns 0 if it isn't valid, and otherwise the length of the unwrapped key.

@aguinetqb
Copy link
Author

Grepping quickly in the code of OSSL: it looks like the check here

if (AES_unwrap_key(&aes_key, nullptr, (byte*)res->c_str(), (const byte*)data->c_str(), (unsigned int)data->length()) < 0)
should be <= 0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants