------------------------------------------------------------------------
Subject:
Gen-art review: draft-ietf-secsh-break-04
From:
Elwyn Davies <elwynd%dial.pipex.com@localhost>
Date:
Tue, 30 Aug 2005 15:29:05 +0100
To:
gen-art%ietf.org@localhost
To:
gen-art%ietf.org@localhost
CC:
Mary Barnes <mary.barnes%nortel.com@localhost>, Sam Hartman
<hartmans-ietf%mit.edu@localhost>, galb-list%vandyke.com@localhost, remaker%cisco.com@localhost
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/