rxrpc: Don't retain the server key in the connection
authorDavid Howells <dhowells@redhat.com>
Wed, 16 Sep 2020 07:00:44 +0000 (08:00 +0100)
committerDavid Howells <dhowells@redhat.com>
Mon, 23 Nov 2020 18:09:29 +0000 (18:09 +0000)
Don't retain a pointer to the server key in the connection, but rather get
it on demand when the server has to deal with a response packet.

This is necessary to implement RxGK (GSSAPI-mediated transport class),
where we can't know which key we'll need until we've challenged the client
and got back the response.

This also means that we don't need to do a key search in the accept path in
softirq mode.

Also, whilst we're at it, allow the security class to ask for a kvno and
encoding-type variant of a server key as RxGK needs different keys for
different encoding types.  Keys of this type have an extra bit in the
description:

"<service-id>:<security-index>:<kvno>:<enctype>"

Signed-off-by: David Howells <dhowells@redhat.com>
net/rxrpc/ar-internal.h
net/rxrpc/call_accept.c
net/rxrpc/conn_event.c
net/rxrpc/conn_object.c
net/rxrpc/conn_service.c
net/rxrpc/rxkad.c
net/rxrpc/security.c

index 3c417ec94e4c9fb5e91065f921a11045b452381c..db6e754743fbef837976bd038b3ecb1db43784ce 100644 (file)
@@ -441,7 +441,6 @@ struct rxrpc_connection {
        struct list_head        link;           /* link in master connection list */
        struct sk_buff_head     rx_queue;       /* received conn-level packets */
        const struct rxrpc_security *security;  /* applied security module */
-       struct key              *server_key;    /* security for this service */
        struct crypto_sync_skcipher *cipher;    /* encryption handle */
        struct rxrpc_crypt      csum_iv;        /* packet checksum base */
        unsigned long           flags;
@@ -890,8 +889,7 @@ 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 *,
-                                  const struct rxrpc_security *, struct key *,
-                                  struct sk_buff *);
+                                  const struct rxrpc_security *, struct sk_buff *);
 void rxrpc_unpublish_service_conn(struct rxrpc_connection *);
 
 /*
@@ -1056,9 +1054,10 @@ 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 *);
-bool rxrpc_look_up_server_security(struct rxrpc_local *, struct rxrpc_sock *,
-                                  const struct rxrpc_security **, struct key **,
-                                  struct sk_buff *);
+const struct rxrpc_security *rxrpc_get_incoming_security(struct rxrpc_sock *,
+                                                        struct sk_buff *);
+struct key *rxrpc_look_up_server_security(struct rxrpc_connection *,
+                                         struct sk_buff *, u32, u32);
 
 /*
  * sendmsg.c
index 8df1964db33328b44006b2aab2a2b953029afc86..382add72c66f6df606c552d4109fa3aa9bfcb6aa 100644 (file)
@@ -261,7 +261,6 @@ static struct rxrpc_call *rxrpc_alloc_incoming_call(struct rxrpc_sock *rx,
                                                    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;
@@ -309,7 +308,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, sec, key, skb);
+               rxrpc_new_incoming_connection(rx, conn, sec, skb);
        } else {
                rxrpc_get_connection(conn);
        }
@@ -353,7 +352,6 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
        struct rxrpc_connection *conn;
        struct rxrpc_peer *peer = NULL;
        struct rxrpc_call *call = NULL;
-       struct key *key = NULL;
 
        _enter("");
 
@@ -374,11 +372,13 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
         */
        conn = rxrpc_find_connection_rcu(local, skb, &peer);
 
-       if (!conn && !rxrpc_look_up_server_security(local, rx, &sec, &key, skb))
-               goto no_call;
+       if (!conn) {
+               sec = rxrpc_get_incoming_security(rx, skb);
+               if (!sec)
+                       goto no_call;
+       }
 
