Subject: new drafts

From: der Mouse <[email-masked]>
Date: Sat, 20 Nov 2004 02:47:04 -0500 (EST)

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).

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".

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.

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".

Around line 870, I see

   authentication" if, in order to prove its autenticity, the server

s/autenticity/authenticity/ surely?

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".)

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.

/~\ The ASCII				der Mouse
\ / Ribbon Campaign
 X  Against HTML	       [email-masked]
/ \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B

---

From: Chris Lonvick <[email-masked]>
Date: Mon, 29 Nov 2004 08:45:52 -0800 (PST)

Hi,

I appreciate the review.
Comments in-line.

> 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. :)

> There is a plural/singular clash here between "the...instructions" and

OK.

> little oddly to me, at least.

OK.

> it just looks confused - it certainly does use a key "of 128 bits or

I added text and referenced [SCHNEIER].

> 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.

> preference, from most to least".)

OK.

> 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

---

From: "denis bider" <[email-masked]>
Date: Mon, 29 Nov 2004 23:26:38 +0100

This looks very wrong.

string = word32 len, byte[len] data

There are no word32 components in the version string. The version string is
actually not a 'string' as defined in [SSH-ARCH]. Its format doesn't fit in
with the rest of the SSH2 protocol in any way, hence the different
definition.

The below proposal is rather confusing. I think the way it was was fine.

Best regards,

denis

> 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
> I just submitted new versions which - since the ID Editors 
> are getting really speedy - may appear later today.
> 
> Thanks,
> Chris
>

---

From: Jacob Nevins <[email-masked]>
Date: Mon, 29 Nov 2004 23:35:58 +0000

(Oops, didn't spot this until after the drafts were published.)

> Section 4.2.  Is this acceptable:
              [...]

`string' is defined in the architecture draft as having a uint32 length
field prepended, which these components of the version string do not
have, so this is wrong. I don't see anything wrong with the text in -20.