rxrpc: Fix missing security check on incoming calls

Fix rxrpc_new_incoming_call() to check that we have a suitable service key
available for the combination of service ID and security class of a new
incoming call - and to reject calls for which we don't.

This causes an assertion like the following to appear:

	rxrpc: Assertion failed - 6(0x6) == 12(0xc) is false
	kernel BUG at net/rxrpc/call_object.c:456!

Where call->state is RXRPC_CALL_SERVER_SECURING (6) rather than
RXRPC_CALL_COMPLETE (12).

Fixes: 248f219cb8bc ("rxrpc: Rewrite the data and ack handling code")
Reported-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 7c7d10f2e0c181776611cd54d6048a17ed512d5f..5e99df80e80a775b26e0de82eec5738454398abc 100644 (file)
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -209,6 +209,7 @@ struct rxrpc_skb_priv {
 struct rxrpc_security {
        const char              *name;          /* name of this service */
        u8                      security_index; /* security type provided */
+       u32                     no_key_abort;   /* Abort code indicating no key */
 
        /* Initialise a security service */
        int (*init)(void);
@@ -977,8 +978,9 @@ static inline void rxrpc_reduce_conn_timer(struct rxrpc_connection *conn,
 struct rxrpc_connection *rxrpc_find_service_conn_rcu(struct rxrpc_peer *,
                                                     struct sk_buff *);
 struct rxrpc_connection *rxrpc_prealloc_service_connection(struct rxrpc_net *, gfp_t);
-void rxrpc_new_incoming_connection(struct rxrpc_sock *,
-                                  struct rxrpc_connection *, struct sk_buff *);
+void rxrpc_new_incoming_connection(struct rxrpc_sock *, struct rxrpc_connection *,
+                                  const struct rxrpc_security *, struct key *,
+                                  struct sk_buff *);
 void rxrpc_unpublish_service_conn(struct rxrpc_connection *);
 
 /*
@@ -1103,7 +1105,9 @@ extern const struct rxrpc_security rxkad;
 int __init rxrpc_init_security(void);
 void rxrpc_exit_security(void);
 int rxrpc_init_client_conn_security(struct rxrpc_connection *);
-int rxrpc_init_server_conn_security(struct rxrpc_connection *);
+bool rxrpc_look_up_server_security(struct rxrpc_local *, struct rxrpc_sock *,
+                                  const struct rxrpc_security **, struct key **,
+                                  struct sk_buff *);
 
 /*
  * sendmsg.c

diff --git a/net/rxrpc/call_accept.c b/net/rxrpc/call_accept.c
index 44fa22b020ef97ad474a3e513eeac3fe34ff6b15..70e44abf106c86c731162a7a40be9963ba9b25f4 100644 (file)
--- a/net/rxrpc/call_accept.c
+++ b/net/rxrpc/call_accept.c
@@ -263,6 +263,8 @@ static struct rxrpc_call *rxrpc_alloc_incoming_call(struct rxrpc_sock *rx,
                                                    struct rxrpc_local *local,
                                                    struct rxrpc_peer *peer,
                                                    struct rxrpc_connection *conn,
+                                                   const struct rxrpc_security *sec,
+                                                   struct key *key,
                                                    struct sk_buff *skb)
 {
        struct rxrpc_backlog *b = rx->backlog;
@@ -310,7 +312,7 @@ static struct rxrpc_call *rxrpc_alloc_incoming_call(struct rxrpc_sock *rx,
                conn->params.local = rxrpc_get_local(local);
                conn->params.peer = peer;
                rxrpc_see_connection(conn);
-               rxrpc_new_incoming_connection(rx, conn, skb);
+               rxrpc_new_incoming_connection(rx, conn, sec, key, skb);
        } else {
                rxrpc_get_connection(conn);
        }
@@ -349,9 +351,11 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
                                           struct sk_buff *skb)
 {
        struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
+       const struct rxrpc_security *sec = NULL;
        struct rxrpc_connection *conn;
        struct rxrpc_peer *peer = NULL;
-       struct rxrpc_call *call;
+       struct rxrpc_call *call = NULL;
+       struct key *key = NULL;
 
        _enter("");
 
@@ -372,7 +376,11 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
         */
        conn = rxrpc_find_connection_rcu(local, skb, &peer);
 
-       call = rxrpc_alloc_incoming_call(rx, local, peer, conn, skb);
+       if (!conn && !rxrpc_look_up_server_security(local, rx, &sec, &key, skb))
+               goto no_call;
+
+       call = rxrpc_alloc_incoming_call(rx, local, peer, conn, sec, key, skb);
+       key_put(key);
        if (!call) {
                skb->mark = RXRPC_SKB_MARK_REJECT_BUSY;
                goto no_call;

diff --git a/net/rxrpc/conn_event.c b/net/rxrpc/conn_event.c
index a1ceef4f5cd07754fff2fa5e1472ff60a50a272e..808a4723f8684c849da712808938df11f3ce4872 100644 (file)
--- a/net/rxrpc/conn_event.c
+++ b/net/rxrpc/conn_event.c
@@ -376,21 +376,7 @@ static void rxrpc_secure_connection(struct rxrpc_connection *conn)
        _enter("{%d}", conn->debug_id);
 
        ASSERT(conn->security_ix != 0);
-
-       if (!conn->params.key) {
-               _debug("set up security");
-               ret = rxrpc_init_server_conn_security(conn);
-               switch (ret) {
-               case 0:
-                       break;
-               case -ENOENT:
-                       abort_code = RX_CALL_DEAD;
-                       goto abort;
-               default:
-                       abort_code = RXKADNOAUTH;
-                       goto abort;
-               }
-       }
+       ASSERT(conn->server_key);
 
        if (conn->security->issue_challenge(conn) < 0) {
                abort_code = RX_CALL_DEAD;

diff --git a/net/rxrpc/conn_service.c b/net/rxrpc/conn_service.c
index 123d6ceab15cb0b00ccf65aec61370b64cda811b..21da48e3d2e5188b79370910753dac54cfd14760 100644 (file)
--- a/net/rxrpc/conn_service.c
+++ b/net/rxrpc/conn_service.c
@@ -148,6 +148,8 @@ struct rxrpc_connection *rxrpc_prealloc_service_connection(struct rxrpc_net *rxn
  */
 void rxrpc_new_incoming_connection(struct rxrpc_sock *rx,
                                   struct rxrpc_connection *conn,
+                                  const struct rxrpc_security *sec,
+                                  struct key *key,
                                   struct sk_buff *skb)
 {
        struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
@@ -160,6 +162,8 @@ void rxrpc_new_incoming_connection(struct rxrpc_sock *rx,
        conn->service_id        = sp->hdr.serviceId;
        conn->security_ix       = sp->hdr.securityIndex;
        conn->out_clientflag    = 0;
+       conn->security          = sec;
+       conn->server_key        = key_get(key);
        if (conn->security_ix)
                conn->state     = RXRPC_CONN_SERVICE_UNSECURED;
        else

diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
index 8d8aa3c230b5515d8cde676432c77a733f14afe5..098f1f9ec53ba10642dc0c3d2c608a44de688e5b 100644 (file)
--- a/net/rxrpc/rxkad.c
+++ b/net/rxrpc/rxkad.c
@@ -648,9 +648,9 @@ static int rxkad_issue_challenge(struct rxrpc_connection *conn)
        u32 serial;
        int ret;
 
-       _enter("{%d,%x}", conn->debug_id, key_serial(conn->params.key));
+       _enter("{%d,%x}", conn->debug_id, key_serial(conn->server_key));
 
-       ret = key_validate(conn->params.key);
+       ret = key_validate(conn->server_key);
        if (ret < 0)
                return ret;
 
@@ -1293,6 +1293,7 @@ static void rxkad_exit(void)
 const struct rxrpc_security rxkad = {
        .name                           = "rxkad",
        .security_index                 = RXRPC_SECURITY_RXKAD,
+       .no_key_abort                   = RXKADUNKNOWNKEY,
        .init                           = rxkad_init,
        .exit                           = rxkad_exit,
        .init_connection_security       = rxkad_init_connection_security,

diff --git a/net/rxrpc/security.c b/net/rxrpc/security.c
index a4c47d2b705478eabf87b699da6852207f525f56..9b1fb9ed07177215aa533666d9af0fbc8bda791e 100644 (file)
--- a/net/rxrpc/security.c
+++ b/net/rxrpc/security.c
@@ -101,62 +101,58 @@ int rxrpc_init_client_conn_security(struct rxrpc_connection *conn)
 }
 
 /*
- * initialise the security on a server connection
+ * Find the security key for a server connection.
  */
-int rxrpc_init_server_conn_security(struct rxrpc_connection *conn)
+bool rxrpc_look_up_server_security(struct rxrpc_local *local, struct rxrpc_sock *rx,
+                                  const struct rxrpc_security **_sec,
+                                  struct key **_key,
+                                  struct sk_buff *skb)
 {
        const struct rxrpc_security *sec;
-       struct rxrpc_local *local = conn->params.local;
-       struct rxrpc_sock *rx;
-       struct key *key;
-       key_ref_t kref;
+       struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
+       key_ref_t kref = NULL;
        char kdesc[5 + 1 + 3 + 1];
 
        _enter("");
 
-       sprintf(kdesc, "%u:%u", conn->service_id, conn->security_ix);
+       sprintf(kdesc, "%u:%u", sp->hdr.serviceId, sp->hdr.securityIndex);
 
-       sec = rxrpc_security_lookup(conn->security_ix);
+       sec = rxrpc_security_lookup(sp->hdr.securityIndex);
        if (!sec) {
-               _leave(" = -ENOKEY [lookup]");
-               return -ENOKEY;
+               trace_rxrpc_abort(0, "SVS",
+                                 sp->hdr.cid, sp->hdr.callNumber, sp->hdr.seq,
+                                 RX_INVALID_OPERATION, EKEYREJECTED);
+               skb->mark = RXRPC_SKB_MARK_REJECT_ABORT;
+               skb->priority = RX_INVALID_OPERATION;
+               return false;
        }
 
-       /* find the service */
-       read_lock(&local->services_lock);
-       rx = rcu_dereference_protected(local->service,
-                                      lockdep_is_held(&local->services_lock));
-       if (rx && (rx->srx.srx_service == conn->service_id ||
-                  rx->second_service == conn->service_id))
-               goto found_service;
+       if (sp->hdr.securityIndex == RXRPC_SECURITY_NONE)
+               goto out;
 
-       /* the service appears to have died */
-       read_unlock(&local->services_lock);
-       _leave(" = -ENOENT");
-       return -ENOENT;
-
-found_service:
        if (!rx->securities) {
-               read_unlock(&local->services_lock);
-               _leave(" = -ENOKEY");
-               return -ENOKEY;
+               trace_rxrpc_abort(0, "SVR",
+                                 sp->hdr.cid, sp->hdr.callNumber, sp->hdr.seq,
+                                 RX_INVALID_OPERATION, EKEYREJECTED);
+               skb->mark = RXRPC_SKB_MARK_REJECT_ABORT;
+               skb->priority = RX_INVALID_OPERATION;
+               return false;
        }
 
        /* look through the service's keyring */
        kref = keyring_search(make_key_ref(rx->securities, 1UL),
                              &key_type_rxrpc_s, kdesc, true);
        if (IS_ERR(kref)) {
-               read_unlock(&local->services_lock);
-               _leave(" = %ld [search]", PTR_ERR(kref));
-               return PTR_ERR(kref);
+               trace_rxrpc_abort(0, "SVK",
+                                 sp->hdr.cid, sp->hdr.callNumber, sp->hdr.seq,
+                                 sec->no_key_abort, EKEYREJECTED);
+               skb->mark = RXRPC_SKB_MARK_REJECT_ABORT;
+               skb->priority = sec->no_key_abort;
+               return false;
        }
 
-       key = key_ref_to_ptr(kref);
-       read_unlock(&local->services_lock);
-
-       conn->server_key = key;
-       conn->security = sec;
-
-       _leave(" = 0");
-       return 0;
+out:
+       *_sec = sec;
+       *_key = key_ref_to_ptr(kref);
+       return true;
 }