-       call = rxrpc_alloc_incoming_call(rx, local, peer, conn, sec, key, skb);
-       key_put(key);
+       call = rxrpc_alloc_incoming_call(rx, local, peer, conn, sec, skb);
        if (!call) {
                skb->mark = RXRPC_SKB_MARK_REJECT_BUSY;
                goto no_call;
index bbf86203ed257abbf6ec83e702b3f1f790e4046b..03a482ba770f77b217b8b18bd72483d0ae467bc1 100644 (file)
@@ -378,7 +378,6 @@ static void rxrpc_secure_connection(struct rxrpc_connection *conn)
        _enter("{%d}", conn->debug_id);
 
        ASSERT(conn->security_ix != 0);
-       ASSERT(conn->server_key);
 
        if (conn->security->issue_challenge(conn) < 0) {
                abort_code = RX_CALL_DEAD;
index 3bcbe0665f9152dae2e1074418b0cac1eba37025..8dd1ef25b98f1b19abf9092e78d9228f52e92984 100644 (file)
@@ -363,7 +363,6 @@ static void rxrpc_destroy_connection(struct rcu_head *rcu)
 
        conn->security->clear(conn);
        key_put(conn->params.key);
-       key_put(conn->server_key);
        rxrpc_put_bundle(conn->bundle);
        rxrpc_put_peer(conn->params.peer);
 
index 6c847720494fae5b268619cbb8230a618de676b3..e1966dfc915274a79daaafa710e3fd69419b8f94 100644 (file)
@@ -156,7 +156,6 @@ 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);
@@ -170,7 +169,6 @@ void rxrpc_new_incoming_connection(struct rxrpc_sock *rx,
        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
index 404d1323c23988bd45c50bde3e5888a38a90015c..0d21935dac2706bfe40d2dbf1130b2111e7e1095 100644 (file)
@@ -647,11 +647,7 @@ static int rxkad_issue_challenge(struct rxrpc_connection *conn)
        u32 serial;
        int ret;
 
-       _enter("{%d,%x}", conn->debug_id, key_serial(conn->server_key));
-
-       ret = key_validate(conn->server_key);
-       if (ret < 0)
-               return ret;
+       _enter("{%d}", conn->debug_id);
 
        get_random_bytes(&conn->security_nonce, sizeof(conn->security_nonce));
 
@@ -891,6 +887,7 @@ other_error:
  * decrypt the kerberos IV ticket in the response
  */
 static int rxkad_decrypt_ticket(struct rxrpc_connection *conn,
+                               struct key *server_key,
                                struct sk_buff *skb,
                                void *ticket, size_t ticket_len,
                                struct rxrpc_crypt *_session_key,
@@ -910,30 +907,17 @@ static int rxkad_decrypt_ticket(struct rxrpc_connection *conn,
        u32 abort_code;
        u8 *p, *q, *name, *end;
 
-       _enter("{%d},{%x}", conn->debug_id, key_serial(conn->server_key));
+       _enter("{%d},{%x}", conn->debug_id, key_serial(server_key));
 
        *_expiry = 0;
 
-       ret = key_validate(conn->server_key);
-       if (ret < 0) {
-               switch (ret) {
-               case -EKEYEXPIRED:
-                       abort_code = RXKADEXPIRED;
-                       goto other_error;
-               default:
-                       abort_code = RXKADNOAUTH;
-                       goto other_error;
-               }
-       }
-
-       ASSERT(conn->server_key->payload.data[0] != NULL);
+       ASSERT(server_key->payload.data[0] != NULL);
        ASSERTCMP((unsigned long) ticket & 7UL, ==, 0);
 
-       memcpy(&iv, &conn->server_key->payload.data[2], sizeof(iv));
+       memcpy(&iv, &server_key->payload.data[2], sizeof(iv));
 
        ret = -ENOMEM;
-       req = skcipher_request_alloc(conn->server_key->payload.data[0],
-                                    GFP_NOFS);
+       req = skcipher_request_alloc(server_key->payload.data[0], GFP_NOFS);
        if (!req)
                goto temporary_error;
 
@@ -1089,6 +1073,7 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
        struct rxkad_response *response;
        struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
        struct rxrpc_crypt session_key;
+       struct key *server_key;
        const char *eproto;
        time64_t expiry;
        void *ticket;
@@ -1096,7 +1081,27 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
        __be32 csum;
        int ret, i;
 
-       _enter("{%d,%x}", conn->debug_id, key_serial(conn->server_key));
+       _enter("{%d}", conn->debug_id);
+
+       server_key = rxrpc_look_up_server_security(conn, skb, 0, 0);
+       if (IS_ERR(server_key)) {
+               switch (PTR_ERR(server_key)) {
+               case -ENOKEY:
+                       abort_code = RXKADUNKNOWNKEY;
+                       break;
+               case -EKEYEXPIRED:
+                       abort_code = RXKADEXPIRED;
+                       break;
+               default:
+                       abort_code = RXKADNOAUTH;
+                       break;
+               }
+               trace_rxrpc_abort(0, "SVK",
+                                 sp->hdr.cid, sp->hdr.callNumber, sp->hdr.seq,
+                                 abort_code, PTR_ERR(server_key));
+               *_abort_code = abort_code;
+               return -EPROTO;
+       }
 
        ret = -ENOMEM;
        response = kzalloc(sizeof(struct rxkad_response), GFP_NOFS);
@@ -1144,8 +1149,8 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
                          ticket, ticket_len) < 0)
                goto protocol_error_free;
 
-       ret = rxkad_decrypt_ticket(conn, skb, ticket, ticket_len, &session_key,
-                                  &expiry, _abort_code);
+       ret = rxkad_decrypt_ticket(conn, server_key, skb, ticket, ticket_len,
+                                  &session_key, &expiry, _abort_code);
        if (ret < 0)
                goto temporary_error_free_ticket;
 
@@ -1224,6 +1229,7 @@ protocol_error_free:
 protocol_error:
        kfree(response);
        trace_rxrpc_rx_eproto(NULL, sp->hdr.serial, eproto);
+       key_put(server_key);
        *_abort_code = abort_code;
        return -EPROTO;
 
@@ -1236,6 +1242,7 @@ temporary_error:
         * ENOMEM.  We just want to send the challenge again.  Note that we
         * also come out this way if the ticket decryption fails.
         */
+       key_put(server_key);
        return ret;
 }
 
index 0c5168f52bd6c08728520dc4b9ddedc658849be8..bef9971e15cdba6c2bb33885134fa697af57dcf8 100644 (file)
@@ -102,22 +102,16 @@ found:
 }
 
 /*
- * Find the security key for a server connection.
+ * Set the ops a server connection.
  */
