-3

I'm trying to reproduce an old encryption/decryption done in Java to a new one in Ruby one because I'm rebuilding the whole app. All this to change this encryption asap, obviously.

Here is the Java code:

public class MyClass {
    private static String algo;
    private static SecretKey key;

    static {
        algo = "AES";
        String keyString = "someString";

        byte[] decodedKey = Base64.getDecoder().decode(keyString);
        key = new SecretKeySpec(decodedKey, 0, decodedKey.length, algo);
    }

    private static String decrypt(String encrypted) {
        try {
            Cipher cipher = Cipher.getInstance(algo);
            cipher.init(Cipher.DECRYPT_MODE, key);

            byte[] decodedBytes = Base64.getDecoder().decode(encrypted.getBytes());
            byte[] original = cipher.doFinal(decodedBytes);

            return new String(original);
        }
        catch (Exception e) {
            // Some error
            return "bad";
        }
    }

    private static String encrypt(String toEncrypt) {
        try {
            Cipher cipher = Cipher.getInstance(algo);
            cipher.init(Cipher.ENCRYPT_MODE, key);

            byte[] encrypted = cipher.doFinal(toEncrypt.getBytes());
            byte[] encryptedValue = Base64.getEncoder().encode(encrypted);

            return new String(encryptedValue);
        }
        catch (Exception e) {
            // Some error
            return "bad";
        }
    }
}

Java code comes from here

I have a problem with the decryption. Here is my Ruby code:

key = Digest::SHA256.digest(key)
aes = OpenSSL::Cipher.new('AES-256-CBC')
aes.decrypt
aes.key = Digest::SHA256.digest(key)
aes.update(secretdata) + aes.final

# => OpenSSL::Cipher::CipherError: bad decrypt

What am I doing wrong?

halfer
  • 19,824
  • 17
  • 99
  • 186
brcebn
  • 1,571
  • 1
  • 23
  • 46
  • That one implicitly uses ECB mode, and it didn't receive 6 downvotes for nothing. The upvotes are probably from people that copy security related code willy-nilly. Like you are doing now. – Maarten Bodewes Feb 26 '18 at 12:04
  • @MaartenBodewes I've just taken this code. It's not from me. I'm just trying to understand what's the magical part that I don't get. I get that this encryption is not usable and the previous dev didn't really know what he was doing. But I'm currently stuck with this language and this encryption. First step is the reproduce the encryption in a more friendly usable one. Seconde is to change it. – brcebn Feb 26 '18 at 12:14
  • 1
    OK, I'd definitely try `"AES-256-ECB"` in that case. ECB is the insecure default for the `"AES"` cipher - for the Sun/Oracle provider anyway. For all other modes such as CBC or GCM you would need an IV. – Maarten Bodewes Feb 26 '18 at 12:20
  • Does that work at all? I hope that ECB is available for AES... – Maarten Bodewes Feb 26 '18 at 12:30
  • This is currently working but I don't really know how. I've tried with `AES-256-ECB` but on `decipher.final` I still have `OpenSSL::Cipher::CipherError: bad decrypt`. – brcebn Feb 26 '18 at 12:37
  • 1
    Maybe because you're hashing the key twice? Without the exact Java code we cannot tell. – Maarten Bodewes Feb 26 '18 at 13:28
  • I've tried both with and without the double hash. Not working either. I've added the java code to the main subject. – brcebn Feb 26 '18 at 14:29
  • 1
    You should really be more careful when creating the question. Now you've left out the class and constructor, while that's where the algo & key are determined. This is why creating an MCVE is so important. – Maarten Bodewes Feb 26 '18 at 15:17
  • I get it. I've changed it with as much details as I can. Now waiting for reopening.. Thanks anyway! – brcebn Feb 26 '18 at 15:24
  • @MaartenBodewes I finally got to solution. I cannot add it as answer for now. I was close and you got the key with "ECB". Thanks! – brcebn Feb 26 '18 at 15:37
  • Ah, base 64 decoding instead of hashing + ECB. – Maarten Bodewes Feb 26 '18 at 17:30
  • @Maarten: this is now open again, so you can post an answer if you wish to. – halfer Feb 26 '18 at 23:31

2 Answers2

1

It was not that easy but here is my way to do it.

class ManualEncryption
  class << self
    attr_accessor :configuration

    def config
      @configuration ||= Configuration.new
    end

    def aes
      return @aes if @aes
      @aes = OpenSSL::Cipher.new(config.algo) # "AES-256-ECB"
      @aes
    end

    def decodedKey
      return @decodedKey if @decodedKey
      @decodedKey = Base64.decode64(config.key_string) # "mySecretString"
    end

    def configure
      yield(config)
      raise 'Algo not specified' unless config.algo
      raise 'key not specified' unless config.key_string
    end

    def encrypt(value)
      raise 'No configuration done' unless config.algo && config.key_string
      aes_perform('encrypt', value)
    end

    def decrypt(value)
      raise 'No configuration done' unless config.algo && config.key_string
      return value unless value
      aes_perform('decrypt', value)
    end

    def aes_perform(status, value)
      aes.reset
      if status.eql?('encrypt')
        aes.encrypt
        aes.key = decodedKey
        aes_val = aes.update(value) + aes.final
        Base64::encode64(aes_val)
      else
        aes.decrypt
        aes.key = decodedKey
        decoded_value = Base64::decode64(value)
        aes.update(decoded_value) + aes.final
      end
    end
  end

  class Configuration
    attr_accessor :algo, :key_string
    attr_reader :aes
  end
end

Note: I still have a problem with the encryption. It creates \n inside my encrypted value and I don't know why. I'm working on it.

brcebn
  • 1,571
  • 1
  • 23
  • 46
1

The "AES" algorithm description isn't complete; it will use a default mode of operation and padding scheme. In the provider included in the JRE this will default to ECB and PKCS#7 padding ("AES/ECB/PKCS5Padding"). This is obviously insecure as ECB is insecure, but due to applications relying on this default it cannot be changed (which is one of the reasons why having defaults was a mistake in the first place).

Furthermore, in the code you've provided there is no hashing involved. This is a good thing as a single secure hash over a key is not enough to provide a good amount of security. Storing a key in a string is almost as bad, but not quite. Instead however the key is base 64 encoded.

So you have to switch to 'AES-256-ECB' and remove the double hashing of the key, replacing it with base 64 decoding instead.

Maarten Bodewes
  • 90,524
  • 13
  • 150
  • 263