[erlang-bugs] Erlang R13B01 ssh and crypto module patch of aes128-cbc support on ssh

Niclas Eklund nick@REDACTED
Sat Sep 5 19:55:57 CEST 2009


Hello!

The crypto stuff will make into R13B02 and hopefullly also the SSH fixes 
as well.

/Niclas @ Erlang/OTP

On Thu, 13 Aug 2009, Kenji Rikitake wrote:

> The following patch includes a feature enhancement of Erlang R13B01 ssh
> and crypto modules to support aes128-cbc mode on SSH.
>
> BUG FOUND: ssh_transport:unpack/3 causes crypto:aes_cbc_ivec/1 to crash,
> by passing a null binary (<<>>), which causes erlang:split_binary/2 to
> crash by badarg.
>
> By tracing the exchange between a FreeBSD OpenSSH implementation, I
> found a case where the internal variable SshLength in
> ssh_transport:unpack/3 goes to zero, which leads to passing a null
> binary as an argument to ssh_transport:decrypt_blocks/3 and to
> aes_cbc_ivec/1.  So I added a case statement to avoid calling the
> decrypt_blocks/3 when SshLength = 0.
>
> Patches follow.
>
> Kenji Rikitake
>
> --- lib/ssh/src/ssh_transport.erl.FCS	2009-06-05 21:55:34.000000000 +0900
> +++ lib/ssh/src/ssh_transport.erl	2009-08-13 17:42:28.000000000 +0900
> @@ -228,8 +228,10 @@
> 		  cookie = Random,
> 		  kex_algorithms = ["diffie-hellman-group1-sha1"],
> 		  server_host_key_algorithms = ["ssh-rsa", "ssh-dss"],
> -		  encryption_algorithms_client_to_server = ["3des-cbc"],
> -		  encryption_algorithms_server_to_client = ["3des-cbc"],
> +		  %% encryption_algorithms_client_to_server = ["3des-cbc"],
> +		  %% encryption_algorithms_server_to_client = ["3des-cbc"],
> +		  encryption_algorithms_client_to_server = ["aes128-cbc","3des-cbc"],
> +		  encryption_algorithms_server_to_client = ["aes128-cbc","3des-cbc"],
> 		  mac_algorithms_client_to_server = ["hmac-sha1"],
> 		  mac_algorithms_server_to_client = ["hmac-sha1"],
> 		  compression_algorithms_client_to_server = Compression,
> @@ -243,8 +245,10 @@
> 		  cookie = Random,
> 		  kex_algorithms = ["diffie-hellman-group1-sha1"],
> 		  server_host_key_algorithms = ["ssh-dss"],
> -		  encryption_algorithms_client_to_server = ["3des-cbc"],
> -		  encryption_algorithms_server_to_client = ["3des-cbc"],
> +		  %% encryption_algorithms_client_to_server = ["3des-cbc"],
> +		  %% encryption_algorithms_server_to_client = ["3des-cbc"],
> +		  encryption_algorithms_client_to_server = ["aes128-cbc","3des-cbc"],
> +		  encryption_algorithms_server_to_client = ["aes128-cbc","3des-cbc"],
> 		  mac_algorithms_client_to_server = ["hmac-sha1"],
> 		  mac_algorithms_server_to_client = ["hmac-sha1"],
> 		  compression_algorithms_client_to_server = Compression,
> @@ -703,6 +707,9 @@
>
> unpack(EncodedSoFar, ReminingLenght, #ssh{recv_mac_size = MacSize} = Ssh0) ->
>     SshLength = ReminingLenght - MacSize,
> +    %%?dbg(?DBG_CRYPTO,
> +    %%     "unpack: EncodedsoFar=~p SshLength=~p ReminingLenght=~p MacSize=~p ~n",
> +    %%     [EncodedSoFar, SshLength, ReminingLenght, MacSize]),
>     {NoMac, Mac, Rest} = case MacSize of
> 			     0 ->
> 				 <<NoMac0:SshLength/binary,
> @@ -714,8 +721,16 @@
> 				  Rest0/binary>> = EncodedSoFar,
> 				 {NoMac0, Mac0, Rest0}
> 			 end,
> -    {Ssh1, DecData, <<>>} =
> -	ssh_transport:decrypt_blocks(NoMac, SshLength, Ssh0),
> +    %%?dbg(?DBG_CRYPTO, "unpack: NoMac=~p Mac=~p Rest=~p ~n",
> +    %%     [NoMac, Mac, Rest]),
> +    {Ssh1, DecData, <<>>} = case SshLength of
> +			        0 ->
> +				    {Ssh0, <<>>, <<>>};
> +				_ ->
> +				     ssh_transport:decrypt_blocks(NoMac, SshLength, Ssh0)
> +			    end,
> +    %%?dbg(?DBG_CRYPTO, "unpack: Ssh1=~p DecData=~p Rest=~p Mac=~p ~n",
> +    %%     [Ssh1, DecData, Rest, Mac]),
>     {Ssh1, DecData, Rest, Mac}.
>
> msg_data(PacketData) ->
> @@ -800,6 +815,18 @@
>     <<K1:8/binary, K2:8/binary, K3:8/binary>> = hash(Ssh, "D", 192),
>     {ok, Ssh#ssh{encrypt_keys = {K1,K2,K3},
> 		 encrypt_block_size = 8,
> +		 encrypt_ctx = IV}};
> +encrypt_init(#ssh{encrypt = 'aes128-cbc', role = client} = Ssh) ->
> +    IV = hash(Ssh, "A", 128),
> +    <<K:16/binary>> = hash(Ssh, "C", 128),
> +    {ok, Ssh#ssh{encrypt_keys = K,
> +		 encrypt_block_size = 16,
> +		 encrypt_ctx = IV}};
> +encrypt_init(#ssh{encrypt = 'aes128-cbc', role = server} = Ssh) ->
> +    IV = hash(Ssh, "B", 128),
> +    <<K:16/binary>> = hash(Ssh, "D", 128),
> +    {ok, Ssh#ssh{encrypt_keys = K,
> +		 encrypt_block_size = 16,
> 		 encrypt_ctx = IV}}.
>
> encrypt_final(Ssh) ->
> @@ -815,10 +842,19 @@
> 	     encrypt_keys = {K1,K2,K3},
> 	     encrypt_ctx = IV0} = Ssh, Data) ->
>     %%?dbg(?DBG_CRYPTO, "encrypt: IV=~p K1=~p, K2=~p, K3=~p ~n",
> -    %%	 [IV0,K1,K2,K3]),
> +    %%         [IV0,K1,K2,K3]),
>     Enc = crypto:des3_cbc_encrypt(K1,K2,K3,IV0,Data),
>     %%?dbg(?DBG_CRYPTO, "encrypt: ~p -> ~p ~n", [Data, Enc]),
>     IV = crypto:des_cbc_ivec(Enc),
> +    {Ssh#ssh{encrypt_ctx = IV}, Enc};
> +encrypt(#ssh{encrypt = 'aes128-cbc',
> +	     encrypt_keys = K,
> +	     encrypt_ctx = IV0} = Ssh, Data) ->
> +    %%?dbg(?DBG_CRYPTO, "encrypt: IV=~p K=~p ~n",
> +    %%     [IV0,K]),
> +    Enc = crypto:aes_cbc_128_encrypt(K,IV0,Data),
> +    %%?dbg(?DBG_CRYPTO, "encrypt: ~p -> ~p ~n", [Data, Enc]),
> +    IV = crypto:aes_cbc_ivec(Enc),
>     {Ssh#ssh{encrypt_ctx = IV}, Enc}.
>
>
> @@ -840,7 +876,21 @@
> 		hash(Ssh, "C", 192)},
>     <<K1:8/binary, K2:8/binary, K3:8/binary>> = KD,
>     {ok, Ssh#ssh{decrypt_keys = {K1, K2, K3}, decrypt_ctx = IV,
> -		 decrypt_block_size = 8}}.
> +		 decrypt_block_size = 8}};
> +
> +decrypt_init(#ssh{decrypt = 'aes128-cbc', role = client} = Ssh) ->
> +    {IV, KD} = {hash(Ssh, "B", 128),
> +		hash(Ssh, "D", 128)},
> +    <<K:16/binary>> = KD,
> +    {ok, Ssh#ssh{decrypt_keys = K, decrypt_ctx = IV,
> +			 decrypt_block_size = 16}};
> +
> +decrypt_init(#ssh{decrypt = 'aes128-cbc', role = server} = Ssh) ->
> +    {IV, KD} = {hash(Ssh, "A", 128),
> +		hash(Ssh, "C", 128)},
> +    <<K:16/binary>> = KD,
> +    {ok, Ssh#ssh{decrypt_keys = K, decrypt_ctx = IV,
> +		 decrypt_block_size = 16}}.
>
> decrypt_final(Ssh) ->
>     {ok, Ssh#ssh {decrypt = none,
> @@ -858,6 +908,14 @@
>     Dec = crypto:des3_cbc_decrypt(K1,K2,K3,IV0,Data),
>     %%?dbg(?DBG_CRYPTO, "decrypt: ~p -> ~p ~n", [Data, Dec]),
>     IV = crypto:des_cbc_ivec(Data),
> +    {Ssh#ssh{decrypt_ctx = IV}, Dec};
> +decrypt(#ssh{decrypt = 'aes128-cbc', decrypt_keys = Key,
> +	     decrypt_ctx = IV0} = Ssh, Data) ->
> +    %%?dbg(?DBG_CRYPTO, "decrypt: IV=~p Key=~p ~n",
> +    %%     [IV0,Key]),
> +    Dec = crypto:aes_cbc_128_decrypt(Key,IV0,Data),
> +    %%?dbg(?DBG_CRYPTO, "decrypt: ~p -> ~p ~n", [Data, Dec]),
> +    IV = crypto:aes_cbc_ivec(Data),
>     {Ssh#ssh{decrypt_ctx = IV}, Dec}.
>
> %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
> --- lib/crypto/src/crypto.erl.FCS	2009-03-12 21:28:35.000000000 +0900
> +++ lib/crypto/src/crypto.erl	2009-08-13 13:27:09.000000000 +0900
> @@ -45,6 +45,7 @@
> %% -export([idea_cbc_encrypt/3, idea_cbc_decrypt/3]).
> -export([aes_cbc_128_encrypt/3, aes_cbc_128_decrypt/3]).
> -export([aes_cbc_256_encrypt/3, aes_cbc_256_decrypt/3]).
> +-export([aes_cbc_ivec/1]).
>
> -export([dh_generate_parameters/2, dh_check/1]). %% Testing see below
>
> @@ -469,6 +470,19 @@
>     control(?AES_CBC_256_DECRYPT, [Key, IVec, Data]).
>
> %%
> +%% aes_cbc_ivec(Data) -> binary()
> +%%
> +%% Returns the IVec to be used in the next iteration of
> +%% aes_cbc_*_[encrypt|decrypt].
> +%% IVec size: 16 bytes
> +%%
> +aes_cbc_ivec(Data) when is_binary(Data) ->
> +    {_, IVec} = split_binary(Data, size(Data) - 16),
> +    IVec;
> +aes_cbc_ivec(Data) when is_list(Data) ->
> +    aes_cbc_ivec(list_to_binary(Data)).
> +
> +%%
> %% XOR - xor to iolists and return a binary
> %% NB doesn't check that they are the same size, just concatenates
> %% them and sends them to the driver
>




More information about the erlang-patches mailing list