[AFS3-std] Last Call: afs3-rxgk-04, afs3-rxgk-afs-02

Simon Wilkinson simon@sxw.org.uk
Mon, 6 May 2013 16:47:03 +0100


Hi,

Just wanted to raise the following further, as it's not been addressed =
in any of the recent changes:

>> 7.3 RPC Definition
>> ------------------
>>=20
>>>      struct RXGK_TokenInfo {
>>>          int errorcode;
>>=20
>> I'm not convinced that we need a separate error code here. Clients =
shouldn't be making negotiation decisions based upon the results of =
CombineTokens - a failure of this function should just be reported up to =
the user.
>=20
> This came in back in November when we realized that (at that time) =
CombineTokens as written was useless, and we pulled in all of the =
interesting bits from AFSCombineTokens.  At the moment, the =
RXGK_TokenInfo data structure is shared between CombineTokens and =
AFSCombineTokens; the latter surely does need an in-band errorcode =
field.  I would kind of like to avoid having an abundance of =
similar-but-different data structures and keep this sharing, even if the =
errorcode might not be strictly necessary here.  (I haven't implemented =
a CombineTokens yet, so I don't have insight to add from experience.)

I still have problems with errorcode in this structure definition. =
Firstly, I think returning an 'secure' errorcode from CombineTokens, as =
opposed to an RX abort, is poorly defined. I don't think that it is =
clear the circumstances where a client would be using multiple calls to =
CombineTokens to negotiate, either within rxgk itself, or to negotiate =
between multiple encryption types. So, I still believe, having =
implemented this, that a secure means of transmitting an error here is =
not required.

Secondly, if an errorcode is required, then I'm not sure that it should =
be part of the RXGK_TokenInfo structure. The errorcode isn't information =
about the returned token, it is a direct parameter of the RPC. So, if it =
is included, it should be a separate argument, rather than being buried =
within a structure.

Cheers,

Simon.