From 157742be7deb5e5e12d962b20907b8e2e7733ac6 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 8 Mar 2017 11:00:22 +0100 Subject: [PATCH 01/21] fuzz: Add fuzzing boilerplate --- Makefile.am | 4 ++++ configure.ac | 24 +++++++++++++++++------- fuzz/.gitignore | 1 + fuzz/Makefile.am | 12 ++++++++++++ fuzz/fuzz_certs.c | 40 ++++++++++++++++++++++++++++++++++++++++ scripts/test.sh | 2 +- 6 files changed, 75 insertions(+), 8 deletions(-) create mode 100644 fuzz/.gitignore create mode 100644 fuzz/Makefile.am create mode 100644 fuzz/fuzz_certs.c diff --git a/Makefile.am b/Makefile.am index a02c576e0..47e9e8ce6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,5 +1,9 @@ SUBDIRS = src man conf init testing +if USE_FUZZING + SUBDIRS += fuzz +endif + if USE_SCRIPTS SUBDIRS += scripts endif diff --git a/configure.ac b/configure.ac index 8b071ec59..2c05df2ba 100644 --- a/configure.ac +++ b/configure.ac @@ -277,6 +277,7 @@ ARG_ENABL_SET([cmd], [enable the command line IKE client charon-cmd.] ARG_ENABL_SET([conftest], [enforce Suite B conformance test framework.]) ARG_ENABL_SET([dumm], [enable the DUMM UML test framework.]) ARG_ENABL_SET([fast], [enable libfast (FastCGI Application Server w/ templates.]) +ARG_ENABL_SET([fuzzing], [enable fuzzing scripts (found in directory fuzz).]) ARG_ENABL_SET([libipsec], [enable user space IPsec implementation.]) ARG_ENABL_SET([manager], [enable web management console (proof of concept).]) ARG_ENABL_SET([medcli], [enable mediation client configuration database plugin.]) @@ -1263,6 +1264,11 @@ if test x$coverage = xtrue; then CFLAGS="${CFLAGS} -g -O0" fi +if test x$fuzzing = xtrue; then + AC_MSG_NOTICE([fuzzing enabled, modifying CFLAGS]) + CFLAGS="${CFLAGS} -fsanitize-coverage=trace-pc-guard -fsanitize=address" +fi + if test x$ruby_gems = xtrue; then AC_PATH_PROG([GEM], [gem], [], [$PATH:/bin:/usr/bin:/usr/local/bin]) if test x$GEM = x; then @@ -1309,6 +1315,7 @@ attest_plugins= scepclient_plugins= pki_plugins= scripts_plugins= +fuzz_plugins= manager_plugins= medsrv_plugins= nm_plugins= @@ -1332,21 +1339,21 @@ ADD_PLUGIN([aes], [s charon scepclient pki scripts nm cmd]) ADD_PLUGIN([des], [s charon scepclient pki scripts nm cmd]) ADD_PLUGIN([blowfish], [s charon scepclient pki scripts nm cmd]) ADD_PLUGIN([rc2], [s charon scepclient pki scripts nm cmd]) -ADD_PLUGIN([sha2], [s charon scepclient pki scripts medsrv attest nm cmd aikgen]) -ADD_PLUGIN([sha3], [s charon scepclient pki scripts medsrv attest nm cmd aikgen]) -ADD_PLUGIN([sha1], [s charon scepclient pki scripts manager medsrv attest nm cmd aikgen]) +ADD_PLUGIN([sha2], [s charon scepclient pki scripts medsrv attest nm cmd aikgen fuzz]) +ADD_PLUGIN([sha3], [s charon scepclient pki scripts medsrv attest nm cmd aikgen fuzz]) +ADD_PLUGIN([sha1], [s charon scepclient pki scripts manager medsrv attest nm cmd aikgen fuzz]) ADD_PLUGIN([md4], [s charon scepclient pki nm cmd]) ADD_PLUGIN([md5], [s charon scepclient pki scripts attest nm cmd aikgen]) ADD_PLUGIN([mgf1], [s charon scepclient pki scripts medsrv attest nm cmd aikgen]) ADD_PLUGIN([rdrand], [s charon scepclient pki scripts medsrv attest nm cmd aikgen]) ADD_PLUGIN([random], [s charon scepclient pki scripts manager medsrv attest nm cmd aikgen]) ADD_PLUGIN([nonce], [s charon nm cmd aikgen]) -ADD_PLUGIN([x509], [s charon scepclient pki scripts attest nm cmd aikgen]) +ADD_PLUGIN([x509], [s charon scepclient pki scripts attest nm cmd aikgen fuzz]) ADD_PLUGIN([revocation], [s charon pki nm cmd]) ADD_PLUGIN([constraints], [s charon nm cmd]) ADD_PLUGIN([acert], [s charon]) ADD_PLUGIN([pubkey], [s charon cmd aikgen]) -ADD_PLUGIN([pkcs1], [s charon scepclient pki scripts manager medsrv attest nm cmd aikgen]) +ADD_PLUGIN([pkcs1], [s charon scepclient pki scripts manager medsrv attest nm cmd aikgen fuzz]) ADD_PLUGIN([pkcs7], [s charon scepclient pki scripts nm cmd]) ADD_PLUGIN([pkcs8], [s charon scepclient pki scripts manager medsrv attest nm cmd]) ADD_PLUGIN([pkcs12], [s charon scepclient pki scripts cmd]) @@ -1355,13 +1362,13 @@ ADD_PLUGIN([dnskey], [s charon pki]) ADD_PLUGIN([sshkey], [s charon pki nm cmd]) ADD_PLUGIN([dnscert], [c charon]) ADD_PLUGIN([ipseckey], [c charon]) -ADD_PLUGIN([pem], [s charon scepclient pki scripts manager medsrv attest nm cmd aikgen]) +ADD_PLUGIN([pem], [s charon scepclient pki scripts manager medsrv attest nm cmd aikgen fuzz]) ADD_PLUGIN([padlock], [s charon]) ADD_PLUGIN([openssl], [s charon scepclient pki scripts manager medsrv attest nm cmd aikgen]) ADD_PLUGIN([gcrypt], [s charon scepclient pki scripts manager medsrv attest nm cmd aikgen]) ADD_PLUGIN([af-alg], [s charon scepclient pki scripts medsrv attest nm cmd aikgen]) ADD_PLUGIN([fips-prf], [s charon nm cmd]) -ADD_PLUGIN([gmp], [s charon scepclient pki scripts manager medsrv attest nm cmd aikgen]) +ADD_PLUGIN([gmp], [s charon scepclient pki scripts manager medsrv attest nm cmd aikgen fuzz]) ADD_PLUGIN([curve25519], [s charon pki scripts nm cmd]) ADD_PLUGIN([agent], [s charon nm cmd]) ADD_PLUGIN([keychain], [s charon cmd]) @@ -1462,6 +1469,7 @@ AC_SUBST(attest_plugins) AC_SUBST(scepclient_plugins) AC_SUBST(pki_plugins) AC_SUBST(scripts_plugins) +AC_SUBST(fuzz_plugins) AC_SUBST(manager_plugins) AC_SUBST(medsrv_plugins) AC_SUBST(nm_plugins) @@ -1646,6 +1654,7 @@ AM_CONDITIONAL(USE_NM, test x$nm = xtrue) AM_CONDITIONAL(USE_PKI, test x$pki = xtrue) AM_CONDITIONAL(USE_SCEPCLIENT, test x$scepclient = xtrue) AM_CONDITIONAL(USE_SCRIPTS, test x$scripts = xtrue) +AM_CONDITIONAL(USE_FUZZING, test x$fuzzing = xtrue) AM_CONDITIONAL(USE_CONFTEST, test x$conftest = xtrue) AM_CONDITIONAL(USE_LIBSTRONGSWAN, test x$charon = xtrue -o x$pki = xtrue -o x$scepclient = xtrue -o x$conftest = xtrue -o x$fast = xtrue -o x$imcv = xtrue -o x$nm = xtrue -o x$tkm = xtrue -o x$cmd = xtrue -o x$tls = xtrue -o x$tnc_tnccs = xtrue -o x$aikgen = xtrue -o x$svc = xtrue -o x$systemd = xtrue) AM_CONDITIONAL(USE_LIBCHARON, test x$charon = xtrue -o x$conftest = xtrue -o x$nm = xtrue -o x$tkm = xtrue -o x$cmd = xtrue -o x$svc = xtrue -o x$systemd = xtrue) @@ -1734,6 +1743,7 @@ AC_SUBST(strongswan_options) AC_CONFIG_FILES([ Makefile conf/Makefile + fuzz/Makefile man/Makefile init/Makefile init/systemd/Makefile diff --git a/fuzz/.gitignore b/fuzz/.gitignore new file mode 100644 index 000000000..64271a6dd --- /dev/null +++ b/fuzz/.gitignore @@ -0,0 +1 @@ +fuzz_certs \ No newline at end of file diff --git a/fuzz/Makefile.am b/fuzz/Makefile.am new file mode 100644 index 000000000..f2a84c1e7 --- /dev/null +++ b/fuzz/Makefile.am @@ -0,0 +1,12 @@ +AM_CPPFLAGS = \ + -I$(top_srcdir)/src/libstrongswan \ + -DPLUGINDIR=\""$(abs_top_builddir)/src/libstrongswan/plugins\"" \ + -DPLUGINS="\"${fuzz_plugins}\"" + +noinst_PROGRAMS = fuzz_certs + +fuzz_certs_SOURCES = fuzz_certs.c + +fuzz_certs_LDFLAGS = libFuzzer.a -lstdc++ + +fuzz_certs_LDADD = $(top_builddir)/src/libstrongswan/libstrongswan.la diff --git a/fuzz/fuzz_certs.c b/fuzz/fuzz_certs.c new file mode 100644 index 000000000..3100eb5f9 --- /dev/null +++ b/fuzz/fuzz_certs.c @@ -0,0 +1,40 @@ +/* + * Copyright (C) 2017 Tobias Brunner + * HSR Hochschule fuer Technik Rapperswil + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. See . + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ + +#include +#include + +int LLVMFuzzerTestOneInput(const uint8_t *buf, size_t len) +{ + certificate_t *cert; + chunk_t chunk; + + dbg_default_set_level(-1); + library_init(NULL, "fuzz_certs"); + plugin_loader_add_plugindirs(PLUGINDIR, PLUGINS); + if (!lib->plugins->load(lib->plugins, PLUGINS)) + { + return 1; + } + + chunk = chunk_create((u_char*)buf, len); + cert = lib->creds->create(lib->creds, CRED_CERTIFICATE, CERT_X509, + BUILD_BLOB, chunk, BUILD_END); + DESTROY_IF(cert); + + lib->plugins->unload(lib->plugins); + library_deinit(); + return 0; +} diff --git a/scripts/test.sh b/scripts/test.sh index 81f4935fc..22e74033e 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -32,7 +32,7 @@ printf-builtin) all|coverage) CONFIG="--enable-all --disable-android-dns --disable-android-log --disable-dumm --disable-kernel-pfroute --disable-keychain - --disable-lock-profiler --disable-padlock + --disable-lock-profiler --disable-padlock --disable-fuzzing --disable-osx-attr --disable-tkm --disable-uci --disable-systemd --disable-soup --disable-unwind-backtraces --disable-svc --disable-dbghelp-backtraces --disable-socket-win From 5e37f7e55041929a4b7458bd7e1f24cd8a0edd8a Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 15 Mar 2017 11:16:35 +0100 Subject: [PATCH 02/21] x509: Fix leak if a certificate contains multiple authorityKeyIdentifiers --- src/libstrongswan/plugins/x509/x509_cert.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libstrongswan/plugins/x509/x509_cert.c b/src/libstrongswan/plugins/x509/x509_cert.c index b3d90c5f6..7068853a8 100644 --- a/src/libstrongswan/plugins/x509/x509_cert.c +++ b/src/libstrongswan/plugins/x509/x509_cert.c @@ -1461,6 +1461,7 @@ static bool parse_certificate(private_x509_cert_t *this) } break; case OID_AUTHORITY_KEY_ID: + chunk_free(&this->authKeyIdentifier); this->authKeyIdentifier = x509_parse_authorityKeyIdentifier( object, level, &this->authKeySerialNumber); break; From 388351609dad7a96b6ac8857864e59642810bc56 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 29 Mar 2017 11:16:34 +0200 Subject: [PATCH 03/21] x509: Fix leak if there is an empty CDP --- src/libstrongswan/plugins/x509/x509_cert.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libstrongswan/plugins/x509/x509_cert.c b/src/libstrongswan/plugins/x509/x509_cert.c index 7068853a8..3027e4387 100644 --- a/src/libstrongswan/plugins/x509/x509_cert.c +++ b/src/libstrongswan/plugins/x509/x509_cert.c @@ -818,10 +818,12 @@ static void add_cdps(linked_list_t *list, linked_list_t *uris, enumerator_t *enumerator; x509_cdp_t *cdp; char *uri; + int len; while (uris->remove_last(uris, (void**)&id) == SUCCESS) { - if (asprintf(&uri, "%Y", id) > 0) + len = asprintf(&uri, "%Y", id); + if (len > 0) { if (issuers->get_count(issuers)) { @@ -845,6 +847,10 @@ static void add_cdps(linked_list_t *list, linked_list_t *uris, list->insert_last(list, cdp); } } + else if (!len) + { + free(uri); + } id->destroy(id); } while (issuers->remove_last(issuers, (void**)&id) == SUCCESS) From 30c03a7df9a638b3c94d0405dd40a9fa1a57832c Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 29 Mar 2017 11:19:30 +0200 Subject: [PATCH 04/21] pem: Don't read beyond line ends --- src/libstrongswan/plugins/pem/pem_builder.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstrongswan/plugins/pem/pem_builder.c b/src/libstrongswan/plugins/pem/pem_builder.c index 719a2a69e..cda703741 100644 --- a/src/libstrongswan/plugins/pem/pem_builder.c +++ b/src/libstrongswan/plugins/pem/pem_builder.c @@ -61,7 +61,7 @@ static bool find_boundary(char* tag, chunk_t *line) if (!present("-----", line) || !present(tag, line) || - *line->ptr != ' ') + !line->len || *line->ptr != ' ') { return FALSE; } @@ -306,7 +306,7 @@ static status_t pem_to_bin(chunk_t *blob, bool *pgp) } /* check for PGP armor checksum */ - if (*data.ptr == '=') + if (data.len && *data.ptr == '=') { *pgp = TRUE; data.ptr++; From 8806b00f43895b948a0514c550c2beca99723edf Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 29 Mar 2017 11:32:25 +0200 Subject: [PATCH 05/21] fuzz: Make path to libFuzzer.a configurable --- configure.ac | 1 + fuzz/Makefile.am | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 2c05df2ba..2dfd4264a 100644 --- a/configure.ac +++ b/configure.ac @@ -62,6 +62,7 @@ ARG_WITH_SUBST([routing-table], [220], [set routing table to use for IPsec ARG_WITH_SUBST([routing-table-prio], [220], [set priority for IPsec routing table]) ARG_WITH_SUBST([ipsec-script], [ipsec], [change the name of the ipsec script]) ARG_WITH_SUBST([fips-mode], [0], [set openssl FIPS mode: disabled(0), enabled(1), Suite B enabled(2)]) +ARG_WITH_SUBST([libfuzzer], [], [path to libFuzzer.a]) ARG_WITH_SET([capabilities], [no], [set capability dropping library. Currently supported values are "libcap" and "native"]) ARG_WITH_SET([mpz_powm_sec], [yes], [use the more side-channel resistant mpz_powm_sec in libgmp, if available]) ARG_WITH_SET([dev-headers], [no], [install strongSwan development headers to directory.]) diff --git a/fuzz/Makefile.am b/fuzz/Makefile.am index f2a84c1e7..a2c225ff3 100644 --- a/fuzz/Makefile.am +++ b/fuzz/Makefile.am @@ -7,6 +7,6 @@ noinst_PROGRAMS = fuzz_certs fuzz_certs_SOURCES = fuzz_certs.c -fuzz_certs_LDFLAGS = libFuzzer.a -lstdc++ +fuzz_certs_LDFLAGS = ${libfuzzer} -lstdc++ fuzz_certs_LDADD = $(top_builddir)/src/libstrongswan/libstrongswan.la From 98b55c8b838675fa8e598701a4b50308b41c6a9d Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 19 Apr 2017 10:40:40 +0200 Subject: [PATCH 06/21] library: Add compile option to disable memwipe() check --- src/libstrongswan/library.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libstrongswan/library.c b/src/libstrongswan/library.c index 4f79dcc5b..1d1e1f07e 100644 --- a/src/libstrongswan/library.c +++ b/src/libstrongswan/library.c @@ -241,6 +241,8 @@ static bool equals(char *a, char *b) */ #define MEMWIPE_WIPE_WORDS 16 +#ifndef NO_CHECK_MEMWIPE + /** * Write magic to memory, and try to clear it with memwipe() */ @@ -281,6 +283,8 @@ static bool check_memwipe() return TRUE; } +#endif + /* * see header file */ @@ -387,10 +391,12 @@ bool library_init(char *settings, const char *namespace) this->public.streams = stream_manager_create(); this->public.plugins = plugin_loader_create(); +#ifndef NO_CHECK_MEMWIPE if (!check_memwipe()) { return FALSE; } +#endif if (lib->settings->get_bool(lib->settings, "%s.integrity_test", FALSE, lib->ns)) From 6ce649a8a6f913a56ccf5b4e60d44f01f21ff8a3 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 28 Apr 2017 15:39:01 +0200 Subject: [PATCH 07/21] configure: Don't build static libraries by default This way we can actually detect if someone wants to build strongSwan statically because --enable-static has to be passed explicitly. --- configure.ac | 1 + 1 file changed, 1 insertion(+) diff --git a/configure.ac b/configure.ac index 2dfd4264a..56738bdf3 100644 --- a/configure.ac +++ b/configure.ac @@ -35,6 +35,7 @@ m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES]) AC_CONFIG_MACRO_DIR([m4/config]) AC_CONFIG_HEADERS([config.h]) AC_DEFINE([CONFIG_H_INCLUDED], [], [defined if config.h included]) +AC_DISABLE_STATIC PKG_PROG_PKG_CONFIG m4_include(m4/macros/split-package-version.m4) From 1a06bf03f9a0be7f54bdc412475f537f623ca99f Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 28 Apr 2017 17:41:57 +0200 Subject: [PATCH 08/21] plugin-loader: Add facility to register plugin constructors Enabled when building monolithically and statically. This should allow us to work around the -whole-archive issue with libtool. If the libraries register the plugin constructors they provide they reference the constructors and will therefore prevent the linker from removing these seemingly unused symbols from the final executable. For use cases where dlsym() can be used, e.g. because the static libraries are manually linked with -whole-archive (Linux) or -force-load (Apple), this can be disabled by passing ss_cv_static_plugin_constructors=no to the configure script. --- configure.ac | 19 ++++++- src/libstrongswan/plugins/plugin_loader.c | 62 +++++++++++++++++++++-- src/libstrongswan/plugins/plugin_loader.h | 10 ++++ 3 files changed, 87 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index 56738bdf3..07df3a672 100644 --- a/configure.ac +++ b/configure.ac @@ -1,5 +1,5 @@ # -# Copyright (C) 2007-2015 Tobias Brunner +# Copyright (C) 2007-2017 Tobias Brunner # Copyright (C) 2006-2016 Andreas Steffen # Copyright (C) 2006-2014 Martin Willi # HSR Hochschule fuer Technik Rapperswil @@ -1303,6 +1303,19 @@ AM_CONDITIONAL(PYTHON_EGGS_INSTALL, [test "x$python_eggs_install" = xtrue]) AM_CONDITIONAL(PERL_CPAN_INSTALL, [test "x$perl_cpan_install" = xtrue]) +AC_CACHE_CHECK( + [if plugin constructors should be resolved statically], + [ss_cv_static_plugin_constructors], + [if test x$monolithic = xtrue -a x$enable_static = xyes; then + ss_cv_static_plugin_constructors=yes + else + ss_cv_static_plugin_constructors="no (enabled for static, monolithic builds)" + fi] +) +if test "x$ss_cv_static_plugin_constructors" = xyes; then + static_plugin_constructors=true +fi + # =============================================== # collect plugin list for strongSwan components # =============================================== @@ -1678,6 +1691,7 @@ AM_CONDITIONAL(USE_IMCV, test x$imcv = xtrue) AM_CONDITIONAL(USE_TROUSERS, test x$tss_trousers = xtrue) AM_CONDITIONAL(USE_TSS2, test x$tss_tss2 = xtrue) AM_CONDITIONAL(MONOLITHIC, test x$monolithic = xtrue) +AM_CONDITIONAL(STATIC_PLUGIN_CONSTRUCTORS, test x$static_plugin_constructors = xtrue) AM_CONDITIONAL(USE_SILENT_RULES, test x$enable_silent_rules = xyes) AM_CONDITIONAL(COVERAGE, test x$coverage = xtrue) AM_CONDITIONAL(USE_DBGHELP, test x$dbghelp_backtraces = xtrue) @@ -1709,6 +1723,9 @@ fi if test x$monolithic = xtrue; then AC_DEFINE([MONOLITHIC], [], [monolithic build embedding plugins]) fi +if test x$static_plugin_constructors = xtrue; then + AC_DEFINE([STATIC_PLUGIN_CONSTRUCTORS], [], [static plugin constructors]) +fi if test x$ikev1 = xtrue; then AC_DEFINE([USE_IKEV1], [], [support for IKEv1 protocol]) fi diff --git a/src/libstrongswan/plugins/plugin_loader.c b/src/libstrongswan/plugins/plugin_loader.c index e4698fac0..e78762b2c 100644 --- a/src/libstrongswan/plugins/plugin_loader.c +++ b/src/libstrongswan/plugins/plugin_loader.c @@ -40,6 +40,13 @@ typedef struct registered_feature_t registered_feature_t; typedef struct provided_feature_t provided_feature_t; typedef struct plugin_entry_t plugin_entry_t; +#ifdef STATIC_PLUGIN_CONSTRUCTORS +/** + * Statically registered constructors + */ +static hashtable_t *plugin_constructors = NULL; +#endif + /** * private data of plugin_loader */ @@ -298,6 +305,46 @@ static plugin_t *static_features_create(const char *name, return &this->public; } +#ifdef STATIC_PLUGIN_CONSTRUCTORS +/* + * Described in header. + */ +void plugin_constructor_register(char *name, void *constructor) +{ + bool old = FALSE; + + if (lib && lib->leak_detective) + { + old = lib->leak_detective->set_state(lib->leak_detective, FALSE); + } + + if (!plugin_constructors) + { + chunk_hash_seed(); + plugin_constructors = hashtable_create(hashtable_hash_str, + hashtable_equals_str, 32); + } + if (constructor) + { + plugin_constructors->put(plugin_constructors, name, constructor); + } + else + { + plugin_constructors->remove(plugin_constructors, name); + if (!plugin_constructors->get_count(plugin_constructors)) + { + plugin_constructors->destroy(plugin_constructors); + plugin_constructors = NULL; + } + } + + if (lib && lib->leak_detective) + { + lib->leak_detective->set_state(lib->leak_detective, old); + } +} +#endif + /** * create a plugin * returns: NOT_FOUND, if the constructor was not found @@ -309,7 +356,7 @@ static status_t create_plugin(private_plugin_loader_t *this, void *handle, { char create[128]; plugin_t *plugin; - plugin_constructor_t constructor; + plugin_constructor_t constructor = NULL; if (snprintf(create, sizeof(create), "%s_plugin_create", name) >= sizeof(create)) @@ -317,8 +364,17 @@ static status_t create_plugin(private_plugin_loader_t *this, void *handle, return FAILED; } translate(create, "-", "_"); - constructor = dlsym(handle, create); - if (constructor == NULL) +#ifdef STATIC_PLUGIN_CONSTRUCTORS + if (plugin_constructors) + { + constructor = plugin_constructors->get(plugin_constructors, name); + } + if (!constructor) +#endif + { + constructor = dlsym(handle, create); + } + if (!constructor) { return NOT_FOUND; } diff --git a/src/libstrongswan/plugins/plugin_loader.h b/src/libstrongswan/plugins/plugin_loader.h index 6be6a909c..92a860615 100644 --- a/src/libstrongswan/plugins/plugin_loader.h +++ b/src/libstrongswan/plugins/plugin_loader.h @@ -168,4 +168,14 @@ plugin_loader_t *plugin_loader_create(); */ void plugin_loader_add_plugindirs(char *basedir, char *plugins); +#ifdef STATIC_PLUGIN_CONSTRUCTORS +/** + * Register a plugin constructor in case of static builds. + * + * @param name name of the plugin + * @param constructor constructor to register (set to NULL to unregister) + */ +void plugin_constructor_register(char *name, void *constructor); +#endif + #endif /** PLUGIN_LOADER_H_ @}*/ From 8699275ed6954037f598a1824319f2cdf9deb4b1 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 28 Apr 2017 17:46:57 +0200 Subject: [PATCH 09/21] plugin-constructors: Add script to generate constructor registration Using a Python script so this works in cross-compilation situations. --- src/libstrongswan/Makefile.am | 1 + .../plugins/plugin_constructors.py | 60 +++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 src/libstrongswan/plugins/plugin_constructors.py diff --git a/src/libstrongswan/Makefile.am b/src/libstrongswan/Makefile.am index f6d6f5465..69abfbe0e 100644 --- a/src/libstrongswan/Makefile.am +++ b/src/libstrongswan/Makefile.am @@ -195,6 +195,7 @@ endif EXTRA_DIST = \ asn1/oid.txt asn1/oid.pl \ crypto/proposal/proposal_keywords_static.txt \ +plugins/plugin_constructors.py \ Android.mk BUILT_SOURCES = \ diff --git a/src/libstrongswan/plugins/plugin_constructors.py b/src/libstrongswan/plugins/plugin_constructors.py new file mode 100644 index 000000000..d9c40e383 --- /dev/null +++ b/src/libstrongswan/plugins/plugin_constructors.py @@ -0,0 +1,60 @@ +#!/usr/bin/env python +# +# Copyright (C) 2017 Tobias Brunner +# HSR Hochschule fuer Technik Rapperswil +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by the +# Free Software Foundation; either version 2 of the License, or (at your +# option) any later version. See . +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY +# or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +# for more details. + +import sys +from argparse import ArgumentParser + +def generate_output(plugins): + """Generate a source file containing plugin constructor registrations""" + print("/**") + print(" * Register plugin constructors for static libraries") + print(" * Created by {0}".format(__file__)) + print(" */") + print("") + print("#include ") + print("#include ") + print("") + + for plugin in plugins: + print("plugin_t *{0}_plugin_create();".format(plugin.replace('-', '_'))) + + print("") + print("static void register_plugins() __attribute__ ((constructor));") + print("static void register_plugins()") + print("{") + + for plugin in plugins: + print(' plugin_constructor_register("{0}", {1}_plugin_create);'.format(plugin, plugin.replace('-', '_'))) + + print("}") + + print("") + print("static void unregister_plugins() __attribute__ ((destructor));") + print("static void unregister_plugins()") + print("{") + + for plugin in plugins: + print(' plugin_constructor_register("{0}", NULL);'.format(plugin)) + + print("}") + print("") + +parser = ArgumentParser(description = "Generate constructor registration for a list of plugins") +parser.add_argument('plugins', metavar="plugin", nargs="*", + help = "name of a plugin for which to generate constructor registration") + + +args = parser.parse_args() +generate_output(args.plugins); From 4a0b6d659d2fb0473b75cf53bb1a0934983d270c Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 28 Apr 2017 17:49:50 +0200 Subject: [PATCH 10/21] Add plugin constructor registration for all libraries that provide plugins Unfortunately, we can't just add the generated C file to the sources in Makefile.am as the linker would remove that object file when it notices that no symbol in it is ever referenced. So we include it in the file that contains the library initialization, which will definitely be referenced by the executable. This allows building an almost stand-alone static version of e.g. charon when building with `--enable-monolithic --enable-static --disable-shared` (without `--disable-shared` libtool will only build a version that links the libraries dynamically). External libraries (e.g. gmp or openssl) are not linked statically this way, though. --- .gitignore | 1 + src/libcharon/Makefile.am | 9 +++++++++ src/libcharon/daemon.c | 7 +++++++ src/libstrongswan/Makefile.am | 9 +++++++++ src/libstrongswan/library.c | 7 +++++++ src/libtnccs/Makefile.am | 9 +++++++++ src/libtnccs/tnc/tnc.c | 7 +++++++ src/libtpmtss/Makefile.am | 9 +++++++++ src/libtpmtss/tpm_tss.c | 7 +++++++ 9 files changed, 65 insertions(+) diff --git a/.gitignore b/.gitignore index 2e4541576..cd0c30728 100644 --- a/.gitignore +++ b/.gitignore @@ -27,6 +27,7 @@ libtool y.tab.[ch] lex.yy.c *keywords.c +plugin_constructors.c Doxyfile apidoc/ *~ diff --git a/src/libcharon/Makefile.am b/src/libcharon/Makefile.am index 8461d6230..3fcaedc3b 100644 --- a/src/libcharon/Makefile.am +++ b/src/libcharon/Makefile.am @@ -184,6 +184,15 @@ if USE_ME sa/ikev2/tasks/ike_me.c sa/ikev2/tasks/ike_me.h endif +if STATIC_PLUGIN_CONSTRUCTORS +BUILT_SOURCES = $(srcdir)/plugin_constructors.c +CLEANFILES = $(srcdir)/plugin_constructors.c + +$(srcdir)/plugin_constructors.c: $(top_srcdir)/src/libstrongswan/plugins/plugin_constructors.py + $(AM_V_GEN) \ + $(PYTHON) $(top_srcdir)/src/libstrongswan/plugins/plugin_constructors.py ${c_plugins} > $@ +endif + # build optional plugins ######################## diff --git a/src/libcharon/daemon.c b/src/libcharon/daemon.c index eadc10a6a..8daea783f 100644 --- a/src/libcharon/daemon.c +++ b/src/libcharon/daemon.c @@ -117,6 +117,13 @@ struct private_daemon_t { refcount_t ref; }; +/** + * Register plugins if built statically + */ +#ifdef STATIC_PLUGIN_CONSTRUCTORS +#include "plugin_constructors.c" +#endif + /** * One and only instance of the daemon. */ diff --git a/src/libstrongswan/Makefile.am b/src/libstrongswan/Makefile.am index 69abfbe0e..b4d8452f1 100644 --- a/src/libstrongswan/Makefile.am +++ b/src/libstrongswan/Makefile.am @@ -221,6 +221,15 @@ $(srcdir)/crypto/proposal/proposal_keywords_static.c: $(srcdir)/crypto/proposal/ $(GPERF) -N proposal_get_token_static -m 10 -C -G -c -t -D < \ $(srcdir)/crypto/proposal/proposal_keywords_static.txt > $@ +if STATIC_PLUGIN_CONSTRUCTORS +BUILT_SOURCES += $(srcdir)/plugin_constructors.c +CLEANFILES = $(srcdir)/plugin_constructors.c + +$(srcdir)/plugin_constructors.c: $(srcdir)/plugins/plugin_constructors.py + $(AM_V_GEN) \ + $(PYTHON) $(srcdir)/plugins/plugin_constructors.py ${s_plugins} > $@ +endif + if MONOLITHIC SUBDIRS = else diff --git a/src/libstrongswan/library.c b/src/libstrongswan/library.c index 1d1e1f07e..7944b9356 100644 --- a/src/libstrongswan/library.c +++ b/src/libstrongswan/library.c @@ -93,6 +93,13 @@ void library_add_namespace(char *ns) } } +/** + * Register plugins if built statically + */ +#ifdef STATIC_PLUGIN_CONSTRUCTORS +#include "plugin_constructors.c" +#endif + /** * library instance */ diff --git a/src/libtnccs/Makefile.am b/src/libtnccs/Makefile.am index 7a630fe54..ff7b54f6a 100644 --- a/src/libtnccs/Makefile.am +++ b/src/libtnccs/Makefile.am @@ -26,6 +26,15 @@ tnc/tnccs/tnccs_manager.h tnc/tnccs/tnccs_manager.c EXTRA_DIST = Android.mk +if STATIC_PLUGIN_CONSTRUCTORS +BUILT_SOURCES = $(srcdir)/plugin_constructors.c +CLEANFILES = $(srcdir)/plugin_constructors.c + +$(srcdir)/plugin_constructors.c: $(top_srcdir)/src/libstrongswan/plugins/plugin_constructors.py + $(AM_V_GEN) \ + $(PYTHON) $(top_srcdir)/src/libstrongswan/plugins/plugin_constructors.py ${t_plugins} > $@ +endif + # build optional plugins ######################## diff --git a/src/libtnccs/tnc/tnc.c b/src/libtnccs/tnc/tnc.c index 80ba61c5a..9627be862 100644 --- a/src/libtnccs/tnc/tnc.c +++ b/src/libtnccs/tnc/tnc.c @@ -54,6 +54,13 @@ struct private_tnc_t { refcount_t ref; }; +/** + * Register plugins if built statically + */ +#ifdef STATIC_PLUGIN_CONSTRUCTORS +#include "plugin_constructors.c" +#endif + /** * Single instance of tnc_t. */ diff --git a/src/libtpmtss/Makefile.am b/src/libtpmtss/Makefile.am index c7ac39a09..5f3a97a99 100644 --- a/src/libtpmtss/Makefile.am +++ b/src/libtpmtss/Makefile.am @@ -33,6 +33,15 @@ else SUBDIRS = . endif +if STATIC_PLUGIN_CONSTRUCTORS +BUILT_SOURCES = $(srcdir)/plugin_constructors.c +CLEANFILES = $(srcdir)/plugin_constructors.c + +$(srcdir)/plugin_constructors.c: $(top_srcdir)/src/libstrongswan/plugins/plugin_constructors.py + $(AM_V_GEN) \ + $(PYTHON) $(top_srcdir)/src/libstrongswan/plugins/plugin_constructors.py ${p_plugins} > $@ +endif + if USE_TPM SUBDIRS += plugins/tpm if MONOLITHIC diff --git a/src/libtpmtss/tpm_tss.c b/src/libtpmtss/tpm_tss.c index b7b970c8d..42a341896 100644 --- a/src/libtpmtss/tpm_tss.c +++ b/src/libtpmtss/tpm_tss.c @@ -17,6 +17,13 @@ #include "tpm_tss_tss2.h" #include "tpm_tss_trousers.h" +/** + * Register plugins if built statically + */ +#ifdef STATIC_PLUGIN_CONSTRUCTORS +#include "plugin_constructors.c" +#endif + /** * Described in header. */ From 05eeffb2f2f42f3f66a1b9f3c128768af756fac2 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Mon, 1 May 2017 16:23:55 +0200 Subject: [PATCH 11/21] fuzz: Change how fuzz_certs is built We mainly do this because we have to create a self-contained executable and it isn't so easy to actually get libtool to link e.g. libgmp statically. --- fuzz/Makefile.am | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/fuzz/Makefile.am b/fuzz/Makefile.am index a2c225ff3..66debc226 100644 --- a/fuzz/Makefile.am +++ b/fuzz/Makefile.am @@ -1,12 +1,18 @@ -AM_CPPFLAGS = \ +CPPFLAGS = @CPPFLAGS@ \ -I$(top_srcdir)/src/libstrongswan \ -DPLUGINDIR=\""$(abs_top_builddir)/src/libstrongswan/plugins\"" \ -DPLUGINS="\"${fuzz_plugins}\"" -noinst_PROGRAMS = fuzz_certs +LDFLAGS = @LDFLAGS@ ${libfuzzer} \ + $(top_builddir)/src/libstrongswan/.libs/libstrongswan.a \ + -Wl,-Bstatic -lgmp -Wl,-Bdynamic \ + -stdlib=libc++ -lstdc++ -fuzz_certs_SOURCES = fuzz_certs.c +FUZZ_TARGETS=fuzz_certs -fuzz_certs_LDFLAGS = ${libfuzzer} -lstdc++ +all-local: $(FUZZ_TARGETS) -fuzz_certs_LDADD = $(top_builddir)/src/libstrongswan/libstrongswan.la +CLEANFILES=$(FUZZ_TARGETS) + +fuzz_certs: fuzz_certs.c + $(CC) $(CPPFLAGS) $(CFLAGS) -o $@ $< $(LDFLAGS) From 83eacc448c94e7cb8cd11720aab505d865eeb6ef Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 3 May 2017 17:35:45 +0200 Subject: [PATCH 12/21] processor: Move priority threads assignment to set_threads() This avoids the evaluation of %N even if the thread pool is never used. We need to avoid as many custom printf specifiers as possible when fuzzing our code to avoid excessive log messages. --- src/libstrongswan/processing/processor.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libstrongswan/processing/processor.c b/src/libstrongswan/processing/processor.c index 27e5ab5f6..bd8d534a5 100644 --- a/src/libstrongswan/processing/processor.c +++ b/src/libstrongswan/processing/processor.c @@ -429,7 +429,15 @@ METHOD(processor_t, execute_job, void, METHOD(processor_t, set_threads, void, private_processor_t *this, u_int count) { + int i; + this->mutex->lock(this->mutex); + for (i = 0; i < JOB_PRIO_MAX; i++) + { + this->prio_threads[i] = lib->settings->get_int(lib->settings, + "%s.processor.priority_threads.%N", 0, lib->ns, + job_priority_names, i); + } if (count > this->total_threads) { /* increase thread count */ worker_thread_t *worker; @@ -551,13 +559,10 @@ processor_t *processor_create() .job_added = condvar_create(CONDVAR_TYPE_DEFAULT), .thread_terminated = condvar_create(CONDVAR_TYPE_DEFAULT), ); + for (i = 0; i < JOB_PRIO_MAX; i++) { this->jobs[i] = linked_list_create(); - this->prio_threads[i] = lib->settings->get_int(lib->settings, - "%s.processor.priority_threads.%N", 0, lib->ns, - job_priority_names, i); } - return &this->public; } From 92a10e4645af25b390b7c7e05fe9650d4e83a070 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 3 May 2017 18:10:17 +0200 Subject: [PATCH 13/21] x509: Manually print CRL/OCSP URIs when fuzzing This avoids a warning about the custom %Y printf specifier. --- configure.ac | 3 ++ src/libstrongswan/plugins/x509/x509_cert.c | 33 ++++++++++++++++------ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/configure.ac b/configure.ac index 07df3a672..827c6913e 100644 --- a/configure.ac +++ b/configure.ac @@ -1732,6 +1732,9 @@ fi if test x$ikev2 = xtrue; then AC_DEFINE([USE_IKEV2], [], [support for IKEv2 protocol]) fi +if test x$fuzzing = xtrue; then + AC_DEFINE([USE_FUZZING], [], [build code for fuzzing]) +fi # ==================================================== # options for enabled modules (see conf/Makefile.am) diff --git a/src/libstrongswan/plugins/x509/x509_cert.c b/src/libstrongswan/plugins/x509/x509_cert.c index 3027e4387..19ba1f863 100644 --- a/src/libstrongswan/plugins/x509/x509_cert.c +++ b/src/libstrongswan/plugins/x509/x509_cert.c @@ -217,6 +217,29 @@ struct private_x509_cert_t { refcount_t ref; }; +/** + * Convert a generalName to a string + */ +static bool gn_to_string(identification_t *id, char **uri) +{ + int len; + +#ifdef USE_FUZZING + chunk_t proper; + chunk_printable(id->get_encoding(id), &proper, '?'); + len = asprintf(uri, "%.*s", (int)proper.len, proper.ptr); + chunk_free(&proper); +#else + len = asprintf(uri, "%Y", id); +#endif + if (!len) + { + free(*uri); + return FALSE; + } + return len > 0; +} + /** * Destroy a CertificateDistributionPoint */ @@ -649,7 +672,7 @@ static bool parse_authorityInfoAccess(chunk_t blob, int level0, } DBG2(DBG_ASN, " '%Y'", id); if (accessMethod == OID_OCSP && - asprintf(&uri, "%Y", id) > 0) + gn_to_string(id, &uri)) { this->ocsp_uris->insert_last(this->ocsp_uris, uri); } @@ -818,12 +841,10 @@ static void add_cdps(linked_list_t *list, linked_list_t *uris, enumerator_t *enumerator; x509_cdp_t *cdp; char *uri; - int len; while (uris->remove_last(uris, (void**)&id) == SUCCESS) { - len = asprintf(&uri, "%Y", id); - if (len > 0) + if (gn_to_string(id, &uri)) { if (issuers->get_count(issuers)) { @@ -847,10 +868,6 @@ static void add_cdps(linked_list_t *list, linked_list_t *uris, list->insert_last(list, cdp); } } - else if (!len) - { - free(uri); - } id->destroy(id); } while (issuers->remove_last(issuers, (void**)&id) == SUCCESS) From 3963dbbde513164d95fb069a8d11008e872094bd Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 3 May 2017 17:55:34 +0200 Subject: [PATCH 14/21] plugin-loader: Disable some logging output when building fuzz targets This avoids evaluating %N. An alternative would be to define a printf-hook for plugin features. --- src/libstrongswan/plugins/plugin_loader.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/libstrongswan/plugins/plugin_loader.c b/src/libstrongswan/plugins/plugin_loader.c index e78762b2c..4daf3f13e 100644 --- a/src/libstrongswan/plugins/plugin_loader.c +++ b/src/libstrongswan/plugins/plugin_loader.c @@ -730,9 +730,11 @@ static bool load_dependencies(private_plugin_loader_t *this, if (!find_compatible_feature(this, &provided->feature[i])) { - char *name, *provide, *depend; bool soft = provided->feature[i].kind == FEATURE_SDEPEND; +#ifndef USE_FUZZING + char *name, *provide, *depend; + name = provided->entry->plugin->get_name(provided->entry->plugin); provide = plugin_feature_get_string(&provided->feature[0]); depend = plugin_feature_get_string(&provided->feature[i]); @@ -753,6 +755,8 @@ static bool load_dependencies(private_plugin_loader_t *this, } free(provide); free(depend); +#endif /* !USE_FUZZING */ + if (soft) { /* it's ok if we can't resolve soft dependencies */ continue; @@ -772,8 +776,6 @@ static void load_feature(private_plugin_loader_t *this, { if (load_dependencies(this, provided, level)) { - char *name, *provide; - if (plugin_feature_load(provided->entry->plugin, provided->feature, provided->reg)) { @@ -783,6 +785,9 @@ static void load_feature(private_plugin_loader_t *this, return; } +#ifndef USE_FUZZING + char *name, *provide; + name = provided->entry->plugin->get_name(provided->entry->plugin); provide = plugin_feature_get_string(&provided->feature[0]); if (provided->entry->critical) @@ -796,6 +801,7 @@ static void load_feature(private_plugin_loader_t *this, provide, name); } free(provide); +#endif /* !USE_FUZZING */ } else { /* TODO: we could check the current level and set a different flag when @@ -815,13 +821,16 @@ static void load_provided(private_plugin_loader_t *this, provided_feature_t *provided, int level) { - char *name, *provide; int indent = level * 2; if (provided->loaded || provided->failed) { return; } + +#ifndef USE_FUZZING + char *name, *provide; + name = provided->entry->plugin->get_name(provided->entry->plugin); provide = plugin_feature_get_string(provided->feature); if (provided->loading) @@ -834,6 +843,12 @@ static void load_provided(private_plugin_loader_t *this, DBG3(DBG_LIB, "%*sloading feature %s in plugin '%s'", indent, "", provide, name); free(provide); +#else + if (provided->loading) + { + return; + } +#endif /* USE_FUZZING */ provided->loading = TRUE; load_feature(this, provided, level + 1); From f5aef3a02028fa986e80187872656b52b3249a6a Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 4 May 2017 16:10:00 +0200 Subject: [PATCH 15/21] configure: Don't modify CFLAGs if fuzzing is enabled Just rely on the flags passed by the build process. --- configure.ac | 5 ----- 1 file changed, 5 deletions(-) diff --git a/configure.ac b/configure.ac index 827c6913e..30edbbac4 100644 --- a/configure.ac +++ b/configure.ac @@ -1266,11 +1266,6 @@ if test x$coverage = xtrue; then CFLAGS="${CFLAGS} -g -O0" fi -if test x$fuzzing = xtrue; then - AC_MSG_NOTICE([fuzzing enabled, modifying CFLAGS]) - CFLAGS="${CFLAGS} -fsanitize-coverage=trace-pc-guard -fsanitize=address" -fi - if test x$ruby_gems = xtrue; then AC_PATH_PROG([GEM], [gem], [], [$PATH:/bin:/usr/bin:/usr/local/bin]) if test x$GEM = x; then From aed77b096191f395e347c4aa00cbb7797c03d0f6 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 4 May 2017 16:16:33 +0200 Subject: [PATCH 16/21] chunk: Correctly parse Base64 text where four = follow in a row That's not correct Base64 but invalid data could trigger this. Since outlen would get reduced four times, but is only ever increased three times per iteration, this could result in an integer underflow and then a potential buffer overflow. --- src/libstrongswan/utils/chunk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstrongswan/utils/chunk.c b/src/libstrongswan/utils/chunk.c index 0c50ab788..8f4b7efff 100644 --- a/src/libstrongswan/utils/chunk.c +++ b/src/libstrongswan/utils/chunk.c @@ -643,7 +643,7 @@ chunk_t chunk_from_base64(chunk_t base64, char *buf) outlen += 3; for (j = 0; j < 4; j++) { - if (*pos == '=') + if (*pos == '=' && outlen > 0) { outlen--; } From b72718f4b808fd6d9bd7c8ed1be0a6d593bde732 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 5 May 2017 09:01:51 +0200 Subject: [PATCH 17/21] pem: Ensure a value before checking Proc-Type in PEM header --- src/libstrongswan/plugins/pem/pem_builder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstrongswan/plugins/pem/pem_builder.c b/src/libstrongswan/plugins/pem/pem_builder.c index cda703741..ec90fb084 100644 --- a/src/libstrongswan/plugins/pem/pem_builder.c +++ b/src/libstrongswan/plugins/pem/pem_builder.c @@ -250,7 +250,7 @@ static status_t pem_to_bin(chunk_t *blob, bool *pgp) { continue; } - if (match("Proc-Type", &name) && *value.ptr == '4') + if (match("Proc-Type", &name) && value.len && *value.ptr == '4') { encrypted = TRUE; } From 9c42126297a4d8db1b6800cdbe7be26c3a63a984 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Sat, 20 May 2017 13:04:56 +0200 Subject: [PATCH 18/21] x509: Fix leak when parsing CDPs if an invalid one follows valid ones --- src/libstrongswan/plugins/x509/x509_cert.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstrongswan/plugins/x509/x509_cert.c b/src/libstrongswan/plugins/x509/x509_cert.c index 19ba1f863..ee630eee0 100644 --- a/src/libstrongswan/plugins/x509/x509_cert.c +++ b/src/libstrongswan/plugins/x509/x509_cert.c @@ -923,8 +923,8 @@ bool x509_parse_crlDistributionPoints(chunk_t blob, int level0, end: parser->destroy(parser); - uris->destroy(uris); - issuers->destroy(issuers); + uris->destroy_offset(uris, offsetof(identification_t, destroy)); + issuers->destroy_offset(issuers, offsetof(identification_t, destroy)); return success; } From 411bda68362685e2830b2e26df9f661eb69f8cc3 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 23 May 2017 12:19:48 +0200 Subject: [PATCH 19/21] asn1: Make sure the first argument to sscanf() is null-terminated --- src/libstrongswan/asn1/asn1.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/libstrongswan/asn1/asn1.c b/src/libstrongswan/asn1/asn1.c index 5ce840325..8b9dc1c48 100644 --- a/src/libstrongswan/asn1/asn1.c +++ b/src/libstrongswan/asn1/asn1.c @@ -350,13 +350,15 @@ time_t asn1_to_time(const chunk_t *utctime, asn1_t type) int tm_leap_4, tm_leap_100, tm_leap_400, tm_leap; int tz_hour, tz_min, tz_offset; time_t tm_days, tm_secs; - u_char *eot = NULL; + char buf[BUF_LEN], *eot = NULL; - if ((eot = memchr(utctime->ptr, 'Z', utctime->len)) != NULL) + snprintf(buf, sizeof(buf), "%.*s", (int)utctime->len, utctime->ptr); + + if ((eot = strchr(buf, 'Z')) != NULL) { tz_offset = 0; /* Zulu time with a zero time zone offset */ } - else if ((eot = memchr(utctime->ptr, '+', utctime->len)) != NULL) + else if ((eot = strchr(buf, '+')) != NULL) { if (sscanf(eot+1, "%2d%2d", &tz_hour, &tz_min) != 2) { @@ -364,7 +366,7 @@ time_t asn1_to_time(const chunk_t *utctime, asn1_t type) } tz_offset = 3600*tz_hour + 60*tz_min; /* positive time zone offset */ } - else if ((eot = memchr(utctime->ptr, '-', utctime->len)) != NULL) + else if ((eot = strchr(buf, '-')) != NULL) { if (sscanf(eot+1, "%2d%2d", &tz_hour, &tz_min) != 2) { @@ -382,15 +384,15 @@ time_t asn1_to_time(const chunk_t *utctime, asn1_t type) const char* format = (type == ASN1_UTCTIME)? "%2d%2d%2d%2d%2d": "%4d%2d%2d%2d%2d"; - if (sscanf(utctime->ptr, format, &tm_year, &tm_mon, &tm_day, - &tm_hour, &tm_min) != 5) + if (sscanf(buf, format, &tm_year, &tm_mon, &tm_day, + &tm_hour, &tm_min) != 5) { return 0; /* error in [yy]yymmddhhmm time format */ } } /* is there a seconds field? */ - if ((eot - utctime->ptr) == ((type == ASN1_UTCTIME)?12:14)) + if ((eot - buf) == ((type == ASN1_UTCTIME)?12:14)) { if (sscanf(eot-2, "%2d", &tm_sec) != 1) { From c0017166420aec2d41406eb0583d76f5e189f683 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 23 May 2017 12:24:01 +0200 Subject: [PATCH 20/21] libimcv: Make sure the first argument to sscanf() is null-terminated --- src/libimcv/ietf/ietf_attr_op_status.c | 4 +++- src/libimcv/tcg/pts/tcg_pts_attr_simple_comp_evid.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libimcv/ietf/ietf_attr_op_status.c b/src/libimcv/ietf/ietf_attr_op_status.c index f04c89b96..1f813b3c6 100644 --- a/src/libimcv/ietf/ietf_attr_op_status.c +++ b/src/libimcv/ietf/ietf_attr_op_status.c @@ -170,6 +170,7 @@ METHOD(pa_tnc_attr_t, process, status_t, chunk_t last_use; uint16_t reserved; struct tm t; + char buf[BUF_LEN]; *offset = 0; @@ -208,7 +209,8 @@ METHOD(pa_tnc_attr_t, process, status_t, *offset = 4; /* Conversion from RFC 3339 ASCII string to time_t */ - if (sscanf(last_use.ptr, "%4d-%2d-%2dT%2d:%2d:%2dZ", &t.tm_year, &t.tm_mon, + snprintf(buf, sizeof(buf), "%.*s", (int)last_use.len, last_use.ptr); + if (sscanf(buf, "%4d-%2d-%2dT%2d:%2d:%2dZ", &t.tm_year, &t.tm_mon, &t.tm_mday, &t.tm_hour, &t.tm_min, &t.tm_sec) != 6) { DBG1(DBG_TNC, "invalid last_use time format in IETF operational status"); diff --git a/src/libimcv/tcg/pts/tcg_pts_attr_simple_comp_evid.c b/src/libimcv/tcg/pts/tcg_pts_attr_simple_comp_evid.c index c249ca151..9438fa062 100644 --- a/src/libimcv/tcg/pts/tcg_pts_attr_simple_comp_evid.c +++ b/src/libimcv/tcg/pts/tcg_pts_attr_simple_comp_evid.c @@ -263,13 +263,15 @@ bool measurement_time_from_utc(time_t *measurement_time, chunk_t utc_time) { int tm_year, tm_mon, tm_day, tm_days, tm_hour, tm_min, tm_sec, tm_secs; int tm_leap_4, tm_leap_100, tm_leap_400, tm_leap; + char buf[BUF_LEN]; if (memeq(utc_undefined_time_str, utc_time.ptr, utc_time.len)) { *measurement_time = 0; return TRUE; } - if (sscanf(utc_time.ptr, "%4d-%2d-%2dT%2d:%2d:%2dZ", + snprintf(buf, sizeof(buf), "%.*s", (int)utc_time.len, utc_time.ptr); + if (sscanf(buf, "%4d-%2d-%2dT%2d:%2d:%2dZ", &tm_year, &tm_mon, &tm_day, &tm_hour, &tm_min, &tm_sec) != 6) { return FALSE; From a9b698f5be2519353d91cd6be52b97ce7f5d6fe6 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 23 May 2017 12:37:05 +0200 Subject: [PATCH 21/21] tnc-ifmap: Null-terminate buffer to make sscanf()-calls safe --- src/libcharon/plugins/tnc_ifmap/tnc_ifmap_soap_msg.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libcharon/plugins/tnc_ifmap/tnc_ifmap_soap_msg.c b/src/libcharon/plugins/tnc_ifmap/tnc_ifmap_soap_msg.c index b86288683..db19bd575 100644 --- a/src/libcharon/plugins/tnc_ifmap/tnc_ifmap_soap_msg.c +++ b/src/libcharon/plugins/tnc_ifmap/tnc_ifmap_soap_msg.c @@ -55,7 +55,7 @@ struct private_tnc_ifmap_soap_msg_t { static xmlNodePtr find_child(xmlNodePtr parent, const xmlChar* name) { xmlNodePtr child; - + child = parent->xmlChildrenNode; while (child) { @@ -80,7 +80,7 @@ METHOD(tnc_ifmap_soap_msg_t, post, bool, xmlChar *xml_str, *errorCode, *errorString; int xml_len, len, written; chunk_t xml, http; - char buf[4096]; + char buf[4096] = { 0 }; status_t status; DBG2(DBG_TNC, "sending ifmap %s", request->name); @@ -131,7 +131,8 @@ METHOD(tnc_ifmap_soap_msg_t, post, bool, xml = chunk_empty; do { - len = this->tls->read(this->tls, buf, sizeof(buf), TRUE); + /* reduce size so the buffer is null-terminated */ + len = this->tls->read(this->tls, buf, sizeof(buf)-1, TRUE); if (len <= 0) { return FALSE; @@ -150,7 +151,7 @@ METHOD(tnc_ifmap_soap_msg_t, post, bool, DBG3(DBG_TNC, "parsing XML message %B", &xml); this->doc = xmlParseMemory(xml.ptr, xml.len); free(xml.ptr); - + if (!this->doc) { DBG1(DBG_TNC, "failed to parse XML message");