Fixed #7045: allow NULL isser and subjects in certificates

This commit is contained in:
akallabeth 2021-05-27 08:38:59 +02:00 committed by akallabeth
parent fa73a2ba55
commit 8e43f90590
4 changed files with 219 additions and 17 deletions

View File

@ -22,6 +22,7 @@
#include "config.h"
#endif
#include <assert.h>
#include <errno.h>
#include <stdio.h>
#include <string.h>
@ -490,10 +491,41 @@ static int certificate_match_data_file(rdpCertificateStore* certificate_store,
return rc;
}
static BOOL useKnownHosts(rdpCertificateStore* certificate_store)
{
BOOL use;
assert(certificate_store);
use = freerdp_settings_get_bool(certificate_store->settings, FreeRDP_CertificateUseKnownHosts);
WLog_INFO(TAG, "known_hosts=%d", use);
return use;
}
#define check_certificate_store_and_data(store, data) \
check_certificate_store_and_data_((store), (data), __FILE__, __FUNCTION__, __LINE__)
static BOOL check_certificate_store_and_data_(const rdpCertificateStore* certificate_store,
const rdpCertificateData* certificate_data,
const char* file, const char* fkt, size_t line)
{
if (!certificate_store)
{
WLog_ERR(TAG, "[%s, %s:%" PRIuz "] certificate_store=NULL", file, fkt, line);
return FALSE;
}
if (!certificate_data)
{
WLog_ERR(TAG, "[%s, %s:%" PRIuz "] certificate_data=NULL", file, fkt, line);
return FALSE;
}
return TRUE;
}
int certificate_store_contains_data(rdpCertificateStore* certificate_store,
const rdpCertificateData* certificate_data)
{
if (freerdp_settings_get_bool(certificate_store->settings, FreeRDP_CertificateUseKnownHosts))
if (!check_certificate_store_and_data(certificate_store, certificate_data))
return -1;
if (useKnownHosts(certificate_store))
{
char* pem = NULL;
int rc =
@ -570,9 +602,15 @@ static char* certificate_data_get_host_file_entry(const rdpCertificateData* data
const char* fingerprint = certificate_data_get_fingerprint(data);
char* pem = encode(certificate_data_get_pem(data));
if (!data || !hostname || !fingerprint || !subject || !issuer)
/* Issuer and subject may be NULL */
if (!data || !hostname || !fingerprint)
goto fail;
if (!subject)
subject = _strdup("");
if (!issuer)
issuer = _strdup("");
if (pem)
buffer = allocated_printf("%s %" PRIu16 " %s %s %s %s\n", hostname, port, fingerprint,
subject, issuer, pem);
@ -609,6 +647,7 @@ static BOOL certificate_data_replace_hosts_file(rdpCertificateStore* certificate
const rdpCertificateData* certificate_data,
BOOL remove, BOOL append)
{
BOOL newfile = TRUE;
HANDLE fp = INVALID_HANDLE_VALUE;
BOOL rc = FALSE;
size_t length;
@ -621,11 +660,14 @@ static BOOL certificate_data_replace_hosts_file(rdpCertificateStore* certificate
DWORD lowSize, highSize;
rdpCertificateData* copy = NULL;
fp = open_file(certificate_store->file, GENERIC_READ | GENERIC_WRITE, 0, OPEN_EXISTING,
fp = open_file(certificate_store->file, GENERIC_READ | GENERIC_WRITE, 0, OPEN_ALWAYS,
FILE_ATTRIBUTE_NORMAL);
if (fp == INVALID_HANDLE_VALUE)
{
WLog_ERR(TAG, "failed to open known_hosts file %s, aborting", certificate_store->file);
return FALSE;
}
/* Create a copy, if a PEM was provided it will replace subject, issuer, fingerprint */
copy = certificate_data_new(certificate_data->hostname, certificate_data->port);
@ -654,7 +696,11 @@ static BOOL certificate_data_replace_hosts_file(rdpCertificateStore* certificate
size = (UINT64)lowSize | ((UINT64)highSize << 32);
if (size < 1)
/* Newly created file, just write the new entry */
if (size > 0)
newfile = FALSE;
if (newfile)
goto fail;
data = (char*)malloc(size + 2);
@ -731,8 +777,12 @@ static BOOL certificate_data_replace_hosts_file(rdpCertificateStore* certificate
fail:
if (!rc && append)
{
char* line = certificate_data_get_host_file_entry(copy);
rc = write_line_and_free(certificate_store->file, fp, line);
if (fp != INVALID_HANDLE_VALUE)
{
char* line = certificate_data_get_host_file_entry(copy);
if (line)
rc = write_line_and_free(certificate_store->file, fp, line);
}
}
CloseHandle(fp);
@ -781,7 +831,9 @@ static BOOL certificate_data_remove_file(rdpCertificateStore* certificate_store,
BOOL certificate_store_remove_data(rdpCertificateStore* certificate_store,
const rdpCertificateData* certificate_data)
{
if (freerdp_settings_get_bool(certificate_store->settings, FreeRDP_CertificateUseKnownHosts))
if (!check_certificate_store_and_data(certificate_store, certificate_data))
return FALSE;
if (useKnownHosts(certificate_store))
{
/* Ignore return, if the entry was invalid just continue */
certificate_data_replace_hosts_file(certificate_store, certificate_data, TRUE, FALSE);
@ -895,12 +947,11 @@ static BOOL update_from_pem(rdpCertificateData* data)
x1 = crypto_cert_from_pem(data->pem, strlen(data->pem), FALSE);
if (!x1)
goto fail;
/* Subject and issuer might be NULL */
subject = crypto_cert_subject(x1);
if (!subject)
goto fail;
issuer = crypto_cert_issuer(x1);
if (!issuer)
goto fail;
fingerprint = crypto_cert_fingerprint(x1);
if (!fingerprint)
goto fail;
@ -919,7 +970,9 @@ fail:
BOOL certificate_store_save_data(rdpCertificateStore* certificate_store,
const rdpCertificateData* certificate_data)
{
if (freerdp_settings_get_bool(certificate_store->settings, FreeRDP_CertificateUseKnownHosts))
if (!check_certificate_store_and_data(certificate_store, certificate_data))
return FALSE;
if (useKnownHosts(certificate_store))
return certificate_data_replace_hosts_file(certificate_store, certificate_data, TRUE, TRUE);
else
return certificate_data_write_to_file(certificate_store, certificate_data);
@ -928,7 +981,7 @@ BOOL certificate_store_save_data(rdpCertificateStore* certificate_store,
rdpCertificateData* certificate_store_load_data(rdpCertificateStore* certificate_store,
const char* host, UINT16 port)
{
if (freerdp_settings_get_bool(certificate_store->settings, FreeRDP_CertificateUseKnownHosts))
if (useKnownHosts(certificate_store))
{
int rc;
rdpCertificateData* data = certificate_data_new(host, port);
@ -1004,7 +1057,9 @@ UINT16 certificate_data_get_port(const rdpCertificateData* cert)
BOOL certificate_data_set_pem(rdpCertificateData* cert, const char* pem)
{
if (!cert)
{
return FALSE;
}
if (!duplicate(&cert->pem, pem))
return FALSE;
if (!pem)

View File

@ -939,6 +939,8 @@ rdpCertificateData* crypto_get_certificate_data(X509* xcert, const char* hostnam
free(pem);
return certdata;
fail:
WLog_WARN(TAG, "Failed to extract PEM from X509=%p for host %s:%" PRIu16, xcert, hostname,
port);
certificate_data_free(certdata);
free(pem);
return NULL;
@ -1074,6 +1076,7 @@ fail:
if (!rc)
{
WLog_ERR(TAG, "Failed to extract PEM from certificate %p", xcert);
free(pemCert);
pemCert = NULL;
}

View File

@ -308,6 +308,49 @@ finish:
return rc;
}
/* Test host add NULL subject, issuer current file. */
static BOOL test_known_hosts_host_add_remove_null(rdpCertificateStore* store)
{
BOOL rc = FALSE;
rdpCertificateData* data;
printf("%s\n", __FUNCTION__);
data = certificate_data_new("somehost", 1234);
if (!data)
{
fprintf(stderr, "Could not create certificate data!\n");
goto finish;
}
if (!certificate_data_set_subject(data, NULL) || !certificate_data_set_issuer(data, NULL) ||
!certificate_data_set_fingerprint(data, "ff:aa:bb:cc"))
goto finish;
if (!certificate_store_save_data(store, data))
{
fprintf(stderr, "Could not add host to file!\n");
goto finish;
}
if (0 != certificate_store_contains_data(store, data))
{
fprintf(stderr, "Could not find host written in v2 file!\n");
goto finish;
}
if (!certificate_store_remove_data(store, data))
{
fprintf(stderr, "Could not remove host written in v2 file!\n");
goto finish;
}
rc = TRUE;
finish:
printf("certificate_data_free %d\n", rc);
certificate_data_free(data);
return rc;
}
/* Test host replace current file. */
static BOOL test_known_hosts_host_replace(rdpCertificateStore* store)
{
@ -353,14 +396,14 @@ static BOOL test_known_hosts_host_replace_invalid(rdpCertificateStore* store)
rdpCertificateData* data;
printf("%s\n", __FUNCTION__);
data = certificate_data_new("somehostXXXX", 1234);
data = certificate_data_new(NULL, 1234);
if (!data)
if (data)
{
fprintf(stderr, "Could not create certificate data!\n");
fprintf(stderr, "Could create invalid certificate data!\n");
goto finish;
}
if (!certificate_data_set_fingerprint(data, "ff:aa:bb:dd:ee"))
if (certificate_data_set_fingerprint(data, "ff:aa:bb:dd:ee"))
goto finish;
if (certificate_store_save_data(store, data))
@ -381,6 +424,99 @@ finish:
return rc;
}
static BOOL test_known_hosts_file_emtpy_single(BOOL (*fkt)(rdpCertificateStore* store))
{
BOOL rc = FALSE;
rdpSettings* settings = NULL;
rdpCertificateStore* store = NULL;
char* currentFileV2 = NULL;
printf("%s", __FUNCTION__);
if (!fkt)
return FALSE;
if (!setup_config(&settings))
goto finish;
if (!freerdp_settings_set_bool(settings, FreeRDP_CertificateUseKnownHosts, TRUE))
goto finish;
currentFileV2 =
GetCombinedPath(freerdp_settings_get_string(settings, FreeRDP_ConfigPath), "known_hosts2");
if (!currentFileV2)
{
fprintf(stderr, "Could not get file path!\n");
goto finish;
}
printf("certificate_store_new\n");
store = certificate_store_new(settings);
if (!store)
{
fprintf(stderr, "Could not create certificate store!\n");
goto finish;
}
rc = fkt(store);
finish:
freerdp_settings_free(settings);
printf("certificate_store_free\n");
certificate_store_free(store);
DeleteFileA(currentFileV2);
free(currentFileV2);
return rc;
}
static BOOL test_known_hosts_file_empty(void)
{
BOOL rc = FALSE;
if (test_known_hosts_file_emtpy_single(test_known_hosts_host_found))
{
fprintf(stderr, "[%s] test_known_hosts_file_emtpy_single failed\n", __FUNCTION__);
goto finish;
}
if (!test_known_hosts_file_emtpy_single(test_known_hosts_host_not_found))
{
fprintf(stderr, "[%s] test_known_hosts_file_emtpy_single failed\n", __FUNCTION__);
goto finish;
}
if (!test_known_hosts_file_emtpy_single(test_known_hosts_host_add))
{
fprintf(stderr, "[%s] test_known_hosts_file_emtpy_single failed\n", __FUNCTION__);
goto finish;
}
if (!test_known_hosts_file_emtpy_single(test_known_hosts_host_add_remove_null))
{
fprintf(stderr, "[%s] test_known_hosts_file_emtpy_single failed\n", __FUNCTION__);
goto finish;
}
if (!test_known_hosts_file_emtpy_single(test_known_hosts_host_replace))
{
fprintf(stderr, "[%s] test_known_hosts_file_emtpy_single failed\n", __FUNCTION__);
goto finish;
}
if (!test_known_hosts_file_emtpy_single(test_known_hosts_host_replace_invalid))
{
fprintf(stderr, "[%s] test_known_hosts_file_emtpy_single failed\n", __FUNCTION__);
goto finish;
}
rc = TRUE;
finish:
return rc;
}
static BOOL test_known_hosts_file(void)
{
BOOL rc = FALSE;
@ -424,6 +560,9 @@ static BOOL test_known_hosts_file(void)
if (!test_known_hosts_host_add(store))
goto finish;
if (!test_known_hosts_host_add_remove_null(store))
goto finish;
if (!test_known_hosts_host_replace(store))
goto finish;
@ -680,6 +819,9 @@ int TestKnownHosts(int argc, char* argv[])
{
WINPR_UNUSED(argc);
WINPR_UNUSED(argv);
if (!test_known_hosts_file_empty())
return -1;
if (!test_known_hosts_file())
return -1;
if (!test_certs_dir(FALSE))

View File

@ -1334,6 +1334,8 @@ int tls_verify_certificate(rdpTls* tls, CryptoCert cert, const char* hostname, U
x509_verify_certificate(cert, certificate_store_get_certs_path(tls->certificate_store));
/* verify certificate name match */
certificate_data = crypto_get_certificate_data(cert->px509, hostname, port);
if (!certificate_data)
goto end;
/* extra common name and alternative names */
common_name = crypto_cert_subject_common_name(cert->px509, &common_name_length);
dns_names = crypto_cert_get_dns_names(cert->px509, &dns_names_count, &dns_names_lengths);