Skip to content

Commit

Permalink
Fix crash in dirmngr when system certificates are loaded after fork().
Browse files Browse the repository at this point in the history
GnuPG adds the system certificates for each gnutls connection when the
connection is configured. To read the system certificates, gnutls uses
Foundation methods. The use of Foundation methods however might lead to
unexpected behavior or worst a crash:
http://www.sealiesoftware.com/blog/archive/2017/6/5/Objective-C_and_fork_in_macOS_1013.html

To avoid this scenario, the certificates are read and cached before
fork() is called, and added to gnutls from the cache.

[#752]
  • Loading branch information
lukele committed Jan 25, 2021
1 parent 89226a5 commit 0bce831
Show file tree
Hide file tree
Showing 2 changed files with 186 additions and 0 deletions.
1 change: 1 addition & 0 deletions create_gpg.sh
Expand Up @@ -164,6 +164,7 @@ function customize_build_for_gnupg {
build_cflags="${build_cflags} -I${arch_dist_dir}/include -I${arch_dist_dir}/include/libusb-1.0/"
build_cxxflags="${build_cflags}"
build_cppflags="${build_cflags}"
build_ldflags="${build_ldflags} -framework Foundation -framework Security"
cache_file="${WORKING_DIR}/config.${dest_arch}.gnupg.cache"

configure_args="$configure_args \
Expand Down
185 changes: 185 additions & 0 deletions patches/gnupg/hkp-add-system-certificate-authorities.patch
@@ -0,0 +1,185 @@
diff --git a/dirmngr/dirmngr.c b/dirmngr/dirmngr.c
index ae967dd..53a6d91 100644
--- a/dirmngr/dirmngr.c
+++ b/dirmngr/dirmngr.c
@@ -1300,6 +1300,20 @@ main (int argc, char **argv)
es_printf ("set %s=%s;%lu;1\n",
DIRMNGR_INFO_NAME, socket_name, (ulong) pid);
#else
+ #ifdef __APPLE__
+ /* Load macOS system certificates before fork() to avoid a
+ crash if Foundation methods are first used after fork.
+ For details see: http://www.sealiesoftware.com/blog/archive/2017/6/5/Objective-C_and_fork_in_macOS_1013.html
+
+ The system certificates are added to each gnutls session,
+ but the session is only established after the fork and thus
+ a call to get_system_certificate_authorities () would crash.
+
+ By calling get_system_certificate_authorities () before fork
+ the certificate authorities are already cached when the gnutls
+ session is configured. */
+ get_system_certificate_authorities (0);
+ #endif
pid = fork();
if (pid == (pid_t)-1)
{
@@ -1541,6 +1555,7 @@ main (int argc, char **argv)
static void
cleanup (void)
{
+ clear_system_certificate_authorities ();
crl_cache_deinit ();
cert_cache_deinit (1);
reload_dns_stuff (1);
diff --git a/dirmngr/http-common.c b/dirmngr/http-common.c
index 3b6cd44..3e27c7a 100644
--- a/dirmngr/http-common.c
+++ b/dirmngr/http-common.c
@@ -23,6 +23,10 @@
#include <stdlib.h>
#include <string.h>

+#ifdef __APPLE__
+# include <Security/Security.h>
+#endif
+
#include "dirmngr.h"
#include "http-common.h"

@@ -48,3 +52,96 @@ get_default_keyserver (int name_only)
}
return result;
}
+
+CFMutableArrayRef
+get_system_certificate_authorities (int clear) {
+#ifdef __APPLE__
+ static CFMutableArrayRef certificate_authorities = NULL;
+
+ if(certificate_authorities != NULL || clear == 1) {
+ if(certificate_authorities != NULL && clear == 1) {
+ log_debug ("clearing system certificate authorities cache\n");
+ CFRelease (certificate_authorities);
+ certificate_authorities = NULL;
+ log_debug ("system certificate authorities cache: %p\n", certificate_authorities);
+ }
+
+ return certificate_authorities;
+ }
+
+ log_debug ("load system certificate authorities into the cache\n");
+ certificate_authorities = CFArrayCreateMutable (NULL, 0, &kCFTypeArrayCallBacks);
+
+ SecTrustSettingsDomain domain[] = {
+ kSecTrustSettingsDomainUser,
+ kSecTrustSettingsDomainAdmin,
+ kSecTrustSettingsDomainSystem
+ };
+
+ for (size_t d = 0; d < sizeof (domain) / sizeof (*domain); d++) {
+ CFArrayRef certs = NULL;
+ OSStatus status = SecTrustSettingsCopyCertificates (domain[d], &certs);
+ if (status != errSecSuccess) {
+ continue;
+ }
+
+ CFIndex cert_count = CFArrayGetCount (certs);
+ for (int i = 0; i < cert_count; i++) {
+ SecCertificateRef cert = (void *)CFArrayGetValueAtIndex (certs, i);
+ CFDataRef der;
+ status = SecItemExport (cert, kSecFormatX509Cert, 0, NULL, &der);
+
+ if (status != errSecSuccess) {
+ CFRelease (der);
+ continue;
+ }
+
+ CFArrayAppendValue (certificate_authorities, der);
+ }
+
+ CFRelease (certs);
+ }
+
+ log_info ("system certificate authorities caching complete - found %ld certificate authorities\n",
+ (long)CFArrayGetCount (certificate_authorities));
+
+ return certificate_authorities;
+#else
+ return NULL;
+#endif
+}
+
+#ifdef HTTP_USE_GNUTLS
+int
+add_system_certificate_authorities (gnutls_certificate_credentials_t cred) {
+#ifdef __APPLE__
+ CFArrayRef certificate_authorities = get_system_certificate_authorities (0);
+
+ CFIndex ca_count = CFArrayGetCount (certificate_authorities);
+ log_debug ("gnutls: adding %ld system certificate authorities\n", (long)ca_count);
+
+ int valid_cas = 0;
+ for (CFIndex i = 0; i < ca_count; i++) {
+ CFDataRef cert = CFArrayGetValueAtIndex (certificate_authorities, i);
+
+ int rc = gnutls_certificate_set_x509_trust_mem (cred,
+ &(gnutls_datum_t) {
+ .data = (void *)CFDataGetBytePtr (cert),
+ .size = CFDataGetLength (cert),
+ }, GNUTLS_X509_FMT_DER);
+ if (rc > 0) {
+ valid_cas++;
+ }
+ }
+
+ return valid_cas > 0 ? valid_cas : GNUTLS_E_FILE_ERROR;
+#else
+ return gnutls_certificate_set_x509_system_trust (cred);
+#endif
+}
+#endif
+
+void
+clear_system_certificate_authorities () {
+ get_system_certificate_authorities (1);
+}
diff --git a/dirmngr/http-common.h b/dirmngr/http-common.h
index 5e6657b..d20f31c 100644
--- a/dirmngr/http-common.h
+++ b/dirmngr/http-common.h
@@ -17,9 +17,22 @@
* along with this program; if not, see <https://www.gnu.org/licenses/>.
*/

+#ifdef __APPLE__
+# include <CoreFoundation/CoreFoundation.h>
+#endif
+
+#ifdef HTTP_USE_GNUTLS
+# include <gnutls/gnutls.h>
+# include <gnutls/x509.h>
+#endif /*HTTP_USE_GNUTLS*/
+
#ifndef HTTP_COMMON_H
#define HTTP_COMMON_H

const char *get_default_keyserver (int name_only);

+CFMutableArrayRef get_system_certificate_authorities (int clear);
+int add_system_certificate_authorities(gnutls_certificate_credentials_t cred);
+void clear_system_certificate_authorities ();
+
#endif /* HTTP_COMMON_H */
diff --git a/dirmngr/http.c b/dirmngr/http.c
index 5e3f17c..3a749c2 100644
--- a/dirmngr/http.c
+++ b/dirmngr/http.c
@@ -821,7 +821,7 @@ http_session_new (http_session_t *r_session,
#if GNUTLS_VERSION_NUMBER >= 0x030014
static int shown;

- rc = gnutls_certificate_set_x509_system_trust (sess->certcred);
+ rc = add_system_certificate_authorities (sess->certcred);
if (rc < 0)
log_info ("setting system CAs failed: %s\n", gnutls_strerror (rc));
else if (!shown)

0 comments on commit 0bce831

Please sign in to comment.