Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

node_crypto: AES-256-cbc yields a partial result #1395

Closed
aklt opened this issue Jul 25, 2011 · 5 comments
Closed

node_crypto: AES-256-cbc yields a partial result #1395

aklt opened this issue Jul 25, 2011 · 5 comments
Labels

Comments

@aklt
Copy link

aklt commented Jul 25, 2011

Hi,

This is an issue that is possibly related to #738

A 32 character message is encoded producing 48 bytes.
Decoding it yields the first 16 bytes of the original message. Adding
a single character (any will do!) to the end of the the encoded 32 characters
and decoding the result yields the original 32 char message as expected.

I have written a test script to illustrate the issue. I have also tried to
reproduce the same script in perl and it behaves differently.

First of all, the perl version disallows encrypting data that does not
have a size that is a multiple of the blocksize. Node crypto does not
have this restriction.

The encrypted result of the perl version has length 32 and not 48.

And finally the perl version does decrypts the 32 characters successfully.

The scripts below illustrate the issue.

#!/usr/bin/env node

var crypto = require('crypto');

function decode(cryptkey, iv, secretdata) {
    var decipher = crypto.createDecipheriv('aes-256-cbc', cryptkey, iv),
        decoded  = decipher.update(secretdata);

    decoded += decipher.final();
    return decoded;
}

function encode(cryptkey, iv, cleardata) {
    var encipher = crypto.createCipheriv('aes-256-cbc', cryptkey, iv),
        encoded  = encipher.update(cleardata);

    encoded += encipher.final();
    return encoded;
}

var cryptkey   = crypto.createHash('sha256').update('Nixnogen').digest(),
    iv         = 'a2xhcgAAAAAAAAAA',
    buf        = "Here is some data for the coding", // 32 chars
    enc        = encode(cryptkey, iv, buf);

// But ...
var enc1       = enc.slice(0, 32),
    dec1       = decode(cryptkey, iv, enc1),        // decode 32 bytes
    dec2       = decode(cryptkey, iv, enc1 + ' ');  // decode 32 bytes + space, but any character will do.

function b64enc(data) {
    var b   = new Buffer(data, 'binary');
    return b.toString('base64');
}

console.warn("Encoded length", enc.length);
console.warn("Decode 32 bytes " + dec1);
console.warn("Decode 33 bytes " + dec2);

And the perl version:

#! /usr/bin/perl -w

use Crypt::Rijndael;
use Digest::SHA;

my $cleartext = "Here is some data for the coding"; # 32 bytes
my $iv = 'a2xhcgAAAAAAAAAA';

my $hash_1 = Digest::SHA->new(256);  # SHA256
$hash_1->add("Nixnogen");
my $cryptkey = $hash_1->digest;

sub encode { 
    my ($cryptkey, $iv, $cleartext) = @_;
    my $cipher = new Crypt::Rijndael $cryptkey, Crypt::Rijndael::MODE_CBC;

    $cipher->set_iv($iv); # we assume this will change for each cookie anyway.
    my $encrypted = $cipher->encrypt($cleartext);
    return $encrypted;
}   

sub decode {
    my ($cryptkey, $iv, $secretdata) = @_;
    my $cipher = new Crypt::Rijndael $cryptkey, Crypt::Rijndael::MODE_CBC;
    $cipher->set_iv($iv);
    my $decrypted = $cipher->decrypt($secretdata);
    return $decrypted;
}   

my $enc = encode($cryptkey, $iv, $cleartext);
my $dec = decode($cryptkey, $iv, $enc);

print "Encoded length " . length($enc) . "\n";
print "Decode 32 bytes: $dec\n"
@cesare
Copy link

cesare commented Jul 25, 2011

Hi aklt,

I'm sorry that I didn't respond to your question in the previous issue.
I examined your problem.

First of all, we should divide whole problem into two pieces, encryption and decryption.
Let's examine the first one, encryption.

I edited your original script into something like this:

#!/usr/bin/env node

var crypto = require('crypto');

function encode(cryptkey, iv, cleardata) {
    var encipher = crypto.createCipheriv('aes-256-cbc', cryptkey, iv),
        encoded  = encipher.update(cleardata);

    encoded += encipher.final();
    return encoded;
}

var cryptkey   = crypto.createHash('sha256').update('Nixnogen').digest(),
    iv         = 'a2xhcgAAAAAAAAAA',
    buf        = "Here is some data for the coding", // 32 chars
    enc        = encode(cryptkey, iv, buf);

