Resolutions below.

On Thu, Sep 8, 2016 at 11:08 AM, Hubert Kario <hka...@redhat.com> wrote:

> On Monday, 5 September 2016 11:02:57 CEST Eric Rescorla wrote:
> > PR: https://github.com/tlswg/tls13-spec/pull/625
>
> Finally found some time to take a look at it.
>
> So in general I like the change to "abort the handshake with a <type>
> alert",
> the "send a fatal <type> alert and close the connection" was a bit clunky.
> I
> was actually planning to do something like this myself.
>
> Few other items carried over from my proposed PR[1]:
>
> ---
>
> I still think we should include generic advice for handling parser errors
> in
> the parser (Presentation Language/Vectors and Constructed Types) section;
> something like this:
>
>    Peers that receive a encoding of a vector in a message with a length
> that
>    does not match specification MUST abort the connection with a
>    "decode_error" alert unless more specific section states otherwise.
>
> and this:
>
>    Peers that receive messages that doesn't match the expected constructed
>    type in expected message MUST abort the connection with a "decode_error"
>    alert unless more specific section states otherwise.
>

I added some text in the syntax section.


> I think it's a bit confusing to have this sentence:
>
>    Servers MUST select the lower of the highest supported server version
> and
>    the version offered by the client in the ClientHello. In particular,
>    servers MUST accept ClientHello messages with versions higher than those
>    supported and negotiate the highest mutually supported version.
>
> in the Server Hello section. Everything before the very last "and" is more
> applicable to ClientHello.


I am going to leave this because I anticipate adopting the new version
syntax.


I think that this sentence in Handshake Protocol:

>
>    Sending handshake messages in an unexpected order results in an
>    "unexpected_message" fatal error.
>
> should use a MUST and be prescribed to the receiving side, for example:
>
>    Peer that receives handshake messages in unexpected order MUST abort the
>    handshake with an "unexpected_message" alert.
>

I changed this.



Description what should client do when the ServerHello.cipher_suite is the
> one
> it did not advertise in ClientHello.cipher_suites is missing.
>

Changed..


> So is description of how to handle ServerHello.version being higher than
> ClientHello.max_supported_version.
>
> At least, I don't see it in Server Hello section.
>

See above re: version


missing_extension description in Alert Protocol probably should be extended
> from:
>
>    Sent by endpoints that receive a hello message not containing an
> extension
>    that is mandatory to send for the offered TLS version.
>
> to:
>
>    Sent by endpoints that receive a hello message not containing an
> extension
>    that is mandatory to send for the offered TLS version or the offered key
>    exchange mechanisms.
>
> (thinking of handling situations where "key_share" or "pre_shared_key" is
> missing)
>

Updated a bit.


> ---
>
> Hello Extensions section doesn't specify how to handle messages with
> duplicate
> extensions (e.g. second "key_share" extension added in the second
> ClientHello
> message in connection)
>
> ---
>
> Signature Algorithms doesn't say how to handle RSA-SSA signatures with salt
> length that doesn't match hash size.
>
> ---
>
> Key Share doesn't say how to handle server_share that doesn't match
> client_shares.
>

I'm comfortable leaving these as-is.



> Finished section doesn't describe what to do if the contents don't match
> the
> expected value.
>
> Should it be illegal_parameter or bad_record_mac is more appropriate?
>

This is specified in the decrypt_error section.


> ---
>
> Record Layer doesn't describe what to do if a record with zero-length
> payload
> and handshake or alert type is received.


I'm comfortable leaving these as-is.


> ---
>
> Record Layer includes
>
>    legacy_record_version : (...) This field is deprecated and MUST be
> ignored
>    for all purposes.
>
> Record Layer Protection does not.
>

I don't follow.



>
> ---
>
> 1 - https://github.com/tlswg/tls13-spec/pull/621
> --
> Regards,
> Hubert Kario
> Senior Quality Engineer, QE BaseOS Security team
> Web: www.cz.redhat.com
> Red Hat Czech s.r.o., Purky┼łova 99/71, 612 45, Brno, Czech Republic
>
_______________________________________________
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls

Reply via email to