Subject: [Elwyn Davies] Gen-art review: draft-ietf-secsh-break-04

From: Sam Hartman <[email-masked]>
Date: Tue, 30 Aug 2005 11:08:53 -0400

In break, was it the intent of the WG for the break length parameter
to be optional?

--Sam

Subject: Gen-art review: draft-ietf-secsh-break-04
From: Elwyn Davies <[email-masked]>
To: [email-masked]
Cc: Mary Barnes <[email-masked]>, Sam Hartman <[email-masked]>, [email-masked], [email-masked]
Date: Tue, 30 Aug 2005 15:29:05 +0100

Background for those on the CC list, who may be unaware of GenART:
GenART is the Area Review Team for the General Area of the IETF.  We 
advise the General Area Director (i.e. the IETF/IESG chair) by providing 
more in depth reviews than he could do himself of documents that come up 
for final decision in IESG telechat.  I was selected as the GenART 
member to review this document.  Below is my review, which was written 
specifically with an eye to the GenART process, but since I believe that 
it will be useful to have these comments more widely distributed, others 
outside the GenART group are being copied.

Document: draft-ietf-secsh-break-04.txt
Intended Status: Proposed Standard
Shepherding AD: Sam Hartman
Review Trigger: IESG Telechat 1/9/05

Review:
This document is almost ready for publication as a proposed standard but 
it has one possible (minor) issue and a couple of editorial nits.

Possible issue:
[I say 'possible' because I am not an ssh expert but there is an 
apparent inconsistency with other ssh documents which makes me wonder].
s2: para 4: The text says 'If the BREAK-length parameter is 0 *or not 
present*, the BREAK SHOULD be interpreted...'. As far as I can see no 
other ssh message has optional parameters in this way. Although it would 
obviously be possible to cope with both cases, it seems to be unusual 
and makes parsing the message more complex than it needs to be, given 
that this message is going to be a relative rarity. Was this intended? 
If so I think it would be desirable to add an explicit note closer to 
the message definition to point out that the parameter is optional. 
Otherwise just delete 'or not present'.

Editorial:
s1: para 1: Add a reference to the SSH Connection protocol [5] after 
'session channel'.
s3: Choose between 'break-length' (as in message format) and 
'BREAK-length' (as in para 4).
s3: next to last para: (2 places) s/preformed/performed/

---

From: Joseph Galbraith <[email-masked]>
Date: Tue, 30 Aug 2005 09:30:07 -0600

> In break, was it the intent of the WG for the break length parameter

I don't recall... it probably just happened.

I, for one, would be happy to remove the offending
phrase.

Does anyone recall differently?

Thanks,

Joseph

> ------------------------------------------------------------------------
> 
> Subject:
> Gen-art review: draft-ietf-secsh-break-04
> From:
> To:
> [email-masked]
> To:
> [email-masked]
> CC:
> Mary Barnes <[email-masked]>, Sam Hartman 
> <[email-masked]>, [email-masked], [email-masked]
> 
>

---

From: der Mouse <[email-masked]>
Date: Tue, 30 Aug 2005 11:31:46 -0400 (EDT)

> The text says 'If the BREAK-length parameter is 0 *or not present*,

The "or not present" is absent in break-03 and break-01.  The
break-length parameter is present in all three versions (-01, -03, -04).

I see no reason for the language implying that it's optional,
especially since it's not described as optional in the packet layout
early in section 3.

/~\ 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: Jacob Nevins <[email-masked]>
Date: Tue, 30 Aug 2005 16:45:56 +0100

> other ssh message has optional parameters in this way. Although it would

To save anyone else grovelling round:

I wondered if it was a backwards compatibility measure. However, the
"break-length" parameter has been present in every published draft from
"draft-ietf-secsh-break-00" (according to tools.ietf.org), and the "or
not present" language only appeared in the latest draft (-04).

That language doesn't appear to have come from any of the suggested
changes from the WG that fed into -04 (e.g., from Ben Harris or
der Mouse); it doesn't appear to have been proposed before the
publication of -04.

---

From: Sam Hartman <[email-masked]>
Date: Tue, 30 Aug 2005 17:48:18 -0400

>>>>> "Jacob" == Jacob Nevins <[email-masked]> writes:

    Jacob> That language doesn't appear to have come from any of the
    Jacob> suggested changes from the WG that fed into -04 (e.g., from
    Jacob> Ben Harris or der Mouse); it doesn't appear to have been
    Jacob> proposed before the publication of -04.

Not a response to any of my comments either.

OK, unless someone screams by noon ET tomorrow, it's gone.

---

From: Joseph Galbraith <[email-masked]>
Date: Tue, 30 Aug 2005 16:01:51 -0600

> OK, unless someone screams by noon ET tomorrow, it's gone.

I believe the intent was that the bits MUST be present in
the packet.

So ax away.

Thanks,

Joseph