Attachment #255463 for bug #344939

View | Details | Raw Unified | Return to bug 344939
Collapse All | Expand All




#include <string.h>

#include <openssl/ssl.h>
#include <openssl/x509v3.h>
#include <openssl/err.h>
#include <openssl/rand.h>


ssl_check_certificate (int fd, const char *host)
{
  X509 *cert;
  GENERAL_NAMES *subjectAltNames;
  char common_name[256];
  long vresult;
  bool success = true;
  bool alt_name_checked = false;

  /* If the user has specified --no-check-cert, we still want to warn
     him about problems with the server's certificate.  */

          break;
        case X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN:
        case X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT:
          logprintf (LOG_NOTQUIET,
                     _("  Self-signed certificate encountered.\n"));
          break;
        case X509_V_ERR_CERT_NOT_YET_VALID:
          logprintf (LOG_NOTQUIET, _("  Issued certificate not yet valid.\n"));

  /* Check that HOST matches the common name in the certificate.
     #### The following remains to be done:

     - It should use dNSName/ipAddress subjectAltName extensions if
       available; according to rfc2818: "If a subjectAltName extension
       of type dNSName is present, that MUST be used as the identity."

     - When matching against common names, it should loop over all
       common names and choose the most specific one, i.e. the last
       one, not the first one, which the current code picks.

     - Ensure that ASN1 strings from the certificate are encoded as
       UTF-8 which can be meaningfully compared to HOST.  */

  subjectAltNames = X509_get_ext_d2i (cert, NID_subject_alt_name, NULL, NULL);
  common_name[0] = '\0';
  X509_NAME_get_text_by_NID (xname, NID_commonName, common_name,
                             sizeof (common_name));

  if (subjectAltNames)
    {
      /* Test subject alternative names */

      /* Do we want to check for dNSNAmes or ipAddresses (see RFC 2818)?
       * Signal it by host_in_octet_string. */
      ASN1_OCTET_STRING *host_in_octet_string = NULL;
      host_in_octet_string = a2i_IPADDRESS (host);

      int numaltnames = sk_GENERAL_NAME_num (subjectAltNames);
      int i;
      for (i=0; i < numaltnames; i++)
        {
          const GENERAL_NAME *name =
            sk_GENERAL_NAME_value (subjectAltNames, i);
          if (name)
            {
              if (host_in_octet_string)
                {
                  if (name->type == GEN_IPADD)
                    {
                      /* Check for ipAddress */
                      /* TODO: Should we convert between IPv4-mapped IPv6
                       * addresses and IPv4 addresses? */
                      alt_name_checked = true;
                      if (!ASN1_STRING_cmp (host_in_octet_string,
                            name->d.iPAddress))
                        break;
                    }
                }
              else if (name->type == GEN_DNS)
                {
                  /* Check for dNSName */
                  alt_name_checked = true;
                  /* dNSName should be IA5String (i.e. ASCII), however who
                   * does trust CA? Convert it into UTF-8 for sure. */
                  unsigned char *name_in_utf8 = NULL;
                  if (0 <= ASN1_STRING_to_UTF8 (&name_in_utf8, name->d.dNSName))
                    {
                      /* Compare and check for NULL attack in ASN1_STRING */
                      if (pattern_match ((char *)name_in_utf8, host) &&
                            (strlen ((char *)name_in_utf8) ==
                                ASN1_STRING_length (name->d.dNSName)))
                        {
                          OPENSSL_free (name_in_utf8);
                          break;
                        }
                      OPENSSL_free (name_in_utf8);
                    }
                }
            }
        }
      sk_GENERAL_NAME_free (subjectAltNames);
      if (host_in_octet_string)
        ASN1_OCTET_STRING_free(host_in_octet_string);

      if (alt_name_checked == true && i >= numaltnames)
        {
          logprintf (LOG_NOTQUIET,
              _("%s: no certificate subject alternative name matches\n"
                "\trequested host name %s.\n"),
                     severity, quote_n (1, host));
          success = false;
        }
    }
  
  if (alt_name_checked == false)
    {
      /* Test commomName */
      X509_NAME *xname = X509_get_subject_name(cert);
      common_name[0] = '\0';
      X509_NAME_get_text_by_NID (xname, NID_commonName, common_name,
                                 sizeof (common_name));

      if (!pattern_match (common_name, host))
        {
          logprintf (LOG_NOTQUIET, _("\
    %s: certificate common name %s doesn't match requested host name %s.\n"),
                     severity, quote_n (0, common_name), quote_n (1, host));
          success = false;
        }
      else
        {
          /* We now determine the length of the ASN1 string. If it
           * differs from common_name's length, then there is a \0
           * before the string terminates.  This can be an instance of a
           * null-prefix attack.
           *
           * https://www.blackhat.com/html/bh-usa-09/bh-usa-09-archives.html#Marlinspike
           * */

          int i = -1, j;
          X509_NAME_ENTRY *xentry;
          ASN1_STRING *sdata;

          if (xname) {
            for (;;)
              {
                j = X509_NAME_get_index_by_NID (xname, NID_commonName, i);
                if (j == -1) break;
                i = j;
              }
          }
      }

          xentry = X509_NAME_get_entry(xname,i);
          sdata = X509_NAME_ENTRY_get_data(xentry);
          if (strlen (common_name) != ASN1_STRING_length (sdata))
            {
              logprintf (LOG_NOTQUIET, _("\
    %s: certificate common name is invalid (contains a NUL character).\n\
    This may be an indication that the host is not who it claims to be\n\
    (that is, it is not the real %s).\n"),
                         severity, quote (host));
              success = false;
            }
        }
    }


  /* Allow --no-check-cert to disable certificate checking. */
  return opt.check_cert ? success : true;
}

