Skip to content
This repository has been archived by the owner on Jan 9, 2018. It is now read-only.

Fix vulnerabilities #16

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ spec/reports
test/tmp
test/version_tmp
tmp
.idea
103 changes: 58 additions & 45 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,79 +1,92 @@
# AESCrypt - Simple AES encryption / decryption for Ruby
# AESCrypt
> Simple AES encryption/decryption for Ruby

AESCrypt is a simple to use, opinionated AES encryption / decryption Ruby gem that just works.

AESCrypt uses the AES-256-CBC cipher and encodes the encrypted data with Base64.

A corresponding gem to easily handle AES encryption / decryption in Objective-C is available at http://github.com/Gurpartap/AESCrypt-ObjC.

## Installation
This version of AESCrypt is a fork of the original, and is maintained by Charcoal. The original has not been updated
since 2013, and was suffering from a few deprecations and lack of security best practice, which this version fixes.

## Installation
Add this line to your application's Gemfile:

gem 'aescrypt'
```ruby
gem 'aescrypt', github: 'Charcoal-SE/aescrypt'
```

And then execute:

$ bundle

Or install it yourself as:

$ gem install aescrypt
$ bundle install

## Usage
**Encrypting:**
```ruby
message = 'my secret message'
password = 'h4xx0r3d'
salt, iv, encrypted = AESCrypt.encrypt(message, password)
```

message = "top secret message"
password = "p4ssw0rd"

Encrypting

encrypted_data = AESCrypt.encrypt(message, password)

Decrypting
Store `salt` and `iv` safely - you'll need them again to decrypt the encrypted text.

message = AESCrypt.decrypt(encrypted_data, password)
**Decrypting:**
```ruby
decrypted = AESCrypt.decrypt(encrypted, password, salt, iv)
```

## Advanced usage
**Note:** If you want to use SHA1 as your key derivation function, you will need to go down the
advanced usage route. AESCrypt uses SHA256 by default for better security.

Encrypting
**Encrypting:**
```ruby
message = 'my other secret'
password = 'h4xx0r3d'

encrypted_data = encrypt_data(data, key, iv, cipher_type)
# I recommend AES-256-CBC, but you can pick another cipher type/mode like this.
# This can be anything that OpenSSL::Cipher.new will accept. You'll be warned if you
# pick ECB mode.
cipher_mode = 'AES-256-CBC'
iv = OpenSSL::Random.random_bytes(16)

Decrypting
# Not all implementations support PBKDF2-HMAC with SHA256. For maximum compatibility,
# use SHA1 as your key digest. Bear in mind that SHA1 has been broken; for maximum
# security, use a stronger algorithm like SHA256.
key_digest = OpenSSL::Digest::SHA256.new
salt = OpenSSL::Random.random_bytes(32)
key = OpenSSL::PKCS5.pbkdf2_hmac(password, salt, 10000, key_digest.digest_length, key_digest)

decrypted_data = decrypt_data(encrypted_data, key, iv, cipher_type)
# Don't use the password directly as the encryption key. Generate a key as shown above.
encrypted = AESCrypt.encrypt_data(message, key, iv, cipher_mode)
```

## Corresponding usage in Objective-C
**Decrypting:**
```ruby
decrypted = AESCrypt.decrypt_data(encrypted, key, iv, cipher_mode)
```

The AESCrypt Objective-C class, available at https://github.com/Gurpartap/AESCrypt-ObjC, understands what you're talking about in your Ruby code. The purpose of the Ruby gem and Objective-C class is to have something that works out of the box across the server (Ruby) and client (Objective-C). However, a standard encryption technique is implemented, which ensures that you can handle the data with any AES compatible library available across the web. So, you're not locked-in.
## Migrating from 1.x
AESCrypt 1.x is obsolete; it hasn't been updated in a while and uses outdated security practices.
AESCrypt 2.x fixes these issues, but is not backwards-compatible with v1.x because of that.

Here's how you would use the AESCrypt Objective-C class:
To migrate encrypted data from 1.x to using 2.x, you can use the `AESCrypt::Migrator` class:

NSString *message = @"top secret message";
NSString *password = @"p4ssw0rd";
```ruby
decrypted = AESCrypt::Migrator.decrypt_from_v1(v1_encrypted_data, v1_encryption_password)
```

Encrypting

NSString *encryptedData = [AESCrypt encrypt:message password:password];

Decrypting

NSString *message = [AESCrypt decrypt:encryptedData password:password];

See the Objective-C class README at http://github.com/Gurpartap/AESCrypt-ObjC for more details.
Once you've got the cleartext back using the above snippet, you can re-encrypt it using v2.x and
save it again.

## License
Copyright (c) 2012-17 Gurpartap Singh and Charcoal.

Copyright (c) 2012 Gurpartap Singh

