amend cryptosystem: take two

README.md

@@ -9,23 +9,28 @@ Usage
 A typical use case is to provide means to keep secrets put in public domain, e.g. secure cookies.
 
 ```erlang
+% generate a token
 Term = {this, is, an, [erlang, <<"term">>]}.
+Cookie = termit:encode_base64(Term, <<"cekpet">>).
+
+% token is ok
+{ok, Term} = termit:decode_base64(Cookie, <<"cekpet">>).
+
+% check whether token is not forged
+{error, forged} = termit:decode_base64(<<Cookie/binary, "1">>, <<"cekpet">>).
+{error, forged} = termit:decode_base64(Cookie, <<"secret">>).
+{error, forged} = termit:decode_base64(undefined, <<"cekpet">>).
+
+% generate expiring token
+Term = {this, is, another, [erlang, <<"term">>]}.
 % time-to-live is 10 seconds =:= secret valid no more than 10 seconds
 Cookie = termit:encode_base64(Term, <<"cekpet">>, 10).
 
-% secret is alive
-{ok, Term} = termit:decode_base64(Cookie, <<"cekpet">>, 10).
-
-% secret's time-to-live must fit
-{error, forged} = termit:decode_base64(Cookie, <<"cekpet">>, 11).
+% secret is ok within 10 seconds interval
+{ok, Term} = termit:decode_base64(Cookie, <<"cekpet">>).
 
 % after 10 seconds elapsed
-{error, expired} = termit:decode_base64(Cookie, <<"cekpet">>, 10).
-
-% check whether secret was not forged
-{error, forged} = termit:decode_base64(<<Cookie/binary, "1">>, <<"cekpet">>, 10).
-{error, forged} = termit:decode_base64(Cookie, <<"secret">>, 10).
-{error, forged} = termit:decode_base64(undefined, <<"cekpet">>, 10).
+{error, expired} = termit:decode_base64(Cookie, <<"cekpet">>).
 ```
 
 [License](termit/blob/master/LICENSE.txt)

src/termit.erl

@@ -1,14 +1,21 @@
 %%
 %% @doc Serialize an Erlang term to signed encrypted binary and
-%% deserialize it back ensuring it's not been forged or expired.
+%% deserialize it back ensuring it's not been forged.
+%%
+%% Some code extracted from
+%%   https://github.com/mochi/mochiweb/blob/master/src/mochiweb_session.erl.
 %%
 
 -module(termit).
 -author('Vladimir Dronnikov <[email protected]>').
 
 -export([
-    encode/3, decode/3,
-    encode_base64/3, decode_base64/3
+    decode/2,
+    decode_base64/2,
+    encode/2,
+    encode/3,
+    encode_base64/2,
+    encode_base64/3
   ]).
 
 %%
@@ -18,158 +25,128 @@
 %% -----------------------------------------------------------------------------
 %%
 
--spec encode(
-    Term :: any(),
-    Secret :: binary(),
-    Ttl :: non_neg_integer()) ->
-  Cipher :: binary().
+-spec encode(Term :: any(), Secret :: binary()) -> Cipher :: binary().
+encode(Term, Secret) ->
+  Key = key(Secret),
+  Enc = encrypt(term_to_binary(Term), Key),
+  << (sign(<< Key/binary, Enc/binary >>, Key))/binary, Enc/binary >>.
 
+-spec encode(Term :: any(), Secret :: binary(),
+    Ttl :: non_neg_integer()) -> Cipher :: binary().
 encode(Term, Secret, Ttl) ->
-  ExpiresAt = timestamp(Ttl),
-  ExpiresAtBin = list_to_binary(integer_to_list(ExpiresAt)),
-  Key = key(Secret, Ttl),
-  Enc = encrypt(term_to_binary(Term), Key),
-  Sig = sign(<< ExpiresAtBin/binary, Key/binary, Enc/binary >>, Secret),
-  << Sig/binary, ExpiresAt:32/integer, Enc/binary >>.
+  Key = key(Secret),
+  Enc = encrypt(term_to_binary(expiring(Term, Ttl)), Key),
+  << (sign(<< Key/binary, Enc/binary >>, Key))/binary, Enc/binary >>.
 
 %%
 %% -----------------------------------------------------------------------------
-%% @doc Given a result of encode/3, i.e. a signed encrypted binary,
+%% @doc Given a result of encode/2, i.e. a signed encrypted binary,
 %% check the signature, uncrypt and deserialize into original term.
-%% Check it timestamp encoded into the data is not older than Ttl.
 %% -----------------------------------------------------------------------------
 %%
 
