ssh: tighten DH bounds to 1<e<p-1 and add K check

Tighten e/f validation from inclusive [1, p-1] to strict (1 < e < p-1)
on all plain-DH and GEX paths. Add missing 1 < K < p-1 check on the
plain-DH client path (handle_kexdh_reply), matching what the GEX code
already enforced.

The RFCs (4253 §8, 4419 §3) only require inclusive bounds for e/f, but
OpenSSH and Go both enforce strict bounds. The K check on plain DH is
defense-in-depth — with safe primes and strict e/f, K ∈ {1, p-1} cannot
occur, but the check guards against crypto backend bugs at zero cost.

No interop risk: no legitimate peer sends e ∈ {0, 1, p-1, p}.

Author: garazdawi

lib/ssh/src/ssh_transport.erl

@@ -602,28 +602,34 @@ handle_kexdh_init(#ssh_msg_kexdh_init{e = E},
     %% server
     {G, P} = dh_group(Kex),
     if
-	1=<E, E=<(P-1) ->
+	1<E, E<(P-1) ->
             Sz = dh_bits(Algs),
 	    {Public, Private} = generate_key(dh, [P,G,2*Sz]),
 	    K = compute_key(dh, E, Private, [P,G]),
-	    MyPrivHostKey = get_host_key(SignAlg, Opts),
-	    MyPubHostKey = ssh_file:extract_public_key(MyPrivHostKey),
-            H = kex_hash(Ssh0, MyPubHostKey, sha(Kex), {E,Public,K}),
-            case sign(H, SignAlg, MyPrivHostKey, Ssh0) of
-                {ok,H_SIG} ->
-                    {SshPacket, Ssh1} =
-                        ssh_packet(#ssh_msg_kexdh_reply{public_host_key = {MyPubHostKey,SignAlg},
-                                                        f = Public,
-                                                        h_sig = H_SIG
-                                                       }, Ssh0),
-                    {ok, SshPacket, Ssh1#ssh{keyex_key = {{Private, Public}, {G, P}},
-                                             shared_secret = ssh_bits:mpint(K),
-                                             exchanged_hash = H,
-                                             session_id = sid(Ssh1, H)}};
-                {error,unsupported_sign_alg} ->
+            if
+                1<K, K<(P-1) ->
+                    MyPrivHostKey = get_host_key(SignAlg, Opts),
+                    MyPubHostKey = ssh_file:extract_public_key(MyPrivHostKey),
+                    H = kex_hash(Ssh0, MyPubHostKey, sha(Kex), {E,Public,K}),
+                    case sign(H, SignAlg, MyPrivHostKey, Ssh0) of
+                        {ok,H_SIG} ->
+                            {SshPacket, Ssh1} =
+                                ssh_packet(#ssh_msg_kexdh_reply{public_host_key = {MyPubHostKey,SignAlg},
+                                                                f = Public,
+                                                                h_sig = H_SIG
+                                                               }, Ssh0),
+                            {ok, SshPacket, Ssh1#ssh{keyex_key = {{Private, Public}, {G, P}},
+                                                     shared_secret = ssh_bits:mpint(K),
+                                                     exchanged_hash = H,
+                                                     session_id = sid(Ssh1, H)}};
+                        {error,unsupported_sign_alg} ->
+                            ?DISCONNECT(?SSH_DISCONNECT_KEY_EXCHANGE_FAILED,
+                                        io_lib:format("Unsupported algorithm ~p", [SignAlg],
+                                                      [{chars_limit, ssh_lib:max_log_len(Opts)}]))
+                    end;
+                true ->
                     ?DISCONNECT(?SSH_DISCONNECT_KEY_EXCHANGE_FAILED,
-                                io_lib:format("Unsupported algorithm ~p", [SignAlg],
-                                              [{chars_limit, ssh_lib:max_log_len(Opts)}]))
+                                "Key exchange failed, 'K' out of bounds")
             end;
 	true ->
             MsgFun =
