IETF-SSH archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: new drafts



Hi,

I appreciate the review.
Comments in-line.

On Sat, 20 Nov 2004, der Mouse wrote:

> I just sucked over the new drafts.  On diffing them with the previous
> versions, I see some things of note.
>
> In assignednumbers-08, fourth line of 4.3.4:
>
>    o  The range of 0xFF000000 to 0xFEFFFFFF is to be used in conjunction
>
> Surely that should be FE000000.  The same error is repeated six lines
> further down.
>
> In connect-21, I see the line
>
>    to 0x0xFDFFFFFF MUST be done through the IETF CONSENSUS method as
>
> s/0x0x/0x/, surely.
>
> The description surrounding and following the previous line appears to
> repeat the "0xFF000000 instead of 0xFE000000" error from above,
> multiple times.  Specifically, these lines
>
>    range of 0xFF000000 to 0xFFFFFFFF, this range will be split in two
>    o  The range of 0xFF000000 to 0xFEFFFFFF is to be used in conjunction
>       the range of 0xFF000000 to 0xFEFFFFFF.  Naturally, if the server
>
> look to me as though they need fixing (even though the first of them
> makes sense in isolation, in context, I believe it is wrong).

OK.  (I really liked it a lot better when I was mistakenly using a BYTE
rather than a uint32. :)


>
> In connect-21 again, just before the beginning of 5.3, I see
>
>    described in [RFC2434].  As is noted, the actual instructions to the
>    IANA is in [SSH-NUMBERS].
>
> There is a plural/singular clash here between "the...instructions" and
> "is".

OK.

>
> In transport-20, near line 515, I find
>
>     implementations MUST allow the algorithm for each direction to be
>     independently selected for each direction, if multiple algorithms are
>
> Isn't the repetition of "each direction" redundant here?  It reads a
> little oddly to me, at least.

OK.

>
> Later in transport-20, around line 550, I see
>
>    and the following 8 bytes for the final encryption.  This requires 24
>    bytes of key data (of which 168 bits are actually used).  To
>    implement CBC mode, outer chaining MUST be used (i.e., there is only
>    one initialization vector).  This is a block cipher with 8 byte
>    blocks.  This algorithm is defined in [FIPS-46-3].  Note that this
>    algorithm does not meet the specifications that SSH encryption
>    algorithms should use keys of 128 bits or more.  However, this
>
> It seems..odd..to me to describe an algorithm and note that it uses 168
> bits of key data, then note that this does not meet a spec requiring
> 128 bits or more - it looks to me as though it meets it with 40 bits to
> spare.  (It may not have 128 bits worth of security, but if that's the
> issue, it seems to me it should be made clear in the text.  As it is,
> it just looks confused - it certainly does use a key "of 128 bits or
> more".

I added text and referenced [SCHNEIER].


>
> Around line 870, I see
>
>    authentication" if, in order to prove its autenticity, the server
>
> s/autenticity/authenticity/ surely?

Yup.  I ran all of the IDs back through a spell checker and corrected a
few more that had crept in.

>
> Somewhere near line 905, I see text (itself unchanged) which says that
> supported algorithms "MUST be listed in order of preference", but does
> not make it explicit whether this is most-preferred-first or
> least-preferred-first.  (The first sentence of the next paragraph
> helps, but I'd prefer to see this change to something like "...order of
> preference, from most to least".)

OK.

>
> In userauth-23, lines 208 and 209 are
>
>       string    user name in UTF-8, normalized according to
>       [I-D.ietf-sasl-saslprep]
>
> which, while not excessively problematic in isolation, would look
> significantly better to me in context if the bracketed I-D reference
> were indented so its open bracket falls under the u of "user name".
>
> Similar remarks apply to lines 454 and 455, and lines 539 and 540.

Those will go away when the reference becomes an RFC - which should be
soon as it's in the queue.


I also aligned all of the references in [NUMBERS] which were out of whack.

The group had described "name-list" as the way to comma-separate lists in
[ARCH] Section 5.  However, "name-list"s were not used in any other
document.  "string"s were used with additional text saying that they were
to be "comma-separated strings".  I _BELIEVE_ that I've found all of the
appropriate places (in [TRANS] and [USERAUTH]) and have converted them to
"name-list"s.  I would really appreciate a review of those to make sure
that the things I converted really should be "name-list"s rather than
"string"s.

I modified the "identification string" from the line explanation to the
format used throughout the rest of the documents.  This is in [TRANS]
Section 4.2.  Is this acceptable:

            string    "SSH-"
            string    protoversion
            string    "-"
            string    softwareversion
            string    <SP> (Space character) only used if 'comments'
                      are used in the identification string
            string    comments CR LF - Optional
            string    <CR><LF>

   Since the protocol being defined in this set of documents is version
   2.0, the 'protoversion' MUST be "2.0".  The 'comments' string is
   OPTIONAL.  If the 'comments' string is included, a 'space' character
   (denoted above as SP, "<SP>", ASCII 32) MUST separate the
   'softwareversion' and 'comments' strings.  The identification MUST be
   terminated by a single Carriage Return and a single Line Feed
   character (ASCII (denoted above as "<CR><LF>", ASCII 13 and 10,
   respectively).  Implementors who wish to maintain compatibility with
   older, undocumented versions of this protocol, may want to process
   the identification string without expecting the presence of the
   carriage return character for reasons described in Section 5 of this
   document.  The null character MUST NOT be sent.  The maximum length
   of the string is 255 characters, including the Carriage Return and
   Line Feed.



The files (.txt, .xml) and the diffs (.html) are all to be found here:
  http://www.employees.org/~lonvick/secsh-wg/2004-nov-29/

I just submitted new versions which - since the ID Editors are getting
really speedy - may appear later today.

Thanks,
Chris



Home | Main Index | Thread Index | Old Index