From f5f9e8da7167d996b889a2265fed4e7ea1bf3190 Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Mon, 11 Feb 2013 21:22:34 +0100
Subject: [PATCH] crypto: improve cipher/decipher error messages

Throw the OpenSSL error string instead of the rather less informative
error message "fail".
---
 src/node_crypto.cc         | 59 +++++++++++++++++---------------------
 test/simple/test-crypto.js |  5 ++++
 2 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 9cbc577b5..e861bb34f 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -119,6 +119,22 @@ static void crypto_lock_cb(int mode, int n, const char* file, int line) {
 }
 
 
+Handle<Value> ThrowCryptoErrorHelper(unsigned long err, bool is_type_error) {
+  HandleScope scope;
+  char errmsg[128];
+  ERR_error_string_n(err, errmsg, sizeof(errmsg));
+  return is_type_error ? ThrowTypeError(errmsg) : ThrowError(errmsg);
+}
+
+
+Handle<Value> ThrowCryptoError(unsigned long err) {
+  return ThrowCryptoErrorHelper(err, false);
+}
+
+
+Handle<Value> ThrowCryptoTypeError(unsigned long err) {
+  return ThrowCryptoErrorHelper(err, true);
+}
 
 
 void SecureContext::Initialize(Handle<Object> target) {
@@ -347,9 +363,7 @@ Handle<Value> SecureContext::SetKey(const Arguments& args) {
       return ThrowException(Exception::Error(
           String::New("PEM_read_bio_PrivateKey")));
     }
-    char string[120];
-    ERR_error_string_n(err, string, sizeof string);
-    return ThrowException(Exception::Error(String::New(string)));
+    return ThrowCryptoError(err);
   }
 
   SSL_CTX_use_PrivateKey(sc->ctx_, key);
@@ -449,9 +463,7 @@ Handle<Value> SecureContext::SetCert(const Arguments& args) {
       return ThrowException(Exception::Error(
           String::New("SSL_CTX_use_certificate_chain")));
     }
-    char string[120];
-    ERR_error_string_n(err, string, sizeof string);
-    return ThrowException(Exception::Error(String::New(string)));
+    return ThrowCryptoError(err);
   }
 
   return True(node_isolate);
@@ -2095,9 +2107,7 @@ class Cipher : public ObjectWrap {
     if (!initialised_) return 0;
     *out_len=len+EVP_CIPHER_CTX_block_size(&ctx);
     *out= new unsigned char[*out_len];
-
-    EVP_CipherUpdate(&ctx, *out, out_len, (unsigned char*)data, len);
-    return 1;
+    return EVP_CipherUpdate(&ctx, *out, out_len, (unsigned char*)data, len);
   }
 
   int SetAutoPadding(bool auto_padding) {
@@ -2156,9 +2166,7 @@ class Cipher : public ObjectWrap {
 
     delete [] key_buf;
 
-    if (!r) {
-      return ThrowException(Exception::Error(String::New("CipherInit error")));
-    }
+    if (!r) return ThrowCryptoError(ERR_get_error());
 
     return args.This();
   }
@@ -2210,9 +2218,7 @@ class Cipher : public ObjectWrap {
     delete [] key_buf;
     delete [] iv_buf;
 
-    if (!r) {
-      return ThrowException(Exception::Error(String::New("CipherInitIv error")));
-    }
+    if (!r) return ThrowCryptoError(ERR_get_error());
 
     return args.This();
   }
@@ -2231,10 +2237,9 @@ class Cipher : public ObjectWrap {
 
     r = cipher->CipherUpdate(buffer_data, buffer_length, &out, &out_len);
 
-    if (!r) {
+    if (r == 0) {
       delete [] out;
-      Local<Value> exception = Exception::TypeError(String::New("DecipherUpdate fail"));
-      return ThrowException(exception);
+      return ThrowCryptoTypeError(ERR_get_error());
     }
 
     Local<Value> outString;
@@ -2272,11 +2277,7 @@ class Cipher : public ObjectWrap {
       // out_value always get allocated.
       delete[] out_value;
       out_value = NULL;
-      if (r == 0) {
-        Local<Value> exception = Exception::TypeError(
-          String::New("CipherFinal fail"));
-        return ThrowException(exception);
-      }
+      if (r == 0) return ThrowCryptoTypeError(ERR_get_error());
     }
 
     outString = Encode(out_value, out_len, BUFFER);
@@ -2400,8 +2401,7 @@ class Decipher : public ObjectWrap {
     *out_len=len+EVP_CIPHER_CTX_block_size(&ctx);
     *out= new unsigned char[*out_len];
 
-    EVP_CipherUpdate(&ctx, *out, out_len, (unsigned char*)data, len);
-    return 1;
+    return EVP_CipherUpdate(&ctx, *out, out_len, (unsigned char*)data, len);
   }
 
   int SetAutoPadding(bool auto_padding) {
@@ -2551,8 +2551,7 @@ class Decipher : public ObjectWrap {
 
     if (!r) {
       delete [] out;
-      Local<Value> exception = Exception::TypeError(String::New("DecipherUpdate fail"));
-      return ThrowException(exception);
+      return ThrowCryptoTypeError(ERR_get_error());
     }
 
     Local<Value> outString;
@@ -2591,11 +2590,7 @@ class Decipher : public ObjectWrap {
     if (out_len == 0 || r == 0) {
       delete [] out_value; // allocated even if out_len == 0
       out_value = NULL;
-      if (r == 0) {
-        Local<Value> exception = Exception::TypeError(
-          String::New("DecipherFinal fail"));
-        return ThrowException(exception);
-      }
+      if (r == 0) return ThrowCryptoTypeError(ERR_get_error());
     }
 
     outString = Encode(out_value, out_len, BUFFER);
diff --git a/test/simple/test-crypto.js b/test/simple/test-crypto.js
index 8f5043ef7..a0aa6bd14 100644
--- a/test/simple/test-crypto.js
+++ b/test/simple/test-crypto.js
@@ -797,3 +797,8 @@ assert.notEqual(-1, crypto.getHashes().indexOf('sha'));
 assert.equal(-1, crypto.getHashes().indexOf('SHA1'));
 assert.equal(-1, crypto.getHashes().indexOf('SHA'));
 assertSorted(crypto.getHashes());
+
+(function() {
+  var c = crypto.createDecipher('aes-128-ecb', '');
+  assert.throws(function() { c.final('utf8') }, /invalid public key/);
+})();