Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AES Key Wrap Segfault #15009

Closed
EternalDeiwos opened this issue Aug 24, 2017 · 4 comments
Closed

AES Key Wrap Segfault #15009

EternalDeiwos opened this issue Aug 24, 2017 · 4 comments
Labels
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem. openssl Issues and PRs related to the OpenSSL dependency.

Comments

@EternalDeiwos
Copy link

EternalDeiwos commented Aug 24, 2017

CC @thelunararmy

  • Version: 6.11.2
  • Platform: Linux PCName 4.8.0-56-generic # 61~16.04.1-Ubuntu SMP Wed Jun 14 11:58:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
  • Subsystem: crypto -> ciphers -> id-aesXXX-wrap

We're trying to do aes key wrapping for our nodejs webcrypto implementation. We were having some trouble so we made the following test script (using data from RFC3394):

'use strict'

const crypto = require('crypto')

let cipherName = 'id-aes128-wrap'

// Ripped from RFC3394
let kekHex = '000102030405060708090A0B0C0D0E0F' // for 256: '000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F'
let keyHex = '00112233445566778899AABBCCDDEEFF' // for 256: '00112233445566778899AABBCCDDEEFF000102030405060708090A0B0C0D0E0F'
let ivHex = 'A6A6A6A6A6A6A6A6'

let iv = Buffer.from(ivHex, 'hex')
let kek = Buffer.from(kekHex, 'hex')
let key = Buffer.from(keyHex, 'hex')

try {
  let cipher = crypto.createCipheriv(cipherName, kek, iv)
  let result = cipher.update(key)
  let final = cipher.final()

  // TODO process result

  console.log('RESULT', result, final)
} catch (error) {
  console.error('ERROR', error)
}

Running this script causes node to segfault. Any ideas?

@joyeecheung joyeecheung added crypto Issues and PRs related to the crypto subsystem. openssl Issues and PRs related to the OpenSSL dependency. labels Aug 24, 2017
@joyeecheung
Copy link
Member

joyeecheung commented Aug 24, 2017

I can reproduce this with v8.4.0.

Process 14998 stopped
* thread #1, name = 'node', stop reason = signal SIGSEGV: invalid address (fault address: 0x24dc000)
    frame #0: 0x0000000000a7cbe0 node`_x86_64_AES_encrypt_compact + 48