function b64enc(data) {
    var b   = new Buffer(data, 'binary');
    return b.toString('base64');
}

console.warn("Encoded length: ", enc.length);
console.warn("Encoded in Base64:", b64enc(enc));

This script yields:

Encoded length:  48
Encoded in Base64: ux+Flze2MzL1gvrXVvhmow7SukcIrWfg7MGbWmKvLe1lDoOe5IgaKEahwh1c1Nfm

And equivalent Ruby script (I'm not familiar with Perl libraries, Sorry.)

#!/usr/bin/env ruby
# -*- coding: utf-8 -*-

require 'base64'
require 'digest'
require 'openssl'

def encode(cryptkey, iv, cleardata)
  cipher = OpenSSL::Cipher.new('AES-256-CBC')
  cipher.encrypt  # set cipher to be encryption mode
  cipher.key = cryptkey
  cipher.iv  = iv

  encrypted = ''
  encrypted << cipher.update(cleardata)
  encrypted << cipher.final
  encrypted
end

def b64enc(data)
  Base64.encode64(data).gsub(/\n/, '')
end

cryptkey = Digest::SHA256.digest('Nixnogen')
iv       = 'a2xhcgAAAAAAAAAA'
buf      = "Here is some data for the coding" # 32 chars
enc      = encode(cryptkey, iv, buf)

puts "Encoded length: #{enc.length}"
puts "Encoded in Base64: " + b64enc(enc)

yields the same results:

Encoded length: 48
Encoded in Base64: ux+Flze2MzL1gvrXVvhmow7SukcIrWfg7MGbWmKvLe1lDoOe5IgaKEahwh1c1Nfm

These two results suggest that encryption process in Node is not wrong.

Let's examine decryption. I tried decrypting whole encrypted string. Like this:

#!/usr/bin/env node

var crypto = require('crypto');

function decode(cryptkey, iv, secretdata) {
    var decipher = crypto.createDecipheriv('aes-256-cbc', cryptkey, iv),
        decoded  = decipher.update(secretdata);

    decoded += decipher.final();
    return decoded;
}

function encode(cryptkey, iv, cleardata) {
    var encipher = crypto.createCipheriv('aes-256-cbc', cryptkey, iv),
        encoded  = encipher.update(cleardata);

    encoded += encipher.final();
    return encoded;
}

var cryptkey   = crypto.createHash('sha256').update('Nixnogen').digest(),
    iv         = 'a2xhcgAAAAAAAAAA',
    buf        = "Here is some data for the coding", // 32 chars
    enc        = encode(cryptkey, iv, buf);

var dec        = decode(cryptkey, iv, enc);

function b64enc(data) {
    var b   = new Buffer(data, 'binary');
    return b.toString('base64');
}

console.warn("Encoded length: ", enc.length);
console.warn("Encoded in Base64:", b64enc(enc));
console.warn("Decoded all: " + dec);

This yields:

Encoded length:  48
Encoded in Base64: ux+Flze2MzL1gvrXVvhmow7SukcIrWfg7MGbWmKvLe1lDoOe5IgaKEahwh1c1Nfm
Decoded all: Here is some data for the coding

The decoded string is identical to the original one. Decrypting whole string also seems to be correct.

So, the problem lie in how you decrypt encrypted data partially.
I think calling final() before passing whole encrypted data to update() is wrong. Try this one:

#!/usr/bin/env node

var crypto = require('crypto');

function decodePartially(cryptkey, iv, secretdata) {
    var decipher = crypto.createDecipheriv('aes-256-cbc', cryptkey, iv);
    var decoded = ''

    console.log("**** decodePartially() start ****");

    var blockSize = 3; // change this variable to arbitrary number (which is greater than 0)

    var datalength = secretdata.length
    for (var i = 0; i < datalength; i += blockSize) {
      partialData = secretdata.slice(i, i + blockSize);
      decoded += decipher.update(partialData);
      console.log("  decoded=" + decoded);
    }
    decoded += decipher.final();
    console.log("  final: decoded=" + decoded);

    console.log("**** decodePartially() end ****");
    return decoded;
}

function encode(cryptkey, iv, cleardata) {
    var encipher = crypto.createCipheriv('aes-256-cbc', cryptkey, iv),
        encoded  = encipher.update(cleardata);

    encoded += encipher.final();
    return encoded;
}

var cryptkey   = crypto.createHash('sha256').update('Nixnogen').digest(),
    iv         = 'a2xhcgAAAAAAAAAA',
    buf        = "Here is some data for the coding", // 32 chars
    enc        = encode(cryptkey, iv, buf);

var dec        = decodePartially(cryptkey, iv, enc);

function b64enc(data) {
    var b   = new Buffer(data, 'binary');
    return b.toString('base64');
}

console.warn("Encoded length: ", enc.length);
console.warn("Encoded in Base64:", b64enc(enc));
console.warn("Decoded: " + dec);

yields something like this:

**** decodePartially() start ****
  decoded=
  decoded=
  decoded=
  decoded=
  decoded=
  decoded=Here is some dat
  decoded=Here is some dat
  decoded=Here is some dat
  decoded=Here is some dat
  decoded=Here is some dat
  decoded=Here is some data for the coding
  decoded=Here is some data for the coding
  decoded=Here is some data for the coding
  decoded=Here is some data for the coding
  decoded=Here is some data for the coding
  decoded=Here is some data for the coding
  final: decoded=Here is some data for the coding
**** decodePartially() end ****
Encoded length:  48
Encoded in Base64: ux+Flze2MzL1gvrXVvhmow7SukcIrWfg7MGbWmKvLe1lDoOe5IgaKEahwh1c1Nfm
Decoded: Here is some data for the coding

You can pass partial data to update() in arbitrary size. This method sometimes returns decrypted value, sometimes does not, depending on how much data it accumulates.
And, you should call final() at the end of the decryption. Calling this method in the midst of decrypting may yield unexpected results, because it doesn't have enough data to complete decrypting. You have to pass whole encrypted data to update() before calling final().

hope this helps.

Regards,

@aklt
Copy link
Author

aklt commented Jul 26, 2011

Hi cesare,

Thanks for your reply. It is interesting to see that ruby, which
uses openssl also produces a key that is 48 bytes.

There must be some difference between the implementation of Rijndael
which is used in the perl script and the implementation that openssl
uses since it is only the perl version that returns 32 bytes of
output and not 48 bytes as openssl does.

In your reply you emphasize that the complete encrypted data should be
passed to update before calling final() and imply that final() is called
before all of the data has been passed to update. That is actually not
what happens in the code I provided, if you look at the decode() method.

What I think is interesting is that the first 32 bytes of the 48 byte message
returned by the encode() method contain all of the information that is needed
to get back to the original message, i.e. in pseudo code:

decode(<32 bytes encrypted message> + 'Some char')  === <original message>

So the additional 16 bytes that are in the 48 byte message that decode()
returns in the node script and in your ruby script do not contain any
significant information and may as well be discarded.

This could have something to do with the blocksize used, but I am no expert
on openssl. If this is the case should it be possible to specify the blocksize
in node_crypt?

Maybe the data to encode is padded? Crypt::Rijndael disallows encryption of data
that is not a multiple of the blocksize, but openssl in the .js and .rb scripts
allows any data size. Wikipedia mentions:

...whereas Rijndael can be specified with block and key sizes in any multiple of 32 bits, with a minimum of 128 bits.

Let me know what you think,
Anders

@nahi
Copy link

nahi commented Aug 11, 2011

Hi, I'm an maintainer of CRuby and JRuby's openssl module.

The difference you're facing is from padding scheme. Symmetric block cipher needs a padding to adjust the length of plain text to fit its block length. Ruby's openssl uses standard PKCS#5 padding scheme by default, and node should behave as the same according to node_crypto.cc.

With PKCS#5 padding, every plain text is padded with at least 1 byte to fit block length. AES is a 128bit (16byte) block cipher so 31 bytes plain text is padded to 32bytes, and 32 bytes plain text is padded to 48 bytes. Here 'at least 1 byte' is important to detect the end of the plain text.

I think the Perl module you're using are configured to NO_PADDING which means user must do padding before passing it to the library. Do you understand what's happening?

Back to node topic, it would be good to have an interface to set padding parameter. Calling EVP_CIPHER_CTX_set_padding allow you to change padding scheme in OpenSSL. (easy, eh?) It should solve #361 and #1067 I guess.

@aklt
Copy link
Author

aklt commented Aug 12, 2011

Thanks for the explanation. I thought it might have been a padding issue.

+1 for your suggestion to enable chenge of padding scheme!

@sh1mmer
Copy link

sh1mmer commented Nov 3, 2011

There is already a bug for controlling cipher padding (#361).

Since this isn't a bug I'm closing. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants