From e90f38270c65e33de4148abcc3c2ad020af7edf4 Mon Sep 17 00:00:00 2001
From: Kirill Fomichev <fanatid@ya.ru>
Date: Tue, 1 Nov 2016 23:02:20 +0300
Subject: [PATCH] crypto: throw error in CipherBase::SetAutoPadding

Throw error after calling CipherBase#final

PR-URL: https://github.com/nodejs/node/pull/9405
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
---
 doc/api/crypto.md                            | 12 +++++--
 src/node_crypto.cc                           |  6 +++-
 test/parallel/test-crypto-cipher-decipher.js | 36 ++++++++++++++++++++
 3 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/doc/api/crypto.md b/doc/api/crypto.md
index 9102f2e89c03c4..69ba3662645158 100644
--- 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
index 283d84f82d37c7..060cbe725faf65 100644
--- 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
index e53f02edfa0c86..dfb68a7f920844 100644
--- 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$/);
+}