Subject: Problems with draft

From: der Mouse <[email-masked]>
Date: Sat, 29 Nov 2003 05:40:25 -0500 (EST)

I find a few minor problems with the drafts.  As far as I know I'm
using the latest drafts (transport-17, dh-group-exchange-04,
architecture-15).

I've been digging into ssh, trying to actually understand it rather
than just using it.  To this end, I've been experimenting against a
machine I set up for the purpose, writing bits of code and seeing how
they work (or don't).  One of the things this has involved is trying to
code based on nothing but the drafts; I've used existing code only to
figure out where the problem lies when my code turns out to not
interoperate.

In transport-17, section 6.2 writes of a "shared secret K", treating it
as an opaque blob which is passed as part of a HASH argument.  But
section 7 writes of K as a large integer.  It's not entirely clear how
the one is converted into the other; looking at existing code, I find
that the kex code treates it as a large integer (which will need fixing
if-and-when key exchange methods that produce a K that's not
conceptually a large integer, though that's an implementation matter
rather than a spec matter), but when serializing it for hashing it uses
the same code it uses for "mpint" serialization.  I would like to see
the draft clearly state exactly how the large integer K of section 7 is
converted into the opaque blob K of section 6.2.  The description of
H's computation seems to imply that this should be done as the code
does it, but it's not clearly stated.

Also, section 7 writes

   The hash H is computed as the HASH hash of the concatenation of the
   following:
[...]
     string    I_C, the payload of the client's SSH_MSG_KEXINIT
     string    I_S, the payload of the server's SSH_MSG_KEXINIT
[...]

It is not clear whether these are compressed or uncompressed; while
this doesn't matter for the first key exchange, it matters for
re-exchange, and I'd like to see the draft state it clearly.

transport-17 section 5.6 says

   The "ssh-rsa" key format has the following specific encoding:
[...]
   Signing and verifying using this key format is done according to
   [SCHNEIER] and [PKCS1] using the SHA-1 hash.

I have been unable to find any description of a signature algorithm for
RSA in Schneier, and I have found no reference anywhere to explain what
[PKCS1] is supposed to refer to - I assume this is as described in
RFC2437?

In dh-group-exchange-04, there's a typo: SSH_MSG_KEY_DH_GEX_REQUEST is
used once where SSH_MSG_KEX_DH_GEX_REQUEST is (from the rest of the
draft) clearly intended.  (I wouldn't even have noticed this except
that I cut-and-pasted fragments of the draft into my code.)

/~\ 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: Markus Friedl <[email-masked]>
Date: Sun, 30 Nov 2003 11:07:14 +0100

> that the kex code treates it as a large integer (which will need fixing

from the draft (section 6.2)

   ...
   o  Initial IV client to server: HASH(K || H || "A" || session_id)
      (Here K is encoded as mpint and "A" as byte and session_id as raw
      data."A" means the single character A, ASCII 65).
   ...

So I don't think the core drafts need to changed at this point.

> re-exchange, and I'd like to see the draft state it clearly.

It refers to the payload after the transport layer transformations,
i.e. after decryption and decompression, with padding removed.

Again, I don't think a change in the core drafts is necessary.

> [PKCS1] is supposed to refer to - I assume this is as described in

Yes, it seems the reference is missing. AFAIK it refers to
section 8.2 from RFC 3447.

-m

---

From: der Mouse <[email-masked]>
Date: Sun, 30 Nov 2003 12:26:28 -0500 (EST)

>> In transport-17, section 6.2 writes of a "shared secret K", treating
>> it as an opaque blob which is passed as part of a HASH argument.
>> But section 7 writes of K as a large integer.  It's not entirely
>> clear how the one is converted into the other;

> So I don't think the core drafts need to changed at this point.

Thanks.  I must have missed that.

>> Also, section 7 writes
>>    The hash H is computed as the HASH hash of [...]
>> It is not clear whether these are compressed or uncompressed;

Perhaps.  I think I disagree, though; I don't see how someone
implementing from scratch is supposed to know, from the drafts, that
I_C and I_S are the decompressed forms.  (Decrypted makes sense, on
reflection, since if a block cipher is in use, then before decryption
the beginning of the payload is generally mixed with the length
values.)

I still think less ambiguity on each point would be good.  This is,
after all, supposed to be a standard, so clarity is a Good Thing.  Is
there some high overhead associated with changing the drafts or
something?

/~\ The ASCII				der Mouse
\ / Ribbon Campaign

---

From: [email-masked] (Niels Möller)
Date: 30 Nov 2003 20:11:02 +0100

der Mouse <[email-masked]> writes:

> I_C and I_S are the decompressed forms.

Except for section 4, which describes the transport-level mangling of
compression, padding and encryption, all mention of "payload" in the
specs refer to the uncompressed, unencrypted, unpadded payloads. It
may be a little unfortunate that both section 4 and section 6 of the
transport spec use the word "payload" for different things, but I
don't think it's a big problem.

> after all, supposed to be a standard, so clarity is a Good Thing.

And conciseness is also important; if every detail that someone might
ever misunderstand should be explained full, a spec can easily get too
large to be readable.

> Is there some high overhead associated with changing the drafts or
> something?

We've been trying to get this stuff to be published as a proper
standard for ages, and every little change somebody asks for delays it
even further. 

That said, it's of course a good thing to get feedback on the draft.
If you really think this needs additional clarification, please write
the text you want and the wg should consider it. I'd recommend
rewording section 4 so that it doesn't use the word "payload" to refer
to a string that sometimes contain the payload in compressed form.

Regards,
/Niels

---

From: der Mouse <[email-masked]>
Date: Sun, 30 Nov 2003 17:18:03 -0500 (EST)

>>> [uncompressed]
> specs refer to the uncompressed, unencrypted, unpadded payloads.

Good - but how can one tell that?  Indeed, I would be tempted to read
section 5's (not 4's - section 4 is "Connection Setup")

     byte[n1]  payload; n1 = packet_length - padding_length - 1
[...]
      payload
         The useful contents of the packet.  If compression has been
         negotiated, this field is compressed.  Initially, compression
         MUST be "none".

as being the _definition_ of "payload" for the rest of the draft, by
which interpretation "payload" would be unencrypted but compressed.

> And conciseness is also important;

True enough.

> rewording section 4 so that it doesn't use the word "payload" to
> refer to a string that sometimes contain the payload in compressed
> form.

Here's a patch against transport-17.  In the interests of keeping the
diff small, I made no attempt to re-fill paragraphs or move page
breaks; I assume the draft was generated from a master file in some
other format, and presumably the changes need to be made to that other
format and the distributed draft file regenerated.

--- transport-17.txt	Wed Nov 26 21:04:37 2003
+++ new-transport-17.txt	Sun Nov 30 17:35:37 2003
@@ -265,7 +265,7 @@
 
      uint32    packet_length
      byte      padding_length
-     byte[n1]  payload; n1 = packet_length - padding_length - 1
+     byte[n1]  compressed-payload; n1 = packet_length - padding_length - 1
      byte[n2]  random padding; n2 = padding_length
      byte[m]   mac (message authentication code); m = mac_length
 
@@ -285,14 +285,15 @@
       padding_length
          Length of padding (bytes).
 
-      payload
+      compressed-payload
          The useful contents of the packet.  If compression has been
          negotiated, this field is compressed.  Initially, compression
-         MUST be "none".
+         MUST be "none".  (When "payload" is used without adjective
+         elsewhere, it refers to the uncompressed form.)
 
       random padding
          Arbitrary-length padding, such that the total length of
-         (packet_length || padding_length || payload || padding) is a
+         (packet_length || padding_length || compressed-payload || padding) is a
          multiple of the cipher block size or 8, whichever is larger.
          There MUST be at least four bytes of padding.  The padding
          SHOULD consist of random bytes.  The maximum amount of padding
@@ -305,7 +306,7 @@

    Note that length of the concatenation of packet length, padding
-   length, payload, and padding MUST be a multiple of the cipher block
+   length, compressed-payload, and padding MUST be a multiple of the cipher block
    size or 8, whichever is larger.  This constraint MUST be enforced
    even when using stream ciphers.  Note that the packet length field is
    also encrypted, and processing it requires special care when sending
@@ -318,9 +319,9 @@
 
 5.1 Maximum Packet Length
 
-   All implementations MUST be able to process packets with uncompressed
+   All implementations MUST be able to process packets with (uncompressed)
    payload length of 32768 bytes or less and total packet size of 35000
-   bytes or less (including length, padding length, payload, padding,
+   bytes or less (including length, padding length, compressed-payload, padding,
    and MAC.). The maximum of 35000 bytes is an arbitrary chosen value
    larger than uncompressed size. Implementations SHOULD support longer
    packets, where they might be needed, e.g. if an implementation wants
@@ -362,7 +363,7 @@
    current block is not a stored block, one or more empty blocks are
    added after the current block to ensure that there are at least 8
    bits counting from the start of the end-of-block code of the current
-   block to the end of the packet payload.
+   block to the end of the (compressed) packet payload.
 
    Additional methods may be defined as specified in [SSH-ARCH].
 
@@ -370,7 +371,7 @@
 
    An encryption algorithm and a key will be negotiated during the key
    exchange.  When encryption is in effect, the packet length, padding
-   length, payload and padding fields of each packet MUST be encrypted
+   length, compressed-payload and padding fields of each packet MUST be encrypted
    with the given algorithm.
 
    The encrypted data in all packets sent in one direction SHOULD be
@@ -491,7 +492,7 @@
      mac = MAC(key, sequence_number || unencrypted_packet)
 
    where unencrypted_packet is the entire packet without MAC (the length
-   fields, payload and padding), and sequence_number is an implicit
+   fields, compressed-payload and padding), and sequence_number is an implicit
    packet sequence number represented as uint32.  The sequence number is
    initialized to zero for the first packet, and is incremented after
    every packet (regardless of whether encryption or MAC is in use).  It

/~\ The ASCII				der Mouse
\ / Ribbon Campaign