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

[Michael Meffie]

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

> Hi,
> 
> Just wanted to raise the following further, as it's not been addressed in any of the recent changes:
> 
> >> 7.3 RPC Definition
> >> ------------------
> >> 
> >>>      struct RXGK_TokenInfo {
> >>>          int errorcode;
> >> 
> >> 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.
> > 
> > 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.

That is a good point and does seem like a better API choice. If the errorcode
is present, none of the other fields are valid, so that does imply a more
understanble choice would be to have the errorcode (if it stays) as a separate
out arg.



-- 
Michael Meffie <mmeffie@sinenomine.net>