Re: WG Last Call: draft-ietf-sasl-gs2-02.txt

[Alexey Melnikov <alexey.melnikov@xxxxxxxxx>]

My review comments are below. Some of them you probably already 
addressed and some of them are nitpicking.

>4.1.  SASL Tokens
>
>   All top-level SASL packets for the GS2 mechanism family follow the
>   following format:

I would suggest avoid using the term SASL packet, as it usually refers to
SASL security layer. Something like "SASL authentication exchange packets"
would be better here.

>4.2.  Context Token

[...]

>   The client calls GSS_Init_sec_context, passing in
>   input_context_handle of 0 (initially), mech_type of the GSSAPI
>   mechanism for which this SASL mechanism is registered, the
>   chan_binding is set to NULL, and targ_name equal to output_name from
>   GSS_Import_Name called with input_name_type of
>   GSS_C_NT_HOSTBASED_SERVICE and input_name_string of

The GS1 has the note marker (*) after the "GSS_C_NT_HOSTBASED_SERVICE",
the note reads:

  (*) - Clients MAY use name types other than
  GSS_C_NT_HOSTBASED_SERVICE to import servers' acceptor names, but
  only when they have a priori knowledge that the servers support
  alternate name types.  Otherwise clients MUST use
  GSS_C_NT_HOSTBASED_SERVICE for importing acceptor names.

This is to address recent Nico's proposal to allow for domain based names.

[...]

>   If the client will be requesting a security
>   layer, it MUST also supply to the GSS_Init_sec_context a
>   mutual_req_flag of TRUE, a sequence_req_flag of TRUE, and an
>   integ_req_flag of TRUE.  If the client will be requesting a security
>   layer providing confidentiality protection, it MUST also supply to
>   the GSS_Init_sec_context a conf_req_flag of TRUE.

This text got replaced with the following in GS1:

  When calling the GSS_Init_sec_context the client
  MUST pass the integ_req_flag of TRUE.  If the client will be
  requesting a security layer, it MUST also supply to the
  GSS_Init_sec_context a mutual_req_flag of TRUE, and a
  sequence_req_flag of TRUE.  If the client will be requesting a
  security layer providing confidentiality protection, it MUST also
  supply to the GSS_Init_sec_context a conf_req_flag of TRUE.


>   If GSS_Init_sec_context returns GSS_S_CONTINUE_NEEDED, then the client
>   expect the server to issue a token in a subsequent challenge or as
>   additional information to the outcome of the authentication.

I suggest adding before this sentence (as in GS1):

  The client then responds with the resulting output_token.

otherwise it is not clear what happens to the generated token.

[...]

>   The server passes the first client response to GSS_Accept_sec_context

I suggest s/first/initial, as "initial client response" is a SASL term.

>   as input_token, setting input_context_handle to 0 (initially),
>   mech_type of the GSSAPI mechanism for which this SASL mechanism is
>   registered,

Delete description of the mech_type parameter, as it is an output parameter
of GSS_Accept_sec_context.

> the chan_binding set to NULL, and acceptor_cred_handle
>   equal to output_cred_handle from GSS_Acquire_cred called with
>   desired_name equal to output_name from GSS_Import_name with
>   input_name_type of GSS_C_NT_HOSTBASED_SERVICE and input_name_string
>   of "service@hostname" where "service" is the service name specified
>   in the protocol's profile, and "hostname" is the fully qualified host
>   name of the server.

The text about acceptor_cred_handle got replaced with the following in GS1:

  and a suitable acceptor_cred_handle (see below).

and later:

  Servers SHOULD use a credential obtained by calling GSS_Acquire_cred
  or GSS_Add_cred for the GSS_C_NO_NAME desired_name and the OID of the
  GSS-API mechanism for which this SASL mechanism is registered (*). 
Servers MAY use
  GSS_C_NO_CREDENTIAL as an acceptor credential handle.  Servers MAY
  use a credential obtained by calling GSS_Acquire_cred or GSS_Add_cred
  for the server's principal name(s) (**) and the
  GSS-API mechanism for which this SASL mechanism is registered.

  (*) - Unlike GSS_Add_cred the GSS_Acquire_cred uses an OID set of
  GSS-API mechanism as an input parameter.  The OID set can be created
  by using GSS_Create_empty_OID_set and GSS_Add_OID_set_member.  It can
  be freed by calling the GSS_Release_oid_set.

  (**) - Use of server's principal names having
  GSS_C_NT_HOSTBASED_SERVICE name type and "service@hostname" format,
  where "service" is the service name specified in the protocol's
  profile, is RECOMMENDED.

  Upon successful establishment of the security context (i.e.
  GSS_Accept_sec_context returns GSS_S_COMPLETE) the server SHOULD
  verify that the negotiated GSS-API mechanism is indeed the registered
  one.  This is done by examining the value of the mech_type
  parameter returned from the GSS_Accept_sec_context call.  If the
  value differ SASL authentication MUST be aborted.