--spec decode(
-    Cipher :: binary(),
-    Secret :: binary(),
-    Ttl :: non_neg_integer()) ->
-  {ok, Term :: any()} |
-  {error, expired} |
-  {error, forged} |
-  {error, badarg}.
-
-decode(<< Sig:32/binary, ExpiresAt:32/integer, Enc/binary >>, Secret, Ttl) ->
-  ExpiresAtBin = list_to_binary(integer_to_list(ExpiresAt)),
-  Key = key(Secret, Ttl),
-  % @todo constant time comparison
-  case sign(<< ExpiresAtBin/binary, Key/binary, Enc/binary >>, Secret) of
-    % signature ok?
-    Sig ->
-      Bin = uncrypt(Enc, Key),
+-spec decode(Cipher :: binary(), Secret :: binary()) ->
+  {ok, Term :: any()} | {error, forged} | {error, badarg}.
+decode(<< Sig:20/binary, Enc/binary >>, Secret) ->
+  Key = key(Secret),
+  % NB constant time signature verification
+  case equal(Sig, sign(<< Key/binary, Enc/binary >>, Key)) of
+    true ->
       % deserialize
-      try binary_to_term(Bin, [safe]) of
-        Term ->
-          % not yet expired?
-          case ExpiresAt > timestamp(0) of
-            true ->
-              {ok, Term};
-            false ->
-              {error, expired}
-          end
+      try check_expired(binary_to_term(uncrypt(Enc, Key), [safe])) of
+        Any -> Any
       catch _:_ ->
         {error, badarg}
       end;
-    _ ->
+    false ->
       {error, forged}
   end;
 
 %% N.B. unmatched binaries are forged
-decode(Bin, _, _) when is_binary(Bin) ->
+decode(Bin, _) when is_binary(Bin) ->
   {error, forged}.
 
-%%
-%% -----------------------------------------------------------------------------
-%% @doc Get current OS time plus Delta in seconds as unsigned integer.
-%% -----------------------------------------------------------------------------
-%%
-
--spec timestamp(
-    Delta :: integer()) ->
-  non_neg_integer().
-
-timestamp(Delta) when is_integer(Delta) ->
-  {MegaSecs, Secs, _} = os:timestamp(),
-  MegaSecs * 1000000 + Secs + Delta.
-
-
-%%
-%% -----------------------------------------------------------------------------
-%% @doc Get 16-octet binary from given arbitrary Secret and integer TTL.
-%% -----------------------------------------------------------------------------
-%%
-
--spec key(
-    Secret :: binary(),
-    Ttl :: non_neg_integer()) ->
-  MAC16 :: binary().
-
-key(Secret, Ttl) ->
-  crypto:md5_mac(Secret, integer_to_list(Ttl)).
-
-%%
-%% -----------------------------------------------------------------------------
-%% @doc Get 32-octet hash of Data salted with Secret.
-%% -----------------------------------------------------------------------------
-%%
+-spec key(Secret :: binary()) -> MAC16 :: binary().
+key(Secret) ->
+  crypto:md5_mac(Secret, []).
 
--spec sign(
-    Data :: binary(),
-    Secret :: binary()) ->
-  Signature32 :: binary().
-
-sign(Data, Secret) ->
-  crypto:sha256([Data, Secret]).
-
-%%
-%% -----------------------------------------------------------------------------
-%% @doc Encrypt Bin using Secret.
-%% -----------------------------------------------------------------------------
-%%
-
--spec encrypt(
-    Data :: binary(),
-    Key :: binary()) ->
-  Cipher :: binary().
+-spec sign(Data :: binary(), Secret :: binary()) -> MAC20 :: binary().
+sign(Data, Key) ->
+  crypto:sha_mac(Key, Data).
 
+-spec encrypt(Data :: binary(), Key :: binary()) -> Cipher :: binary().
 encrypt(Data, Key) ->
   IV = crypto:rand_bytes(16),
+  % @todo pad up to 16 octets and use CBC mode
   << IV/binary, (crypto:aes_cfb_128_encrypt(Key, IV, Data))/binary >>.
 
+-spec uncrypt(Cipher :: binary(), Key :: binary()) -> Uncrypted :: binary().
+uncrypt(<< IV:16/binary, Data/binary >>, Key) ->
+  % @todo pad up to 16 octets and use CBC mode
+  crypto:aes_cfb_128_decrypt(Key, IV, Data).
+
 %%
 %% -----------------------------------------------------------------------------
-%% @doc Uncrypt Bin using Secret.
+%% @doc 'Constant' time =:= operator for binaries, to mitigate timing attacks.
 %% -----------------------------------------------------------------------------
 %%
 
--spec uncrypt(
-    Cipher :: binary(),
-    Key :: binary()) ->
-  Uncrypted :: binary().
+-spec equal(A :: binary(), B :: binary()) -> true | false.
+equal(A, B) ->
+  equal(A, B, 0).
 
-uncrypt(<< IV:16/binary, Data/binary >>, Key) ->
-  crypto:aes_cfb_128_decrypt(Key, IV, Data).
+equal(<< A, As/binary >>, << B, Bs/binary >>, Acc) ->
+  equal(As, Bs, Acc bor (A bxor B));
+equal(<<>>, <<>>, 0) ->
+  true;
+equal(_As, _Bs, _Acc) ->
+  false.
 
 %%
 %% -----------------------------------------------------------------------------
 %% Conversion helpers
 %% -----------------------------------------------------------------------------
 %%
 