The encrypt_data and decrypt_data methods are Copyright (c) 2007 Brent Sowers and have been included in the gem with prior permission. Thanks Brent! :)

See LICENSE for license terms.
AESCrypt is available under the terms of the MIT license; see LICENSE for full license terms.

## Contributing
If you'd like to contribute code, go ahead and fork this repo, add your changes, and send us a
pull request. We'll review it and keep you updated.

1. Fork it
2. Create your feature branch (`git checkout -b my-new-feature`)
3. Commit your changes (`git commit -am 'Added some feature'`)
4. Push to the branch (`git push origin my-new-feature`)
5. Create new Pull Request
If you've got questions or feedback, please open a new issue on this repository.
9 changes: 5 additions & 4 deletions aescrypt.gemspec
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
# -*- encoding: utf-8 -*-

Gem::Specification.new do |gem|
gem.authors = ["Gurpartap Singh"]
gem.email = ["contact@gurpartap.com"]
gem.authors = ["Gurpartap Singh", "Charcoal"]
gem.email = ["contact@gurpartap.com", "art@charcoal-se.org"]
gem.description = "Simple AES encryption / decryption for Ruby"
gem.summary = "AESCrypt is a simple to use, opinionated AES encryption / decryption Ruby gem that just works."
gem.homepage = "http://github.com/Gurpartap/aescrypt"
gem.licenses = ['MIT']

gem.files = `git ls-files`.split("\n")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mildly prefer the git ls-files-based version, as it honours .gitignore.

gem.files = Dir.glob("lib/**/*")
gem.name = "aescrypt"
gem.require_paths = ["lib"]
gem.version = "1.0.1"
gem.version = "2.0.2"

gem.add_development_dependency "rake"
end
39 changes: 31 additions & 8 deletions lib/aescrypt.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,27 @@

require 'openssl'
require 'base64'
require 'securerandom'

require_relative 'migrator'

module AESCrypt
def self.encrypt(message, password)
Base64.encode64(self.encrypt_data(message.to_s.strip, self.key_digest(password), nil, "AES-256-CBC"))
def self.encrypt(message, password, salt = nil, iv = nil)
iv ||= SecureRandom.bytes 16
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know the SecureRandom implementation without checking. But I notice that in the README.md you suggest using OpenSSL::Random.random_bytes(16). Based on knowing OpenSSL a bit, I'd use the latter.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SecureRandom.bytes only for ruby 2.4 ?

salt, key = self.key_digest(password, salt)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment below; an HMAC (with salt) is unnecessary for generating cipher keys.

The point of a key derivation function is to expand or contract the bit length of the key to something that's the block length of cipher. If slightly different keys yield vastly different results from the key derivation function, that's also a benefit. So running the key through such a function is great!

But the use of an HMAC requires you to deal with the salt; a simple SHA2-256 for AES-256 would be sufficient and no salt would be required. You can hash for a few iterations much like the OpenSSL::PKCS5.pbkdf2_hmac function does, but that's about all that's required.

return salt, iv, Base64.encode64(self.encrypt_data(message.to_s.strip, key, iv, 'AES-256-CBC'))
end

def self.decrypt(message, password)
def self.decrypt(message, password, salt, iv)
base64_decoded = Base64.decode64(message.to_s.strip)
self.decrypt_data(base64_decoded, self.key_digest(password), nil, "AES-256-CBC")
slt, key = self.key_digest(password, salt)
self.decrypt_data(base64_decoded, key, iv, 'AES-256-CBC')
end

def self.key_digest(password)
OpenSSL::Digest::SHA256.new(password).digest
def self.key_digest(password, salt = nil)
salt ||= SecureRandom.bytes 32
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above: choose either SecureRandom or OpenSSL::Random.

digest = OpenSSL::Digest::SHA256.new
return salt, OpenSSL::PKCS5.pbkdf2_hmac(password, salt, 10000, digest.digest_length, digest)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An HMAC is a message authentication code. You probably took this from the PKCS5 example.

It's not that what you're doing is wrong, exactly, but I'm not sure it is what you need. The purposes of an HMAC is that given a shared secret (here: the salt), you can authenticate that a message (here: the password) hasn't been tampered with.

