[gnutls-devel] simplifying certificate verification

[Daniel Kahn Gillmor]

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.

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?

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.
>  */

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:

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?

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?

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

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
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.

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 ?

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:

 (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)

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

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.

I'm sure there are some homebrew CA setups that use CA:True certs for
servers.

>  * 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?

>    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.

>     For a more advanced verification function check
>  * gnutls_session_set_verify_cert2().
>  *
>  * That function is intended to be used by clients.

which function?  It's unclear whether this last sentence applies to
g_s_set_verify_cert() or g_s_set_verify_cert2().

is "by clients" supposed to mean "only TLS clients"?  What happens if a
server calls it?


> /**
>  * gnutls_session_set_verify_cert2:
>  * @session: is a gnutls session
>  * @data: an array of typed data
>  * @elements: the number of data elements
>  * @flags: flags for certificate verification -- #gnutls_certificate_verify_flags
>  *
>  * This function instructs GnuTLS to verify the peer's certificate
>  * using the provided typed data information. 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 acceptable typed data are the same as in gnutls_certificate_verify_peers(),
>  * and once set must remain valid for the lifetime of the session. More precisely
>  * they should be available during any subsequent handshakes.
>  *
>  * Since: 3.5.0
>  **/
> void gnutls_session_set_verify_cert2(gnutls_session_t session,
> 				     gnutls_typed_vdata_st * data,
> 				     unsigned elements,
> 				     unsigned flags)
> {

Would it help to comment here that this is equivalent to calling
gnutls_certificate_verify_peers with the same data object?

Regards,

      --dkg