-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 *rxrpc_get_incoming_security(struct rxrpc_sock *rx,
+                                                        struct sk_buff *skb)
 {
        const struct rxrpc_security *sec;
        struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
-       key_ref_t kref = NULL;
-       char kdesc[5 + 1 + 3 + 1];
 
        _enter("");
 
-       sprintf(kdesc, "%u:%u", sp->hdr.serviceId, sp->hdr.securityIndex);
-
        sec = rxrpc_security_lookup(sp->hdr.securityIndex);
        if (!sec) {
                trace_rxrpc_abort(0, "SVS",
@@ -125,35 +119,72 @@ bool rxrpc_look_up_server_security(struct rxrpc_local *local, struct rxrpc_sock
                                  RX_INVALID_OPERATION, EKEYREJECTED);
                skb->mark = RXRPC_SKB_MARK_REJECT_ABORT;
                skb->priority = RX_INVALID_OPERATION;
-               return false;
+               return NULL;
        }
 
-       if (sp->hdr.securityIndex == RXRPC_SECURITY_NONE)
-               goto out;
-
-       if (!rx->securities) {
+       if (sp->hdr.securityIndex != RXRPC_SECURITY_NONE &&
+           !rx->securities) {
                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;
+               skb->priority = sec->no_key_abort;
+               return NULL;
        }
 
+       return sec;
+}
+
+/*
+ * Find the security key for a server connection.
+ */
+struct key *rxrpc_look_up_server_security(struct rxrpc_connection *conn,
+                                         struct sk_buff *skb,
+                                         u32 kvno, u32 enctype)
+{
+       struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
+       struct rxrpc_sock *rx;
+       struct key *key = ERR_PTR(-EKEYREJECTED);
+       key_ref_t kref = NULL;
+       char kdesc[5 + 1 + 3 + 1 + 12 + 1 + 12 + 1];
+       int ret;
+
+       _enter("");
+
+       if (enctype)
+               sprintf(kdesc, "%u:%u:%u:%u",
+                       sp->hdr.serviceId, sp->hdr.securityIndex, kvno, enctype);
+       else if (kvno)
+               sprintf(kdesc, "%u:%u:%u",
+                       sp->hdr.serviceId, sp->hdr.securityIndex, kvno);
+       else
+               sprintf(kdesc, "%u:%u",
+                       sp->hdr.serviceId, sp->hdr.securityIndex);
+
+       rcu_read_lock();
+
+       rx = rcu_dereference(conn->params.local->service);
+       if (!rx)
+               goto out;
+
        /* 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)) {
-               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 = ERR_CAST(kref);
+               goto out;
+       }
+
+       key = key_ref_to_ptr(kref);
+
+       ret = key_validate(key);
+       if (ret < 0) {
+               key_put(key);
+               key = ERR_PTR(ret);
+               goto out;
        }
 
 out:
-       *_sec = sec;
-       *_key = key_ref_to_ptr(kref);
-       return true;
+       rcu_read_unlock();
+       return key;
 }