[AFS3-std] Last Call: afs3-rxgk-04, afs3-rxgk-afs-02
Simon Wilkinson
simon@sxw.org.uk
Thu, 25 Apr 2013 16:03:07 +0100
Here are some comments on afs3-rxgk-04. Many of these are editing notes, =
but I think this document needs further work with regards to the =
description of the GSSAPI handshake, and a fuller discussion of limits =
to the sizes of the various opaque data types is required.
Abstract
--------
> This document provides a general description of
> rxgk. A further document will provide details of integration with
> specific RX applications.
I think we're now at the point where this document provides information =
on how to integrate rxgk with generic RX applications - the only =
information in rxgk-afs is how to integrate it with the AFS-3 protocol =
family - so I think this could be clearer. Perhaps:
"This document provides a general description of rxgk and how to =
integrate it into generic RX applications. Application specific =
behaviour will
be described, as necessary, in future documents"
1. Introduction
---------------
> It builds on the Kerberos crypto framework
There should probably be an RFC3961 reference here.
3.1 Key Usage Values
--------------------
> const RXGK_SERVER_ENC_TOKEN =3D 1036;
Indentation is a bit messed up here
5. Token Format
---------------
> The token is completely opaque to the client, which just transmits it =
from server to server
I had some trouble parsing "server to server" in this sentence. I wonder =
if "which just receives it from one server and passes it to another" =
might make the intent clearer.
6. Key Negotiation
------------------
> const RXGK_MAXENCTYPES =3D 10;
Limiting to a maximum of 10 encryption types seems somewhat low, given =
that this is a hard limit that can't be negotiated around. Each =
encryption type only takes 4 bytes on the wire, so there doesn't seem to =
be a resource-based limit here. Given the sprawling proliferation of =
Kerberos encryption types, I can see somebody, at some point, wanting to =
use more than 10 here.
> const RXGK_MAXLEVELS =3D 10;
Whilst we only support 3 levels today, again a hard limit of 10 seems =
low.
How about a limit of 255 for each of these?
> const RXGK_MAXMIC =3D 16384;
By contrast, this seems large. The MIC is going to be the length of the =
chosen encryption type's checksum, plus a small amount of header =
material. For aes-256, that's a mere 28 octets (16 of CFX token, 12 of =
checksum). So, I think we could get away with something smaller here.
> typedef opaque RXGK_Data<>;
[ ... ]
> RXGK_Data token;
Given we're limiting everything else, should we also put some kind of =
upper limit on the size of the token? We do need to consider, though, =
that a token may contain non-Kerberos identities, and X509 (in =
particular) identities can be very large.
> GSSNegotiate(IN RXGK_StartParams *client_start,
> IN RXGK_Data *input_token_buffer,
> IN RXGK_Data *opaque_in,
> OUT RXGK_Data *output_token_buffer,
> OUT RXGK_Data *opaque_out,
> OUT unsigned int *gss_major_status,
> OUT unsigned int *gss_minor_status,
> OUT RXGK_Data *rxgk_info) =3D 1;
Same question here - RXGK_Data has no limits.
> This nonce SHOULD be at least 20 octets in length.
Is SHOULD strong enough here? Using a nonce that is too short will =
reduce the security of the resulting key.
> The GSS service name is application dependent.
We've described how to construct the acceptor name earlier in this =
section - perhaps refer back to that?
> The client then calls RXGK_GSSNegotiate, as defined above.
The client should only call RXGK_GSSNegotiate if the call to =
GSS_Init_sec_context succeeded
> client_start The client params structure detailed above. This
> should remain constant across the negotiation
"should" or SHOULD, or even MUST?
> Each call to GSSNegotiate will return an output token from
> GSS_Accept_sec_context() and/or an output opaque
I don't think that "and/or" is appropriate here. Output opaques are =
separate from the negotiation itself - they just serve as a means for =
the server to preserve state across multiple RPCs.
> If the major status code from GSS_Init_sec_context() includes =
a
> fatal error code, the negotiation loop is in an error =
condition
> and terminates.
"includes a fatal error code" doesn't seem clearly defined. How about =
just "indicates a GSSAPI error" =20
> If the major status code is GSS_S_COMPLETE and
> the mutual_state flag is not true, or the major status code is
> GSS_S_COMPLETE and the conf_avail flag is not true, or the
> major status code is GSS_S_COMPLETE and the integ_avail flag =
is
> not true, the negotiation loop is in an error condition and
> terminates. =20
This could be expressed much more succinctly as:
"If the major status code is GSS_S_COMPLETE and the mutual_state, =
conf_avail flag and integ_avail flags are not all true, the negotiation =
loop is in an error condition and terminates"
> If the major status code is GSS_S_COMPLETE and the
> output token is zero length, this is a success condition and
> the negotiation loop terminates (this cannot happen on the
> first iteration of the loop). Otherwise, if the major status
> code does not include GSS_S_CONTINUE_NEEDED, the negotiation
> loop is in an error condition and terminates. If the major
> status code includes GSS_S_CONTINUE_NEEDED, the output token =
is
> sent to the server, per the next step.
This paragraph suggests that returning GSS_S_COMPLETE from =
GSS_Init_sec_context() and a non-empty output token is an error. All of =
the other GSS negotiation loops that I'm aware of permit this behaviour =
(the output token is simply sent to the server). This behaviour is also =
specifically permitted by RFC 4462.
> If the most recent call to
> GSS_Init_sec_context() returned a major status code including
> GSS_S_CONTINUE_NEEDED and the output_token_buffer is zero
> length, the negotiation loop is in error and terminates.
This doesn't need to be explicitly stated - if it is an error, then =
Gss_Init_sec_context() will handle it when we feed it the zero length =
token.=20
Also:
When the server receives GSS_S_COMPLETE from Gss_Accept_sec_context, it =
also needs to check the context flags and make sure that at least =
conf_avail and integ_avail are true. It needs to do that before it =
constructs the ClientInfo structure.
I found the description of the loop hard going, and I know what it =
should do! I'm not sure how best to improve this, but as I've said =
before, I believe the language, and layout, of the description in the =
SSH GSSAPI RFC to be a good place to start.
> If the returned value of conf_state is zero, the
> negotiation has failed
It needs to be clearer that this is an output parameter from gss_unwrap. =
I started out grepping the document to work out where it was coming =
from!
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.
8.5 The Response
----------------
> const RXGK_MAXAUTHENTICATOR =3D 1500 /* better fit in a packet! */
The normal maximum size of an RX payload is 1416 bytes, so an =
authenticator of 1500 definitely won't fit in a packet! rxkad ends up =
taking liberties here, and just extends the maximum size of packet when =
the data it needs to transmit is too large, but that causes issues on =
networks that drop fragmented UDP. It would be good to avoid those =
here...
12.3 Nonce Lengths
------------------
> The client_nonce is important for using the MIC of the StartParams
> structure as a defense against man-in-the-middle attacks,
I'm not sure what this sentence is trying to say?
Cheers,
Simon