From dc9820bcf8ce6ff4f8da9bdcd0ff353b704be44f Mon Sep 17 00:00:00 2001 From: tanjiro Kamado Date: Mon, 26 Oct 2026 19:06:22 +0530 Subject: [PATCH 1/2] create credential private key files with mode 0600 --- CHANGES.md | 4 ++++ cups/file.c | 2 +- cups/testcreds.c | 7 +++++++ cups/tls-gnutls.c | 4 ++-- cups/tls-openssl.c | 26 ++++++++++++++++++++++++-- cups/tls.c | 2 +- 6 files changed, 39 insertions(+), 6 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 00e3340fa..5c3fc1735 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -50,6 +50,10 @@ v3.0.4 - YYYY-MM-DD - Fixed a memory leak in `httpGetSecurity` when using GNU TLS. - Fixed a memory leak in `cupsJWTImportString`. - Fixed a bug in `cupsJWTExportString` with JSON output. +- Fixed private key files written by `cupsCreateCredentials`, + `cupsCreateCredentialsRequest`, and `cupsSaveCredentials` being readable by + other users. +- Fixed the "m###" permissions suffix in `cupsFileOpen`. v3.0.3 - 2026-08-20 diff --git a/cups/file.c b/cups/file.c index 8ec4bbb04..1fc1b9b2f 100644 --- a/cups/file.c +++ b/cups/file.c @@ -642,7 +642,7 @@ cupsFileOpen(const char *filename, // I - Name of file if ((ptr = strchr(mode, 'm')) != NULL && ptr[1] >= '0' && ptr[1] <= '7') { // Get permissions from mode string... - perm = (int)strtol(mode + 1, NULL, 8); + perm = (int)strtol(ptr + 1, NULL, 8); } // Open the file... diff --git a/cups/testcreds.c b/cups/testcreds.c index 176d3aeeb..065251001 100644 --- a/cups/testcreds.c +++ b/cups/testcreds.c @@ -409,6 +409,7 @@ do_unit_tests(void) *chain; // Cert chain data size_t chainlen; // Length of cert chain data http_trust_t trust; // Trust in credentials + struct stat fileinfo; // Key file information static const char * const alt_names[] = { // subjectAltName values "printer.example.com", @@ -442,6 +443,9 @@ do_unit_tests(void) data = cupsCopyCredentialsKey(TEST_CERT_PATH, "_site_"); testEnd(data != NULL); free(data); + + testBegin("_site_.key permissions"); + testEnd(!stat(TEST_CERT_PATH "/_site_.key", &fileinfo) && !(fileinfo.st_mode & 077)); } else { @@ -520,6 +524,9 @@ do_unit_tests(void) testEnd(data != NULL); free(data); + testBegin("altprinter.ktm permissions"); + testEnd(!stat(TEST_CERT_PATH "/altprinter.ktm", &fileinfo) && !(fileinfo.st_mode & 077)); + testBegin("cupsCopyCredentialsRequest(altprinter w/alt names)"); data = cupsCopyCredentialsRequest(TEST_CERT_PATH, "altprinter"); testEnd(data != NULL); diff --git a/cups/tls-gnutls.c b/cups/tls-gnutls.c index 3939068f2..7c4e74f58 100644 --- a/cups/tls-gnutls.c +++ b/cups/tls-gnutls.c @@ -231,7 +231,7 @@ cupsCreateCredentials( _cupsSetError(IPP_STATUS_ERROR_INTERNAL, gnutls_strerror(err), 0); goto done; } - else if ((fp = cupsFileOpen(keyfile, "w")) != NULL) + else if ((fp = cupsFileOpen(keyfile, "wm600")) != NULL) { DEBUG_printf("1cupsCreateCredentials: Writing private key to \"%s\".", keyfile); cupsFileWrite(fp, (char *)buffer, bytes); @@ -586,7 +586,7 @@ cupsCreateCredentialsRequest( DEBUG_printf("1cupsCreateCredentialsRequest: Unable to export private key: %s", gnutls_strerror(err)); goto done; } - else if ((fp = cupsFileOpen(keyfile, "w")) != NULL) + else if ((fp = cupsFileOpen(keyfile, "wm600")) != NULL) { DEBUG_printf("1cupsCreateCredentialsRequest: Writing private key to \"%s\".", keyfile); cupsFileWrite(fp, (char *)buffer, bytes); diff --git a/cups/tls-openssl.c b/cups/tls-openssl.c index fa415e484..327936992 100644 --- a/cups/tls-openssl.c +++ b/cups/tls-openssl.c @@ -35,6 +35,7 @@ static X509_EXTENSION *openssl_create_san(const char *common_name, size_t num_al static time_t openssl_get_date(X509 *cert, int which); //static void openssl_load_crl(void); static STACK_OF(X509 *) openssl_load_x509(const char *credentials); +static BIO *openssl_open_key(const char *keyfile); static bool openssl_validate_name(X509 *cert, const char *common_name); @@ -370,7 +371,7 @@ cupsCreateCredentials( http_make_path(keyfile, sizeof(keyfile), path, common_name, "key"); http_make_path(pubfile, sizeof(pubfile), path, common_name, "pub"); - if ((bio = BIO_new_file(keyfile, "wb")) == NULL) + if ((bio = openssl_open_key(keyfile)) == NULL) { _cupsSetError(IPP_STATUS_ERROR_INTERNAL, strerror(errno), 0); goto done; @@ -606,7 +607,7 @@ cupsCreateCredentialsRequest( sk_X509_EXTENSION_pop_free(exts, X509_EXTENSION_free); // Save them... - if ((bio = BIO_new_file(keyfile, "wb")) == NULL) + if ((bio = openssl_open_key(keyfile)) == NULL) { _cupsSetError(IPP_STATUS_ERROR_INTERNAL, strerror(errno), 0); goto done; @@ -2609,6 +2610,27 @@ openssl_load_x509( } +// +// 'openssl_open_key()' - Create a private key file that only the owner can read. +// + +static BIO * // O - Key file BIO or `NULL` on error +openssl_open_key(const char *keyfile) // I - Key filename +{ + int fd; // File descriptor + BIO *bio; // Key file BIO + + + if ((fd = open(keyfile, O_CREAT | O_WRONLY | O_TRUNC, 0600)) < 0) + return (NULL); + + if ((bio = BIO_new_fd(fd, BIO_CLOSE)) == NULL) + close(fd); + + return (bio); +} + + // // 'openssl_validate_name()' - Check that the certificate is valid for the given name. // diff --git a/cups/tls.c b/cups/tls.c index eeacb653b..79753cd8d 100644 --- a/cups/tls.c +++ b/cups/tls.c @@ -811,7 +811,7 @@ http_save_file(const char *path, // I - Directory path for certificate/key store return (true); } - if ((fd = open(filename, O_CREAT | O_WRONLY | O_TRUNC, 0644)) < 0) + if ((fd = open(filename, O_CREAT | O_WRONLY | O_TRUNC, strcmp(ext, "key") ? 0644 : 0600)) < 0) return (false); if (write(fd, value, strlen(value)) < 0) From 89be684d87220393aac90ef01cc2d63cbc3076dd Mon Sep 17 00:00:00 2001 From: tanjiro Kamado Date: Tue, 27 Oct 2026 12:53:46 +0530 Subject: [PATCH 2/2] address review: openssl_open_file with mode, handle .ktm, split test checks, drop CHANGES.md --- CHANGES.md | 4 ---- cups/testcreds.c | 14 ++++++++++++-- cups/tls-openssl.c | 27 ++++++++++++++------------- cups/tls.c | 2 +- 4 files changed, 27 insertions(+), 20 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 5c3fc1735..00e3340fa 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -50,10 +50,6 @@ v3.0.4 - YYYY-MM-DD - Fixed a memory leak in `httpGetSecurity` when using GNU TLS. - Fixed a memory leak in `cupsJWTImportString`. - Fixed a bug in `cupsJWTExportString` with JSON output. -- Fixed private key files written by `cupsCreateCredentials`, - `cupsCreateCredentialsRequest`, and `cupsSaveCredentials` being readable by - other users. -- Fixed the "m###" permissions suffix in `cupsFileOpen`. v3.0.3 - 2026-08-20 diff --git a/cups/testcreds.c b/cups/testcreds.c index 065251001..8f7a5d161 100644 --- a/cups/testcreds.c +++ b/cups/testcreds.c @@ -445,7 +445,12 @@ do_unit_tests(void) free(data); testBegin("_site_.key permissions"); - testEnd(!stat(TEST_CERT_PATH "/_site_.key", &fileinfo) && !(fileinfo.st_mode & 077)); + if (stat(TEST_CERT_PATH "/_site_.key", &fileinfo)) + testEndMessage(false, "%s", strerror(errno)); + else if (fileinfo.st_mode & 077) + testEndMessage(false, "wrong permissions, got %03o expected %03o.", (unsigned)(fileinfo.st_mode & 0777), (unsigned)(fileinfo.st_mode & 0700)); + else + testEnd(true); } else { @@ -525,7 +530,12 @@ do_unit_tests(void) free(data); testBegin("altprinter.ktm permissions"); - testEnd(!stat(TEST_CERT_PATH "/altprinter.ktm", &fileinfo) && !(fileinfo.st_mode & 077)); + if (stat(TEST_CERT_PATH "/altprinter.ktm", &fileinfo)) + testEndMessage(false, "%s", strerror(errno)); + else if (fileinfo.st_mode & 077) + testEndMessage(false, "wrong permissions, got %03o expected %03o.", (unsigned)(fileinfo.st_mode & 0777), (unsigned)(fileinfo.st_mode & 0700)); + else + testEnd(true); testBegin("cupsCopyCredentialsRequest(altprinter w/alt names)"); data = cupsCopyCredentialsRequest(TEST_CERT_PATH, "altprinter"); diff --git a/cups/tls-openssl.c b/cups/tls-openssl.c index 327936992..ee079de6a 100644 --- a/cups/tls-openssl.c +++ b/cups/tls-openssl.c @@ -35,7 +35,7 @@ static X509_EXTENSION *openssl_create_san(const char *common_name, size_t num_al static time_t openssl_get_date(X509 *cert, int which); //static void openssl_load_crl(void); static STACK_OF(X509 *) openssl_load_x509(const char *credentials); -static BIO *openssl_open_key(const char *keyfile); +static BIO *openssl_open_file(const char *filename, int mode); static bool openssl_validate_name(X509 *cert, const char *common_name); @@ -371,7 +371,7 @@ cupsCreateCredentials( http_make_path(keyfile, sizeof(keyfile), path, common_name, "key"); http_make_path(pubfile, sizeof(pubfile), path, common_name, "pub"); - if ((bio = openssl_open_key(keyfile)) == NULL) + if ((bio = openssl_open_file(keyfile, 0600)) == NULL) { _cupsSetError(IPP_STATUS_ERROR_INTERNAL, strerror(errno), 0); goto done; @@ -386,7 +386,7 @@ cupsCreateCredentials( BIO_free(bio); - if ((bio = BIO_new_file(pubfile, "wb")) == NULL) + if ((bio = openssl_open_file(pubfile, 0644)) == NULL) { _cupsSetError(IPP_STATUS_ERROR_INTERNAL, strerror(errno), 0); goto done; @@ -401,7 +401,7 @@ cupsCreateCredentials( BIO_free(bio); - if ((bio = BIO_new_file(crtfile, "wb")) == NULL) + if ((bio = openssl_open_file(crtfile, 0644)) == NULL) { _cupsSetError(IPP_STATUS_ERROR_INTERNAL, strerror(errno), 0); goto done; @@ -607,7 +607,7 @@ cupsCreateCredentialsRequest( sk_X509_EXTENSION_pop_free(exts, X509_EXTENSION_free); // Save them... - if ((bio = openssl_open_key(keyfile)) == NULL) + if ((bio = openssl_open_file(keyfile, 0600)) == NULL) { _cupsSetError(IPP_STATUS_ERROR_INTERNAL, strerror(errno), 0); goto done; @@ -622,7 +622,7 @@ cupsCreateCredentialsRequest( BIO_free(bio); - if ((bio = BIO_new_file(pubfile, "wb")) == NULL) + if ((bio = openssl_open_file(pubfile, 0644)) == NULL) { _cupsSetError(IPP_STATUS_ERROR_INTERNAL, strerror(errno), 0); goto done; @@ -637,7 +637,7 @@ cupsCreateCredentialsRequest( BIO_free(bio); - if ((bio = BIO_new_file(csrfile, "wb")) == NULL) + if ((bio = openssl_open_file(csrfile, 0644)) == NULL) { _cupsSetError(IPP_STATUS_ERROR_INTERNAL, strerror(errno), 0); goto done; @@ -1412,7 +1412,7 @@ cupsSignCredentialsRequest( // Save the certificate... http_make_path(crtfile, sizeof(crtfile), path, common_name, "crt"); - if ((bio = BIO_new_file(crtfile, "wb")) == NULL) + if ((bio = openssl_open_file(crtfile, 0644)) == NULL) { _cupsSetError(IPP_STATUS_ERROR_INTERNAL, strerror(errno), 0); goto done; @@ -2611,17 +2611,18 @@ openssl_load_x509( // -// 'openssl_open_key()' - Create a private key file that only the owner can read. +// 'openssl_open_file()' - Create a file for writing with the given permissions. // -static BIO * // O - Key file BIO or `NULL` on error -openssl_open_key(const char *keyfile) // I - Key filename +static BIO * // O - File BIO or `NULL` on error +openssl_open_file(const char *filename, // I - Filename + int mode) // I - Permissions { int fd; // File descriptor - BIO *bio; // Key file BIO + BIO *bio; // File BIO - if ((fd = open(keyfile, O_CREAT | O_WRONLY | O_TRUNC, 0600)) < 0) + if ((fd = open(filename, O_CREAT | O_WRONLY | O_TRUNC, mode)) < 0) return (NULL); if ((bio = BIO_new_fd(fd, BIO_CLOSE)) == NULL) diff --git a/cups/tls.c b/cups/tls.c index 79753cd8d..75ee9f9f4 100644 --- a/cups/tls.c +++ b/cups/tls.c @@ -811,7 +811,7 @@ http_save_file(const char *path, // I - Directory path for certificate/key store return (true); } - if ((fd = open(filename, O_CREAT | O_WRONLY | O_TRUNC, strcmp(ext, "key") ? 0644 : 0600)) < 0) + if ((fd = open(filename, O_CREAT | O_WRONLY | O_TRUNC, (strcmp(ext, "key") && strcmp(ext, "ktm")) ? 0644 : 0600)) < 0) return (false); if (write(fd, value, strlen(value)) < 0)