Update the AES module according to ToB's recommendations

Author: alcuadrado

packages/ethereum-cryptography/README.md

@@ -267,16 +267,15 @@ like [pbkdf2](#pbkdf2-submodule) or [scrypt](#scrypt-submodule).
 
 ### Operation modes
 
-This submodule works with different [block cipher modes of operation](https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation).
-To choose one of them, you should pass the `mode` parameter a string with the
-same format as OpenSSL and Node use. You can take a look at them by running
-`openssl list -cipher-algorithms`.
+This submodule works with different [block cipher modes of operation](https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation). If you are using this module in a new
+application, we recommend using the default.
 
-In Node, any mode that its OpenSSL version supports can be used.
+While this module may work with any mode supported by OpenSSL, we only test it
+with `aes-128-ctr`, `aes-128-cbc`, and `aes-256-cbc`. If you use another module
+a warning will be printed in the console.
 
-In the browser, we test it to work with the modes that are normally used in
-Ethereum libraries and applications. Those are `aes-128-ctr`, `aes-126-cbc`, and
-`aes-256-cbc`, but other modes may work.
+We only recommend using `aes-128-cbc` and `aes-256-cbc` to decrypt already
+encrypted data.
 
 ### Padding plaintext messages
 
@@ -292,14 +291,38 @@ If you need to encrypt without padding or want to use another padding scheme,
 you can disable PKCS#7 padding by passing `false` as the last argument and
 handling padding yourself. Note that if you do this and your operation mode
 requires padding, `encrypt` will throw if your plaintext message isn't a
-multiple of `16.
+multiple of `16`.
+
+This option is only present to enable the decryption of already encrypted data.
+To encrypt new data, we recommend using the default.
+
+### IV reusing
+
+The `iv` parameter of the `encrypt` function must be unique, or the security
+of the encryption algorithm can be compromissed.
+
+You can generate a new `iv` using the `random` module.
+
+Note that to decrypt a value, you have to provide the same `iv` used to encrypt
+it.
+
+### How to handle errors with this module
+
+Sensitive information can be leaked via error messages when using this module.
+To avoid this, you should make sure that the errors you return don't
+contain the exact reason for the error. Instead, errors must report general
+encryption/decryption failures.
+
+Note that implementing this can mean catching all errors that can be thrown
+when calling on of this module's functions, and just throwing a new generic
+exception.
 
 ### Function types
 
 ```ts
-function encrypt(msg: Buffer, key: Buffer, iv: Buffer, mode: string, pkcs7PaddingEnabled = true): Buffer;
+function encrypt(msg: Buffer, key: Buffer, iv: Buffer, mode = "aes-128-ctr", pkcs7PaddingEnabled = true): Buffer;
 
-function decrypt(cypherText: Buffer, key: Buffer, iv: Buffer, mode: string, pkcs7PaddingEnabled = true): Buffer
+function decrypt(cypherText: Buffer, key: Buffer, iv: Buffer, mode = "aes-128-ctr", pkcs7PaddingEnabled = true): Buffer
 ```
 
 ### Example usage
@@ -311,8 +334,7 @@ console.log(
   encrypt(
     Buffer.from("message", "ascii"),
     Buffer.from("2b7e151628aed2a6abf7158809cf4f3c", "hex"),
-    Buffer.from("f0f1f2f3f4f5f6f7f8f9fafbfcfdfeff", "hex"),
-    "aes-128-cbc"
+    Buffer.from("f0f1f2f3f4f5f6f7f8f9fafbfcfdfeff", "hex")
   ).toString("hex")
 );
 ```

packages/ethereum-cryptography/src/aes.ts

@@ -1,16 +1,24 @@
 const browserifyAes = require("browserify-aes");
 
+const SUPPORTED_MODES = ["aes-128-ctr", "aes-128-cbc", "aes-256-cbc"];
+
 function ensureAesMode(mode: string) {
   if (!mode.startsWith("aes-")) {
     throw new Error(`AES submodule doesn't support mode ${mode}`);
   }
 }
 
+function warnIfUnsuportedMode(mode: string) {
+  if (!SUPPORTED_MODES.includes(mode)) {
+    console.warn("Using an unsupported AES mode. Consider using aes-128-ctr.");
+  }
+}
+
 export function encrypt(
   msg: Buffer,
   key: Buffer,
   iv: Buffer,
-  mode: string,
+  mode = "aes-128-ctr",
   pkcs7PaddingEnabled = true
 ): Buffer {
   ensureAesMode(mode);
@@ -28,7 +36,7 @@ export function decrypt(
   cypherText: Buffer,
   key: Buffer,
   iv: Buffer,
-  mode: string,
+  mode = "aes-128-ctr",
   pkcs7PaddingEnabled = true
 ): Buffer {
   ensureAesMode(mode);

packages/ethereum-cryptography/test/test-vectors/aes.ts

@@ -23,6 +23,17 @@ const TEST_VECTORS = [
       "874d6191b620e3261bef6864990db6ce9806f66b7970fdff8617187bb9fffdff5ae4df3edbd5d35e5b4f09020db03eab1e031dda2fbe03d1792170a0f3009cee",
     pkcs7PaddingEnabled: true
   },
+  // Same as the previous one, but with default params
+  {
+    mode: undefined,
+    key: "2b7e151628aed2a6abf7158809cf4f3c",
+    iv: "f0f1f2f3f4f5f6f7f8f9fafbfcfdfeff",
+    msg:
+      "6bc1bee22e409f96e93d7e117393172aae2d8a571e03ac9c9eb76fac45af8e5130c81c46a35ce411e5fbc1191a0a52eff69f2445df4f9b17ad2b417be66c3710",
+    cypherText:
+      "874d6191b620e3261bef6864990db6ce9806f66b7970fdff8617187bb9fffdff5ae4df3edbd5d35e5b4f09020db03eab1e031dda2fbe03d1792170a0f3009cee",
+    pkcs7PaddingEnabled: undefined
+  },
   // CBC uses padding, but the NIST test vectors don't
   {
     mode: "aes-128-cbc",
@@ -94,15 +105,15 @@ export function createTests(
     msg: Buffer,
     key: Buffer,
     iv: Buffer,
-    mode: string,
-    pkcs7PaddingEnabled: boolean
+    mode?: string,
+    pkcs7PaddingEnabled?: boolean
   ) => Buffer,
   decrypt: (
     cypherText: Buffer,
     key: Buffer,
     iv: Buffer,
-    mode: string,
-    pkcs7PaddingEnabled: boolean
+    mode?: string,
+    pkcs7PaddingEnabled?: boolean
   ) => Buffer
 ) {
   describe("aes", function() {