From 23a7c82e56668709a3127730f95a83c72fcfdc2c Mon Sep 17 00:00:00 2001 From: Michael R Sweet Date: Fri, 9 Sep 2022 19:25:43 -0400 Subject: [PATCH] Fix certificate generation bugs that affect Chrome compatibility, among other things. --- CHANGES.md | 1 + cups/tls-darwin.c | 5 ++ cups/tls-gnutls.c | 83 ++++++------------- cups/tls-openssl.c | 119 ++++++++++++++++++++------- xcode/CUPS.xcodeproj/project.pbxproj | 6 +- 5 files changed, 123 insertions(+), 91 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 81aef4e680..4c06bbf9bf 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,6 +13,7 @@ Changes in CUPS v2.4.3 (TBA) - Fixed an OpenSSL crash bug (Issue #409) - Fixed a potential SNMP OID value overflow issue (Issue #431) - Fixed an OpenSSL certificate loading issue (Issue #465) +- Fixed TLS certificate generation bugs. - Look for default printer on network if needed (Issue ##452) - Now localize HTTP responses using the Content-Language value (Issue #426) - Raised file size limit for importing PPD via Web UI (Issue #433) diff --git a/cups/tls-darwin.c b/cups/tls-darwin.c index 23376dae8a..dfc6ea51b8 100644 --- a/cups/tls-darwin.c +++ b/cups/tls-darwin.c @@ -1522,6 +1522,8 @@ _httpTLSStart(http_t *http) /* I - HTTP connection */ if (isdigit(hostname[0] & 255) || hostname[0] == '[') hostname[0] = '\0'; /* Don't allow numeric addresses */ + _cupsMutexLock(&tls_mutex); + if (hostname[0]) http->tls_credentials = http_cdsa_copy_server(hostname); else if (tls_common_name) @@ -1537,6 +1539,7 @@ _httpTLSStart(http_t *http) /* I - HTTP connection */ http->error = errno = EINVAL; http->status = HTTP_STATUS_ERROR; _cupsSetError(IPP_STATUS_ERROR_INTERNAL, _("Unable to create server credentials."), 1); + _cupsMutexUnlock(&tls_mutex); return (-1); } @@ -1544,6 +1547,8 @@ _httpTLSStart(http_t *http) /* I - HTTP connection */ http->tls_credentials = http_cdsa_copy_server(hostname[0] ? hostname : tls_common_name); } + _cupsMutexUnlock(&tls_mutex); + if (!http->tls_credentials) { DEBUG_puts("4_httpTLSStart: Unable to find server credentials."); diff --git a/cups/tls-gnutls.c b/cups/tls-gnutls.c index f87b4f4df1..24f968d88f 100644 --- a/cups/tls-gnutls.c +++ b/cups/tls-gnutls.c @@ -167,7 +167,7 @@ cupsMakeServerCredentials( gnutls_x509_crt_set_key(crt, key); gnutls_x509_crt_set_serial(crt, serial, sizeof(serial)); gnutls_x509_crt_set_activation_time(crt, curtime); - gnutls_x509_crt_set_expiration_time(crt, curtime + 10 * 365 * 86400); + gnutls_x509_crt_set_expiration_time(crt, expiration_date); gnutls_x509_crt_set_ca_status(crt, 0); gnutls_x509_crt_set_subject_alt_name(crt, GNUTLS_SAN_DNSNAME, common_name, (unsigned)strlen(common_name), GNUTLS_FSAN_SET); if (!strchr(common_name, '.')) @@ -1368,6 +1368,8 @@ _httpTLSStart(http_t *http) /* I - Connection to server */ char crtfile[1024], /* Certificate file */ keyfile[1024]; /* Private key file */ + const char *cn, // Common name to lookup + *cnptr; // Pointer into common name int have_creds = 0; /* Have credentials? */ if (http->fields[HTTP_FIELD_HOST]) @@ -1405,61 +1407,21 @@ _httpTLSStart(http_t *http) /* I - Connection to server */ if (isdigit(hostname[0] & 255) || hostname[0] == '[') hostname[0] = '\0'; /* Don't allow numeric addresses */ - if (hostname[0]) - { - /* - * First look in the CUPS keystore... - */ - - http_gnutls_make_path(crtfile, sizeof(crtfile), tls_keypath, hostname, "crt"); - http_gnutls_make_path(keyfile, sizeof(keyfile), tls_keypath, hostname, "key"); - - if (access(crtfile, R_OK) || access(keyfile, R_OK)) - { - /* - * No CUPS-managed certs, look for CA certs... - */ - - char cacrtfile[1024], cakeyfile[1024]; /* CA cert files */ - - snprintf(cacrtfile, sizeof(cacrtfile), "/etc/letsencrypt/live/%s/fullchain.pem", hostname); - snprintf(cakeyfile, sizeof(cakeyfile), "/etc/letsencrypt/live/%s/privkey.pem", hostname); - - if ((access(cacrtfile, R_OK) || access(cakeyfile, R_OK)) && (hostptr = strchr(hostname, '.')) != NULL) - { - /* - * Try just domain name... - */ + _cupsMutexLock(&tls_mutex); - hostptr ++; - if (strchr(hostptr, '.')) - { - snprintf(cacrtfile, sizeof(cacrtfile), "/etc/letsencrypt/live/%s/fullchain.pem", hostptr); - snprintf(cakeyfile, sizeof(cakeyfile), "/etc/letsencrypt/live/%s/privkey.pem", hostptr); - } - } - - if (!access(cacrtfile, R_OK) && !access(cakeyfile, R_OK)) - { - /* - * Use the CA certs... - */ - - strlcpy(crtfile, cacrtfile, sizeof(crtfile)); - strlcpy(keyfile, cakeyfile, sizeof(keyfile)); - } - } + if (hostname[0]) + cn = hostname; + else + cn = tls_common_name; - have_creds = !access(crtfile, R_OK) && !access(keyfile, R_OK); - } - else if (tls_common_name) + if (cn) { /* * First look in the CUPS keystore... */ - http_gnutls_make_path(crtfile, sizeof(crtfile), tls_keypath, tls_common_name, "crt"); - http_gnutls_make_path(keyfile, sizeof(keyfile), tls_keypath, tls_common_name, "key"); + http_gnutls_make_path(crtfile, sizeof(crtfile), tls_keypath, cn, "crt"); + http_gnutls_make_path(keyfile, sizeof(keyfile), tls_keypath, cn, "key"); if (access(crtfile, R_OK) || access(keyfile, R_OK)) { @@ -1469,20 +1431,20 @@ _httpTLSStart(http_t *http) /* I - Connection to server */ char cacrtfile[1024], cakeyfile[1024]; /* CA cert files */ - snprintf(cacrtfile, sizeof(cacrtfile), "/etc/letsencrypt/live/%s/fullchain.pem", tls_common_name); - snprintf(cakeyfile, sizeof(cakeyfile), "/etc/letsencrypt/live/%s/privkey.pem", tls_common_name); + snprintf(cacrtfile, sizeof(cacrtfile), "/etc/letsencrypt/live/%s/fullchain.pem", cn); + snprintf(cakeyfile, sizeof(cakeyfile), "/etc/letsencrypt/live/%s/privkey.pem", cn); - if ((access(cacrtfile, R_OK) || access(cakeyfile, R_OK)) && (hostptr = strchr(tls_common_name, '.')) != NULL) + if ((access(cacrtfile, R_OK) || access(cakeyfile, R_OK)) && (cnptr = strchr(cn, '.')) != NULL) { /* * Try just domain name... */ - hostptr ++; - if (strchr(hostptr, '.')) + cnptr ++; + if (strchr(cnptr, '.')) { - snprintf(cacrtfile, sizeof(cacrtfile), "/etc/letsencrypt/live/%s/fullchain.pem", hostptr); - snprintf(cakeyfile, sizeof(cakeyfile), "/etc/letsencrypt/live/%s/privkey.pem", hostptr); + snprintf(cacrtfile, sizeof(cacrtfile), "/etc/letsencrypt/live/%s/fullchain.pem", cnptr); + snprintf(cakeyfile, sizeof(cakeyfile), "/etc/letsencrypt/live/%s/privkey.pem", cnptr); } } @@ -1500,16 +1462,17 @@ _httpTLSStart(http_t *http) /* I - Connection to server */ have_creds = !access(crtfile, R_OK) && !access(keyfile, R_OK); } - if (!have_creds && tls_auto_create && (hostname[0] || tls_common_name)) + if (!have_creds && tls_auto_create && cn) { - DEBUG_printf(("4_httpTLSStart: Auto-create credentials for \"%s\".", hostname[0] ? hostname : tls_common_name)); + DEBUG_printf(("4_httpTLSStart: Auto-create credentials for \"%s\".", cn)); - if (!cupsMakeServerCredentials(tls_keypath, hostname[0] ? hostname : tls_common_name, 0, NULL, time(NULL) + 365 * 86400)) + if (!cupsMakeServerCredentials(tls_keypath, cn, 0, NULL, time(NULL) + 3650 * 86400)) { DEBUG_puts("4_httpTLSStart: cupsMakeServerCredentials failed."); http->error = errno = EINVAL; http->status = HTTP_STATUS_ERROR; _cupsSetError(IPP_STATUS_ERROR_INTERNAL, _("Unable to create server credentials."), 1); + _cupsMutexUnlock(&tls_mutex); return (-1); } @@ -1517,6 +1480,8 @@ _httpTLSStart(http_t *http) /* I - Connection to server */ DEBUG_printf(("4_httpTLSStart: Using certificate \"%s\" and private key \"%s\".", crtfile, keyfile)); + _cupsMutexUnlock(&tls_mutex); + status = gnutls_certificate_set_x509_key_file(*credentials, crtfile, keyfile, GNUTLS_X509_FMT_PEM); } diff --git a/cups/tls-openssl.c b/cups/tls-openssl.c index acc10fc420..21a2535f45 100644 --- a/cups/tls-openssl.c +++ b/cups/tls-openssl.c @@ -35,6 +35,7 @@ static const char *http_default_path(char *buffer, size_t bufsize); static time_t http_get_date(X509 *cert, int which); //static void http_load_crl(void); static const char *http_make_path(char *buffer, size_t bufsize, const char *dirname, const char *filename, const char *ext); +static int http_x509_add_ext(X509 *cert, int nid, const char *value); static void http_x509_add_san(X509 *cert, const char *name); @@ -79,6 +80,9 @@ cupsMakeServerCredentials( cups_lang_t *language; // Default language info time_t curtime; // Current time X509_NAME *name; // Subject/issuer name + ASN1_INTEGER *serial; // Serial number + ASN1_TIME *notBefore, // Initial date + *notAfter; // Expiration date BIO *bio; // Output file char temp[1024], // Temporary directory name crtfile[1024], // Certificate filename @@ -104,7 +108,7 @@ cupsMakeServerCredentials( // Create the encryption key... DEBUG_puts("1cupsMakeServerCredentials: Creating key pair."); - if ((rsa = RSA_generate_key(2048, RSA_F4, NULL, NULL)) == NULL) + if ((rsa = RSA_generate_key(3072, RSA_F4, NULL, NULL)) == NULL) { _cupsSetError(IPP_STATUS_ERROR_INTERNAL, _("Unable to create key pair."), 1); return (0); @@ -131,23 +135,40 @@ cupsMakeServerCredentials( return (0); } - curtime = time(NULL); - language = cupsLangDefault(); + curtime = time(NULL); + + notBefore = ASN1_TIME_new(); + ASN1_TIME_set(notBefore, curtime); + X509_set_notBefore(cert, notBefore); + ASN1_TIME_free(notBefore); + + notAfter = ASN1_TIME_new(); + ASN1_TIME_set(notAfter, expiration_date); + X509_set_notAfter(cert, notAfter); + ASN1_TIME_free(notAfter); + + serial = ASN1_INTEGER_new(); + ASN1_INTEGER_set(serial, (int)curtime); + X509_set_serialNumber(cert, serial); + ASN1_INTEGER_free(serial); - ASN1_TIME_set(X509_get_notBefore(cert), curtime); - ASN1_TIME_set(X509_get_notAfter(cert), expiration_date); - ASN1_INTEGER_set(X509_get_serialNumber(cert), (int)curtime); X509_set_pubkey(cert, pkey); - name = X509_get_subject_name(cert); + language = cupsLangDefault(); + name = X509_NAME_new(); if (strlen(language->language) == 5) - X509_NAME_add_entry_by_txt(name, "C", MBSTRING_ASC, (unsigned char *)language->language + 3, -1, -1, 0); + X509_NAME_add_entry_by_txt(name, SN_countryName, MBSTRING_ASC, (unsigned char *)language->language + 3, -1, -1, 0); else - X509_NAME_add_entry_by_txt(name, "C", MBSTRING_ASC, (unsigned char *)"US", -1, -1, 0); - X509_NAME_add_entry_by_txt(name, "O", MBSTRING_ASC, (unsigned char *)"Unknown", -1, -1, 0); - X509_NAME_add_entry_by_txt(name, "CN", MBSTRING_ASC, (unsigned char *)common_name, -1, -1, 0); + X509_NAME_add_entry_by_txt(name, SN_countryName, MBSTRING_ASC, (unsigned char *)"US", -1, -1, 0); + X509_NAME_add_entry_by_txt(name, SN_commonName, MBSTRING_ASC, (unsigned char *)common_name, -1, -1, 0); + X509_NAME_add_entry_by_txt(name, SN_organizationName, MBSTRING_ASC, (unsigned char *)common_name, -1, -1, 0); + X509_NAME_add_entry_by_txt(name, SN_organizationalUnitName, MBSTRING_ASC, (unsigned char *)"Unknown", -1, -1, 0); + X509_NAME_add_entry_by_txt(name, SN_stateOrProvinceName, MBSTRING_ASC, (unsigned char *)"Unknown", -1, -1, 0); + X509_NAME_add_entry_by_txt(name, SN_localityName, MBSTRING_ASC, (unsigned char *)"Unknown", -1, -1, 0); X509_set_issuer_name(cert, name); + X509_set_subject_name(cert, name); + X509_NAME_free(name); http_x509_add_san(cert, common_name); if ((common_ptr = strstr(common_name, ".local")) == NULL) @@ -175,6 +196,14 @@ cupsMakeServerCredentials( } } + // Add extensions that are required to make Chrome happy... + http_x509_add_ext(cert, NID_basic_constraints, "critical,CA:FALSE,pathlen:0"); + http_x509_add_ext(cert, NID_key_usage, "critical,digitalSignature,keyEncipherment"); + http_x509_add_ext(cert, NID_ext_key_usage, "1.3.6.1.5.5.7.3.1"); + http_x509_add_ext(cert, NID_subject_key_identifier, "hash"); + http_x509_add_ext(cert, NID_authority_key_identifier, "keyid,issuer"); + X509_set_version(cert, 2); // v3 + X509_sign(cert, pkey, EVP_sha256()); // Save them... @@ -1002,6 +1031,8 @@ _httpTLSStart(http_t *http) // I - Connection to server else cn = tls_common_name; + _cupsMutexLock(&tls_mutex); + if (cn) { // First look in the CUPS keystore... @@ -1042,18 +1073,21 @@ _httpTLSStart(http_t *http) // I - Connection to server { DEBUG_printf(("4_httpTLSStart: Auto-create credentials for \"%s\".", cn)); - if (!cupsMakeServerCredentials(tls_keypath, cn, 0, NULL, time(NULL) + 365 * 86400)) + if (!cupsMakeServerCredentials(tls_keypath, cn, 0, NULL, time(NULL) + 3650 * 86400)) { DEBUG_puts("4_httpTLSStart: cupsMakeServerCredentials failed."); http->error = errno = EINVAL; http->status = HTTP_STATUS_ERROR; _cupsSetError(IPP_STATUS_ERROR_INTERNAL, _("Unable to create server credentials."), 1); - SSL_CTX_free(context); + SSL_CTX_free(context); + _cupsMutexUnlock(&tls_mutex); return (-1); } } + _cupsMutexUnlock(&tls_mutex); + SSL_CTX_use_PrivateKey_file(context, keyfile, SSL_FILETYPE_PEM); SSL_CTX_use_certificate_chain_file(context, crtfile); } @@ -1565,6 +1599,46 @@ http_make_path( } +// +// 'http_x509_add_ext()' - Add an extension to a certificate. +// + +static int // O - 1 on success, 0 on failure +http_x509_add_ext(X509 *cert, // I - Certificate + int nid, // I - Extension ID + const char *value) // I - Value +{ + int ret; // Return value + X509_EXTENSION *ex = NULL; // Extension + X509V3_CTX ctx; // Certificate context + + + DEBUG_printf(("3http_x509_add_ext(cert=%p, nid=%d, value=\"%s\")", (void *)cert, nid, value)); + + // Don't use a configuration database... + X509V3_set_ctx_nodb(&ctx); + + // Self-signed certificates use the same issuer and subject... + X509V3_set_ctx(&ctx, /*issuer*/cert, /*subject*/cert, /*req*/NULL, /*crl*/NULL, /*flags*/0); + + // Create and add the extension... + if ((ex = X509V3_EXT_conf_nid(/*conf*/NULL, &ctx, nid, value)) == NULL) + { + DEBUG_puts("4http_x509_add_ext: Unable to create extension, returning false."); + return (0); + } + + ret = X509_add_ext(cert, ex, -1) != 0; + + DEBUG_printf(("4http_x509_add_ext: X509_add_ext returned %s.", ret ? "true" : "false")); + + // Free the extension and return... + X509_EXTENSION_free(ex); + + return (ret); +} + + // // 'http_x509_add_san()' - Add a subjectAltName extension to an X.509 certificate. // @@ -1574,24 +1648,9 @@ http_x509_add_san(X509 *cert, // I - Certificate const char *name) // I - Hostname { char dns_name[1024]; // DNS: prefixed hostname - X509_EXTENSION *san_ext; // Extension for subjectAltName - ASN1_OCTET_STRING *san_asn1; // ASN1 string // The subjectAltName value for DNS names starts with a DNS: prefix... - snprintf(dns_name, sizeof(dns_name), "DNS: %s", name); - - if ((san_asn1 = ASN1_OCTET_STRING_new()) == NULL) - return; - - ASN1_OCTET_STRING_set(san_asn1, (unsigned char *)dns_name, strlen(dns_name)); - if ((san_ext = X509_EXTENSION_create_by_NID(NULL, NID_subject_alt_name, 0, san_asn1)) == NULL) - { - ASN1_OCTET_STRING_free(san_asn1); - return; - } - - X509_add_ext(cert, san_ext, -1); - X509_EXTENSION_free(san_ext); - ASN1_OCTET_STRING_free(san_asn1); + snprintf(dns_name, sizeof(dns_name), "DNS:%s", name); + http_x509_add_ext(cert, NID_subject_alt_name, dns_name); } diff --git a/xcode/CUPS.xcodeproj/project.pbxproj b/xcode/CUPS.xcodeproj/project.pbxproj index f4311664ae..8440b69733 100644 --- a/xcode/CUPS.xcodeproj/project.pbxproj +++ b/xcode/CUPS.xcodeproj/project.pbxproj @@ -3309,10 +3309,11 @@ 278C58E8136B64B000836530 /* SystemConfiguration.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = SystemConfiguration.framework; path = /System/Library/Frameworks/SystemConfiguration.framework; sourceTree = ""; }; 279AE6F42395B80F004DD600 /* libpam.tbd */ = {isa = PBXFileReference; lastKnownFileType = "sourcecode.text-based-dylib-definition"; name = libpam.tbd; path = usr/lib/libpam.tbd; sourceTree = SDKROOT; }; 27A0347B1A8BDB1300650675 /* lpadmin */ = {isa = PBXFileReference; explicitFileType = "compiled.mach-o.executable"; includeInIndex = 0; path = lpadmin; sourceTree = BUILT_PRODUCTS_DIR; }; - 27C89C8F2613E7C300A58F43 /* cups-snap.m4 */ = {isa = PBXFileReference; lastKnownFileType = text; name = "cups-snap.m4"; path = "../config-scripts/cups-snap.m4"; sourceTree = ""; }; 27C89C902613E7C300A58F43 /* cups-tls.m4 */ = {isa = PBXFileReference; lastKnownFileType = text; name = "cups-tls.m4"; path = "../config-scripts/cups-tls.m4"; sourceTree = ""; }; 27D3037D134148CB00F022B1 /* libcups2.def */ = {isa = PBXFileReference; lastKnownFileType = text; name = libcups2.def; path = ../cups/libcups2.def; sourceTree = ""; }; 27F89DA21B3AC43B00E5A4B7 /* testraster.c */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.c; name = testraster.c; path = ../cups/testraster.c; sourceTree = ""; }; + 27F9A76D28CBFC03002CCEE0 /* tls-openssl.c */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.c; name = "tls-openssl.c"; path = "../cups/tls-openssl.c"; sourceTree = ""; }; + 27F9A76E28CBFC03002CCEE0 /* tls-darwin.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = "tls-darwin.h"; path = "../cups/tls-darwin.h"; sourceTree = ""; }; 720DD6C21358FD5F0064AA82 /* snmp */ = {isa = PBXFileReference; explicitFileType = "compiled.mach-o.executable"; includeInIndex = 0; path = snmp; sourceTree = BUILT_PRODUCTS_DIR; }; 720DD6D21358FDDE0064AA82 /* snmp.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; name = snmp.c; path = ../backend/snmp.c; sourceTree = ""; }; 720E854120164E7A00C6C411 /* ipp-file.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; name = "ipp-file.c"; path = "../cups/ipp-file.c"; sourceTree = ""; }; @@ -4996,8 +4997,10 @@ 72220F02133305BB00FCA411 /* string.c */, 72220F03133305BB00FCA411 /* tempfile.c */, 72220F05133305BB00FCA411 /* thread.c */, + 27F9A76E28CBFC03002CCEE0 /* tls-darwin.h */, 270B267D17F5C06700C8A3A9 /* tls-darwin.c */, 270B267E17F5C06700C8A3A9 /* tls-gnutls.c */, + 27F9A76D28CBFC03002CCEE0 /* tls-openssl.c */, 270B268117F5C5D600C8A3A9 /* tls-sspi.c */, 727AD5B619100A58009F6862 /* tls.c */, 72220F06133305BB00FCA411 /* transcode.c */, @@ -5273,7 +5276,6 @@ 72E65BB018DC799B00097E89 /* cups-pam.m4 */, 72E65BB118DC799B00097E89 /* cups-poll.m4 */, 72E65BB318DC799B00097E89 /* cups-sharedlibs.m4 */, - 27C89C8F2613E7C300A58F43 /* cups-snap.m4 */, 72E65BB518DC799B00097E89 /* cups-threads.m4 */, 27C89C902613E7C300A58F43 /* cups-tls.m4 */, ); -- 2.47.2