crypto: throw error in CipherBase::SetAutoPadding

Throw error after calling CipherBase#final PR-URL: nodejs#9405 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
krydos · Feb 25, 2017 · cc34a0c · cc34a0c
1 parent 13571db
commit cc34a0c
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 3 deletions.
diff --git a/doc/api/crypto.md b/doc/api/crypto.md
@@ -194,6 +194,8 @@ When using an authenticated encryption mode (only `GCM` is currently
 supported), the `cipher.setAAD()` method sets the value used for the
 _additional authenticated data_ (AAD) input parameter.
 
+The `cipher.setAAD()` method must be called before [`cipher.update()`][].
+
 Returns `this` for method chaining.
 
 ### cipher.getAuthTag()
@@ -222,7 +224,8 @@ multiple of the cipher's block size or [`cipher.final()`][] will throw an Error.
 Disabling automatic padding is useful for non-standard padding, for instance
 using `0x0` instead of PKCS padding.
 
-The `cipher.setAutoPadding()` method must be called before [`cipher.final()`][].
+The `cipher.setAutoPadding()` method must be called before
+[`cipher.final()`][].
 
 Returns `this` for method chaining.
 
@@ -333,6 +336,8 @@ When using an authenticated encryption mode (only `GCM` is currently
 supported), the `decipher.setAAD()` method sets the value used for the
 _additional authenticated data_ (AAD) input parameter.
 
+The `decipher.setAAD()` method must be called before [`decipher.update()`][].
+
 Returns `this` for method chaining.
 
 ### decipher.setAuthTag(buffer)
@@ -346,6 +351,9 @@ received _authentication tag_. If no tag is provided, or if the cipher text
 has been tampered with, [`decipher.final()`][] with throw, indicating that the
 cipher text should be discarded due to failed authentication.
 
+The `decipher.setAuthTag()` method must be called before
+[`decipher.final()`][].
+
 Returns `this` for method chaining.
 
 ### decipher.setAutoPadding(auto_padding=true)
@@ -361,7 +369,7 @@ Turning auto padding off will only work if the input data's length is a
 multiple of the ciphers block size.
 
 The `decipher.setAutoPadding()` method must be called before
-[`decipher.update()`][].
+[`decipher.final()`][].
 
 Returns `this` for method chaining.
 

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -3598,9 +3598,13 @@ bool CipherBase::SetAutoPadding(bool auto_padding) {
 
 
 void CipherBase::SetAutoPadding(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+
   CipherBase* cipher;
   ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder());
-  cipher->SetAutoPadding(args.Length() < 1 || args[0]->BooleanValue());
+
+  if (!cipher->SetAutoPadding(args.Length() < 1 || args[0]->BooleanValue()))
+    env->ThrowError("Attempting to set auto padding in unsupported state");
 }
 
 

diff --git a/test/parallel/test-crypto-cipher-decipher.js b/test/parallel/test-crypto-cipher-decipher.js
@@ -150,3 +150,39 @@ testCipher2(Buffer.from('0123456789abcdef'));
   assert.strictEqual(decipher.setAuthTag(tagbuf), decipher);
   assert.strictEqual(decipher.setAAD(aadbuf), decipher);
 }
+
+// error throwing in setAAD/setAuthTag/getAuthTag/setAutoPadding
+{
+  const key = '0123456789';
+  const aadbuf = Buffer.from('aadbuf');
+  const data = Buffer.from('test-crypto-cipher-decipher');
+
+  const cipher = crypto.createCipher('aes-256-gcm', key);
+  cipher.setAAD(aadbuf);
+  cipher.setAutoPadding();
+
+  assert.throws(() => {
+    cipher.getAuthTag();
+  }, /^Error: Attempting to get auth tag in unsupported state$/);
+
+  const encrypted = Buffer.concat([cipher.update(data), cipher.final()]);
+
+  const decipher = crypto.createDecipher('aes-256-gcm', key);
+  decipher.setAAD(aadbuf);
+  decipher.setAuthTag(cipher.getAuthTag());
+  decipher.setAutoPadding();
+  decipher.update(encrypted);
+  decipher.final();
+
+  assert.throws(() => {
+    decipher.setAAD(aadbuf);
+  }, /^Error: Attempting to set AAD in unsupported state$/);
+
+  assert.throws(() => {
+    decipher.setAuthTag(cipher.getAuthTag());
+  }, /^Error: Attempting to set auth tag in unsupported state$/);
+
+  assert.throws(() => {
+    decipher.setAutoPadding();
+  }, /^Error: Attempting to set auto padding in unsupported state$/);
+}