From 997a9980e33ae92abdaa81d07d0857d41dd6e9ae Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Mon, 14 Jan 2013 12:45:04 -0600 Subject: [PATCH] rx: Honor RXS_PreparePacket errors rxi_PrepareSendPacket calls RXS_PreparePacket to allow the security class to modify the given packet appropriately (to be undone by CheckPacket on the other endpoint). However, currently rxi_PrepareSendPacket ignores all errors generated by RXS_PreparePacket, and processing continues as if there was no error. For rxkad, an error often results in the given packet being untouched. This means that the security checksum is not calculated, and thus not populated in the packet, and for encrypted connections means that the packet contents are not encrypted. This occurs for any error generated by the security class PreparePacket routine. For rxkad, the most common error is probably RXKADEXPIRED, though some other internal errors are possible as well. This behavior has a few effects for rxkad: 1. When any error is generated by PreparePacket, the other endpoint generally bails out with the error RXKADSEALEDINCON, since the security checksum of the packet is 0, which does not match what the checksum should be. This results in error messages like 'rxk: sealed data inconsistent'. This can be very confusing if the actual error is, say, just that the given credentials have expired. 2. For connections requiring encryption (rxkad_crypt), an error from PreparePacket means that the packet payload is sent in the clear. This can happen for about a window size's worth of packets. 3. If a client ignores errors/inconsistencies with the checksum and encryption, etc, they can keep reading data for the call forever, even after their credentials have expired. To fix this, make an error from RXS_PreparePacket cause a connection error for the given connection, and immediately send a connection abort. No further error checking should be necessary for the callers of rxi_PrepareSendPacket, since they already check for call/conn errors before sending any actual packets. Reviewed-on: http://gerrit.openafs.org/8909 Tested-by: BuildBot Reviewed-by: Simon Wilkinson Reviewed-by: Derrick Brashear (cherry picked from commit 03d3dacae16847352af754ac13c854ca0df0c08c) Change-Id: I5d0f421d22ca2e4d723df2d698088b6bbdc85f7b Reviewed-on: http://gerrit.openafs.org/9279 Tested-by: BuildBot Reviewed-by: Andrew Deason Reviewed-by: Jeffrey Altman Reviewed-by: Stephan Wiesand --- src/rx/rx_packet.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/rx/rx_packet.c b/src/rx/rx_packet.c index 89f56da8e..218cdd4bb 100644 --- a/src/rx/rx_packet.c +++ b/src/rx/rx_packet.c @@ -2747,6 +2747,7 @@ rxi_PrepareSendPacket(struct rx_call *call, afs_uint32 seq = call->tnext++; unsigned int i; afs_int32 len; /* len must be a signed type; it can go negative */ + int code; /* No data packets on call 0. Where do these come from? */ if (*call->callNumber == 0) @@ -2798,7 +2799,15 @@ rxi_PrepareSendPacket(struct rx_call *call, if (len) p->wirevec[i - 1].iov_len += len; MUTEX_ENTER(&call->lock); - RXS_PreparePacket(conn->securityObject, call, p); + code = RXS_PreparePacket(conn->securityObject, call, p); + if (code) { + MUTEX_EXIT(&call->lock); + rxi_ConnectionError(conn, code); + MUTEX_ENTER(&conn->conn_data_lock); + p = rxi_SendConnectionAbort(conn, p, 0, 0); + MUTEX_EXIT(&conn->conn_data_lock); + MUTEX_ENTER(&call->lock); + } } /* Given an interface MTU size, calculate an adjusted MTU size that -- 2.39.5