Merge branch 'maint'

Author: u3s

lib/ssh/src/ssh_options.erl

@@ -558,7 +558,7 @@ default(server) ->
            },
 
       dh_gex_limits =>
-          #{default => {0, infinity},
+          #{default => {2047, infinity},
             chk => fun({I1,I2}) ->
                            check_pos_integer(I1) andalso
                                check_pos_integer(I2) andalso
@@ -705,7 +705,7 @@ default(client) ->
            },
 
       dh_gex_limits =>
-          #{default => {1024, 6144, 8192},      % FIXME: Is this true nowadays?
+          #{default => {2047, 6144, 8192},
             chk => fun({Min,I,Max}) ->
                            lists:all(fun check_pos_integer/1,
                                      [Min,I,Max]);

lib/ssh/src/ssh_transport.erl

@@ -777,14 +777,22 @@ adjust_gex_min_max(Min0, Max0, Opts) ->
     end.
 
 
-handle_kex_dh_gex_group(#ssh_msg_kex_dh_gex_group{p = P, g = G}, Ssh0) ->
-    %% client
-    Sz = dh_bits(Ssh0#ssh.algorithms),
-    {Public, Private} = generate_key(dh, [P,G,2*Sz]),
-    {SshPacket, Ssh1} = 
-	ssh_packet(#ssh_msg_kex_dh_gex_init{e = Public}, Ssh0),	% Pub = G^Priv mod P (def)
-    {ok, SshPacket, 
-     Ssh1#ssh{keyex_key = {{Private, Public}, {G, P}}}}.
+handle_kex_dh_gex_group(#ssh_msg_kex_dh_gex_group{p = P, g = G},
+                        #ssh{keyex_info = {Min, _Max, _NBits}} = Ssh0) ->
+    %% client — validate group parameters from server (RFC 4419 §3)
+    PBits = nbits(P),
+    MinBits = max(Min, ?DH_GEX_MIN_BITS),
+    case validate_dh_gex_group(P, G, PBits, MinBits) of
+        ok ->
+            Sz = dh_bits(Ssh0#ssh.algorithms),
+            {Public, Private} = generate_key(dh, [P, G, 2*Sz]),
+            {SshPacket, Ssh1} =
+                ssh_packet(#ssh_msg_kex_dh_gex_init{e = Public}, Ssh0),
+            {ok, SshPacket,
+             Ssh1#ssh{keyex_key = {{Private, Public}, {G, P}}}};
+        {error, Reason} ->
+            ?DISCONNECT(?SSH_DISCONNECT_KEY_EXCHANGE_FAILED, Reason)
+    end.
 
 handle_kex_dh_gex_init(#ssh_msg_kex_dh_gex_init{e = E}, 
 		       #ssh{keyex_key = {{Private, Public}, {G, P}},
@@ -2411,6 +2419,24 @@ dh_bits(#alg{encrypt = Encrypt,
                    mac_key_bytes(SendMac)
                   ]).
 
+%% Validate DH GEX group parameters received from the server.
+%% Checks generator bounds and prime size against requested limits.
+validate_dh_gex_group(P, G, _PBits, _MinBits)
+  when G =< 1;
+       G >= P - 1 ->
+    {error, "DH GEX invalid generator"};
+validate_dh_gex_group(_P, _G, PBits, MinBits) when PBits < MinBits ->
+    {error, io_lib:format("DH GEX group too small: ~p bits (minimum ~p)",
+                          [PBits, MinBits])};
+validate_dh_gex_group(_P, _G, _PBits, _MinBits) ->
+    ok.
+
+%% Number of significant bits in a positive integer.
+nbits(N) when is_integer(N), N > 0 ->
+    bit_size(binary:encode_unsigned(N));
+nbits(_) ->
+    0.
+
 ecdh_curve('ecdh-sha2-nistp256') -> secp256r1;
 ecdh_curve('ecdh-sha2-nistp384') -> secp384r1;
 ecdh_curve('ecdh-sha2-nistp521') -> secp521r1;

lib/ssh/src/ssh_transport.hrl

@@ -34,10 +34,16 @@
 
 -define(MAX_NUM_ALGORITHMS, 200).
 
--define(DEFAULT_DH_GROUP_MIN,   1024).
+-define(DEFAULT_DH_GROUP_MIN,   2047).
 -define(DEFAULT_DH_GROUP_NBITS, 2048).
 -define(DEFAULT_DH_GROUP_MAX,   8192).
 
+%% Absolute minimum DH group size the client will accept from a server
+%% during GEX, regardless of dh_gex_limits configuration.
+%% Matches the smallest built-in group in pubkey_moduli.hrl.
+%% OpenSSH uses 2048 (DH_GRP_MIN); our moduli are labeled 2047.
+-define(DH_GEX_MIN_BITS, 2047).
+
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %%
 %% BASIC transport messages

lib/ssh/test/ssh_protocol_SUITE.erl

@@ -32,6 +32,10 @@
 -include("ssh_auth.hrl").
 -include("ssh_test_lib.hrl").
 
+%% RFC 3526 Group 14 — 2048-bit MODP prime (generator = 2).
+-define(RFC3526_GROUP14_PRIME,
+        16#FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE45B3DC2007CB8A163BF0598DA48361C55D39A69163FA8FD24CF5F83655D23DCA3AD961C62F356208552BB9ED529077096966D670C354E4ABC9804F1746C08CA18217C32905E462E36CE3BE39E772C180E86039B2783A2EC07A28FB5C55DF06F4C52C9DE2BCBF6955817183995497CEA956AE515D2261898FA051015728E5A8AACAA68FFFFFFFFFFFFFFFF).
+
 -export([
          suite/0,
          all/0,
@@ -82,6 +86,8 @@
          gex_client_init_option_groups_moduli_file/1,
          gex_client_old_request_exact/1,
          gex_client_old_request_noexact/1,
+         gex_client_rejects_small_group/1,
+         gex_client_rejects_bad_generator/1,
          gex_server_gex_limit/1,
          lib_match/1,
          lib_no_match/1,
@@ -209,6 +215,8 @@ groups() ->
 		gex_client_init_option_groups_file,
 		gex_client_old_request_exact,
 		gex_client_old_request_noexact,
+                gex_client_rejects_small_group,
+                gex_client_rejects_bad_generator,
                 kex_strict_negotiated,
                 kex_strict_violation_key_exchange,
                 kex_strict_violation_new_keys,
@@ -301,6 +309,9 @@ init_per_testcase(TC, Config) when TC == kex_strict_negotiated;
     Level = ssh_test_lib:get_log_level(),
     ssh_test_lib:set_log_level(debug),
     [{saved_log_level, Level} | Config];
+init_per_testcase(TC, Config) when TC == gex_client_rejects_small_group;
+                                   TC == gex_client_rejects_bad_generator ->
+    Config;
 init_per_testcase(TC, Config) when TC == gex_client_init_option_groups ;
 				   TC == gex_client_init_option_groups_moduli_file ;
 				   TC == gex_client_init_option_groups_file ;
@@ -309,10 +320,8 @@ init_per_testcase(TC, Config) when TC == gex_client_init_option_groups ;
 				   TC == gex_client_old_request_noexact ->
     Opts = case TC of
 	       gex_client_init_option_groups ->
-		   [{dh_gex_groups, 
-                     [{1023, 5, 
-                       16#D9277DAA27DB131C03B108D41A76B4DA8ACEECCCAE73D2E48CEDAAA70B09EF9F04FB020DCF36C51B8E485B26FABE0337E24232BE4F4E693548310244937433FB1A5758195DC73B84ADEF8237472C46747D79DC0A2CF8A57CE8DBD8F466A20F8551E7B1B824B2E4987A8816D9BC0741C2798F3EBAD3ADEBCC78FCE6A770E2EC9F
-                      }]}];
+                   [{dh_gex_groups,
+                     [{2048, 2, ?RFC3526_GROUP14_PRIME}]}];
 	       gex_client_init_option_groups_file ->
 		   DataDir = proplists:get_value(data_dir, Config),
 		   F = filename:join(DataDir, "dh_group_test"),
@@ -663,9 +672,8 @@ no_common_alg_client_disconnects(Config) ->
 
 %%%--------------------------------------------------------------------
 gex_client_init_option_groups(Config) ->
-    do_gex_client_init(Config, {512, 2048, 4000},
-		       {5,16#D9277DAA27DB131C03B108D41A76B4DA8ACEECCCAE73D2E48CEDAAA70B09EF9F04FB020DCF36C51B8E485B26FABE0337E24232BE4F4E693548310244937433FB1A5758195DC73B84ADEF8237472C46747D79DC0A2CF8A57CE8DBD8F466A20F8551E7B1B824B2E4987A8816D9BC0741C2798F3EBAD3ADEBCC78FCE6A770E2EC9F}
-                      ).
+    do_gex_client_init(Config, {2048, 2048, 4000},
+                       {2, ?RFC3526_GROUP14_PRIME}).
 
 gex_client_init_option_groups_file(Config) ->
     do_gex_client_init(Config, {2000, 2048, 4000},
@@ -742,6 +750,67 @@ do_gex_client_init_old(Config, N, {G,P}) ->
 	  ]
 	 ).
 
+%%%--------------------------------------------------------------------
+%%% Client rejects a DH GEX group with too-small prime (512 bits).
+%%% The test lib acts as server and sends a bad GEX_GROUP; the real
+%%% OTP client must disconnect with KEY_EXCHANGE_FAILED.
+gex_client_rejects_small_group(Config) ->
+    %% 512-bit number (not even prime — doesn't matter, size check rejects first)
+    SmallP = 16#D4BCD52406F2C926B7E8BE5FF5D2B2E3B956F79441CE5B2E35,
+    gex_client_rejects_group(Config, SmallP, 2).
+
+%%% Client rejects a DH GEX group with invalid generator (G=1).
+gex_client_rejects_bad_generator(Config) ->
+    %% Valid 2048-bit prime (RFC 3526 group 14), but generator = 1
+    gex_client_rejects_group(Config, ?RFC3526_GROUP14_PRIME, 1).
+
+gex_client_rejects_group(Config, P, G) ->
+    {ok, InitialState} = ssh_trpt_test_lib:exec(listen),
+    HostPort = ssh_trpt_test_lib:server_host_port(InitialState),
+    Parent = self(),
+
+    %% Server side: accept, negotiate DH-GEX, send bad group
+    Pid =
+        spawn_link(
+          fun() ->
+                  Parent !
+                      {result, self(),
+                       ssh_trpt_test_lib:exec(
+                         [{set_options, [print_ops, print_seqnums, print_messages]},
+                          {accept, [{system_dir, ssh_test_lib:system_dir(Config)},
+                                    {user_dir, ssh_test_lib:user_dir(Config)}]},
+                          receive_hello,
+                          {send, hello},
+                          {send, ssh_msg_kexinit},
+                          {match, #ssh_msg_kexinit{_='_'}, receive_msg},
+                          {match, #ssh_msg_kex_dh_gex_request{_='_'}, receive_msg},
+                          {send, #ssh_msg_kex_dh_gex_group{p = P, g = G}},
+                          {match, disconnect(?SSH_DISCONNECT_KEY_EXCHANGE_FAILED),
+                           receive_msg}
+                         ],
+                         InitialState)}
+          end),
+
+    %% Client side: connect forcing DH-GEX
+    Result = std_connect(HostPort, Config,
+                         [{preferred_algorithms,
+                           [{kex, ['diffie-hellman-group-exchange-sha256']},
+                            {cipher, ?DEFAULT_CIPHERS}]}]),
+    ct:log("Client connect result: ~p", [Result]),
+
+    receive
+        {result, Pid, {ok, _}} ->
+            ok;
+        {result, Pid, {error, {Op, ExecResult, S}}} ->
+            ct:log("ERROR!~nOp = ~p~nExecResult = ~p~nState =~n~s",
+                   [Op, ExecResult, ssh_trpt_test_lib:format_msg(S)]),
+            {fail, ExecResult};
+        {result, Pid, X} ->
+            ct:fail(X)
+    after
+        30000 -> ct:fail("timeout ~p:~p", [?MODULE, ?LINE])
+    end.
+
 %%%--------------------------------------------------------------------
 bad_service_name(Config) -> 
     bad_service_name(Config, "kfglkjf").