+encode_base64(Term, Secret) ->
+  base64:encode(encode(Term, Secret)).
+
 encode_base64(Term, Secret, Ttl) ->
   base64:encode(encode(Term, Secret, Ttl)).
 
-decode_base64(undefined, _, _) ->
+decode_base64(undefined, _) ->
   {error, forged};
 
-decode_base64(Bin, Secret, Ttl) when is_binary(Bin) ->
+decode_base64(Bin, Secret) when is_binary(Bin) ->
   try base64:decode(Bin) of
     Decoded ->
-      decode(Decoded, Secret, Ttl)
+      decode(Decoded, Secret)
   catch _:_ ->
     {error, forged}
   end.
 
+%%
+%% -----------------------------------------------------------------------------
+%% Expiration helpers
+%% -----------------------------------------------------------------------------
+%%
+
+-spec timestamp(Delta :: integer()) -> non_neg_integer().
+timestamp(Delta) when is_integer(Delta) ->
+  {MegaSecs, Secs, _} = os:timestamp(),
+  MegaSecs * 1000000 + Secs + Delta.
+
+expiring(Term, Ttl) ->
+  {expires, timestamp(Ttl), Term}.
+
+check_expired({expires, ExpiresAt, Term}) ->
+  case ExpiresAt > timestamp(0) of
+    true -> {ok, Term};
+    false -> {error, expired}
+  end;
+check_expired(Term) ->
+  {ok, Term}.
+
 %%
 %% -----------------------------------------------------------------------------
 %% Some unit tests
@@ -190,28 +167,36 @@ encrypt_test() ->
   ?assertNotEqual(Bin, uncrypt(encrypt(Bin, <<"0", Secret15/binary>>), Secret)).
 
 smoke_test() ->
+  Term = {a, b, c, [d, "e", <<"foo">>]},
+  Secret = <<"TopSecRet">>,
+  Enc = encode(Term, Secret),
+  ?assertEqual({ok, Term}, decode(Enc, Secret)),
+  % forged data
+  ?assertEqual({error, forged}, decode(<<"1">>, Secret)),
+  ?assertEqual({error, forged}, decode(<<"0", Enc/binary>>, Secret)),
+  ?assertEqual({error, forged}, decode(<<Enc/binary, "1">>, Secret)).
+
+expiry_test() ->
   Term = {a, b, c, [d, "e", <<"foo">>]},
   Secret = <<"TopSecRet">>,
   Enc = encode(Term, Secret, 1),
-  % decode encoded term with valid time to live
-  ?assertEqual({ok, Term}, decode(Enc, Secret, 1)),
+  ?assertEqual({ok, Term}, decode(Enc, Secret)),
   % forged data
-  ?assertEqual({error, forged}, decode(Enc, Secret, 2)),
-  ?assertEqual({error, forged}, decode(<<"1">>, Secret, 1)),
-  ?assertEqual({error, forged}, decode(<<"0", Enc/binary>>, Secret, 1)),
-  ?assertEqual({error, forged}, decode(<<Enc/binary, "1">>, Secret, 1)),
-  % expired data
-  Enc2 = encode(Term, Secret, 1),
+  ?assertEqual({error, forged}, decode(<<"1">>, Secret)),
+  ?assertEqual({error, forged}, decode(<<"0", Enc/binary>>, Secret)),
+  ?assertEqual({error, forged}, decode(<<Enc/binary, "1">>, Secret)),
+  % wait until it expires
   timer:sleep(2000),
-  ?assertEqual({error, expired}, decode(Enc2, Secret, 1)).
+  ?assertEqual({error, expired}, decode(Enc, Secret)).
 
 encode64_test() ->
   Term = {a, b, c, [d, "e", <<"foo">>]},
   Secret = <<"TopSecRet">>,
-  ?assertEqual({error, forged}, decode_base64(undefined, a, b)),
-  ?assertEqual({ok, Term}, decode_base64(encode_base64(Term, Secret, 1), Secret, 1)).
+
+  ?assertEqual({error, forged}, decode_base64(undefined, a)),
+  ?assertEqual({ok, Term}, decode_base64(encode_base64(Term, Secret), Secret)).
 
 decode64_test() ->
-  ?assertEqual({error, forged}, decode_base64(<<"%3A">>, a, b)).
+  ?assertEqual({error, forged}, decode_base64(<<"%3A">>, a)).
 
 -endif.

test/termit_SUITE.erl

@@ -15,9 +15,6 @@ all() ->
 encode_test(_Config) ->
   Term = {a, b, c, [d, "e", <<"foo">>]},
   Secret = <<"TopSecRet">>,
-  {error, forged} = termit:decode_base64(undefined, a, b),
+  {error, forged} = termit:decode_base64(undefined, a),
   {ok, Term} = termit:decode_base64(
-      termit:encode_base64(Term, Secret, 1), Secret, 1),
-  Enc = termit:encode_base64(Term, Secret, 1),
-  timer:sleep(2000),
-  {error, expired} = termit:decode_base64(Enc, Secret, 1).
+      termit:encode_base64(Term, Secret), Secret).