rx: Do not ignore RXS_* op errors 22/13522/4
authorAndrew Deason <adeason@sinenomine.net>
Wed, 13 Mar 2019 23:30:43 +0000 (18:30 -0500)
committerBenjamin Kaduk <kaduk@mit.edu>
Sat, 23 Mar 2019 22:43:40 +0000 (18:43 -0400)
commit635594d6cceba6de4e09be5a9e9b908f7d16697d
tree54de7caf22ba4bb37d8fbe11a2b6422c64d34a51
parent2ee35afa339731f6a60f1e5e99ccaf63baa6c891
rx: Do not ignore RXS_* op errors

Several places in rx call an RXS_* security layer operation, but
ignore the error code. Though errors for these operations are rare or
impossible currently, if they ever do return an error there could be
noticeable consequences, like a connection getting an uninitialized
challenge nonce, or sending a challenge packet with uninitialized
payload.

Change these call sites to record and handle the error. Errors from
the security class normally mean aborting the entire conn, but for
many operations we need to behave differently:

- For RXS_DestroyConnection, errors don't make sense, since we're just
  freeing an object. Change the op to return void, and update our
  implementations of DestroyConnection to match.

- For RXS_GetStats, just clear the relevant stats structure on error
  instead. This change also results in us clearing the stats structure
  when there is no security class associated with the connection;
  previously we just reused the same struct data as the previous conn.

- For RXS_CreateChallenge, aborting the entire conn is difficult,
  because some code paths have callers that potentially lock multiple
  calls on the same conn (rxi_UpdatePeerReach -> TryAttach ->
  rxi_ChallengeOn -> RXS_CreateChallenge), and aborting our conn
  requires locking every call on the conn. So instead we just
  propagate an error up to our callers, and we abort just the call we
  have.

- For RXS_GetChallenge, we cannot abort the conn when
  rxi_ChallengeEvent is called directly, because the caller will have
  the call locked. But when rxi_ChallengeEvent is called as an event
  (when we retry sending the challenge), we can.

- For RXS_SetConfiguration, propagate the error up to our caller.
  Update all rx_SetSecurityConfiguration callers to record and handle
  the error; all of these are during initialization of daemons, so
  have them log an error and exit.

Change-Id: I138b3e06da00470c7d70c458879cc741d296d225
Reviewed-on: https://gerrit.openafs.org/13522
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
12 files changed:
src/bozo/bosserver.c
src/ptserver/ptserver.c
src/rx/rx.c
src/rx/rx.h
src/rx/rx_packet.c
src/rxgk/rxgk_client.c
src/rxgk/rxgk_server.c
src/rxkad/rxkad_common.c
src/rxkad/rxkad_prototypes.h
src/viced/viced.c
src/vlserver/vlserver.c
src/volser/volmain.c