[gnutls-devel] simplifying certificate verification

[Nikos Mavrogiannopoulos]

On Thu, Sep 17, 2015 at 1:56 AM, Daniel Kahn Gillmor
<dkg at fifthhorseman.net> wrote:
> On Mon 2015-08-24 07:58:08 -0400, Nikos Mavrogiannopoulos wrote:
>> One the pains in using gnutls is the fact that there is needed quite
>> some copy-paste code to perform certificate verification. I decided to
>> simplify that from 3.5.0, using a function called
>> gnutls_session_auto_verify_cert(), and the result can be seen on the
>> following example
>> https://gitlab.com/gnutls/gnutls/blob/master/doc/examples/ex-client-x509.c
>> That is about ~60 lines of code less per program using gnutls.
>> https://gitlab.com/gnutls/gnutls/commit/25f2b0814401d1e9c98f3fdc833e09b3c877fc72
>> I'd appreciate any comments or suggestions for improving that interface [0].
> I really like the idea of simplifying this UI to something managable and
> correct by default for the common use case.  In particular, i think the
> common use case is for TLS clients to verify server certificates.
> Handling that simply would make the life of many developers much
> better. :)
> Is this interface intended to work for servers to verify client
> certificates as well?  I think we should avoid trying to make one
> configuration that handles them both, and this improvement should focus
> explicitly on the server certificate use case.

I also considered that, but for the majority of use cases both can be
handled in quite manageable way (at the cost of using the
gnutls_session_set_verify_cert2()).

> But the proposed interface is a bit unclear to me, and i'd be happy to
> help clarify it.  I'm looking at master, at
> 368018efccee25f82149cba47e05f5af6af28ae9.
> Looking at NEWS :
> It says gnutls_session_auto_verify_cert() was added, but
> gnutls_session_set_verify_cert and gnutls_session_set_verify_cert2()
> functions are the only ones i see implemented and exposed in the
> headers. what am i missing?

I've removed the _auto_ from the name and replaced it with _set_. I've
updated the NEWS to reflect that.

> Looking at gnutls.h.in:
>> /**
>>  * gnutls_vdata_types_t:
>>  * @GNUTLS_DT_UNKNOWN: Unknown data type.
>>  * @GNUTLS_DT_DNS_HOSTNAME: The data contain a null-terminated DNS hostname.
>>  * @GNUTLS_DT_RFC822NAME: The data contain a null-terminated email address.
>>  * @GNUTLS_DT_KEY_PURPOSE_OID: The data contain a null-terminated key purpose OID.
>>  *
>>  * Enumeration of different key exchange algorithms.
>>  */

Corrected. Text updated.

> These are not key exchange algorithms.  Maybe they're "X.509 certificate
> data field elements" or something?
>
>> typedef enum {
>>         GNUTLS_DT_UNKNOWN = 0,
>>         GNUTLS_DT_DNS_HOSTNAME = 1,
>>         GNUTLS_DT_KEY_PURPOSE_OID = 2,
>>         GNUTLS_DT_RFC822NAME = 3
>> } gnutls_vdata_types_t;
>>
>> typedef struct {
>>         gnutls_vdata_types_t type;
>>         unsigned char *data;
>>         unsigned int size;
>> } gnutls_typed_vdata_st;
> This seems to be a set of fields that the user is asking GnuTLS to look
> for in the cert, but it's unclear what it means -- for example, with
> GNUTLS_DT_DNS_HOSTNAME:

The new text is:
 * Enumeration of different typed-data options. They are used as input
to certificate
 * verification functions to provide information about the name and
purpose of the
 * certificate.

> Would an element of this type be checked only in SubjectAltNames, or
> would it also checked in the leaf CommonName of the Subject's
> DistinguishedName?  Is it verified case-insensitively?  normalized in
> any way?  how is IDN/punycode handled?

I've added in the text that RFC6125 comparison is being used.

> If multiple GNUTLS_DT_DNS_HOSTNAMEs are listed in an array of
> gnutls_typed_vdata_st, would any of them acceptable for verification?
> or do they all have to be present?

Only a single one is accepted. I've made that clear.

> For GNUTLS_DT_RFC822NAME, are these SubjectAltNames also, or will they
> be looked for in other fields in the cert?

Since there is no specific RFC for that, I've added more text there.
"The data contain a null-terminated email address; the email will be
 matched against the RFC822Name field of the certificate, or the EMAIL
DN component if the
 former isn't available. Prior to matching the email address will be
converted to ACE
  (ASCII-compatible-encoding)"

