From ef68622da9cc0c4e5202f90093a3a5314e41e9e9 Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 6 Apr 2017 10:11:59 +0100 Subject: [PATCH] rxrpc: Handle temporary errors better in rxkad security In the rxkad security module, when we encounter a temporary error (such as ENOMEM) from which we could conceivably recover, don't abort the connection, but rather permit retransmission of the relevant packets to induce a retry. Note that I'm leaving some places that could be merged together to insert tracing in the next patch. Signed-off-by; David Howells --- net/rxrpc/rxkad.c | 78 ++++++++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 38 deletions(-) diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c index 2d5838a3dc24..988903f1dc80 100644 --- a/net/rxrpc/rxkad.c +++ b/net/rxrpc/rxkad.c @@ -759,16 +759,14 @@ static int rxkad_respond_to_challenge(struct rxrpc_connection *conn, _enter("{%d,%x}", conn->debug_id, key_serial(conn->params.key)); - if (!conn->params.key) { - _leave(" = -EPROTO [no key]"); - return -EPROTO; - } + abort_code = RX_PROTOCOL_ERROR; + if (!conn->params.key) + goto protocol_error; + abort_code = RXKADEXPIRED; ret = key_validate(conn->params.key); - if (ret < 0) { - *_abort_code = RXKADEXPIRED; - return ret; - } + if (ret < 0) + goto other_error; abort_code = RXKADPACKETSHORT; if (skb_copy_bits(skb, sizeof(struct rxrpc_wire_header), @@ -787,8 +785,9 @@ static int rxkad_respond_to_challenge(struct rxrpc_connection *conn, goto protocol_error; abort_code = RXKADLEVELFAIL; + ret = -EACCES; if (conn->params.security_level < min_level) - goto protocol_error; + goto other_error; token = conn->params.key->payload.data[0]; @@ -815,9 +814,10 @@ static int rxkad_respond_to_challenge(struct rxrpc_connection *conn, return rxkad_send_response(conn, &sp->hdr, &resp, token->kad); protocol_error: + ret = -EPROTO; +other_error: *_abort_code = abort_code; - _leave(" = -EPROTO [%d]", abort_code); - return -EPROTO; + return ret; } /* @@ -848,10 +848,10 @@ static int rxkad_decrypt_ticket(struct rxrpc_connection *conn, switch (ret) { case -EKEYEXPIRED: *_abort_code = RXKADEXPIRED; - goto error; + goto other_error; default: *_abort_code = RXKADNOAUTH; - goto error; + goto other_error; } } @@ -860,13 +860,11 @@ static int rxkad_decrypt_ticket(struct rxrpc_connection *conn, memcpy(&iv, &conn->server_key->payload.data[2], sizeof(iv)); + ret = -ENOMEM; req = skcipher_request_alloc(conn->server_key->payload.data[0], GFP_NOFS); - if (!req) { - *_abort_code = RXKADNOAUTH; - ret = -ENOMEM; - goto error; - } + if (!req) + goto temporary_error; sg_init_one(&sg[0], ticket, ticket_len); skcipher_request_set_callback(req, 0, NULL, NULL); @@ -943,13 +941,13 @@ static int rxkad_decrypt_ticket(struct rxrpc_connection *conn, if (issue > now) { *_abort_code = RXKADNOAUTH; ret = -EKEYREJECTED; - goto error; + goto other_error; } if (issue < now - life) { *_abort_code = RXKADEXPIRED; ret = -EKEYEXPIRED; - goto error; + goto other_error; } *_expiry = issue + life; @@ -961,16 +959,15 @@ static int rxkad_decrypt_ticket(struct rxrpc_connection *conn, /* get the service instance name */ name = Z(INST_SZ); _debug("KIV SINST: %s", name); - - ret = 0; -error: - _leave(" = %d", ret); - return ret; + return 0; bad_ticket: *_abort_code = RXKADBADTICKET; - ret = -EBADMSG; - goto error; + ret = -EPROTO; +other_error: + return ret; +temporary_error: + return ret; } /* @@ -1054,9 +1051,10 @@ static int rxkad_verify_response(struct rxrpc_connection *conn, goto protocol_error; /* extract the kerberos ticket and decrypt and decode it */ + ret = -ENOMEM; ticket = kmalloc(ticket_len, GFP_NOFS); if (!ticket) - return -ENOMEM; + goto temporary_error; abort_code = RXKADPACKETSHORT; if (skb_copy_bits(skb, sizeof(struct rxrpc_wire_header), @@ -1064,12 +1062,9 @@ static int rxkad_verify_response(struct rxrpc_connection *conn, goto protocol_error_free; ret = rxkad_decrypt_ticket(conn, ticket, ticket_len, &session_key, - &expiry, &abort_code); - if (ret < 0) { - *_abort_code = abort_code; - kfree(ticket); - return ret; - } + &expiry, _abort_code); + if (ret < 0) + goto temporary_error_free; /* use the session key from inside the ticket to decrypt the * response */ @@ -1123,10 +1118,8 @@ static int rxkad_verify_response(struct rxrpc_connection *conn, * this the connection security can be handled in exactly the same way * as for a client connection */ ret = rxrpc_get_server_data_key(conn, &session_key, expiry, kvno); - if (ret < 0) { - kfree(ticket); - return ret; - } + if (ret < 0) + goto temporary_error_free; kfree(ticket); _leave(" = 0"); @@ -1140,6 +1133,15 @@ protocol_error: *_abort_code = abort_code; _leave(" = -EPROTO [%d]", abort_code); return -EPROTO; + +temporary_error_free: + kfree(ticket); +temporary_error: + /* Ignore the response packet if we got a temporary error such as + * ENOMEM. We just want to send the challenge again. Note that we + * also come out this way if the ticket decryption fails. + */ + return ret; } /* -- 2.20.1