Re: [Curdle] Ben Campbell's Discuss on draft-ietf-curdle-ssh-ext-info-12: (with DISCUSS and COMMENT)

[denis bider <[e-mail]>]

> I don't think allowing specific extensions
> to add ordering requirement works.

OK, fair point.

That makes two votes against this. There is currently no known extension
that needs this, and if one does arise, there exist other possible
approaches for what is required. I have removed.


> 2.3: "This message is sent immediately after SSH_MSG_NEWKEYS,
> without delay." That seems to be contradicted in section 2.4, which
> talks about two other potential times.

True. I have substantially rewritten sections 2.3. and 2.4. so as to not
conflict, and to make this clearer.


> 2.5, 2nd paragraph: "or it MAY be sufficient that only one party
> includes it" That seems like a statement of fact rather than a grant
> of permission. If so, the 2119 MAY is not appropriate.

True. Changed to lowercase "can".


> -2.1, first paragraph: "Applications implementing this mechanism
> MUST add to the field "kex_algorithms", in their KEXINIT packet
> sent for the first key exchange, one of the following indicator names:"
> That's hard to parse.

I'm guessing you're not a German speaker. :) The issue raised here seems to
be that one needs to read to the end of the sentence to fully understand
what is being added ("one of the following indicator names").

No problem, rearranged sentence.


> -2.5, 2nd to last paragraph: "... applications MUST tolerate
> any sequence of bytes; including null bytes at any position;
> in an unknown extension’s extension-value."
> Redundant to similar normative statement in 2.3.

Aye, this is redundant on purpose. If this is not followed, it is
devastating to compatibility. OpenSSH in particular has this issue in
currently released versions (including 7.5, the latest). They stated
they'll fix this for 7.6, but a workaround is required to NOT send
extension-values containing null bytes to OpenSSH servers.

The bug is not on purpose, it's because they have multiple functions to
decode an SSH string - some that allow for binary data, some that don't.
They accidentally used the one that doesn't allow binary data to decode
extension-values.

That's bad - and easy to proliferate as long as the most commonly used
extensions don't exercise binary data. Fortunately, OpenSSH doesn't hide
its version string, so the compatibility workaround is straightforward.
That's not the case for all implementations.

This is super important not to miss. The spec is somewhat big, and
implementers might read one part at one time, and another part at another.
I would prefer that this is not missed by anyone.

denis



On Wed, Sep 13, 2017 at 1:30 PM, Ben Campbell <[e-mail]> wrote:

> Ben Campbell has entered the following ballot position for
> draft-ietf-curdle-ssh-ext-info-12: Discuss
>
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
>
>
> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> for more information about IESG DISCUSS and COMMENT positions.
>
>
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-curdle-ssh-ext-info/
>
>
>
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
>
> I plan to ballot "yes", but I want to discuss one point first. Alexey
> mentioned
> this in his comments, but I think it's discuss worthy. Hopefully it's an
> easy
> fix, and it may well be because I've missed something obvious. If people
> think
> it really, really needs to be this way, I will clear--but I want to
> discuss it
> first:
>
> - 2.5: The relative order in which extensions appear in an
>   EXT_INFO message MUST be ignored by default; but an extension MAY
>   specify that the order matters for that extension, in a specific way.
>
> I don't think allowing specific extensions to add ordering requirement
> works.
> It opens up the possibility of incompatible ordering requirements across
> extensions. As far as I can tell, the only control over this is the "IETF
> Consensus" requirement for adding new extensions. I'm open to arguments
> that
> this is good enough, but my knee-jerk response is that it puts an undue
> burden
> on the consensus process for little return.
>
>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> Substantive:
>
> -2.3: "This message is sent immediately after SSH_MSG_NEWKEYS, without
> delay."
> That seems to be contradicted in section 2.4, which talks about two other
> potential times.
>
> -2.4, last paragraph (asterisk note): "The message MUST be sent at this
> point
> for the following reasons:" That's a confusing use of MUST. Do you mean
> that
> the message MUST NOT be sent unless the reasons are true? Or that if the
> reasons are true, the message MUST be sent? Or is this just a statement of
> fact, in which case the 2119 keyword is not appropriate?
>
> -2.5, 2nd paragraph: "or it MAY be sufficient that only one party includes
> it"
> That seems like a statement of fact rather than a grant of permission. If
> so,
> the 2119 MAY is not appropriate.
>
> Editorial:
>
> -2.1, first paragraph: "Applications implementing this mechanism MUST add
> to
> the field
>   "kex_algorithms", in their KEXINIT packet sent for the first key
>   exchange, one of the following indicator names:"
>
> That's hard to parse.  Suggestion:
> "Applications implementing this mechanism MUST add one of the
>  following indicator names to the "kex_algorithms" field for the first
>  key exchange:"
>
> -2.5, 2nd to last paragraph: "... applications MUST
>   tolerate any sequence of bytes; including null bytes at any position;
>   in an unknown extension’s extension-value."
>
> Redundant to similar normative statement in 2.3.
>
>
> _______________________________________________
> Curdle mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/curdle
>