You would normally send the message and the digest across an insecure medium, relying on the HMAC (and the shared secret that's been communicated securely beforehand) to verify the message has not been tampered with.

The example also suggests to generate cipher keys this way (that's questionable), and to store passwords that way.

Storing passwords in the form of salt and HMAC works just fine. If that's your intended use, go for it.

Generating cipher keys with an HMAC is simply unnecessary, and forces you to also deal with the salt.

end

# Decrypts a block of data (encrypted_data) given an encryption key
Expand All @@ -55,7 +63,9 @@ def self.key_digest(password)
#:arg: iv => String
#:arg: cipher_type => String
def self.decrypt_data(encrypted_data, key, iv, cipher_type)
aes = OpenSSL::Cipher::Cipher.new(cipher_type)
self.warn_if_ecb cipher_type

aes = OpenSSL::Cipher.new(cipher_type)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, aes is not necessarily the right variable name, as cipher_type might specify non-AES.

aes.decrypt
aes.key = key
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps run the key derivation function (mentioned above) here, so that all uses of decrypt_data are safe, not just the simple uses. But perhaps you want the advanced functions to stay advanced. So alternatively assert that they key is non-nil and of the cipher's block length (some ciphers require different lengths).

aes.iv = iv if iv != nil
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assert that iv is not nil and of the correct length.

Expand All @@ -74,10 +84,23 @@ def self.decrypt_data(encrypted_data, key, iv, cipher_type)
#:arg: iv => String
#:arg: cipher_type => String
def self.encrypt_data(data, key, iv, cipher_type)
aes = OpenSSL::Cipher::Cipher.new(cipher_type)
self.warn_if_ecb cipher_type

aes = OpenSSL::Cipher.new(cipher_type)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, aes is not necessarily the right variable name, as cipher_type might specify non-AES.

aes.encrypt
aes.key = key
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps run the key derivation function (mentioned above) here, so that all uses of encrypt_data are safe, not just the simple uses. But perhaps you want the advanced functions to stay advanced. So alternatively assert that they key is non-nil and of the cipher's block length (some ciphers require different lengths).

aes.iv = iv if iv != nil
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assert that iv is not nil and of the correct length.

aes.update(data) + aes.final
end

private
def self.warn_if_ecb(cipher_type)
if cipher_type.downcase.include? 'ecb'
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if it includes 'aes'.

Actually, ECB mode is not a good choice with any cipher :)

warn("AESCrypt WARNING: You are using AES in ECB mode. This mode does not effectively hide patterns in " +
"plaintext. Unless you know what you're doing and are absolutely sure you need ECB mode, you should use " +
"another mode, such as CBC. See " +
"http://ruby-doc.org/stdlib-2.0.0/libdoc/openssl/rdoc/OpenSSL/Cipher.html#class-OpenSSL::Cipher-label-Choosing+an+IV " +
"for further information.")
end
end
end
69 changes: 69 additions & 0 deletions lib/migrator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
require 'openssl'

module AESCrypt
class Migrator
@@legacy = Class.new do
def self.encrypt(message, password)
warn("AESCrypt WARNING: The AESCrypt internal legacy class is not secure, and encryptions performed using " +
"this class are vulnerable to attack. You should only be using this class as part of a migration between " +
"AESCrypt versions, NOT as a permanent part of a production system.")
Base64.encode64(self.encrypt_data(message.to_s.strip, self.key_digest(password), nil, "AES-256-CBC"))
end

def self.decrypt(message, password)
warn("AESCrypt WARNING: The AESCrypt internal legacy class is not secure, and encryptions performed using " +
"this class are vulnerable to attack. You should only be using this class as part of a migration between " +
"AESCrypt versions, NOT as a permanent part of a production system.")
base64_decoded = Base64.decode64(message.to_s.strip)
self.decrypt_data(base64_decoded, self.key_digest(password), nil, "AES-256-CBC")
end

private
def self.key_digest(password)
OpenSSL::Digest::SHA256.new(password).digest
end

# Decrypts a block of data (encrypted_data) given an encryption key
# and an initialization vector (iv). Keys, iv's, and the data
# returned are all binary strings. Cipher_type should be
# "AES-256-CBC", "AES-256-ECB", or any of the cipher types
# supported by OpenSSL. Pass nil for the iv if the encryption type
# doesn't use iv's (like ECB).
#:return: => String
#:arg: encrypted_data => String
#:arg: key => String
#:arg: iv => String
#:arg: cipher_type => String
def self.decrypt_data(encrypted_data, key, iv, cipher_type)
aes = OpenSSL::Cipher::Cipher.new(cipher_type)
aes.decrypt
aes.key = key
aes.iv = iv if iv != nil
aes.update(encrypted_data) + aes.final
end

# Encrypts a block of data given an encryption key and an
# initialization vector (iv). Keys, iv's, and the data returned
# are all binary strings. Cipher_type should be "AES-256-CBC",
# "AES-256-ECB", or any of the cipher types supported by OpenSSL.
# Pass nil for the iv if the encryption type doesn't use iv's (like
# ECB).
#:return: => String
#:arg: data => String
#:arg: key => String
#:arg: iv => String
#:arg: cipher_type => String
def self.encrypt_data(data, key, iv, cipher_type)
aes = OpenSSL::Cipher::Cipher.new(cipher_type)
aes.encrypt
aes.key = key
aes.iv = iv if iv != nil
aes.update(data) + aes.final
end
end

def self.decrypt_from_v1(data, password)
@@legacy.decrypt(data, password)
end
end
end