@@ -643,22 +649,28 @@ handle_kexdh_reply(#ssh_msg_kexdh_reply{public_host_key = PeerPubHostKey,
 		   #ssh{keyex_key = {{Private, Public}, {G, P}},
                         algorithms = #alg{kex=Kex}} = Ssh0) ->
     %% client
-    if 
-	1=<F, F=<(P-1)->
+    if
+	1<F, F<(P-1) ->
 	    K = compute_key(dh, F, Private, [P,G]),
-            H = kex_hash(Ssh0, PeerPubHostKey, sha(Kex), {Public,F,K}),
-	    case verify_host_key(Ssh0, PeerPubHostKey, H, H_SIG) of
-		ok ->
-		    {SshPacket, Ssh} = ssh_packet(#ssh_msg_newkeys{}, Ssh0),
-		    {ok, SshPacket, install_alg(snd, Ssh#ssh{shared_secret  = ssh_bits:mpint(K),
-                                                             exchanged_hash = H,
-                                                             session_id = sid(Ssh, H)})};
-		Error ->
+            if
+                1<K, K<(P-1) ->
+                    H = kex_hash(Ssh0, PeerPubHostKey, sha(Kex), {Public,F,K}),
+                    case verify_host_key(Ssh0, PeerPubHostKey, H, H_SIG) of
+                        ok ->
+                            {SshPacket, Ssh} = ssh_packet(#ssh_msg_newkeys{}, Ssh0),
+                            {ok, SshPacket, install_alg(snd, Ssh#ssh{shared_secret  = ssh_bits:mpint(K),
+                                                                     exchanged_hash = H,
+                                                                     session_id = sid(Ssh, H)})};
+                        Error ->
+                            ?DISCONNECT(?SSH_DISCONNECT_KEY_EXCHANGE_FAILED,
+                                        io_lib:format("Kexdh init failed. Verify host key: ~p",[Error],
+                                                      [{chars_limit, ssh_lib:max_log_len(Ssh0)}])
+                                       )
+                    end;
+                true ->
                     ?DISCONNECT(?SSH_DISCONNECT_KEY_EXCHANGE_FAILED,
-                                io_lib:format("Kexdh init failed. Verify host key: ~p",[Error],
-                                              [{chars_limit, ssh_lib:max_log_len(Ssh0)}])
-                               )
-	    end;
+                                "Key exchange failed, 'K' out of bounds")
+            end;
 
 	true ->
             ?DISCONNECT(?SSH_DISCONNECT_KEY_EXCHANGE_FAILED,
@@ -760,7 +772,7 @@ handle_kex_dh_gex_init(#ssh_msg_kex_dh_gex_init{e = E},
                             opts = Opts} = Ssh0) ->
     %% server
     if
-	1=<E, E=<(P-1) ->
+	1<E, E<(P-1) ->
 	    K = compute_key(dh, E, Private, [P,G]),
 	    if
 		1<K, K<(P-1) ->
@@ -805,8 +817,8 @@ handle_kex_dh_gex_reply(#ssh_msg_kex_dh_gex_reply{public_host_key = PeerPubHostK
                              algorithms = #alg{kex=Kex}} = 
 			    Ssh0) ->
     %% client
-    if 
-	1=<F, F=<(P-1)->
+    if
+	1<F, F<(P-1) ->
 	    K = compute_key(dh, F, Private, [P,G]),
 	    if
 		1<K, K<(P-1) ->

lib/ssh/test/ssh_protocol_SUITE.erl

@@ -68,6 +68,7 @@
          kex_strict_violation/1,
          kex_strict_violation_2/1,
          kex_strict_msg_unknown/1,
+         dh_kexdh_init_e_out_of_bounds/1,
          gex_client_init_option_groups/1,
          gex_client_init_option_groups_file/1,
          gex_client_init_option_groups_moduli_file/1,
@@ -185,7 +186,8 @@ groups() ->
                 kex_strict_violation_new_keys,
                 kex_strict_violation,
                 kex_strict_violation_2,
-                kex_strict_msg_unknown]},
+                kex_strict_msg_unknown,
+                dh_kexdh_init_e_out_of_bounds]},
      {service_requests, [], [bad_service_name,
 			     bad_long_service_name,
 			     bad_very_long_service_name,
@@ -1384,6 +1386,40 @@ kex_strict_msg_unknown(Config) ->
          {match, disconnect(?SSH_DISCONNECT_KEY_EXCHANGE_FAILED), receive_msg}],
     kex_strict_helper(Config, TestMessages, ExpectedReason).
 
+%% RFC 4253 §8 / RFC 4419 §3: a peer's DH value must satisfy 1 < e < p-1.
+%% A peer driving e (or, on the client side, f) to one of {0, 1, p-1, p}
+%% must be rejected with SSH_DISCONNECT_KEY_EXCHANGE_FAILED. Verify this
+%% for the server, which receives e from the client.
+dh_kexdh_init_e_out_of_bounds(Config) ->
+    {_G, P} = ?dh_group14,
+    [verify_kexdh_init_rejected(Config, E) || E <- [0, 1, P-1, P]],
+    ok.
+
+verify_kexdh_init_rejected(Config, E) ->
+    ct:log("Trying kexdh_init with e=~p", [E]),
+    {ok, InitialState} = ssh_trpt_test_lib:exec(
+                          [{set_options, [print_ops, print_seqnums, print_messages]}]),
+    {ok, _} =
+        ssh_trpt_test_lib:exec(
+          [{connect,
+            ssh_test_lib:server_host(Config), ssh_test_lib:server_port(Config),
+            [{preferred_algorithms,
+              [{kex, ['diffie-hellman-group14-sha256']},
+               {cipher, ?DEFAULT_CIPHERS}]},
+             {silently_accept_hosts, true},
+             {recv_ext_info, false},
+             {user_dir, ssh_test_lib:user_dir(Config)},
+             {user_interaction, false}
+             | proplists:get_value(extra_options, Config, [])
+            ]},
+           receive_hello,
+           {send, hello},
+           {send, ssh_msg_kexinit},
+           {match, #ssh_msg_kexinit{_='_'}, receive_msg},
+           {send, #ssh_msg_kexdh_init{e = E}},
+           {match, disconnect(?SSH_DISCONNECT_KEY_EXCHANGE_FAILED), receive_msg}],
+          InitialState).
+
 kex_strict_helper(Config, TestMessages, ExpectedReason) ->
     {ok, TestRef} = ssh_test_lib:add_log_handler(),
     Level = ssh_test_lib:get_log_level(),