alertlogic / erlcloud

***Please submit all new work to upstream***
https://github.com/erlcloud/erlcloud
Other
5 stars 16 forks source link

AWS KMS is broken #101

Closed Lapinskas closed 8 years ago

Lapinskas commented 8 years ago

Hi!

erlcloud_kms is broken as will be shown below. Most probably - due to incorrect conversion to/from JSON with the help of JSX application.

Here is a simple test results:

1> ssl:start(), erlcloud:start().
ok
2> test_encryption:test("This is a test").
{{"test passed:",false},
 {"original text:","This is a test"},
 {"text after encryption/decryption operations:",
  <<"This/is/a/testte">>}}

As you can see, there are two problems:

  1. Non-alphanum characters are corrupted (replaced by slash)
  2. Extra characters are added at the end of the string

For your convenience, below is a simple code to reproduce the error. Please note: encryption key ID should be placed as environmental value AWS_ENCRYPT_KEY_ID


-module(test_encryption).

-export([
        encrypt/1,
    decrypt/1,
    test/1
    ]).

test(Data) ->
    Chipher = encrypt(Data),
    Plain = decrypt(Chipher),
    {
    {"test passed:",string:equal(Data,Plain)},
    {"original text:",Data},
    {"text after encryption/decryption operations:",Plain}
    }.

encrypt (Data) ->
    KEYID   = os:getenv("AWS_ENCRYPT_KEY_ID"),
    {ok,Result} = erlcloud_kms:encrypt(
    list_to_binary([KEYID]),
    list_to_binary([Data])
    ),
    [{_,Chipher},_] = Result,
    Chipher.

decrypt (Chipher) ->
    {ok,Result} = erlcloud_kms:decrypt(
    list_to_binary([Chipher])
    ),
    [_,{_,Plain}] = Result,
    Plain.
motobob commented 8 years ago

@Lapinskas thank you for reporting the issue. we will take a look into . just in case which erlang version are you using?

Lapinskas commented 8 years ago

@motobob , thank you! I'm using Erlang 18 on MacBook and Erlang 17.5 on Desktop PC, both have the same problem.

Lapinskas commented 8 years ago

@motobob , do you have any news regarding this issue? I appreciate your feedback!

zacharyfox commented 8 years ago

For plain strings - it seems you need to base64 encode before encrypting. I've been trying to find reference for this in the AWS docs, but I haven't been able to. I did not include this in the library when I implemented it because I didn't want to encode data that was already binary in nature.

If your encrypt function encodes before encrypting and your decrypt decodes after, it will return the correct string.

In the absence of any documentation from AWS about this, I would be hesitant to include it automatically in the base library.

Lapinskas commented 8 years ago

Thank you, @zacharyfox , with base64 encode/decode it works fine.

However, you're not right that binary data does not need encoding - here you are an example with my little test application:

test_encryption:test(<<"\0This is a test binary data\n\r">>).
{{"test passed:",false},
 {"original text:",
  <<0,84,104,105,115,32,105,115,32,97,32,116,101,115,116,
    32,98,105,110,97,114,121,32,100,97,...>>},
 {"text after encryption/decryption operations:",
  <<"/This/is/a/test/binary/data/">>}}

As you can see, binary \0 \n \r and even space are replaced by slash. However, if I encode binary data BEFORE encryption and decode AFTER encryption, it works fine!

So, I believe your approach is correct, but please mention in documentation or in code that function expects Base24 encoded data.

Thank you for the support and KMS implementation! :)