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.

Change-Id: I87de833730424881dcd3d659870f71191eabafe4
Reviewed-on: http://gerrit.openafs.org/8909
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Simon Wilkinson <simonxwilkinson@gmail.com>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>

diff --git a/src/rx/rx_packet.c b/src/rx/rx_packet.c
index 68d6593..ac10c27 100644 (file)
--- a/src/rx/rx_packet.c
+++ b/src/rx/rx_packet.c
@@ -2725,6 +2725,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)
@@ -2776,7 +2777,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