> For GNUTLS_DT_KEY_PURPOSE_OID, is that keyUsage or extendedKeyUsage?  In
> practice and in specifications, keyUsage and extendedKeyUsage have
> unusual combining behavior -- see, for example, the discussion in

This is the Extended Key Usage.

> https://bugzilla.mozilla.org/show_bug.cgi?id=1049176 about the
> relationship between kU and eKU.  In particular RFC 5280 says:
>    If a certificate contains both a key usage extension and an extended
>    key usage extension, then both extensions MUST be processed
>    independently and the certificate MUST only be used for a purpose
>    consistent with both extensions.  If there is no purpose consistent
>    with both extensions, then the certificate MUST NOT be used for any
>    purpose.

That's an interesting case. I'm not sure I agree that when an extended
key usage is present the key usage extension should be nullified.
That's certainly something to be clarified in PKIX and RFC5280. As it
is now, I don't think that gnutls is strict about the extended key
usage, unless the verifier explicitly specifies a particular usage.

> So what would that requirement look like?
> What about adding something like a SRVNAME option here as well to handle
> https://tools.ietf.org/html/rfc6125#section-6.5.1 ?

That looks interesting. Please open an issue about it in the issue
tracker so I don't forget for 3.5.0.

> Looking at lib/auto-verify.c:
>
>> /**
>>  * gnutls_session_set_verify_cert:
>>  * @session: is a gnutls session
>>  * @hostname: is the expected name of the peer; may be %NULL
>>  * @flags: flags for certificate verification -- #gnutls_certificate_verify_flags
>>  *
>>  * This function instructs GnuTLS to verify the peer's certificate
>>  * using the provided hostname. If the verification fails the handshake
>>  * will also fail with %GNUTLS_E_CERTIFICATE_VERIFICATION_ERROR. In that
>>  * case the verification result can be obtained using gnutls_session_get_verify_cert_status().
>
> The same questions about GNUTLS_DT_DNS_HOSTNAME above (canonicalization;
> SAN vs. Subject leaf CN) apply here.
> Also, it looks to me like this doesn't verify anything about the
> server's certificate's eKU or kU flags -- but i think the simple default should at least:

The kU flags don't really need to be specified the verifier. The
verification routine has enough information to know when to use them.
The eKU cannot be specified indeed.

>  (a) if keyUsage extension exists, require the appropriate flags for the
>      type of key exchange mechanism used in the TLS handshake
>      (digitalSignature for (EC)DHE, keyEncipherment for RSA, or
>      keyAgreement for static DH)

That should be the case already.

>  (b) if extendedKeyUsage extension exists, require the "TLS Server"
>       value set (id-kp-serverAuth)

I have problem with that because RFC5280 specifies "TLS WWW server
authentication". So if for example this is used for IMAP this key
purpose is invalid. That's the reason it is not set by default.

> Or are these requirements already present by default?
> We might also want to review the policies articulated by the CA/B Forum
> for subscriber certificates:
> https://cabforum.org/baseline-requirements-certificate-contents/#Subscriber-Certificates
> I'm no big fan of the CA/B forum, but some of their recommendations may
> be things we want to expect.  At the moment, i'm torn about their
> requirement for BasicConstraints: CA:False for end-entity certificates.
> I think that's an excellent requirement for CAs issuing EE certs -- i'm
> not sure whether we should enforce it as a TLS client as well.

That's too strict to be useful. It is a good practice to be explicit,
but in reality there will be many cases with certificates that have
some fields implicit. We only need to have good defaults for these
cases.

>>  * The @hostname pointer provided must remain valid for the lifetime
>>  * of the session. More precisely it should be available during any subsequent
>>  * handshakes.
> This memory management sounds cumbersome to me for what will likely
> become the default handler for most applications -- though it's similar
> to gnutls_credentials_set.  The difference is that this is likely to be
> called from code that itself received the hostname from elsewhere.  Can
> we have the session object allocate a copy for the case where this
> default function is called so that the default caller doesn't have to
> think about bundling the hostname's memory management with the session
> object?

Talloc would have been very handy here. I agree, but will have to
think how to solve that. Please open a bug, or follow up if that is
not solved prior to 3.5.0.

>>    If not hostname is provided, no hostname verification
>>  * will be performed.
> This should be "If no hostname is provided, ..."  Perhaps it should also
> indicate what else happens -- for example, if no hostname verification
> is performed, is any certificate valid?  or does GnuTLS only check that
> the certificate chains back to a trusted root, which means it can be
> replaced by any other certificate that chains to a trusted root?  We
> should be clear that leaving hostname blank here represents a security
> risk.

Any text suggestion?

regards,
Nikos