/*
 * vim: tabstop=2 shiftwidth=2 softtabstop=2
 */

Return to bug 344939

Lines 39-45 Link Here

(-)src/openssl.c (-44 / +120 lines)
39	#include <string.h>	39	#include <string.h>
40		40
41	#include <openssl/ssl.h>	41	#include <openssl/ssl.h>
42	#include <openssl/x509.h>	42	#include <openssl/x509v3.h>
43	#include <openssl/err.h>	43	#include <openssl/err.h>
44	#include <openssl/rand.h>	44	#include <openssl/rand.h>
45		45
Lines 486-494 Link Here
486	ssl_check_certificate (int fd, const char *host)	486	ssl_check_certificate (int fd, const char *host)
487	{	487	{
488	X509 *cert;	488	X509 *cert;
		489	GENERAL_NAMES *subjectAltNames;
489	char common_name[256];	490	char common_name[256];
490	long vresult;	491	long vresult;
491	bool success = true;	492	bool success = true;
		493	bool alt_name_checked = false;
492		494
493	/* If the user has specified --no-check-cert, we still want to warn	495	/* If the user has specified --no-check-cert, we still want to warn
494	him about problems with the server's certificate. */	496	him about problems with the server's certificate. */
Lines 536-542 Link Here
536	break;	538	break;
537	case X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN:	539	case X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN:
538	case X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT:	540	case X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT:
539	logprintf (LOG_NOTQUIET, _(" Self-signed certificate encountered.\n"));	541	logprintf (LOG_NOTQUIET,
		542	_(" Self-signed certificate encountered.\n"));
540	break;	543	break;
541	case X509_V_ERR_CERT_NOT_YET_VALID:	544	case X509_V_ERR_CERT_NOT_YET_VALID:
542	logprintf (LOG_NOTQUIET, _(" Issued certificate not yet valid.\n"));	545	logprintf (LOG_NOTQUIET, _(" Issued certificate not yet valid.\n"));
Lines 558-567 Link Here
558	/* Check that HOST matches the common name in the certificate.	561	/* Check that HOST matches the common name in the certificate.
559	#### The following remains to be done:	562	#### The following remains to be done:
560		563
561	- It should use dNSName/ipAddress subjectAltName extensions if
562	available; according to rfc2818: "If a subjectAltName extension
563	of type dNSName is present, that MUST be used as the identity."
564
565	- When matching against common names, it should loop over all	564	- When matching against common names, it should loop over all
566	common names and choose the most specific one, i.e. the last	565	common names and choose the most specific one, i.e. the last
567	one, not the first one, which the current code picks.	566	one, not the first one, which the current code picks.
Lines 569-618 Link Here
569	- Ensure that ASN1 strings from the certificate are encoded as	568	- Ensure that ASN1 strings from the certificate are encoded as
570	UTF-8 which can be meaningfully compared to HOST. */	569	UTF-8 which can be meaningfully compared to HOST. */
571		570
572	X509_NAME *xname = X509_get_subject_name(cert);	571	subjectAltNames = X509_get_ext_d2i (cert, NID_subject_alt_name, NULL, NULL);
573	common_name[0] = '\0';
574	X509_NAME_get_text_by_NID (xname, NID_commonName, common_name,
575	sizeof (common_name));
576		572
577	if (!pattern_match (common_name, host))	573	if (subjectAltNames)
578	{	574	{
579	logprintf (LOG_NOTQUIET, _("\	575	/* Test subject alternative names */
580	%s: certificate common name %s doesn't match requested host name %s.\n"),	576
581	severity, quote_n (0, common_name), quote_n (1, host));	577	/* Do we want to check for dNSNAmes or ipAddresses (see RFC 2818)?
582	success = false;	578	* Signal it by host_in_octet_string. */
		579	ASN1_OCTET_STRING *host_in_octet_string = NULL;
		580	host_in_octet_string = a2i_IPADDRESS (host);
		581
		582	int numaltnames = sk_GENERAL_NAME_num (subjectAltNames);
		583	int i;
		584	for (i=0; i < numaltnames; i++)
		585	{
		586	const GENERAL_NAME *name =
		587	sk_GENERAL_NAME_value (subjectAltNames, i);
		588	if (name)
		589	{
		590	if (host_in_octet_string)
		591	{
		592	if (name->type == GEN_IPADD)
		593	{
		594	/* Check for ipAddress */
		595	/* TODO: Should we convert between IPv4-mapped IPv6
		596	* addresses and IPv4 addresses? */
		597	alt_name_checked = true;
		598	if (!ASN1_STRING_cmp (host_in_octet_string,
		599	name->d.iPAddress))
		600	break;
		601	}
		602	}
		603	else if (name->type == GEN_DNS)
		604	{
		605	/* Check for dNSName */
		606	alt_name_checked = true;
		607	/* dNSName should be IA5String (i.e. ASCII), however who
		608	* does trust CA? Convert it into UTF-8 for sure. */
		609	unsigned char *name_in_utf8 = NULL;
		610	if (0 <= ASN1_STRING_to_UTF8 (&name_in_utf8, name->d.dNSName))
		611	{
		612	/* Compare and check for NULL attack in ASN1_STRING */
		613	if (pattern_match ((char *)name_in_utf8, host) &&
		614	(strlen ((char *)name_in_utf8) ==
		615	ASN1_STRING_length (name->d.dNSName)))
		616	{
		617	OPENSSL_free (name_in_utf8);
		618	break;
		619	}
		620	OPENSSL_free (name_in_utf8);
		621	}
		622	}
		623	}
		624	}
		625	sk_GENERAL_NAME_free (subjectAltNames);
		626	if (host_in_octet_string)
		627	ASN1_OCTET_STRING_free(host_in_octet_string);
		628
		629	if (alt_name_checked == true && i >= numaltnames)
		630	{
		631	logprintf (LOG_NOTQUIET,
		632	_("%s: no certificate subject alternative name matches\n"
		633	"\trequested host name %s.\n"),
		634	severity, quote_n (1, host));
		635	success = false;
		636	}
583	}	637	}
584	else	638
		639	if (alt_name_checked == false)
585	{	640	{
586	/* We now determine the length of the ASN1 string. If it differs from	641	/* Test commomName */
587	* common_name's length, then there is a \0 before the string terminates.	642	X509_NAME *xname = X509_get_subject_name(cert);
588	* This can be an instance of a null-prefix attack.	643	common_name[0] = '\0';
589	*	644	X509_NAME_get_text_by_NID (xname, NID_commonName, common_name,
590	* https://www.blackhat.com/html/bh-usa-09/bh-usa-09-archives.html#Marlinspike	645	sizeof (common_name));
591	* */	646
592		647	if (!pattern_match (common_name, host))
593	int i = -1, j;	648	{
594	X509_NAME_ENTRY *xentry;	649	logprintf (LOG_NOTQUIET, _("\
595	ASN1_STRING *sdata;	650	%s: certificate common name %s doesn't match requested host name %s.\n"),
596		651	severity, quote_n (0, common_name), quote_n (1, host));
597	if (xname) {	652	success = false;
598	for (;;)	653	}
599	{	654	else
600	j = X509_NAME_get_index_by_NID (xname, NID_commonName, i);	655	{
601	if (j == -1) break;	656	/* We now determine the length of the ASN1 string. If it
602	i = j;	657	* differs from common_name's length, then there is a \0
		658	* before the string terminates. This can be an instance of a
		659	* null-prefix attack.
		660	*
		661	* https://www.blackhat.com/html/bh-usa-09/bh-usa-09-archives.html#Marlinspike
		662	* */
		663
		664	int i = -1, j;
		665	X509_NAME_ENTRY *xentry;
		666	ASN1_STRING *sdata;
		667
		668	if (xname) {
		669	for (;;)
		670	{
		671	j = X509_NAME_get_index_by_NID (xname, NID_commonName, i);
		672	if (j == -1) break;
		673	i = j;
		674	}
603	}	675	}
604	}
605		676
606	xentry = X509_NAME_get_entry(xname,i);	677	xentry = X509_NAME_get_entry(xname,i);
607	sdata = X509_NAME_ENTRY_get_data(xentry);	678	sdata = X509_NAME_ENTRY_get_data(xentry);
608	if (strlen (common_name) != ASN1_STRING_length (sdata))	679	if (strlen (common_name) != ASN1_STRING_length (sdata))
609	{	680	{
610	logprintf (LOG_NOTQUIET, _("\	681	logprintf (LOG_NOTQUIET, _("\
611	%s: certificate common name is invalid (contains a NUL character).\n\	682	%s: certificate common name is invalid (contains a NUL character).\n\
612	This may be an indication that the host is not who it claims to be\n\	683	This may be an indication that the host is not who it claims to be\n\
613	(that is, it is not the real %s).\n"),	684	(that is, it is not the real %s).\n"),
614	severity, quote (host));	685	severity, quote (host));
615	success = false;	686	success = false;
		687	}
616	}	688	}
617	}	689	}
618		690
Lines 631-633 Link Here
631	/* Allow --no-check-cert to disable certificate checking. */	703	/* Allow --no-check-cert to disable certificate checking. */
632	return opt.check_cert ? success : true;	704	return opt.check_cert ? success : true;
633	}	705	}
		706
		707	/*
		708	* vim: tabstop=2 shiftwidth=2 softtabstop=2
		709	*/