Re: WG Last Call on draft-ietf-secsh-publickey-subsystem-02

[der Mouse <mouse%Rodents.Montreal.QC.CA@localhost>]

> This message marks the start of a Working Group Last Call on
> draft-ietf-secsh-publickey-subsystem-02, for publication as a
> Proposed Standard.

> Please send comments to the WG list as a whole.

Here are mine.  I'll leave it up to the WG and/or the document
author/editor which of these are worth doing; I'm just listing
everything I notice - put it this way: if I had sole responsibility for
the document, these are changes I'd make.

For changes where I think something should be changed but I give no
specific wording, I'll be happy to have a stab at writing up wording if
anyone wants.

In 2.1:
   The public-key subsystem is opened when the clients sends a
   SSH_MSG_CHANNEL_REQUEST over an existing session.
This has so many minor mistakes that rather than try to fix each one
individually, I'll just propose a rewrite:
   The public-key subsystem is opened by a client sending a
   SSH_MSG_CHANNEL_REQUEST over an existing session's channel.

Fourteen lines further down in 2.1:
   Client implementations SHOULD reject this request; it is normally
   only sent by the client.
s/only sent/sent only/

Nine lines further down:
   authenticated with a restricted public key that does not allow access
   to the publickey subsystem.
Proposed rewrite:
   authenticated by a means (such as a restricted public key) that does
   not allow access to the publickey subsystem.

In 2.2:
   The length field describes the length of the request-name field and
   the request-specific data, but not of the length field itself.  The
s/but not of/but does not include/

A few lines further down, I see "the...'data' portion of the packet",
but there is no field called "data":
        uint32    length
        string    request-name
        ... request specific data follows
My preferred fix here is to s/'data'/the data/.

I'd like to see something explicit in 2.3 stating that the length works
the same way it does in 2.2 - perhaps 2.2 and the non-subsectioned
portion of 2.3 could be collapsed, since they otherwise will include a
lot of duplicated, or nearly duplicated, language?

In 2.3.1.1, I see no description of what the intended semantics of the
return codes are, except for the description implicit in their names.
Do we have consensus that this is sufficient?  While I think it
probably is for native anglophones, I don't have any other perspective
on it myself - and even if it is sufficient, I'd prefer to see a short
statement to that effect.

Section 3 ("Public-Key Subsystem Operations") starts by saing that
"[t]he public-key subsystem currently defines four operations: add,
remove, list, and listattributes", and then proceeds to section 3.1,
which describes something (version negotiation) that does not appear to
be any of those four and therefore I would argue does not belong in
section 3 as it currently stands.

In 3.1, in case of an unsupported version, "[b]efore closing the
subsystem, a status message with the status
SSH_PUBLICKEY_VERSION_NOT_SUPPORTED SHOULD be sent".  But 2.3.1, which
describes status messages, writes of them only as responses to
requests, and the version packet is not described as a request (though
it fits the format of requests).

I'm not sure this is worth fixing, though I'd prefer to see a brief
note recognizing this slight mismatch, such as
   ....  Before closing the subsystem, a status message with the status
   SSH_PUBLICKEY_VERSION_NOT_SUPPORTED SHOULD be sent, even though the
   "version" packet is not a normal request.

In 3.2, I see
   algorithm names in [1].  If the server does not implement a mandatory
   attribute, it MUST fail the add.
but, in contrast to the error conditions described in the previous
paragraph, I see no indication of what error code should be used.  (I
also don't see any error code whose name makes it look suited to this
error condition, and GENERAL_FAILURE is notably unhelpful.)

In 3.2, describing the "comment-language" attribute,
   [5].  The client MAY specify more than comment if it additionally
   specifies a different language for each of those comments.  The
Based on the context, I believe s/more than /&one / is needed here.

In 3.2, describing the "subsystem" attribute, on first sight, it
appears that there is no way to specify a key that permits any
subsystem.  (Presumably the way is to not specify the attribute at all.
But this is not obvious at first sight; indeed, it wasn't until I was
writing the first version of this paragraph that I realized it.)

In 3.3, no indication is given of what to do if the removal succeeds,
or for that matter if it fails.  (I assume SUCCESS and KEY_NOT_FOUND
are the principal return codes, with a few others like ACCESS_DENIED
possible, but I'd still rather see it explicit.)

In 3.4, no explicit provision is made for servers that support the
request but choose to deny it for the client in question.  I'd prefer
to see this changed.

In 3.2 and 3.5, no provision is made for a server that supports a
specific attribute but is administratively set to impose a lack of that
attribute - only for imposing the presence of an attribute.  There also
is no provision for cases such as imposing certain hosts' presence or
absence in (say) the "agent" list.

In 5.2.1, there is a restriction in that the length limit applies even
to domain-localized names.  This seems semi-broken, since FQDNs can be
relatively long (though 64 seems generous now, I have little confidence
it will remain so - I already have a private name 43 characters long).
Unless the "names" in "[n]ames are case-sensitive, and MUST NOT be
longer than 64 characters" refers to just the part before the @, in
which case I think the language needs clarification - this section uses
"name" to refer to both the whole string and just the part before the
@, which is confusing; consider "Names with the at-sign in them will
have the format of "name@domainname" (without the double quotes) where
the part preceeding the at-sign is the name.", which uses the word in
both senses in the same sentence.

/~\ The ASCII				der Mouse
\ / Ribbon Campaign
 X  Against HTML	       mouse%rodents.montreal.qc.ca@localhost
/ \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B