mir/sudo
2
0
Fork 0
mirror of https://github.com/sudo-project/sudo.git synced 2025-08-23 02:17:35 +00:00
Code Issues Releases Wiki Activity

Add debugging statements to certificate checks.

Browse Source
This commit is contained in:
Todd C. Miller 2020-01-18 12:57:24 -07:00
parent 5a86073bd0
commit 47d9504716
4 changed files with 72 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
include/hostcheck.h
View File

@ -20,7 +20,6 @@
#if defined(HAVE_OPENSSL) #if defined(HAVE_OPENSSL)
# include <openssl/x509v3.h> # include <openssl/x509v3.h>
# include "sudo_compat.h"
typedef enum { typedef enum {
MatchFound, MatchFound,
@ -35,4 +34,4 @@ validate_hostname(const X509 *cert, const char *hostname, const char *ipaddr, in
#endif /* HAVE_OPENSSL */ #endif /* HAVE_OPENSSL */
#endif /* SUDO_HOSTCHECK_H */ #endif /* SUDO_HOSTCHECK_H */

98
lib/iolog/hostcheck.c
View File

@ -14,15 +14,22 @@
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/ */
/*
* This is an open source non-commercial project. Dear PVS-Studio, please check it.
* PVS-Studio Static Code Analyzer for C, C++ and C#: http://www.viva64.com
*/
#include "config.h" #include "config.h"
#if defined(HAVE_OPENSSL) #if defined(HAVE_OPENSSL)
# include <string.h>
# include <netdb.h>
# include <sys/socket.h> # include <sys/socket.h>
# include <arpa/inet.h> # include <arpa/inet.h>
# include <openssl/x509v3.h> # include <string.h>
# include <netdb.h>
# include "sudo_compat.h"
# include "sudo_debug.h"
# include "sudo_util.h"
# include "hostcheck.h" # include "hostcheck.h"
/** /**
@ -30,14 +37,14 @@
* *
* @param hostname hostname to be resolved * @param hostname hostname to be resolved
* @param ipaddr ip address to be checked * @param ipaddr ip address to be checked
* *
* @return 1 if hostname resolves to the given IP address * @return 1 if hostname resolves to the given IP address
* 0 otherwise * 0 otherwise
*/ */
static int static int
forward_lookup_match(const char *hostname, const char *ipaddr) forward_lookup_match(const char *hostname, const char *ipaddr)
{ {
int ret = 0; int rc, ret = 0;
struct addrinfo *res = NULL, *p; struct addrinfo *res = NULL, *p;
void *addr; void *addr;
struct sockaddr_in *ipv4; struct sockaddr_in *ipv4;
@ -47,17 +54,23 @@ forward_lookup_match(const char *hostname, const char *ipaddr)
#else #else
char ipstr[INET_ADDRSTRLEN]; char ipstr[INET_ADDRSTRLEN];
#endif #endif
debug_decl(forward_lookup_match, SUDO_DEBUG_UTIL);
if (getaddrinfo(hostname, NULL, NULL, &res) != 0) { sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO,
"verify %s resolves to %s", hostname, ipaddr);
if ((rc = getaddrinfo(hostname, NULL, NULL, &res)) != 0) {
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO,
"unable to resolve %s: %s", hostname, gai_strerror(rc));
goto exit; goto exit;
} }
for(p = res ;p != NULL; p = p->ai_next) { for (p = res; p != NULL; p = p->ai_next) {
if(p->ai_family == AF_INET) { if (p->ai_family == AF_INET) {
ipv4 = (struct sockaddr_in *)p->ai_addr; ipv4 = (struct sockaddr_in *)p->ai_addr;
addr = &(ipv4->sin_addr); addr = &(ipv4->sin_addr);
#if defined(HAVE_STRUCT_IN6_ADDR) #if defined(HAVE_STRUCT_IN6_ADDR)
} else if(p->ai_family == AF_INET6) { } else if (p->ai_family == AF_INET6) {
ipv6 = (struct sockaddr_in6 *)p->ai_addr; ipv6 = (struct sockaddr_in6 *)p->ai_addr;
addr = &(ipv6->sin6_addr); addr = &(ipv6->sin6_addr);
#endif #endif
@ -66,6 +79,8 @@ forward_lookup_match(const char *hostname, const char *ipaddr)
} }
if (inet_ntop(p->ai_family, addr, ipstr, sizeof(ipstr)) != 0) { if (inet_ntop(p->ai_family, addr, ipstr, sizeof(ipstr)) != 0) {
sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO,
"comparing %s to %s", ipstr, ipaddr);
if (strcmp(ipaddr, ipstr) == 0) { if (strcmp(ipaddr, ipstr) == 0) {
ret = 1; ret = 1;
break; break;
@ -77,7 +92,7 @@ exit:
if (res != NULL) { if (res != NULL) {
freeaddrinfo(res); freeaddrinfo(res);
} }
return ret; debug_return_int(ret);
} }
/** /**
@ -89,24 +104,30 @@ exit:
* - foo.bar.example.com * - foo.bar.example.com
* - *.example.com * - *.example.com
* - *.bar.example.com * - *.bar.example.com
* *
* @param hostname peer's name * @param hostname peer's name
* @param certname_asn1 hostname in the certificate * @param certname_asn1 hostname in the certificate
* *
* @return MatchFound * @return MatchFound
* MatchNotFound * MatchNotFound
*/ */
static HostnameValidationResult static HostnameValidationResult
validate_name(const char *hostname, ASN1_STRING *certname_asn1) { validate_name(const char *hostname, ASN1_STRING *certname_asn1)
char *certname_s = (char *) ASN1_STRING_get0_data(certname_asn1); {
int certname_len = ASN1_STRING_length(certname_asn1); char *certname_s = (char *) ASN1_STRING_get0_data(certname_asn1);
int certname_len = ASN1_STRING_length(certname_asn1);
int hostname_len = strlen(hostname); int hostname_len = strlen(hostname);
debug_decl(validate_name, SUDO_DEBUG_UTIL);
/* remove last '.' from hostname if exists */ /* remove last '.' from hostname if exists */
if (hostname_len != 0 && hostname[hostname_len - 1] == '.') { if (hostname_len != 0 && hostname[hostname_len - 1] == '.') {
--hostname_len; --hostname_len;
} }
sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO,
"comparing %.*s to %.*s in cert", hostname_len, hostname,
certname_len, certname_s);
/* skip the first label if wildcard */ /* skip the first label if wildcard */
if (certname_len > 2 && certname_s[0] == '*' && certname_s[1] == '.') { if (certname_len > 2 && certname_s[0] == '*' && certname_s[1] == '.') {
if (hostname_len != 0) { if (hostname_len != 0) {
@ -122,18 +143,18 @@ validate_name(const char *hostname, ASN1_STRING *certname_asn1) {
} }
/* Compare expected hostname with the DNS name */ /* Compare expected hostname with the DNS name */
if (certname_len != hostname_len) { if (certname_len != hostname_len) {
return MatchNotFound; debug_return_int(MatchNotFound);
} }
if (strncasecmp(hostname, certname_s, hostname_len) != 0) { if (strncasecmp(hostname, certname_s, hostname_len) != 0) {
return MatchNotFound; debug_return_int(MatchNotFound);
} }
return MatchFound; debug_return_int(MatchFound);
} }
/** /**
* @brief Matches a hostname with the cert's CN. * @brief Matches a hostname with the cert's CN.
* *
* @param hostname peer's name * @param hostname peer's name
* on client side: it is the name where the client is connected to * on client side: it is the name where the client is connected to
* on server side, it is in fact an IP address of the remote client * on server side, it is in fact an IP address of the remote client
@ -141,7 +162,7 @@ validate_name(const char *hostname, ASN1_STRING *certname_asn1) {
* @param cert peer's X509 certificate * @param cert peer's X509 certificate
* @param resolve if the value is not 0, the function checks that the value of the CN * @param resolve if the value is not 0, the function checks that the value of the CN
* resolves to the given ipaddr or not. * resolves to the given ipaddr or not.
* *
* @return MatchFound * @return MatchFound
* MatchNotFound * MatchNotFound
* MalformedCertificate * MalformedCertificate
@ -153,41 +174,42 @@ matches_common_name(const char *hostname, const char *ipaddr, const X509 *cert,
X509_NAME_ENTRY *common_name_entry = NULL; X509_NAME_ENTRY *common_name_entry = NULL;
ASN1_STRING *common_name_asn1 = NULL; ASN1_STRING *common_name_asn1 = NULL;
int common_name_loc = -1; int common_name_loc = -1;
debug_decl(matches_common_name, SUDO_DEBUG_UTIL);
/* Find the position of the CN field in the Subject field of the certificate */ /* Find the position of the CN field in the Subject field of the certificate */
common_name_loc = X509_NAME_get_index_by_NID(X509_get_subject_name((X509 *) cert), NID_commonName, -1); common_name_loc = X509_NAME_get_index_by_NID(X509_get_subject_name((X509 *) cert), NID_commonName, -1);
if (common_name_loc < 0) { if (common_name_loc < 0) {
return Error; debug_return_int(Error);
} }
/* Extract the CN field */ /* Extract the CN field */
common_name_entry = X509_NAME_get_entry(X509_get_subject_name((X509 *) cert), common_name_loc); common_name_entry = X509_NAME_get_entry(X509_get_subject_name((X509 *) cert), common_name_loc);
if (common_name_entry == NULL) { if (common_name_entry == NULL) {
return Error; debug_return_int(Error);
} }
/* Convert the CN field to a C string */ /* Convert the CN field to a C string */
common_name_asn1 = X509_NAME_ENTRY_get_data(common_name_entry); common_name_asn1 = X509_NAME_ENTRY_get_data(common_name_entry);
if (common_name_asn1 == NULL) { if (common_name_asn1 == NULL) {
return Error; debug_return_int(Error);
} }
const unsigned char *common_name_str = ASN1_STRING_get0_data(common_name_asn1); const unsigned char *common_name_str = ASN1_STRING_get0_data(common_name_asn1);
/* Make sure there isn't an embedded NUL character in the CN */ /* Make sure there isn't an embedded NUL character in the CN */
if (memchr(common_name_str, '\0', ASN1_STRING_length(common_name_asn1)) != NULL) { if (memchr(common_name_str, '\0', ASN1_STRING_length(common_name_asn1)) != NULL) {
return MalformedCertificate; debug_return_int(MalformedCertificate);
} }
/* Compare expected hostname with the CN */ /* Compare expected hostname with the CN */
if (validate_name(hostname, common_name_asn1) == MatchFound) { if (validate_name(hostname, common_name_asn1) == MatchFound) {
return MatchFound; debug_return_int(MatchFound);
} }
int common_name_length = ASN1_STRING_length(common_name_asn1); int common_name_length = ASN1_STRING_length(common_name_asn1);
unsigned char *nullterm_common_name = malloc(common_name_length + 1); unsigned char *nullterm_common_name = malloc(common_name_length + 1);
if (nullterm_common_name == NULL) { if (nullterm_common_name == NULL) {
return Error; debug_return_int(Error);
} }
memcpy(nullterm_common_name, common_name_str, common_name_length); memcpy(nullterm_common_name, common_name_str, common_name_length);
@ -197,11 +219,11 @@ matches_common_name(const char *hostname, const char *ipaddr, const X509 *cert,
/* check if hostname in the CN field resolves to the given ip address */ /* check if hostname in the CN field resolves to the given ip address */
if (resolve && forward_lookup_match(nullterm_common_name, ipaddr)) { if (resolve && forward_lookup_match(nullterm_common_name, ipaddr)) {
free(nullterm_common_name); free(nullterm_common_name);
return MatchFound; debug_return_int(MatchFound);
} }
free(nullterm_common_name); free(nullterm_common_name);
return MatchNotFound; debug_return_int(MatchNotFound);
} }
/** /**
@ -211,7 +233,7 @@ matches_common_name(const char *hostname, const char *ipaddr, const X509 *cert,
* for IP address matching, the GEN_IPADD field is used. * for IP address matching, the GEN_IPADD field is used.
* Since SAN is an X503 v3 extension, it can happen that the cert does * Since SAN is an X503 v3 extension, it can happen that the cert does
* not contain SAN at all. * not contain SAN at all.
* *
* @param hostname remote peer's name * @param hostname remote peer's name
* on client side: it is the name where the client is connected to * on client side: it is the name where the client is connected to
* on server side, it is in fact an IP address of the remote client * on server side, it is in fact an IP address of the remote client
@ -219,7 +241,7 @@ matches_common_name(const char *hostname, const char *ipaddr, const X509 *cert,
* @param cert peer's X509 certificate * @param cert peer's X509 certificate
* @param resolve if the value is not 0, the function checks that the value of the * @param resolve if the value is not 0, the function checks that the value of the
* SAN GEN_DNS resolves to the given ipaddr or not. * SAN GEN_DNS resolves to the given ipaddr or not.
* *
* @return MatchFound * @return MatchFound
* MatchNotFound * MatchNotFound
* NoSANPresent * NoSANPresent
@ -233,11 +255,12 @@ matches_subject_alternative_name(const char *hostname, const char *ipaddr, const
int i; int i;
int san_names_nb = -1; int san_names_nb = -1;
STACK_OF(GENERAL_NAME) *san_names = NULL; STACK_OF(GENERAL_NAME) *san_names = NULL;
debug_decl(matches_subject_alternative_name, SUDO_DEBUG_UTIL);
/* Try to extract the names within the SAN extension from the certificate */ /* Try to extract the names within the SAN extension from the certificate */
san_names = X509_get_ext_d2i((X509 *) cert, NID_subject_alt_name, NULL, NULL); san_names = X509_get_ext_d2i((X509 *) cert, NID_subject_alt_name, NULL, NULL);
if (san_names == NULL) { if (san_names == NULL) {
return NoSANPresent; debug_return_int(NoSANPresent);
} }
san_names_nb = sk_GENERAL_NAME_num(san_names); san_names_nb = sk_GENERAL_NAME_num(san_names);
@ -263,7 +286,7 @@ matches_subject_alternative_name(const char *hostname, const char *ipaddr, const
unsigned char *nullterm_dns_name = malloc(dns_name_length + 1); unsigned char *nullterm_dns_name = malloc(dns_name_length + 1);
if (nullterm_dns_name == NULL) { if (nullterm_dns_name == NULL) {
return Error; debug_return_int(Error);
} }
memcpy(nullterm_dns_name, dns_name, dns_name_length); memcpy(nullterm_dns_name, dns_name, dns_name_length);
@ -311,7 +334,7 @@ matches_subject_alternative_name(const char *hostname, const char *ipaddr, const
} }
sk_GENERAL_NAME_pop_free(san_names, GENERAL_NAME_free); sk_GENERAL_NAME_pop_free(san_names, GENERAL_NAME_free);
return result; debug_return_int(result);
} }
/** /**
@ -320,7 +343,7 @@ matches_subject_alternative_name(const char *hostname, const char *ipaddr, const
* According to RFC 6125 section 6.4.4, first the certificate's SAN field * According to RFC 6125 section 6.4.4, first the certificate's SAN field
* has to be checked. If there is no SAN field, the certificate's CN field * has to be checked. If there is no SAN field, the certificate's CN field
* has to be checked. * has to be checked.
* *
* @param cert X509 certificate * @param cert X509 certificate
* @param hostname remote peer's name * @param hostname remote peer's name
* on client side: it is the name where the client is connected to * on client side: it is the name where the client is connected to
@ -328,7 +351,7 @@ matches_subject_alternative_name(const char *hostname, const char *ipaddr, const
* @param ipaddr remote peer's IP address * @param ipaddr remote peer's IP address
* @param resolve if the value is not 0, the function checks that the value of the * @param resolve if the value is not 0, the function checks that the value of the
* SAN GEN_DNS or the value of CN resolves to the given ipaddr or not. * SAN GEN_DNS or the value of CN resolves to the given ipaddr or not.
* *
* @return MatchFound * @return MatchFound
* MatchNotFound * MatchNotFound
* MalformedCertificate * MalformedCertificate
@ -338,12 +361,13 @@ HostnameValidationResult
validate_hostname(const X509 *cert, const char *hostname, const char *ipaddr, int resolve) validate_hostname(const X509 *cert, const char *hostname, const char *ipaddr, int resolve)
{ {
HostnameValidationResult res = MatchFound; HostnameValidationResult res = MatchFound;
debug_decl(validate_hostname, SUDO_DEBUG_UTIL);
/* hostname can be also an ip address, if client connects /* hostname can be also an ip address, if client connects
* to ip instead of FQDN * to ip instead of FQDN
*/ */
if((ipaddr == NULL) || (cert == NULL)) { if((ipaddr == NULL) || (cert == NULL)) {
return Error; debug_return_int(Error);
} }
/* check SAN first if exists */ /* check SAN first if exists */
@ -356,6 +380,6 @@ validate_hostname(const X509 *cert, const char *hostname, const char *ipaddr, in
res = matches_common_name(hostname, ipaddr, cert, resolve); res = matches_common_name(hostname, ipaddr, cert, resolve);
} }
return res; debug_return_int(res);
} }
#endif /* HAVE_OPENSSL */ #endif /* HAVE_OPENSSL */

9
logsrvd/logsrvd.c
View File

@ -919,10 +919,11 @@ verify_peer_identity(int preverify_ok, X509_STORE_CTX *ctx)
SSL *ssl; SSL *ssl;
X509 *current_cert; X509 *current_cert;
X509 *peer_cert; X509 *peer_cert;
debug_decl(verify_peer_identity, SUDO_DEBUG_UTIL);
/* if pre-verification of the cert failed, just propagate that result back */ /* if pre-verification of the cert failed, just propagate that result back */
if (preverify_ok != 1) { if (preverify_ok != 1) {
return 0; debug_return_int(0);
} }
/* since this callback is called for each cert in the chain, /* since this callback is called for each cert in the chain,
@ -932,7 +933,7 @@ verify_peer_identity(int preverify_ok, X509_STORE_CTX *ctx)
peer_cert = X509_STORE_CTX_get0_cert(ctx); peer_cert = X509_STORE_CTX_get0_cert(ctx);
if (current_cert != peer_cert) { if (current_cert != peer_cert) {
return 1; debug_return_int(1);
} }
/* read out the attached object (closure) from the ssl connection object */ /* read out the attached object (closure) from the ssl connection object */
@ -944,9 +945,9 @@ verify_peer_identity(int preverify_ok, X509_STORE_CTX *ctx)
switch(result) switch(result)
{ {
case MatchFound: case MatchFound:
return 1; debug_return_int(1);
default: default:
return 0; debug_return_int(0);
} }
} }

