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 = 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 = 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 = 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 = 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) = 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"
> 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.
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.
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 = 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
_______________________________________________
AFS3-standardization mailing list
[email protected]
http://lists.openafs.org/mailman/listinfo/afs3-standardization