Re: [TLS] Empty CertificateRequest.supported_signature_algorithms in TLS 1.2

[David Benjamin <[e-mail]>]

On Wed, Nov 21, 2018 at 3:50 PM Martin Thomson <[e-mail]>
wrote:

> In attempting to fix a bug related to this, a question came up about
> what the semantics of an empty value is here.  Some implementations
> seem to infer that empty means {*,SHA1}, which effectively assumes
> that an empty value is equivalent to an absent signature_algorithms
> extension (Section 7.4.1.4.1)
>

In the ClientHello, I believe an empty signature_algorithms extension is
invalid. The structure is defined as:

      SignatureAndHashAlgorithm
        supported_signature_algorithms<2..2^16-2>;

BoringSSL enforces this. If your ClientHello has an empty
signature_algorithms extension, we will reject it. If it is missing, we do
interpret it as {*, SHA1}, as TLS 1.2 (mistakenly) prescribes.


> The text on CertificateRequest is less clear about what to do.  That's
> understandable because it doesn't have to deal with the value being
> absent because it's not optional.  All we have to go on is this from
> Section 7.4.8:
>
>    The hash and signature algorithms used in the signature MUST be
>    one of those present in the supported_signature_algorithms field
>    of the CertificateRequest message.
>
> We think that treating an empty supported_signature_algorithms field
> as an error is the best response and plan to implement that change.
> We'll send a fatal alert if we receive one.
>


> This is consistent with our handling of the signature_algorithms
> extension, where we treat an empty list as a failure.
>

Sadly, the structure itself has a syntax error, so it's unclear whether
empty is allowed or not. :-)

      struct {
          ClientCertificateType certificate_types<1..2^8-1>;
          SignatureAndHashAlgorithm
            supported_signature_algorithms<2^16-1>;
          DistinguishedName certificate_authorities<0..2^16-1>;
      } CertificateRequest;

We do tolerate empty and give it the same {*, SHA1} interpretation as
ClientHello, since they go through the same logic, but I agree with you
that the specification does not actually say it applies to both. Rejecting
the empty list sounds like a good idea, assuming it's compatible with
current implementations. The 7.4.8 citation looks right, and rejecting it
makes sense on general principle. IMO, the default {*, SHA1} bit in TLS 1.2
was a mistake. It's caused unnecessary confusion, resulting in buggy TLS
1.2 servers that only sign SHA-1.

Maybe we should errata this by fixing that <2^16-1> to <2..2^16-1>?

David