Padding differences against MRI for cipher modes such as OFB

[iamasmith]

I'm working on an RPC interface at the moment with the POC piece in Ruby, but I wrote a Java client.

I found that using bf-ofb cipher that MRI Ruby used no padding, not going to get into it should or it shouldn't but the current default for MRI is not to pad with the bf-ofb Cipher, hence my JCE implementation uses Cipher.getInstance("Blowfish/OFB64/NoPadding") and this works quite happily with MRI Ruby on default settings and no cipher.padding = 0 added.

The documentation is a little vague but the behaviour is definitely there that padding isn't added by MRI but on JRuby it seems to default the other way.

I did find an artical, way back (can't find it now) that states that OFB shouldn't use padding but I'm not going to get into that since this seems to be the behaviour for MRI Ruby.

I've just been more explicit in my code and added cipher.padding = 0 to my code but if you are interested, for these modes, theres a subtle different.

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/6383790-padding-differences-against-mri-for-cipher-modes-such-as-ofb?utm_campaign=plugin&utm_content=tracker%2F136995&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F136995&utm_medium=issues&utm_source=github).

[kares]

I recall I was looking into this ~ two months back ... not sure I covered (all of) these, could you be more specific in terms on Ruby OpenSSL code ? ... also try out a snapshot of a gem from: https://oss.sonatype.org/content/repositories/snapshots/rubygems/jruby-openssl/ ... although it's a bit outdated (still should include most of the related changes) but I'm sure @mkristian will push us another or let me know if I can ... thanks!

[mkristian]

just pushed a new snapshot. @kares you can push a snapshot anytime -
sonatype will cleanup things after some time.

[iamasmith]

I tried the version from this snapshot. It seems to produce the same padding inconsistencies. If I turn padding off explicitly it does work so it's really just about the consistent default with MRI.

[kares]

@iamasmith thanks ... we would appreciate concrete code samples (cipher names) that are inconsistent

[iamasmith]

Sure, here's the POC Blowfish wrapper where I discovered the inconsistency, it's part of a closed network RPC transport that I developed so it's designed to process presentation protocol frames and is paired against each session.

The module provides a pluggable abstraction of the encryption setup and use to my RPC pieces so I can tell my RPC to use any encryption engine.

Points of note, resetting the Cipher I found necessary when interfacing this to a Java client class that does the same thing since java's doFinal() resets the Cipher, that's more around Ruby to Java differences, is O/T but I thought I would explain that reset.

The critical thing to make this work between Ruby and JRuby in test was to have the .padding = 0 for the Cipher since Ruby assumes padding off for Blowfish OFB and JRuby assumes it on.

# Blowfish cryptography wrapper.

require 'openssl'
require 'securerandom'

class BlowfishCrypto

  def initialize(key)
    @key = key
    @c_enc = nil
    @c_dec = nil
  end

  # Produces encrypted serial stream as string
  def encrypt(data)
    begin
      # On first encrypt we generate the salt, cipher and key and we send the salt
      salt = ''
      unless @c_enc
        salt = SecureRandom.random_bytes(8)
        @c_enc = OpenSSL::Cipher::Cipher.new('bf-ofb').send(:encrypt)
        # Padding is NOT significant for MRI Ruby, it is always turn off for OFB
        # It IS significant is using JRuby which will default it to be on.
        @c_enc.padding = 0
        @c_enc.pkcs5_keyivgen(@key,salt)
      end
      # Cipher must be reset because final doesn't reset it
      @c_enc.reset
      salt + @c_enc.update(data) << @c_enc.final
    rescue OpenSSL::Cipher::CipherError
      nil
    rescue TypeError
      nil
    end
  end

  # Decrypts data with passed key, traps and returns nil if key could not be used
  def decrypt(data)
    begin
      unless @c_dec
        salt, data = data.unpack('a8a*')
        @c_dec = OpenSSL::Cipher::Cipher.new('bf-ofb').send(:decrypt)
        @c_dec.padding = 0
        @c_dec.pkcs5_keyivgen(@key,salt)
      end
      @c_dec.reset
      @c_dec.update(data) << @c_dec.final
    rescue OpenSSL::Cipher::CipherError => e
      nil
    rescue TypeError => e
      nil
    end
  end

end

[dgolombek]

I just ran into this as well. I believe the issue is that OFB, CFB, CFB8, and CTR modes should use padding type of NoPadding. There's already a TODO to this effect at https://github.com/jruby/jruby-openssl/blob/master/src/main/java/org/jruby/ext/openssl/Cipher.java#L559-L562

I believe that ECB SHOULD have padding, despite the comment there already.

[dgolombek]

I opened a bug with BouncyCastle discussing this as well (bcgit/bc-java#291), but I believe the true failure is here in JRuby OpenSSL. Note that this is a incompatibility with MRI -- there's no way in MRI to add padding to CFB/OFB/CTR modes, since OpenSSL won't do so even if asked (cipher.set_padding = 1).

I'm happy to work on a fix if you agree with this.

[kares]

@dgolombek makes sense - feel free to go for it