(I've edited the text not to reference Kerberos)

And the text quoted above is followed by additional text which is not 
currently in GS2:

  Upon successful establishment of the security context and if the
  server used GSS_C_NO_NAME/GSS_C_NO_CREDENTIAL to create acceptor
  credential handle, the server SHOULD also check using the
  GSS_Inquire_context that the target_name used by the client matches:

  -  the GSS_C_NT_HOSTBASED_SERVICE "service@hostname" name syntax,
     where "service" is the service name specified in the application
     protocol's profile.

  When GSS_Accept_sec_context returns GSS_S_COMPLETE, the server
  examines the context to ensure that it provides a level of protection
  permitted by the server's security policy.  In particular, if the
  integ_avail flag is not set in the context, then no security layer
  can be offered or accepted.  If the conf_avail flag is not set in the
  context, then no security layer with confidentiality can be offered
  or accepted.

>4.3.  Wrap Token

[...]
>   The entity that sends its first Wrap token will have to specify a
>   bitmap of supported and preferred quality of protection schemes.  The
>   entity that reply to the Wrap tokens will pick a scheme, based on the

typo: replies

>   bitmask and local policy.

>   Four input formats to the GSS_Wrap function are defined.

This is confusing. Maybe "two pairs of input formats"?

>  The first
>   two input formats are used when the client sends the GSS_Wrap token
>   first and the server reponds.  The other two input formats are used

typo: responds

>   when the server sends the GSS_Wrap token first and the client
>   responds.

>4.3.1.  GSS_Wrap input for client requests

Here and in other 4.3.* sections: using SASL terms, the client sends 
"responses", while the server sends "challenges".

[...]

>   The "client_maxbuf" field indicate the maximum protected buffer size

... in network byte order

>   the client can receive.


>4.3.2.  GSS_Wrap input for server responses

[...]
>   The "server_qop" field integer indicate the selected quality of
>   protection.

I suggest to add:

 It must be one of the values specified by the client in 
"client_qops"/"client_cbqops"

And similar text for "client_qop" in 4.3.4.


>5.  Channel Bindings
>
>   The GS2 mechanism provide its own channel binding mechanism, instead
>   of using the "chan_binding" parameter in the GSS-API context
>   functions.  The reason for this is that the GS2 mechanism provide an
>   option to proceed even if the channel bindings does not match.  The
>   GSS-API framework specifies that authentication cannot proceed if
>   channel bindings does not match.

typo: do not

>   Client and servers MUST set the "chan_binding" parameter in the calls
>   to GSS_Init_sec_context to GSS_Accept_sec_context, respectively, to

missing word "and"?

>   NULL.

>6.  Protocol Overview

[...]

>   Because GSS-API authentication is initiated by the client, the length
>   field will be 0 in the initial token from the server to the client
>   when the protocol profile do not support additional information to be

typo: does not

>   sent together with the authentication request.


You might want to add the following to section 9:

  When decoding any received data with GSS_Unwrap the major_status
  other than the GSS_S_COMPLETE MUST be treated as a fatal error.

>9.1.  Examples

[...]

>   When integrity is negotiated the octet will encode an integer 4 as

I think you meant "confidentiality" here.

>   follows.
>
>       0 1 2 3 4 5 6 7
>      +-+-+-+-+-+-+-+-+
>      |0|0|0|0|0|0|1|1|
>      +-+-+-+-+-+-+-+-+

And the example doesn't match the description above, it shows the value 
3 (no layer + integrity)

>12.  IANA Considerations
>
>   The IANA is advised that SASL mechanism names starting with "GS2-"
>   are reserved for SASL mechanisms which conform to this document.  The
>   IANA is directed to place a statement to that effect in the sasl-
>   mechanisms registry.
>
>     Subject: Registration of SASL mechanism GS2-*
>     Family of SASL mechanisms: YES

I think Kurt took out this line from the IANA registration template in 
RFC 4422.

>     SASL mechanism prefix: GS2-