9
plugins/sudoers/iolog_client.c
View File

@ -228,10 +228,11 @@ verify_peer_identity(int preverify_ok, X509_STORE_CTX *ctx)
SSL *ssl; SSL *ssl;
X509 *current_cert; X509 *current_cert;
X509 *peer_cert; X509 *peer_cert;
debug_decl(verify_peer_identity, SUDOERS_DEBUG_UTIL);
/* if pre-verification of the cert failed, just propagate that result back */ /* if pre-verification of the cert failed, just propagate that result back */
if (preverify_ok != 1) { if (preverify_ok != 1) {
return 0; debug_return_int(0);
} }
/* since this callback is called for each cert in the chain, /* since this callback is called for each cert in the chain,
@ -241,7 +242,7 @@ verify_peer_identity(int preverify_ok, X509_STORE_CTX *ctx)
peer_cert = X509_STORE_CTX_get0_cert(ctx); peer_cert = X509_STORE_CTX_get0_cert(ctx);
if (current_cert != peer_cert) { if (current_cert != peer_cert) {
return 1; debug_return_int(1);
} }
/* read out the attached object (closure) from the ssl connection object */ /* read out the attached object (closure) from the ssl connection object */
@ -253,9 +254,9 @@ verify_peer_identity(int preverify_ok, X509_STORE_CTX *ctx)
switch(result) switch(result)
{ {
case MatchFound: case MatchFound:
return 1; debug_return_int(1);
default: default:
return 0; debug_return_int(0);
} }
} }