node`_x86_64_AES_encrypt_compact:
->  0xa7cbe0 <+48>: xorl   (%r15), %eax
    0xa7cbe3 <+51>: xorl   0x4(%r15), %ebx
    0xa7cbe7 <+55>: xorl   0x8(%r15), %ecx
    0xa7cbeb <+59>: xorl   0xc(%r15), %edx
(lldb) register read r15
     r15 = 0x00000000024dc000
(lldb) memory read 0x00000000024dc000
error: memory read failed for 0x24dc000

@jasnell jasnell added the confirmed-bug Issues with confirmed bugs. label Aug 24, 2017
@jasnell
Copy link
Member

jasnell commented Aug 24, 2017

@nodejs/crypto

@shigeki
Copy link
Contributor

shigeki commented Aug 24, 2017

The following patch would fix this. I'll submit a PR with tests tomorrow.

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 1fa522d..e8391dc 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -3396,13 +3396,18 @@ void CipherBase::InitIv(const char* cipher_type,
   }

   const int expected_iv_len = EVP_CIPHER_iv_length(cipher);
-  const bool is_gcm_mode = (EVP_CIPH_GCM_MODE == EVP_CIPHER_mode(cipher));
+  const int mode = EVP_CIPHER_mode(cipher);
+  const bool is_gcm_mode = (EVP_CIPH_GCM_MODE == mode);

   if (is_gcm_mode == false && iv_len != expected_iv_len) {
     return env()->ThrowError("Invalid IV length");
   }

   EVP_CIPHER_CTX_init(&ctx_);
+
+  if (mode == EVP_CIPH_WRAP_MODE)
+    EVP_CIPHER_CTX_set_flags(&ctx_, EVP_CIPHER_CTX_FLAG_WRAP_ALLOW);
+
   const bool encrypt = (kind_ == kCipher);
   EVP_CipherInit_ex(&ctx_, cipher, nullptr, nullptr, nullptr, encrypt);
$ ./node ~/test_aes_wrap.js
RESULT <Buffer 1f a6 8b 0a 81 12 b4 47 ae f3 4b d8 fb 5a 7b 82 9d 3e 86 23 71 d2 cf e5> <Buffer >

@EternalDeiwos
Copy link
Author

@shigeki thanks for the fix 👍

ghost pushed a commit to ayojs/ayo that referenced this issue Aug 30, 2017
EVP_CIPHER_CTX_FLAG_WRAP_ALLOW flag needs to be set in using wrap mode
ciphers. In `crypto.createCipher()`, AES key wrap mode does not use a
default IV defined in RFC3394 but a generated IV with
`EVP_BytesToKey()` to be consistent API behaviors with other ciphers.

The built-in AES wrap mode in OpenSSL is not supported in FIPS mode as
http://openssl.6102.n7.nabble.com/AES-Key-Wrap-in-FIPS-Mode-td50238.html
so its tests in FIPS mode are skipped.

Fixes: nodejs/node#15009
PR-URL: nodejs/node#15037
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
ghost pushed a commit to ayojs/ayo that referenced this issue Aug 30, 2017
EVP_CIPHER_CTX_FLAG_WRAP_ALLOW flag needs to be set in using wrap mode
ciphers. In `crypto.createCipher()`, AES key wrap mode does not use a
default IV defined in RFC3394 but a generated IV with
`EVP_BytesToKey()` to be consistent API behaviors with other ciphers.

The built-in AES wrap mode in OpenSSL is not supported in FIPS mode as
http://openssl.6102.n7.nabble.com/AES-Key-Wrap-in-FIPS-Mode-td50238.html
so its tests in FIPS mode are skipped.

Fixes: nodejs/node#15009
PR-URL: nodejs/node#15037
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
cjihrig pushed a commit to cjihrig/node that referenced this issue Aug 31, 2017
EVP_CIPHER_CTX_FLAG_WRAP_ALLOW flag needs to be set in using wrap mode
ciphers. In `crypto.createCipher()`, AES key wrap mode does not use a
default IV defined in RFC3394 but a generated IV with
`EVP_BytesToKey()` to be consistent API behaviors with other ciphers.

The built-in AES wrap mode in OpenSSL is not supported in FIPS mode as
http://openssl.6102.n7.nabble.com/AES-Key-Wrap-in-FIPS-Mode-td50238.html
so its tests in FIPS mode are skipped.

Fixes: nodejs#15009
PR-URL: nodejs#15037
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Sep 10, 2017
EVP_CIPHER_CTX_FLAG_WRAP_ALLOW flag needs to be set in using wrap mode
ciphers. In `crypto.createCipher()`, AES key wrap mode does not use a
default IV defined in RFC3394 but a generated IV with
`EVP_BytesToKey()` to be consistent API behaviors with other ciphers.

The built-in AES wrap mode in OpenSSL is not supported in FIPS mode as
http://openssl.6102.n7.nabble.com/AES-Key-Wrap-in-FIPS-Mode-td50238.html
so its tests in FIPS mode are skipped.

Fixes: #15009
PR-URL: #15037
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Sep 11, 2017
EVP_CIPHER_CTX_FLAG_WRAP_ALLOW flag needs to be set in using wrap mode
ciphers. In `crypto.createCipher()`, AES key wrap mode does not use a
default IV defined in RFC3394 but a generated IV with
`EVP_BytesToKey()` to be consistent API behaviors with other ciphers.

The built-in AES wrap mode in OpenSSL is not supported in FIPS mode as
http://openssl.6102.n7.nabble.com/AES-Key-Wrap-in-FIPS-Mode-td50238.html
so its tests in FIPS mode are skipped.

Fixes: #15009
PR-URL: #15037
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Sep 11, 2017
EVP_CIPHER_CTX_FLAG_WRAP_ALLOW flag needs to be set in using wrap mode
ciphers. In `crypto.createCipher()`, AES key wrap mode does not use a
default IV defined in RFC3394 but a generated IV with
`EVP_BytesToKey()` to be consistent API behaviors with other ciphers.

The built-in AES wrap mode in OpenSSL is not supported in FIPS mode as
http://openssl.6102.n7.nabble.com/AES-Key-Wrap-in-FIPS-Mode-td50238.html
so its tests in FIPS mode are skipped.

Fixes: #15009
PR-URL: #15037
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Sep 12, 2017
EVP_CIPHER_CTX_FLAG_WRAP_ALLOW flag needs to be set in using wrap mode
ciphers. In `crypto.createCipher()`, AES key wrap mode does not use a
default IV defined in RFC3394 but a generated IV with
`EVP_BytesToKey()` to be consistent API behaviors with other ciphers.

The built-in AES wrap mode in OpenSSL is not supported in FIPS mode as
http://openssl.6102.n7.nabble.com/AES-Key-Wrap-in-FIPS-Mode-td50238.html
so its tests in FIPS mode are skipped.

Fixes: #15009
PR-URL: #15037
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
bnoordhuis pushed a commit to bnoordhuis/io.js that referenced this issue Oct 29, 2017
EVP_CIPHER_CTX_FLAG_WRAP_ALLOW flag needs to be set in using wrap mode
ciphers. In `crypto.createCipher()`, AES key wrap mode does not use a
default IV defined in RFC3394 but a generated IV with
`EVP_BytesToKey()` to be consistent API behaviors with other ciphers.

The built-in AES wrap mode in OpenSSL is not supported in FIPS mode as
http://openssl.6102.n7.nabble.com/AES-Key-Wrap-in-FIPS-Mode-td50238.html
so its tests in FIPS mode are skipped.

Fixes: nodejs#15009
PR-URL: nodejs#15037
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Nov 14, 2017
EVP_CIPHER_CTX_FLAG_WRAP_ALLOW flag needs to be set in using wrap mode
ciphers. In `crypto.createCipher()`, AES key wrap mode does not use a
default IV defined in RFC3394 but a generated IV with
`EVP_BytesToKey()` to be consistent API behaviors with other ciphers.

The built-in AES wrap mode in OpenSSL is not supported in FIPS mode as
http://openssl.6102.n7.nabble.com/AES-Key-Wrap-in-FIPS-Mode-td50238.html
so its tests in FIPS mode are skipped.

Backport-PR-URL: #16584
Fixes: #15009
PR-URL: #15037
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Nov 21, 2017
EVP_CIPHER_CTX_FLAG_WRAP_ALLOW flag needs to be set in using wrap mode
ciphers. In `crypto.createCipher()`, AES key wrap mode does not use a
default IV defined in RFC3394 but a generated IV with
`EVP_BytesToKey()` to be consistent API behaviors with other ciphers.

The built-in AES wrap mode in OpenSSL is not supported in FIPS mode as
http://openssl.6102.n7.nabble.com/AES-Key-Wrap-in-FIPS-Mode-td50238.html
so its tests in FIPS mode are skipped.

Backport-PR-URL: #16584
Fixes: #15009
PR-URL: #15037
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Nov 28, 2017
EVP_CIPHER_CTX_FLAG_WRAP_ALLOW flag needs to be set in using wrap mode
ciphers. In `crypto.createCipher()`, AES key wrap mode does not use a
default IV defined in RFC3394 but a generated IV with
`EVP_BytesToKey()` to be consistent API behaviors with other ciphers.

The built-in AES wrap mode in OpenSSL is not supported in FIPS mode as
http://openssl.6102.n7.nabble.com/AES-Key-Wrap-in-FIPS-Mode-td50238.html
so its tests in FIPS mode are skipped.

Backport-PR-URL: #16584
Fixes: #15009
PR-URL: #15037
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem. openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants