Convert strings to binaries even if disabled or secret pending

This way the type signature does not depend on the service state and is
consistenly: `decrypt(encrypt(Data)) -> binary()`.

Some users like rabbit_shovel_parameters rely on this
(https://github.com/rabbitmq/rabbitmq-server/blob/master/deps/rabbitmq_shovel/src/rabbit_shovel_parameters.erl#L447)

Author: gomoripeti

README.md

@@ -44,7 +44,11 @@ credentials_obfuscation:decrypt(Encrypted).
 ```
 
 Lists (char lists in Elixir) will be converted to binaries before encryption.
-This means that decrypted values will also be returned as binaries.
+This means that decrypted values will alwyas be returned as binaries.
+
+(Lists hear mean "byte lists" that is unicode characters are not
+supported. But that should be no problem for encrypting for example
+URIs.)
 
 ## License and Copyright
 

src/credentials_obfuscation.erl

@@ -42,13 +42,15 @@ set_secret(Secret) when is_binary(Secret) ->
 set_fallback_secret(Secret) when is_binary(Secret) ->
     ok = credentials_obfuscation_svc:set_fallback_secret(Secret).
 
--spec encrypt(term()) -> {plaintext, term()} | {encrypted, binary()}.
+-spec encrypt(none | undefined) -> none | undefined;
+             (iodata()) -> {plaintext, binary()} | {encrypted, binary()}.
 encrypt(none) -> none;
 encrypt(undefined) -> undefined;
 encrypt(Term) ->
     credentials_obfuscation_svc:encrypt(Term).
 
--spec decrypt({plaintext, term()} | {encrypted, binary()}) -> term().
+-spec decrypt(none | undefined) -> none | undefined;
+             ({plaintext, binary()} | {encrypted, binary()}) -> binary().
 decrypt(none) -> none;
 decrypt(undefined) -> undefined;
 decrypt(Term) ->

src/credentials_obfuscation_pbe.erl

@@ -65,9 +65,9 @@ decrypt_term(Cipher, Hash, Iterations, Secret, Base64Binary) ->
 %% function accepts that same base64 binary.
 
 -spec encrypt(crypto:cipher_iv(), crypto:hash_algorithm(),
-              pos_integer(), iodata() | '$pending-secret', binary()) -> {plaintext, binary()} | {encrypted, binary()}.
+              pos_integer(), iodata() | '$pending-secret', iodata()) -> {plaintext, binary()} | {encrypted, binary()}.
 encrypt(_Cipher, _Hash, _Iterations, ?PENDING_SECRET, ClearText) ->
-    {plaintext, ClearText};
+    {plaintext, iolist_to_binary(ClearText)};
 encrypt(Cipher, Hash, Iterations, Secret, ClearText) when is_list(ClearText) ->
     encrypt(Cipher, Hash, Iterations, Secret, list_to_binary(ClearText));
 encrypt(Cipher, Hash, Iterations, Secret, ClearText) when is_binary(ClearText) ->

src/credentials_obfuscation_svc.erl

@@ -62,7 +62,7 @@ set_fallback_secret(Secret) when is_binary(Secret) ->
     gen_server:call(?MODULE, {set_fallback_secret, Secret}).
 
 
--spec encrypt(term()) -> {plaintext, term()} | {encrypted, binary()}.
+-spec encrypt(iodata()) -> {plaintext, binary()} | {encrypted, binary()} | binary().
 encrypt(Term) when is_binary(Term); is_list(Term) ->
     try
         gen_server:call(?MODULE, {encrypt, Term}, ?TIMEOUT)
@@ -72,13 +72,13 @@ encrypt(Term) when is_binary(Term); is_list(Term) ->
             %% but might be to some. There is no right or wrong answer to whether
             %% availability or security are more important, so the users have to decide
             %% whether using {plaintext, Term} results is appropriate in their specific case.
-            {plaintext, Term};
+            {plaintext, to_binary(Term)};
           _:_ ->
             %% see above
-            {plaintext, Term}
+            {plaintext, to_binary(Term)}
     end.
 
--spec decrypt({plaintext, term()} | {encrypted, binary()}) -> term().
+-spec decrypt({plaintext, binary()} | {encrypted, binary()}) -> binary().
 decrypt(Term) ->
     gen_server:call(?MODULE, {decrypt, Term}, ?TIMEOUT).
 
@@ -103,7 +103,7 @@ handle_call(refresh_config, _From, State0) ->
     {ok, State1} = refresh_config(State0),
     {reply, ok, State1};
 handle_call({encrypt, Term}, _From, #state{enabled=false}=State) ->
-    {reply, Term, State};
+    {reply, to_binary(Term), State};
 handle_call({encrypt, Term}, _From, #state{cipher=Cipher,
                                            hash=Hash,
                                            iterations=Iterations,
@@ -113,9 +113,9 @@ handle_call({encrypt, Term}, _From, #state{cipher=Cipher,
     % upon decryption.
     ClearText = {?VALUE_TAG, to_binary(Term)},
     Encrypted = credentials_obfuscation_pbe:encrypt_term(Cipher, Hash, Iterations, Secret, ClearText),
-    case Encrypted of 
-        {plaintext, _} -> 
-            {reply, {plaintext, Term}, State};
+    case Encrypted of
+        {plaintext, {?VALUE_TAG, Bin}} ->
+            {reply, {plaintext, Bin}, State};
         _ -> {reply, Encrypted, State}
     end;
 handle_call({decrypt, Term}, _From, #state{enabled=false}=State) ->

test/credentials_obfuscation_SUITE.erl

@@ -147,6 +147,12 @@ encryption_happens_only_when_secret_available(_Config) ->
     ?assertEqual({plaintext, Uri}, NotReallyEncryptedUri),
     ?assertEqual(Uri, credentials_obfuscation:decrypt(NotReallyEncryptedUri)),
 
+    %% Strings are converted to binaries even if no secret available
+    UriStr = "amqp://super:secret@localhost:5672",
+    NotReallyEncryptedUri2 = credentials_obfuscation:encrypt(UriStr),
+    ?assertEqual({plaintext, Uri}, NotReallyEncryptedUri2),
+    ?assertEqual(Uri, credentials_obfuscation:decrypt(NotReallyEncryptedUri2)),
+
     %% Start epmd
     os:cmd("epmd -daemon"),
 
@@ -181,6 +187,11 @@ disabled(_Config) ->
     Credentials = <<"guest">>,
     ?assertEqual(Credentials, credentials_obfuscation:encrypt(Credentials)),
     ?assertEqual(Credentials, credentials_obfuscation:decrypt(Credentials)),
+
+    %% Strings are converted to binaries even if no secret available
+    CredentialsStr = "guest",
+    ?assertEqual(Credentials, credentials_obfuscation:encrypt(CredentialsStr)),
+    ?assertEqual(Credentials, credentials_obfuscation:decrypt(Credentials)),
     ok.
 
 refresh_configuration(_Config) ->