server: fix queued USERAUTH_SUCCESS rekey bug

Fix a bug with server-side rekeying where the session state at hand
has been toggled to SSH_SESSION_STATE_AUTHENTICATED before performing
the packet send of the SSH2_MSG_USERAUTH_SUCCESS message.

Before this change, what can happen is that during the packet send,
the SSH2_MSG_USERAUTH_SUCCESS message can end up being queued due
to a small rekey data limit value.  libssh server will then proceed
to attempt to send KEX-related rekeying messages to the client before
the client has received USERAUTH_SUCCESS.  OpenSSH clients do not
expect to undergo rekeying before having been authenticated, and so
will exit with error when this happens.

The behavior before and after can be observed with the pkd test making
use of its new --rekey flag:

    ./pkd_hello -t torture_pkd_openssh_rsa_rsa_default -i1 --rekey=16 -v -v -v

A new CMake test entry is added for the above variation and can be run
with:

    ARGS="-R pkd_hello_rekey" make test

Before the fix, the test will fail; after, the test succeeds while
performing rekeying once every 16 bytes.

Signed-off-by: Jon Simons <jon@jonsimons.org>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>

src/server.c

@@ -895,9 +895,6 @@ int ssh_auth_reply_success(ssh_session session, int partial)
         return ssh_auth_reply_default(session, partial);
     }
 
-    session->session_state = SSH_SESSION_STATE_AUTHENTICATED;
-    session->flags |= SSH_SESSION_FLAG_AUTHENTICATED;
-
     r = ssh_buffer_add_u8(session->out_buffer,SSH2_MSG_USERAUTH_SUCCESS);
     if (r < 0) {
         return SSH_ERROR;
@@ -905,6 +902,15 @@ int ssh_auth_reply_success(ssh_session session, int partial)
 
     r = ssh_packet_send(session);
 
+    /*
+     * Consider the session as having been authenticated only after sending
+     * the USERAUTH_SUCCESS message.  Setting these flags after ssh_packet_send
+     * ensures that a rekey is not triggered prematurely, causing the message
+     * to be queued.
+     */
+    session->session_state = SSH_SESSION_STATE_AUTHENTICATED;
+    session->flags |= SSH_SESSION_FLAG_AUTHENTICATED;
+
     crypto = ssh_packet_get_current_crypto(session, SSH_DIRECTION_OUT);
     if (crypto != NULL && crypto->delayed_compress_out) {
         SSH_LOG(SSH_LOG_PROTOCOL, "Enabling delayed compression OUT");

tests/pkd/CMakeLists.txt

@@ -38,6 +38,11 @@ target_link_libraries(pkd_hello ${pkd_libs})
 #
 add_test(pkd_hello_i1 ${CMAKE_CURRENT_BINARY_DIR}/pkd_hello -e -o -i1 -w /tmp/pkd_socket_wrapper_XXXXXX)
 
+#
+# pkd_hello_rekey is used to test server-side implementation of rekeying.
+#
+add_test(pkd_hello_rekey ${CMAKE_CURRENT_BINARY_DIR}/pkd_hello -t torture_pkd_openssh_rsa_rsa_default -i1 --rekey=16 -v -v -v -w /tmp/pkd_socket_wrapper_XXXXXX)
+
 #
 # Configure environment for cwrap socket wrapper.
 #