[PATCH net] rxrpc: Fix NULL deref on CHALLENGE to an unsecured client conn

[PATCH net] rxrpc: Fix NULL deref on CHALLENGE to an unsecured client conn

[Daniel Wade]

The I/O thread publishes a client connection in the local endpoint's ID
table, where incoming packets can find it, while it is still in
RXRPC_CONN_CLIENT_UNSECURED: the security class is set but
conn->security_ix is 0 and no session key is installed.  The key is set
up later by the sending thread, and if that thread is interrupted while
waiting for a channel the connection can stay unsecured indefinitely.

A CHALLENGE carrying securityIndex 0 then passes the check in
rxrpc_input_packet_on_conn(), and nothing before ->respond_to_challenge()
looks at the connection state.  rxkad encrypts the RESPONSE with a NULL
cipher and the work item oopses in fcrypt_pcbc_encrypt(); rxgk
dereferences a NULL transport key in rxgk_rekey() under
conn->security_lock.  An unprivileged local user can do this against
their own socket: the epoch is in every packet the client sends, and the
connection ID and state are in /proc/net/rxrpc/conns.

Discard a CHALLENGE that arrives on a connection still in
RXRPC_CONN_CLIENT_UNSECURED.  A genuine server cannot challenge before it
has seen DATA, and no DATA is sent until security is set up.  Read the
state under conn->state_lock to pair with
rxrpc_init_client_conn_security(), which installs the key before changing
the state under that lock.

Discovered by an AI code review agent, reproduced on unmodified mainline
with KASAN: a CHALLENGE with security index 0 sent to a connection listed
as ClUnsec in /proc/net/rxrpc/conns oopsed the rxrpc worker with "KASAN:
null-ptr-deref" in fcrypt_pcbc_encrypt().  Same on v6.12.117.  With this
patch the CHALLENGE is dropped and secured connections still answer
theirs.

Fixes: 9d35d880e0e4 ("rxrpc: Move client call connection to the I/O thread")
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Wade <danjwade95@gmail.com>
---
Tested on mainline b3dfbde62cf1 in a 2-vCPU KVM guest with KASAN and
PROVE_LOCKING, AF_RXRPC and RXKAD built in, as an unprivileged user.
Unmodified, the rxrpc worker took a general protection fault with
"KASAN: null-ptr-deref" in fcrypt_pcbc_encrypt() from
rxkad_respond_to_challenge(), and the test process then did not exit
within 300 s.  With this patch there is no oops: the CHALLENGE to the
unsecured connection goes unanswered, and the secured connection answers
its own as before.  On v6.12.117 without debug options the unmodified
kernel gave "BUG: kernel NULL pointer dereference" in the same place.

Stable: the hunk applies wherever ->validate_challenge() and
rxrpc_post_challenge() exist, v6.16 and later.  6.12.y needs a backport:
there the CHALLENGE is validated and answered in the work item, so the
same locked test goes into the CHALLENGE arm of rxrpc_process_event()
and returns 0, as the RESPONSE arm below it already does.  6.6.y and
6.1.y were not examined.

Not tested: rxgk, the RXRPC_MANAGE_RESPONSE path, interoperation with
AFS servers or clients.

The test program is not included here; it is available on request.

 net/rxrpc/conn_event.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)
diff --git a/net/rxrpc/conn_event.c b/net/rxrpc/conn_event.c
index 611c790bc6d..da16521cdfb 100644
--- a/net/rxrpc/conn_event.c
+++ b/net/rxrpc/conn_event.c
@@ -511,6 +511,16 @@ bool rxrpc_input_conn_packet(struct rxrpc_connection *conn, struct sk_buff *skb)
 				rxrpc_send_conn_abort(conn);
 			return true;
 		}
+		/* No genuine peer can challenge before the client has sent
+		 * DATA, and until then there is no session key to respond
+		 * with.  Pairs with rxrpc_init_client_conn_security().
+		 */
+		spin_lock_irq(&conn->state_lock);
+		if (conn->state == RXRPC_CONN_CLIENT_UNSECURED) {
+			spin_unlock_irq(&conn->state_lock);
+			return true;
+		}
+		spin_unlock_irq(&conn->state_lock);
 		if (!conn->security->validate_challenge(conn, skb))
 			return false;
 		return rxrpc_post_challenge(conn, skb);