From 778aca3d251b6a563ffbabef95816fab863825e1 Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Tue, 27 Jun 2017 20:00:47 +0800 Subject: [PATCH 01/31] crypto: correct typ0 in error message Signed-off-by: Antonio Quartulli Acked-by: Steffan Karger Message-Id: <20170627120047.12304-1-a@unstable.cc> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14975.html Signed-off-by: Gert Doering --- src/openvpn/crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index 191fee8e..9f2828a4 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -1261,7 +1261,7 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags) fd = platform_open(file, O_RDONLY, 0); if (fd == -1) { - msg(M_ERR, "Cannot open file key file '%s'", file); + msg(M_ERR, "Cannot open key file '%s'", file); } size = read(fd, in.data, in.capacity); if (size < 0) From 26345ba61b8d5bccb1331894ab6d1468e3b09adf Mon Sep 17 00:00:00 2001 From: Arne Schwabe Date: Mon, 26 Jun 2017 13:13:26 +0200 Subject: [PATCH 02/31] Set tls-cipher restriction before loading certificates OpenSSL 1.1 does not allow MD5 signed certificates by default anymore. This can be enabled again by settings tls-cipher "DEFAULT:@SECLEVEL=0" but only if the cipher list is set before loading the certificates. This patch changes the order of loading. Acked-by: Christian Hesse Acked-by: Steffan Karger Message-Id: <1498475606-8337-1-git-send-email-arne@rfc2549.org> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14961.html Signed-off-by: Gert Doering --- src/openvpn/ssl.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 29280dce..9ca300c0 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -616,6 +616,11 @@ init_ssl(const struct options *options, struct tls_root_ctx *new_ctx) tls_ctx_client_new(new_ctx); } + /* Allowable ciphers */ + /* Since @SECLEVEL also influces loading of certificates, set the + * cipher restrictions before loading certificates */ + tls_ctx_restrict_ciphers(new_ctx, options->cipher_list); + tls_ctx_set_options(new_ctx, options->ssl_flags); if (options->pkcs12_file) @@ -708,9 +713,6 @@ init_ssl(const struct options *options, struct tls_root_ctx *new_ctx) tls_ctx_load_ecdh_params(new_ctx, options->ecdh_curve); } - /* Allowable ciphers */ - tls_ctx_restrict_ciphers(new_ctx, options->cipher_list); - #ifdef ENABLE_CRYPTO_MBEDTLS /* Personalise the random by mixing in the certificate */ tls_ctx_personalise_random(new_ctx); From 5e6e4b7d21150ea2f0738948d5a9bd0c7d910e1a Mon Sep 17 00:00:00 2001 From: Steffan Karger Date: Mon, 19 Jun 2017 13:51:05 +0200 Subject: [PATCH 03/31] init_key_ctx: key and iv arguments can (now) be const In older OpenSSL, the key and iv arguments of EVP_CipherInit_ex() were not const, which meant that our API could not be const either. Since we dropped support for OpenSSL 0.9.8, we can now fix our internal API. Signed-off-by: Steffan Karger Acked-by: Antonio Quartulli Message-Id: <1497873065-2229-1-git-send-email-steffan.karger@fox-it.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14881.html Signed-off-by: Gert Doering --- src/openvpn/crypto.c | 2 +- src/openvpn/crypto.h | 2 +- src/openvpn/crypto_backend.h | 4 ++-- src/openvpn/crypto_mbedtls.c | 4 ++-- src/openvpn/crypto_openssl.c | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index 9f2828a4..78ca4197 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -820,7 +820,7 @@ init_key_type(struct key_type *kt, const char *ciphername, /* given a key and key_type, build a key_ctx */ void -init_key_ctx(struct key_ctx *ctx, struct key *key, +init_key_ctx(struct key_ctx *ctx, const struct key *key, const struct key_type *kt, int enc, const char *prefix) { diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h index 8e2f2b15..fec2eea7 100644 --- a/src/openvpn/crypto.h +++ b/src/openvpn/crypto.h @@ -312,7 +312,7 @@ void init_key_type(struct key_type *kt, const char *ciphername, * Key context functions */ -void init_key_ctx(struct key_ctx *ctx, struct key *key, +void init_key_ctx(struct key_ctx *ctx, const struct key *key, const struct key_type *kt, int enc, const char *prefix); diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h index b7f519b5..567fd9b2 100644 --- a/src/openvpn/crypto_backend.h +++ b/src/openvpn/crypto_backend.h @@ -323,7 +323,7 @@ void cipher_ctx_free(cipher_ctx_t *ctx); * @param enc Whether to encrypt or decrypt (either * \c MBEDTLS_OP_ENCRYPT or \c MBEDTLS_OP_DECRYPT). */ -void cipher_ctx_init(cipher_ctx_t *ctx, uint8_t *key, int key_len, +void cipher_ctx_init(cipher_ctx_t *ctx, const uint8_t *key, int key_len, const cipher_kt_t *kt, int enc); /** @@ -391,7 +391,7 @@ const cipher_kt_t *cipher_ctx_get_cipher_kt(const cipher_ctx_t *ctx); * * @return \c 0 on failure, \c 1 on success. */ -int cipher_ctx_reset(cipher_ctx_t *ctx, uint8_t *iv_buf); +int cipher_ctx_reset(cipher_ctx_t *ctx, const uint8_t *iv_buf); /** * Updates the given cipher context, providing additional data (AD) for diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c index 24bc3158..30b51a5f 100644 --- a/src/openvpn/crypto_mbedtls.c +++ b/src/openvpn/crypto_mbedtls.c @@ -523,7 +523,7 @@ cipher_ctx_free(mbedtls_cipher_context_t *ctx) } void -cipher_ctx_init(mbedtls_cipher_context_t *ctx, uint8_t *key, int key_len, +cipher_ctx_init(mbedtls_cipher_context_t *ctx, const uint8_t *key, int key_len, const mbedtls_cipher_info_t *kt, const mbedtls_operation_t operation) { ASSERT(NULL != kt && NULL != ctx); @@ -597,7 +597,7 @@ cipher_ctx_get_cipher_kt(const cipher_ctx_t *ctx) } int -cipher_ctx_reset(mbedtls_cipher_context_t *ctx, uint8_t *iv_buf) +cipher_ctx_reset(mbedtls_cipher_context_t *ctx, const uint8_t *iv_buf) { if (!mbed_ok(mbedtls_cipher_reset(ctx))) { diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index 9cf3355b..138455a4 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -665,7 +665,7 @@ cipher_ctx_free(EVP_CIPHER_CTX *ctx) } void -cipher_ctx_init(EVP_CIPHER_CTX *ctx, uint8_t *key, int key_len, +cipher_ctx_init(EVP_CIPHER_CTX *ctx, const uint8_t *key, int key_len, const EVP_CIPHER *kt, int enc) { ASSERT(NULL != kt && NULL != ctx); @@ -732,7 +732,7 @@ cipher_ctx_get_cipher_kt(const cipher_ctx_t *ctx) int -cipher_ctx_reset(EVP_CIPHER_CTX *ctx, uint8_t *iv_buf) +cipher_ctx_reset(EVP_CIPHER_CTX *ctx, const uint8_t *iv_buf) { return EVP_CipherInit_ex(ctx, NULL, NULL, NULL, iv_buf, -1); } From 9fc0e963c757ffec3cc9fbf797fb7609f409c370 Mon Sep 17 00:00:00 2001 From: Steffan Karger Date: Wed, 21 Jun 2017 23:10:43 +0200 Subject: [PATCH 04/31] Move adjust_power_of_2() to integer.h misc.c is a mess of incoherent functions, and is therefore included by virtually all our source files. That makes testing harder than it should be. As a first step of cleaning up misc.c, move adjust_power_of_2() to integer.h, which is a more suitable place for a function like this. This allows us to remove the duplicate implementation from test_argv.c. Signed-off-by: Steffan Karger Acked-by: Antonio Quartulli Message-Id: <20170621211043.6490-1-steffan@karger.me> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14940.html Signed-off-by: Gert Doering --- src/openvpn/argv.c | 1 + src/openvpn/integer.h | 18 ++++++++++++++++++ src/openvpn/list.c | 1 + src/openvpn/mbuf.c | 1 + src/openvpn/misc.c | 18 ------------------ src/openvpn/misc.h | 2 -- tests/unit_tests/openvpn/test_argv.c | 18 ------------------ 7 files changed, 21 insertions(+), 38 deletions(-) diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c index a71d261c..95bdfeac 100644 --- a/src/openvpn/argv.c +++ b/src/openvpn/argv.c @@ -36,6 +36,7 @@ #include "syshead.h" #include "argv.h" +#include "integer.h" #include "options.h" static void diff --git a/src/openvpn/integer.h b/src/openvpn/integer.h index 240781b7..9bb00a38 100644 --- a/src/openvpn/integer.h +++ b/src/openvpn/integer.h @@ -118,6 +118,24 @@ modulo_add(int x, int y, int mod) return sum; } +/* + * Return the next largest power of 2 + * or u if u is a power of 2. + */ +static inline size_t +adjust_power_of_2(size_t u) +{ + size_t ret = 1; + + while (ret < u) + { + ret <<= 1; + ASSERT(ret > 0); + } + + return ret; +} + static inline int index_verify(int index, int size, const char *file, int line) { diff --git a/src/openvpn/list.c b/src/openvpn/list.c index edca6f79..91765d20 100644 --- a/src/openvpn/list.c +++ b/src/openvpn/list.c @@ -31,6 +31,7 @@ #if P2MP_SERVER +#include "integer.h" #include "list.h" #include "misc.h" diff --git a/src/openvpn/mbuf.c b/src/openvpn/mbuf.c index fafbce01..f969a2b5 100644 --- a/src/openvpn/mbuf.c +++ b/src/openvpn/mbuf.c @@ -33,6 +33,7 @@ #include "buffer.h" #include "error.h" +#include "integer.h" #include "misc.h" #include "mbuf.h" diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index fbd99385..965869a7 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -1645,24 +1645,6 @@ openvpn_sleep(const int n) sleep(n); } -/* - * Return the next largest power of 2 - * or u if u is a power of 2. - */ -size_t -adjust_power_of_2(size_t u) -{ - size_t ret = 1; - - while (ret < u) - { - ret <<= 1; - ASSERT(ret > 0); - } - - return ret; -} - /* * Remove security-sensitive strings from control message * so that they will not be output to log file. diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index ce965492..3116ec42 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -327,8 +327,6 @@ extern const char *iproute_path; #define SSEC_PW_ENV 3 /* allow calling of built-in programs and user-defined scripts that may receive a password as an environmental variable */ extern int script_security; /* GLOBAL */ -/* return the next largest power of 2 */ -size_t adjust_power_of_2(size_t u); #define COMPAT_FLAG_QUERY 0 /** compat_flags operator: Query for a flag */ #define COMPAT_FLAG_SET (1<<0) /** compat_flags operator: Set a compat flag */ diff --git a/tests/unit_tests/openvpn/test_argv.c b/tests/unit_tests/openvpn/test_argv.c index 8c90eb9c..4a3ba559 100644 --- a/tests/unit_tests/openvpn/test_argv.c +++ b/tests/unit_tests/openvpn/test_argv.c @@ -13,24 +13,6 @@ #include "argv.h" #include "buffer.h" -/* - * This is defined here to prevent #include'ing misc.h - * which makes things difficult beyond any recognition - */ -size_t -adjust_power_of_2(size_t u) -{ - size_t ret = 1; - - while (ret < u) - { - ret <<= 1; - assert(ret > 0); - } - - return ret; -} - /* Defines for use in the tests and the mock parse_line() */ #define PATH1 "/s p a c e" #define PATH2 "/foo bar/baz" From a72d21a56a0223b8a50d05d88af64abcda0fc5dc Mon Sep 17 00:00:00 2001 From: Emmanuel Deloget Date: Thu, 29 Jun 2017 16:21:18 +0200 Subject: [PATCH 05/31] OpenSSL: remove EVP_CIPHER_CTX_new() from the compat layer For unknown reason, the writer of the compat layer seemed to think that this function was only present in OpenSSL 1.1. This is not the case at all, since it has been introduced in OpenSSL before version 0.9.8. Thus, there is no need to add this function to the compat layer, and it can be safely removed. Signed-off-by: Emmanuel Deloget Acked-by: Steffan Karger Message-Id: <20170629142119.29502-1-logout@free.fr> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14989.html Signed-off-by: Gert Doering --- configure.ac | 1 - src/openvpn/openssl_compat.h | 15 --------------- 2 files changed, 16 deletions(-) diff --git a/configure.ac b/configure.ac index 22f91cb6..cb121795 100644 --- a/configure.ac +++ b/configure.ac @@ -919,7 +919,6 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then AC_CHECK_FUNCS( [ \ - EVP_CIPHER_CTX_new \ EVP_CIPHER_CTX_free \ HMAC_CTX_new \ HMAC_CTX_free \ diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h index 617410e0..cd25bd37 100644 --- a/src/openvpn/openssl_compat.h +++ b/src/openvpn/openssl_compat.h @@ -101,21 +101,6 @@ EVP_CIPHER_CTX_free(EVP_CIPHER_CTX *c) } #endif -#if !defined(HAVE_EVP_CIPHER_CTX_NEW) -/** - * Allocate a new cipher context object - * - * @return A zero'ed cipher context object - */ -static inline EVP_CIPHER_CTX * -EVP_CIPHER_CTX_new(void) -{ - EVP_CIPHER_CTX *ctx = NULL; - ALLOC_OBJ_CLEAR(ctx, EVP_CIPHER_CTX); - return ctx; -} -#endif - #if !defined(HAVE_HMAC_CTX_RESET) /** * Reset a HMAC context From 7ee9a94fcbbde941bfed167229a64df0f7cdae0b Mon Sep 17 00:00:00 2001 From: Emmanuel Deloget Date: Thu, 29 Jun 2017 16:21:19 +0200 Subject: [PATCH 06/31] OpenSSL: remove EVP_CIPHER_CTX_free() from the compat layer For unknown reason, the writer of the compat layer seemed to think that this function was only present in OpenSSL 1.1. This is not the case at all, since it has been introduced in OpenSSL before version 0.9.8. Thus, there is no need to add this function to the compat layer, and it can be safely removed. Signed-off-by: Emmanuel Deloget Acked-by: Steffan Karger Message-Id: <20170629142119.29502-2-logout@free.fr> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14988.html Signed-off-by: Gert Doering --- configure.ac | 1 - src/openvpn/openssl_compat.h | 13 ------------- 2 files changed, 14 deletions(-) diff --git a/configure.ac b/configure.ac index cb121795..60bb4658 100644 --- a/configure.ac +++ b/configure.ac @@ -919,7 +919,6 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then AC_CHECK_FUNCS( [ \ - EVP_CIPHER_CTX_free \ HMAC_CTX_new \ HMAC_CTX_free \ HMAC_CTX_reset \ diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h index cd25bd37..36f68b01 100644 --- a/src/openvpn/openssl_compat.h +++ b/src/openvpn/openssl_compat.h @@ -88,19 +88,6 @@ EVP_MD_CTX_new(void) } #endif -#if !defined(HAVE_EVP_CIPHER_CTX_FREE) -/** - * Free an existing cipher context - * - * @param ctx The cipher context - */ -static inline void -EVP_CIPHER_CTX_free(EVP_CIPHER_CTX *c) -{ - free(c); -} -#endif - #if !defined(HAVE_HMAC_CTX_RESET) /** * Reset a HMAC context From 3be9a1c1cd75627c30dca05bed28c84ad4dc1d37 Mon Sep 17 00:00:00 2001 From: Steffan Karger Date: Wed, 28 Jun 2017 00:20:29 +0200 Subject: [PATCH 07/31] Undo cipher push in client options state if cipher is rejected Because of the way we re-use the options parser for both config files and pushed options, we always update the local options state when we accept an option. This resulted in a pushed cipher being rejected the first time it was pushed, but being accepted the second time. This patch is a minimal way to resolve this issue in the master and release/2.4 branches. I'll send a more invasive patch for master, to reset the entire options state on reconnects, later. Trac: #906 Signed-off-by: Steffan Karger Acked-by: Arne Schwabe Acked-by: Gert Doering Message-Id: <20170627222029.26623-1-steffan@karger.me> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14984.html Signed-off-by: Gert Doering --- src/openvpn/ssl.c | 4 +++- src/openvpn/ssl.h | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 9ca300c0..df232894 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1960,7 +1960,7 @@ cleanup: bool tls_session_update_crypto_params(struct tls_session *session, - const struct options *options, struct frame *frame) + struct options *options, struct frame *frame) { if (!session->opt->server && 0 != strcmp(options->ciphername, session->opt->config_ciphername) @@ -1969,6 +1969,8 @@ tls_session_update_crypto_params(struct tls_session *session, msg(D_TLS_ERRORS, "Error: pushed cipher not allowed - %s not in %s or %s", options->ciphername, session->opt->config_ciphername, options->ncp_ciphers); + /* undo cipher push, abort connection setup */ + options->ciphername = session->opt->config_ciphername; return false; } diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 56ea6013..0e0f68fa 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -481,7 +481,7 @@ void tls_update_remote_addr(struct tls_multi *multi, * @return true if updating succeeded, false otherwise. */ bool tls_session_update_crypto_params(struct tls_session *session, - const struct options *options, struct frame *frame); + struct options *options, struct frame *frame); /** * "Poor man's NCP": Use peer cipher if it is an allowed (NCP) cipher. From f9ebfe1b5a011e55fb87a5026b1897c8ffb8f75e Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Wed, 28 Jun 2017 21:15:38 +0200 Subject: [PATCH 08/31] doc: The CRL processing is not a deprecated feature The note related to the CRL processing was somehow put into the deprecated section. This is quite confusing. Since this is a fairly important change, and there have been a noticable amount of supports questions related to OpenVPN not starting due to CRL errors, I put this into the "New features" section labelled as an improvement. Otherwise I fear this would drown in the list of "User-visible Changes" later on. Signed-off-by: David Sommerseth Acked-by: Steffan Karger Message-Id: <20170628191538.9135-1-davids@openvpn.net> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14985.html Signed-off-by: Gert Doering --- Changes.rst | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Changes.rst b/Changes.rst index 9db0a451..0b2b04dd 100644 --- a/Changes.rst +++ b/Changes.rst @@ -44,6 +44,13 @@ ECDH key exchange The TLS control channel now supports for elliptic curve diffie-hellmann key exchange (ECDH). +Improved Certificate Revocation List (CRL) processing + CRLs are now handled by the crypto library (OpenSSL or mbed TLS), instead + of inside OpenVPN itself. The crypto library implementations are more + strict than the OpenVPN implementation was. This might reject peer + certificates that would previously be accepted. If this occurs, OpenVPN + will log the crypto library's error description. + Dualstack round-robin DNS client connect Instead of only using the first address of each ``--remote`` OpenVPN will now try all addresses (IPv6 and IPv4) of a ``--remote`` entry. @@ -160,12 +167,6 @@ Deprecated features will then use ``--key-method 2`` by default. Note that this requires changing the option in both the client and server side configs. -- CRLs are now handled by the crypto library (OpenSSL or mbed TLS), instead of - inside OpenVPN itself. The crypto library implementations are more strict - than the OpenVPN implementation was. This might reject peer certificates - that would previously be accepted. If this occurs, OpenVPN will log the - crypto library's error description. - - ``--tls-remote`` is removed in 2.4, as indicated in the 2.3 man-pages. Similar functionality is provided via ``--verify-x509-name``, which does the same job in a better way. From 56b396dcbc34ffd3cddeb2e65ae55c40eae51831 Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Thu, 13 Jul 2017 16:05:26 +0800 Subject: [PATCH 09/31] use M_ERRNO instead of explicitly printing errno the msg() function will print the errno for us when provided with the M_ERRNO flag. Therefore, don't bother printing errno explicitly and always pass M_ERRNO to msg(). Signed-off-by: Antonio Quartulli Acked-by: Arne Schwabe Message-Id: <20170713080527.13299-1-a@unstable.cc> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15056.html Signed-off-by: Gert Doering --- src/openvpn/manage.c | 14 ++++++-------- src/openvpn/misc.c | 6 ++---- src/openvpn/mtcp.c | 2 +- src/openvpn/mudp.c | 2 +- src/openvpn/multi.c | 2 +- src/openvpn/options.c | 3 +-- src/openvpn/socket.c | 13 +++++-------- src/openvpn/status.c | 2 +- src/openvpn/tun.c | 9 +++------ 9 files changed, 21 insertions(+), 32 deletions(-) diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c index c2e8dc72..39ce8b3b 100644 --- a/src/openvpn/manage.c +++ b/src/openvpn/manage.c @@ -1878,17 +1878,15 @@ man_connect(struct management *man) #if UNIX_SOCK_SUPPORT if (man->settings.flags & MF_UNIX_SOCK) { - msg(D_LINK_ERRORS, - "MANAGEMENT: connect to unix socket %s failed: %s", - sockaddr_unix_name(&man->settings.local_unix, "NULL"), - strerror_ts(status, &gc)); + msg(D_LINK_ERRORS | M_ERRNO, + "MANAGEMENT: connect to unix socket %s failed", + sockaddr_unix_name(&man->settings.local_unix, "NULL")); } else #endif - msg(D_LINK_ERRORS, - "MANAGEMENT: connect to %s failed: %s", - print_sockaddr(man->settings.local->ai_addr, &gc), - strerror_ts(status, &gc)); + msg(D_LINK_ERRORS | M_ERRNO, + "MANAGEMENT: connect to %s failed", + print_sockaddr(man->settings.local->ai_addr, &gc)); throw_signal_soft(SIGTERM, "management-connect-failed"); goto done; } diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index 965869a7..ef779ee3 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -928,10 +928,8 @@ create_temp_file(const char *directory, const char *prefix, struct gc_arena *gc) else if (fd == -1 && errno != EEXIST) { /* Something else went wrong, no need to retry. */ - struct gc_arena gcerr = gc_new(); - msg(M_FATAL, "Could not create temporary file '%s': %s", - retfname, strerror_ts(errno, &gcerr)); - gc_free(&gcerr); + msg(M_FATAL | M_ERRNO, "Could not create temporary file '%s'", + retfname); return NULL; } } diff --git a/src/openvpn/mtcp.c b/src/openvpn/mtcp.c index cb940d8a..851643a9 100644 --- a/src/openvpn/mtcp.c +++ b/src/openvpn/mtcp.c @@ -797,7 +797,7 @@ tunnel_server_tcp(struct context *top) multi.top.c2.inotify_fd = inotify_init(); if (multi.top.c2.inotify_fd < 0) { - msg(D_MULTI_ERRORS, "MULTI: inotify_init error: %s", strerror(errno)); + msg(D_MULTI_ERRORS | M_ERRNO, "MULTI: inotify_init error"); } #endif diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c index 793678d8..eb28ca2b 100644 --- a/src/openvpn/mudp.c +++ b/src/openvpn/mudp.c @@ -325,7 +325,7 @@ tunnel_server_udp_single_threaded(struct context *top) multi.top.c2.inotify_fd = inotify_init(); if (multi.top.c2.inotify_fd < 0) { - msg(D_MULTI_ERRORS, "MULTI: inotify_init error: %s", strerror(errno)); + msg(D_MULTI_ERRORS | M_ERRNO, "MULTI: inotify_init error"); } #endif diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 8d3d67fd..7edfee1b 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -2355,7 +2355,7 @@ multi_process_post(struct multi_context *m, struct multi_instance *mi, const uns } else { - msg(M_NONFATAL, "MULTI: inotify_add_watch error: %s", strerror(errno)); + msg(M_NONFATAL | M_ERRNO, "MULTI: inotify_add_watch error"); } } #endif diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 505c5b2e..1d5b62ca 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3137,8 +3137,7 @@ check_file_access(const int type, const char *file, const int mode, const char * /* Scream if an error is found */ if (errcode > 0) { - msg(M_NOPREFIX|M_OPTERR, "%s fails with '%s': %s", - opt, file, strerror(errno)); + msg(M_NOPREFIX | M_OPTERR | M_ERRNO, "%s fails with '%s'", opt, file); } /* Return true if an error occured */ diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c index 4e7e3f99..a814b952 100644 --- a/src/openvpn/socket.c +++ b/src/openvpn/socket.c @@ -1297,11 +1297,9 @@ socket_bind(socket_descriptor_t sd, } if (bind(sd, cur->ai_addr, cur->ai_addrlen)) { - const int errnum = openvpn_errno(); - msg(M_FATAL, "%s: Socket bind failed on local address %s: %s", + msg(M_FATAL | M_ERRNO, "%s: Socket bind failed on local address %s", prefix, - print_sockaddr_ex(local->ai_addr, ":", PS_SHOW_PORT, &gc), - strerror_ts(errnum, &gc)); + print_sockaddr_ex(local->ai_addr, ":", PS_SHOW_PORT, &gc)); } gc_free(&gc); } @@ -3888,12 +3886,11 @@ socket_bind_unix(socket_descriptor_t sd, if (bind(sd, (struct sockaddr *) local, sizeof(struct sockaddr_un))) { - const int errnum = openvpn_errno(); - msg(M_FATAL, "%s: Socket bind[%d] failed on unix domain socket %s: %s", + msg(M_FATAL | M_ERRNO, + "%s: Socket bind[%d] failed on unix domain socket %s", prefix, (int)sd, - sockaddr_unix_name(local, "NULL"), - strerror_ts(errnum, &gc)); + sockaddr_unix_name(local, "NULL")); } #ifdef HAVE_UMASK diff --git a/src/openvpn/status.c b/src/openvpn/status.c index a1634083..d2f0b133 100644 --- a/src/openvpn/status.c +++ b/src/openvpn/status.c @@ -178,7 +178,7 @@ status_flush(struct status_output *so) const off_t off = lseek(so->fd, (off_t)0, SEEK_CUR); if (ftruncate(so->fd, off) != 0) { - msg(M_WARN, "Failed to truncate status file: %s", strerror(errno)); + msg(M_WARN | M_ERRNO, "Failed to truncate status file"); } } #elif defined(HAVE_CHSIZE) diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 75a156c1..c09f9700 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -3022,16 +3022,14 @@ utun_open_helper(struct ctl_info ctlInfo, int utunnum) if (fd < 0) { - msg(M_INFO, "Opening utun (%s): %s", "socket(SYSPROTO_CONTROL)", - strerror(errno)); + msg(M_INFO | M_ERRNO, "Opening utun (socket(SYSPROTO_CONTROL))"); return -2; } if (ioctl(fd, CTLIOCGINFO, &ctlInfo) == -1) { close(fd); - msg(M_INFO, "Opening utun (%s): %s", "ioctl(CTLIOCGINFO)", - strerror(errno)); + msg(M_INFO | M_ERRNO, "Opening utun (ioctl(CTLIOCGINFO))"); return -2; } @@ -3049,8 +3047,7 @@ utun_open_helper(struct ctl_info ctlInfo, int utunnum) if (connect(fd, (struct sockaddr *)&sc, sizeof(sc)) < 0) { - msg(M_INFO, "Opening utun (%s): %s", "connect(AF_SYS_CONTROL)", - strerror(errno)); + msg(M_INFO | M_ERRNO, "Opening utun (connect(AF_SYS_CONTROL))"); close(fd); return -1; } From e441d861881669c97906652c3278cc9a6c69a417 Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Thu, 13 Jul 2017 16:05:27 +0800 Subject: [PATCH 10/31] don't print errno twice when passing the M_ERRNO flag to msg(), the latter will already print the errno message (in a form of a string and number) for us, hence there is no need to explicitly print it a second time. Signed-off-by: Antonio Quartulli Acked-by: Arne Schwabe Message-Id: <20170713080527.13299-2-a@unstable.cc> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15057.html Signed-off-by: Gert Doering --- src/openvpn/platform.c | 2 +- src/openvpn/tun.c | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c index 2495523f..d936890e 100644 --- a/src/openvpn/platform.c +++ b/src/openvpn/platform.c @@ -159,7 +159,7 @@ platform_nice(int niceval) errno = 0; if (nice(niceval) < 0 && errno != 0) { - msg(M_WARN | M_ERRNO, "WARNING: nice %d failed: %s", niceval, strerror(errno)); + msg(M_WARN | M_ERRNO, "WARNING: nice %d failed", niceval); } else { diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index c09f9700..68fb4889 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -2563,8 +2563,7 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun if (ioctl(tt->fd, TUNGIFINFO, &info) < 0) { - msg(M_WARN | M_ERRNO, "Can't get interface info: %s", - strerror(errno)); + msg(M_WARN | M_ERRNO, "Can't get interface info"); } #ifdef IFF_MULTICAST /* openbsd 4.x doesn't have this */ @@ -2573,8 +2572,7 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun if (ioctl(tt->fd, TUNSIFINFO, &info) < 0) { - msg(M_WARN | M_ERRNO, "Can't set interface info: %s", - strerror(errno)); + msg(M_WARN | M_ERRNO, "Can't set interface info"); } } } @@ -2663,7 +2661,7 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun i = 1; if (ioctl(tt->fd, TUNSIFHEAD, &i) < 0) /* multi-af mode on */ { - msg(M_WARN | M_ERRNO, "ioctl(TUNSIFHEAD): %s", strerror(errno)); + msg(M_WARN | M_ERRNO, "ioctl(TUNSIFHEAD)"); } } } @@ -2796,12 +2794,12 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun if (ioctl(tt->fd, TUNSIFMODE, &i) < 0) { - msg(M_WARN | M_ERRNO, "ioctl(TUNSIFMODE): %s", strerror(errno)); + msg(M_WARN | M_ERRNO, "ioctl(TUNSIFMODE)"); } i = 1; if (ioctl(tt->fd, TUNSIFHEAD, &i) < 0) { - msg(M_WARN | M_ERRNO, "ioctl(TUNSIFHEAD): %s", strerror(errno)); + msg(M_WARN | M_ERRNO, "ioctl(TUNSIFHEAD)"); } } } From 1cdfc9302aad8570360d278aded5fb9f110ca2b6 Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Mon, 10 Jul 2017 12:34:39 +0800 Subject: [PATCH 11/31] ntlm: avoid useless cast The argument passed to my_strupr() is converted to an upper case string by means of toupper(). The latter expects a single signed int as argument, therefore it makes sense to have my_strupr() take a signed argument too and avoid an explicit and an implicit cast. Signed-off-by: Antonio Quartulli Acked-by: Steffan Karger Message-Id: <20170710043441.24770-3-a@unstable.cc> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15031.html Signed-off-by: Gert Doering --- src/openvpn/ntlm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/openvpn/ntlm.c b/src/openvpn/ntlm.c index 0b1163ee..90d48754 100644 --- a/src/openvpn/ntlm.c +++ b/src/openvpn/ntlm.c @@ -130,7 +130,7 @@ gen_nonce(unsigned char *nonce) } void -my_strupr(unsigned char *str) +my_strupr(char *str) { /* converts string to uppercase in place */ @@ -271,7 +271,7 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are int tib_len; /* NTLMv2 hash */ - my_strupr((unsigned char *)strcpy(userdomain, username)); + my_strupr(strcpy(userdomain, username)); if (strlen(username) + strlen(domain) < sizeof(userdomain)) { strcat(userdomain, domain); From ad7f7e56d34bbf477a7e5639f1b78b2c7e58186c Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Wed, 12 Jul 2017 12:30:02 +0800 Subject: [PATCH 12/31] ntlm: unwrap multiple function calls In order to improve code readability it is better to unwrap multiple function calls onto multiple lines. Signed-off-by: Antonio Quartulli Acked-by: Gert Doering Message-Id: <20170712043002.11083-1-a@unstable.cc> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15041.html Signed-off-by: Gert Doering --- src/openvpn/ntlm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/openvpn/ntlm.c b/src/openvpn/ntlm.c index 90d48754..976142dd 100644 --- a/src/openvpn/ntlm.c +++ b/src/openvpn/ntlm.c @@ -271,7 +271,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are int tib_len; /* NTLMv2 hash */ - my_strupr(strcpy(userdomain, username)); + strcpy(userdomain, username); + my_strupr(userdomain); if (strlen(username) + strlen(domain) < sizeof(userdomain)) { strcat(userdomain, domain); From fd2a29ab2668fea9c0ac972d5ec69f00232c88b6 Mon Sep 17 00:00:00 2001 From: Steffan Karger Date: Thu, 20 Jul 2017 13:39:00 +0200 Subject: [PATCH 13/31] Remove strerror_ts() This function was only called in string format functions, which already copy the contents, so all this ever did was adding redundant malloc() and free() calls. Also, this wasn't as thread-safe as it claims: another thread could still change the string value between the strerror() and buf_printf() calls. So, instead of a not needed false sense of thread-safeness, just be honest and use strerror() directly. (I think we should find a better place for everything currently in misc.c, and get rid of it all together. In this case, the better place is /dev/null. This patch is part of that effort.) Signed-off-by: Steffan Karger Acked-by: Gert Doering Message-Id: <1500550740-24773-1-git-send-email-steffan.karger@fox-it.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15105.html Signed-off-by: Gert Doering --- configure.ac | 2 +- src/openvpn/error.c | 15 +++++---------- src/openvpn/manage.c | 5 ++--- src/openvpn/misc.c | 15 --------------- src/openvpn/misc.h | 6 ------ src/openvpn/socket.c | 6 ++---- 6 files changed, 10 insertions(+), 39 deletions(-) diff --git a/configure.ac b/configure.ac index 60bb4658..39d992c0 100644 --- a/configure.ac +++ b/configure.ac @@ -662,7 +662,7 @@ AC_FUNC_FORK AC_CHECK_FUNCS([ \ daemon chroot getpwnam setuid nice system getpid dup dup2 \ - getpass strerror syslog openlog mlockall getgrnam setgid \ + getpass syslog openlog mlockall getgrnam setgid \ setgroups stat flock readv writev time gettimeofday \ ctime memset vsnprintf strdup \ setsid chdir putenv getpeername unlink \ diff --git a/src/openvpn/error.c b/src/openvpn/error.c index ce50ff9e..3817666b 100644 --- a/src/openvpn/error.c +++ b/src/openvpn/error.c @@ -267,7 +267,7 @@ x_msg_va(const unsigned int flags, const char *format, va_list arglist) if ((flags & M_ERRNO) && e) { openvpn_snprintf(m2, ERR_BUF_SIZE, "%s: %s (errno=%d)", - m1, strerror_ts(e, &gc), e); + m1, strerror(e), e); SWAP; } @@ -693,20 +693,15 @@ x_check_status(int status, { if (extended_msg) { - msg(x_cs_info_level, "%s %s [%s]: %s (code=%d)", - description, + msg(x_cs_info_level, "%s %s [%s]: %s (code=%d)", description, sock ? proto2ascii(sock->info.proto, sock->info.af, true) : "", - extended_msg, - strerror_ts(my_errno, &gc), - my_errno); + extended_msg, strerror(my_errno), my_errno); } else { - msg(x_cs_info_level, "%s %s: %s (code=%d)", - description, + msg(x_cs_info_level, "%s %s: %s (code=%d)", description, sock ? proto2ascii(sock->info.proto, sock->info.af, true) : "", - strerror_ts(my_errno, &gc), - my_errno); + strerror(my_errno), my_errno); } if (x_cs_err_delay_ms) diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c index 39ce8b3b..2b85d25c 100644 --- a/src/openvpn/manage.c +++ b/src/openvpn/manage.c @@ -2006,9 +2006,8 @@ man_io_error(struct management *man, const char *prefix) if (!ignore_sys_error(err)) { struct gc_arena gc = gc_new(); - msg(D_MANAGEMENT, "MANAGEMENT: TCP %s error: %s", - prefix, - strerror_ts(err, &gc)); + msg(D_MANAGEMENT, "MANAGEMENT: TCP %s error: %s", prefix, + strerror(err)); gc_free(&gc); return true; } diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index ef779ee3..f6d6c6ad 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -444,21 +444,6 @@ init_random_seed(void) } } -/* thread-safe strerror */ - -const char * -strerror_ts(int errnum, struct gc_arena *gc) -{ -#ifdef HAVE_STRERROR - struct buffer out = alloc_buf_gc(256, gc); - - buf_printf(&out, "%s", openvpn_strerror(errnum, gc)); - return BSTR(&out); -#else - return "[error string unavailable]"; -#endif -} - /* * Set environmental variable (int or string). * diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index 3116ec42..bc267d73 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -95,12 +95,6 @@ openvpn_run_script(const struct argv *a, const struct env_set *es, const unsigne } -#ifdef HAVE_STRERROR -/* a thread-safe version of strerror */ -const char *strerror_ts(int errnum, struct gc_arena *gc); - -#endif - /* Set standard file descriptors to /dev/null */ void set_std_files_to_null(bool stdin_only); diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c index a814b952..846df04b 100644 --- a/src/openvpn/socket.c +++ b/src/openvpn/socket.c @@ -1473,10 +1473,8 @@ socket_connect(socket_descriptor_t *sd, if (status) { - msg(D_LINK_ERRORS, - "TCP: connect to %s failed: %s", - print_sockaddr(dest, &gc), - strerror_ts(status, &gc)); + msg(D_LINK_ERRORS, "TCP: connect to %s failed: %s", + print_sockaddr(dest, &gc), strerror(status)); openvpn_close_socket(*sd); *sd = SOCKET_UNDEFINED; From 45b2af9c7719d9a40c6c2b9d0693e4db0d917a04 Mon Sep 17 00:00:00 2001 From: Steffan Karger Date: Thu, 20 Jul 2017 18:00:35 +0200 Subject: [PATCH 14/31] Move openvpn_sleep() to manage.c openvpn_sleep() is basically "service the management interface for x seconds, then return". Therefore, manage.c is a more suitable location than the random collection of unrelated stuff called misc.c. (I think we should find a better place for everything currently in misc.c, and get rid of it all together. This patch is part of that effort.) Signed-off-by: Steffan Karger Acked-by: Gert Doering Message-Id: <1500566435-29920-1-git-send-email-steffan.karger@fox-it.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15109.html Signed-off-by: Gert Doering --- src/openvpn/forward.c | 2 +- src/openvpn/init.c | 4 ++-- src/openvpn/manage.c | 22 +++++++++++++++++++--- src/openvpn/manage.h | 7 +++++++ src/openvpn/misc.c | 13 ------------- src/openvpn/misc.h | 6 ------ src/openvpn/socket.c | 8 ++++---- 7 files changed, 33 insertions(+), 29 deletions(-) diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 371ddca5..c45d1259 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -756,7 +756,7 @@ read_incoming_link(struct context *c) if (event_timeout_defined(&c->c2.explicit_exit_notification_interval)) { msg(D_STREAM_ERRORS, "Connection reset during exit notification period, ignoring [%d]", status); - openvpn_sleep(1); + management_sleep(1); } else #endif diff --git a/src/openvpn/init.c b/src/openvpn/init.c index a54307ad..bc3b81e3 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -1969,7 +1969,7 @@ do_up(struct context *c, bool pulled_options, unsigned int option_types_found) /* if so, close tun, delete routes, then reinitialize tun and add routes */ msg(M_INFO, "NOTE: Pulled options changed on restart, will need to close and reopen TUN/TAP device."); do_close_tun(c, true); - openvpn_sleep(1); + management_sleep(1); c->c2.did_open_tun = do_open_tun(c); update_time(); } @@ -2263,7 +2263,7 @@ socket_restart_pause(struct context *c) if (sec) { msg(D_RESTART, "Restart pause, %d second(s)", sec); - openvpn_sleep(sec); + management_sleep(sec); } } diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c index 2b85d25c..13be6f6d 100644 --- a/src/openvpn/manage.c +++ b/src/openvpn/manage.c @@ -3997,9 +3997,25 @@ log_history_ref(const struct log_history *h, const int index) } } -#else /* ifdef ENABLE_MANAGEMENT */ -static void -dummy(void) +void +management_sleep(const int n) { + if (management) + { + management_event_loop_n_seconds(management, n); + } + else + { + sleep(n); + } } + +#else /* ifdef ENABLE_MANAGEMENT */ + +void +management_sleep(const int n) +{ + sleep(n); +} + #endif /* ENABLE_MANAGEMENT */ diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h index 542cc07a..676be640 100644 --- a/src/openvpn/manage.h +++ b/src/openvpn/manage.h @@ -605,4 +605,11 @@ management_bytes_server(struct management *man, #endif /* MANAGEMENT_DEF_AUTH */ #endif /* ifdef ENABLE_MANAGEMENT */ + +/** + * A sleep function that services the management layer for n seconds rather + * than doing nothing. + */ +void management_sleep(const int n); + #endif /* ifndef MANAGE_H */ diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index f6d6c6ad..ae96aa69 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -1615,19 +1615,6 @@ make_extended_arg_array(char **p, struct gc_arena *gc) } } -void -openvpn_sleep(const int n) -{ -#ifdef ENABLE_MANAGEMENT - if (management) - { - management_event_loop_n_seconds(management, n); - return; - } -#endif - sleep(n); -} - /* * Remove security-sensitive strings from control message * so that they will not be output to log file. diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index bc267d73..32b64e8b 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -292,12 +292,6 @@ bool env_safe_to_print(const char *str); /* returns true if environmental variable may be passed to an external program */ bool env_allowed(const char *str); -/* - * A sleep function that services the management layer for n - * seconds rather than doing nothing. - */ -void openvpn_sleep(const int n); - void configure_path(void); const char *sanitize_control_message(const char *str, struct gc_arena *gc); diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c index 846df04b..6e01fe10 100644 --- a/src/openvpn/socket.c +++ b/src/openvpn/socket.c @@ -496,7 +496,7 @@ openvpn_getaddrinfo(unsigned int flags, goto done; } - openvpn_sleep(fail_wait_interval); + management_sleep(fail_wait_interval); } ASSERT(res); @@ -1193,7 +1193,7 @@ socket_listen_accept(socket_descriptor_t sd, if (status <= 0) { - openvpn_sleep(1); + management_sleep(1); continue; } @@ -1228,7 +1228,7 @@ socket_listen_accept(socket_descriptor_t sd, break; } } - openvpn_sleep(1); + management_sleep(1); } if (!nowait && openvpn_close_socket(sd)) @@ -1374,7 +1374,7 @@ openvpn_connect(socket_descriptor_t sd, #endif break; } - openvpn_sleep(1); + management_sleep(1); continue; } From cdb262a6c78a29349789b7cf1813feaf7cc6e8c8 Mon Sep 17 00:00:00 2001 From: Steffan Karger Date: Thu, 20 Jul 2017 21:17:02 +0200 Subject: [PATCH 15/31] fixup: also change missed openvpn_sleep() occurrences 45b2af9c missed some openvpn_sleep() occurrences in platform-specific code in tun.c - fix that. Signed-off-by: Steffan Karger Acked-by: Gert Doering Message-Id: <1500578222-21689-1-git-send-email-steffan@karger.me> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15111.html Signed-off-by: Gert Doering --- src/openvpn/tun.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 68fb4889..3639718c 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -1862,7 +1862,7 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun if (oldtunfd >=0 && android_method == ANDROID_OPEN_AFTER_CLOSE) { close(oldtunfd); - openvpn_sleep(2); + management_sleep(2); } if (oldtunfd >=0 && android_method == ANDROID_KEEP_OLD_TUN) @@ -4999,7 +4999,7 @@ netsh_command(const struct argv *a, int n, int msglevel) for (i = 0; i < n; ++i) { bool status; - openvpn_sleep(1); + management_sleep(1); netcmd_semaphore_lock(); argv_msg_prefix(M_INFO, a, "NETSH"); status = openvpn_execve_check(a, NULL, 0, "ERROR: netsh command failed"); @@ -5008,7 +5008,7 @@ netsh_command(const struct argv *a, int n, int msglevel) { return; } - openvpn_sleep(4); + management_sleep(4); } msg(msglevel, "NETSH: command failed"); } @@ -5991,7 +5991,7 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun if (s > 0) { msg(M_INFO, "Sleeping for %d seconds...", s); - openvpn_sleep(s); + management_sleep(s); } } From 20d98427ef37e3b748dbcca2174cd243dcc963dc Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Thu, 20 Jul 2017 16:23:38 +0800 Subject: [PATCH 16/31] route: improve error message - fix typ0 in message: NLSMG -> NLMSG - use strerror() to print a human readable message - don't print error message if error is ENETUNREACH: it means no route found Signed-off-by: Antonio Quartulli Acked-by: Gert Doering Message-Id: <20170720082338.1302-1-a@unstable.cc> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15101.html Signed-off-by: Gert Doering --- src/openvpn/route.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/openvpn/route.c b/src/openvpn/route.c index a1811f41..a8a4c66d 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -3441,7 +3441,14 @@ get_default_gateway_ipv6(struct route_ipv6_gateway_info *rgi6, if (nh->nlmsg_type == NLMSG_ERROR) { struct nlmsgerr *ne = (struct nlmsgerr *)NLMSG_DATA(nh); - msg(M_WARN, "GDG6: NLSMG_ERROR: error %d\n", ne->error); + + /* since linux-4.11 -ENETUNREACH is returned when no route can be + * found. Don't print any error message in this case */ + if (ne->error != -ENETUNREACH) + { + msg(M_WARN, "GDG6: NLMSG_ERROR: error %s\n", + strerror(-ne->error)); + } break; } From 3322c558fa742cb823fa919f682486973abc4f8e Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Fri, 7 Jul 2017 22:01:08 +0800 Subject: [PATCH 17/31] management: preserve wait_for_push field when asking for user/pass With the introduction of the wait_for_push field in the auth_user_pass structure, we have to make sure that such field is not accidentally erased when the management asks the user for user/pass. Erasing such field would mess up the logic introduced by ("Ignore auth-nocache for auth-user-pass if auth-token is pushed"). Thanks to David Sommerseth for the preliminary analysis and debugging. Reported-by: Steven Haigh Signed-off-by: Antonio Quartulli Tested-by: Steven Haigh Acked-by: David Sommerseth Message-Id: <20170707140108.31612-1-a@unstable.cc> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15015.html Signed-off-by: David Sommerseth --- src/openvpn/manage.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c index 13be6f6d..ff948240 100644 --- a/src/openvpn/manage.c +++ b/src/openvpn/manage.c @@ -3501,7 +3501,9 @@ management_query_user_pass(struct management *man, */ if (ret) { - man->connection.up_query.nocache = up->nocache; /* preserve caller's nocache setting */ + /* preserve caller's settings */ + man->connection.up_query.nocache = up->nocache; + man->connection.up_query.wait_for_push = up->wait_for_push; *up = man->connection.up_query; } secure_memzero(&man->connection.up_query, sizeof(man->connection.up_query)); From 2dfbf62b6ace1eb39f1ae7126bc5530a541bed58 Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Fri, 7 Jul 2017 18:22:38 +0800 Subject: [PATCH 18/31] tls-crypt: avoid warnings when --disable-crypto is used Avoid including the content of tls_crypt.h when --disable-crypto is used, as it will trigger some warnings due to missing structures declarations. Signed-off-by: Antonio Quartulli Acked-by: Steffan Karger Acked-by: David Sommerseth Message-Id: <20170707102238.8781-1-a@unstable.cc> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15014.html Signed-off-by: David Sommerseth --- src/openvpn/tls_crypt.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/openvpn/tls_crypt.h b/src/openvpn/tls_crypt.h index e8080df9..4071ac94 100644 --- a/src/openvpn/tls_crypt.h +++ b/src/openvpn/tls_crypt.h @@ -74,6 +74,8 @@ #ifndef TLSCRYPT_H #define TLSCRYPT_H +#ifdef ENABLE_CRYPTO + #include "buffer.h" #include "crypto.h" #include "session_id.h" @@ -140,4 +142,6 @@ bool tls_crypt_unwrap(const struct buffer *src, struct buffer *dst, /** @} */ +#endif /* ENABLE_CRYPTO */ + #endif /* TLSCRYPT_H */ From c5b12817c9aa3ae97fbdd2c2a9a9ab605087dff1 Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Tue, 25 Jul 2017 16:57:18 +0200 Subject: [PATCH 19/31] cleanup: Move write_pid() to where it is being used The write_pid() function is only used in openvpn.c, so no need to have that in the misc.[ch] mixed bag. [on-the-fly change: Added #include "platform.h"] Signed-off-by: David Sommerseth Acked-by: Steffan Karger Message-Id: <20170725145718.13175-1-davids@openvpn.net> URL: https://www.mail-archive.com/search?l=mid&q=20170725145718.13175-1-davids@openvpn.net Signed-off-by: David Sommerseth --- src/openvpn/misc.c | 21 --------------------- src/openvpn/misc.h | 2 -- src/openvpn/openvpn.c | 22 ++++++++++++++++++++++ 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index ae96aa69..8a76bba8 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -142,27 +142,6 @@ run_up_down(const char *command, gc_free(&gc); } -/* Write our PID to a file */ -void -write_pid(const char *filename) -{ - if (filename) - { - unsigned int pid = 0; - FILE *fp = platform_fopen(filename, "w"); - if (!fp) - { - msg(M_ERR, "Open error on pid file %s", filename); - } - - pid = platform_getpid(); - fprintf(fp, "%u\n", pid); - if (fclose(fp)) - { - msg(M_ERR, "Close error on pid file %s", filename); - } - } -} /* * Set standard file descriptors to /dev/null diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index 32b64e8b..734e679c 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -68,8 +68,6 @@ void run_up_down(const char *command, const char *script_type, struct env_set *es); -void write_pid(const char *filename); - /* system flags */ #define S_SCRIPT (1<<0) #define S_FATAL (1<<1) diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c index 08c09e6b..e237ee50 100644 --- a/src/openvpn/openvpn.c +++ b/src/openvpn/openvpn.c @@ -33,6 +33,7 @@ #include "forward.h" #include "multi.h" #include "win32.h" +#include "platform.h" #include "memdbg.h" @@ -47,6 +48,27 @@ process_signal_p2p(struct context *c) return process_signal(c); } +/* Write our PID to a file */ +static void +write_pid(const char *filename) +{ + if (filename) + { + unsigned int pid = 0; + FILE *fp = platform_fopen(filename, "w"); + if (!fp) + { + msg(M_ERR, "Open error on pid file %s", filename); + } + + pid = platform_getpid(); + fprintf(fp, "%u\n", pid); + if (fclose(fp)) + { + msg(M_ERR, "Close error on pid file %s", filename); + } + } +} /**************************************************************************/ From e7e4070cb7b90f4836b65c53360166e11fc3f383 Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Mon, 10 Jul 2017 12:34:38 +0800 Subject: [PATCH 20/31] ntlm: convert binary buffers to uint8_t * Several binary buffers in the ntlm component are stored as char *, however this generates a lot of warnings, because hashing functions expect something unsigned. Convert binary buffers to uint8_t *, while use explicit cast for buffers that are really carrying a string inside. This commit removes several warnings from ntlm.c that you can catch with "-Wall -std=c99". [DS: Done minor typo-fixes in commit message at commit time] Signed-off-by: Antonio Quartulli Acked-by: Steffan Karger Message-Id: <20170710043441.24770-2-a@unstable.cc> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15032.html Signed-off-by: David Sommerseth --- src/openvpn/ntlm.c | 51 ++++++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/src/openvpn/ntlm.c b/src/openvpn/ntlm.c index 976142dd..393368e4 100644 --- a/src/openvpn/ntlm.c +++ b/src/openvpn/ntlm.c @@ -71,31 +71,32 @@ create_des_keys(const unsigned char *hash, unsigned char *key) } static void -gen_md4_hash(const char *data, int data_len, char *result) +gen_md4_hash(const uint8_t *data, int data_len, uint8_t *result) { /* result is 16 byte md4 hash */ const md_kt_t *md4_kt = md_kt_get("MD4"); - char md[MD4_DIGEST_LENGTH]; + uint8_t md[MD4_DIGEST_LENGTH]; md_full(md4_kt, data, data_len, md); memcpy(result, md, MD4_DIGEST_LENGTH); } static void -gen_hmac_md5(const char *data, int data_len, const char *key, int key_len,char *result) +gen_hmac_md5(const uint8_t *data, int data_len, const uint8_t *key, int key_len, + uint8_t *result) { const md_kt_t *md5_kt = md_kt_get("MD5"); hmac_ctx_t *hmac_ctx = hmac_ctx_new(); hmac_ctx_init(hmac_ctx, key, key_len, md5_kt); - hmac_ctx_update(hmac_ctx, (const unsigned char *)data, data_len); - hmac_ctx_final(hmac_ctx, (unsigned char *)result); + hmac_ctx_update(hmac_ctx, data, data_len); + hmac_ctx_final(hmac_ctx, result); hmac_ctx_cleanup(hmac_ctx); hmac_ctx_free(hmac_ctx); } static void -gen_timestamp(unsigned char *timestamp) +gen_timestamp(uint8_t *timestamp) { /* Copies 8 bytes long timestamp into "timestamp" buffer. * Timestamp is Little-endian, 64-bit signed value representing the number of tenths of a microsecond since January 1, 1601. @@ -195,19 +196,19 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are */ char pwbuf[sizeof(p->up.password) * 2]; /* for unicode password */ - unsigned char buf2[128]; /* decoded reply from proxy */ - unsigned char phase3[464]; + uint8_t buf2[128]; /* decoded reply from proxy */ + uint8_t phase3[464]; - char md4_hash[MD4_DIGEST_LENGTH+5]; - char challenge[8], ntlm_response[24]; + uint8_t md4_hash[MD4_DIGEST_LENGTH + 5]; + uint8_t challenge[8], ntlm_response[24]; int i, ret_val; - char ntlmv2_response[144]; + uint8_t ntlmv2_response[144]; char userdomain_u[256]; /* for uppercase unicode username and domain */ char userdomain[128]; /* the same as previous but ascii */ - char ntlmv2_hash[MD5_DIGEST_LENGTH]; - char ntlmv2_hmacmd5[16]; - char *ntlmv2_blob = ntlmv2_response + 16; /* inside ntlmv2_response, length: 128 */ + uint8_t ntlmv2_hash[MD5_DIGEST_LENGTH]; + uint8_t ntlmv2_hmacmd5[16]; + uint8_t *ntlmv2_blob = ntlmv2_response + 16; /* inside ntlmv2_response, length: 128 */ int ntlmv2_blob_size = 0; int phase3_bufpos = 0x40; /* offset to next security buffer data to be added */ size_t len; @@ -246,12 +247,13 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are /* fill 1st 16 bytes with md4 hash, disregard terminating null */ - gen_md4_hash(pwbuf, unicodize(pwbuf, p->up.password) - 2, md4_hash); + gen_md4_hash((uint8_t *)pwbuf, unicodize(pwbuf, p->up.password) - 2, + md4_hash); /* pad to 21 bytes */ memset(md4_hash + MD4_DIGEST_LENGTH, 0, 5); - ret_val = openvpn_base64_decode( phase_2, (void *)buf2, -1); + ret_val = openvpn_base64_decode(phase_2, buf2, -1); if (ret_val < 0) { return NULL; @@ -282,15 +284,16 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are msg(M_INFO, "Warning: Username or domain too long"); } unicodize(userdomain_u, userdomain); - gen_hmac_md5(userdomain_u, 2 * strlen(userdomain), md4_hash, MD5_DIGEST_LENGTH, ntlmv2_hash); + gen_hmac_md5((uint8_t *)userdomain_u, 2 * strlen(userdomain), md4_hash, + MD5_DIGEST_LENGTH, ntlmv2_hash); /* NTLMv2 Blob */ memset(ntlmv2_blob, 0, 128); /* Clear blob buffer */ ntlmv2_blob[0x00] = 1; /* Signature */ ntlmv2_blob[0x01] = 1; /* Signature */ ntlmv2_blob[0x04] = 0; /* Reserved */ - gen_timestamp((unsigned char *)&ntlmv2_blob[0x08]); /* 64-bit Timestamp */ - gen_nonce((unsigned char *)&ntlmv2_blob[0x10]); /* 64-bit Client Nonce */ + gen_timestamp(&ntlmv2_blob[0x08]); /* 64-bit Timestamp */ + gen_nonce(&ntlmv2_blob[0x10]); /* 64-bit Client Nonce */ ntlmv2_blob[0x18] = 0; /* Unknown, zero should work */ /* Add target information block to the blob */ @@ -302,8 +305,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are tib_len = 96; } { - char *tib_ptr; - int tib_pos = buf2[0x2c]; + uint8_t *tib_ptr; + uint8_t tib_pos = buf2[0x2c]; if (tib_pos + tib_len > sizeof(buf2)) { return NULL; @@ -336,13 +339,13 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are { unsigned char key1[DES_KEY_LENGTH], key2[DES_KEY_LENGTH], key3[DES_KEY_LENGTH]; - create_des_keys((unsigned char *)md4_hash, key1); + create_des_keys(md4_hash, key1); cipher_des_encrypt_ecb(key1, challenge, ntlm_response); - create_des_keys((unsigned char *)&(md4_hash[DES_KEY_LENGTH-1]), key2); + create_des_keys(&md4_hash[DES_KEY_LENGTH - 1], key2); cipher_des_encrypt_ecb(key2, challenge, &ntlm_response[DES_KEY_LENGTH]); - create_des_keys((unsigned char *)&(md4_hash[2*(DES_KEY_LENGTH-1)]), key3); + create_des_keys(&md4_hash[2 * (DES_KEY_LENGTH - 1)], key3); cipher_des_encrypt_ecb(key3, challenge, &ntlm_response[DES_KEY_LENGTH*2]); } From c2d08916f1b7933bec81422d1f14f84e9b1ef878 Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Mon, 10 Jul 2017 12:34:40 +0800 Subject: [PATCH 21/31] ntlm: restyle compressed multiple function calls The gen_md4_hash() function is receiving as first argument a buffer that is filled by a function invoked when evaluating the second argument. Although this is proper C, it makes the call invocation a bit obscure because it is not immediately easy to grasp how the 'pwbuf' buffer is filled. Unroll the multiple function call onto lines and make the core more readable. Signed-off-by: Antonio Quartulli Acked-by: Steffan Karger Message-Id: <20170710043441.24770-4-a@unstable.cc> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15030.html Signed-off-by: David Sommerseth --- src/openvpn/ntlm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/openvpn/ntlm.c b/src/openvpn/ntlm.c index 393368e4..8c322f74 100644 --- a/src/openvpn/ntlm.c +++ b/src/openvpn/ntlm.c @@ -247,8 +247,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are /* fill 1st 16 bytes with md4 hash, disregard terminating null */ - gen_md4_hash((uint8_t *)pwbuf, unicodize(pwbuf, p->up.password) - 2, - md4_hash); + int unicode_len = unicodize(pwbuf, p->up.password) - 2; + gen_md4_hash((uint8_t *)pwbuf, unicode_len, md4_hash); /* pad to 21 bytes */ memset(md4_hash + MD4_DIGEST_LENGTH, 0, 5); From c310f1ecba905f091e3a31cb3e6cba5ae75e996b Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Mon, 10 Jul 2017 12:34:41 +0800 Subject: [PATCH 22/31] ntlm: improve code style and readability This patch does not introduce any functional or behavioural change. The code in ntlm.c has been restyled to better to obey to the new coding style and its readability has been a improved a bit. Signed-off-by: Antonio Quartulli Acked-by: Steffan Karger Message-Id: <20170710043441.24770-5-a@unstable.cc> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15028.html Signed-off-by: David Sommerseth --- src/openvpn/ntlm.c | 74 ++++++++++++++++++++++++++++------------------ 1 file changed, 45 insertions(+), 29 deletions(-) diff --git a/src/openvpn/ntlm.c b/src/openvpn/ntlm.c index 8c322f74..65c1cbf5 100644 --- a/src/openvpn/ntlm.c +++ b/src/openvpn/ntlm.c @@ -60,13 +60,13 @@ static void create_des_keys(const unsigned char *hash, unsigned char *key) { key[0] = hash[0]; - key[1] = ((hash[0]&1)<<7)|(hash[1]>>1); - key[2] = ((hash[1]&3)<<6)|(hash[2]>>2); - key[3] = ((hash[2]&7)<<5)|(hash[3]>>3); - key[4] = ((hash[3]&15)<<4)|(hash[4]>>4); - key[5] = ((hash[4]&31)<<3)|(hash[5]>>5); - key[6] = ((hash[5]&63)<<2)|(hash[6]>>6); - key[7] = ((hash[6]&127)<<1); + key[1] = ((hash[0] & 1) << 7) | (hash[1] >> 1); + key[2] = ((hash[1] & 3) << 6) | (hash[2] >> 2); + key[3] = ((hash[2] & 7) << 5) | (hash[3] >> 3); + key[4] = ((hash[3] & 15) << 4) | (hash[4] >> 4); + key[5] = ((hash[4] & 31) << 3) | (hash[5] >> 5); + key[6] = ((hash[5] & 63) << 2) | (hash[6] >> 6); + key[7] = ((hash[6] & 127) << 1); key_des_fixup(key, 8, 1); } @@ -99,7 +99,8 @@ static void gen_timestamp(uint8_t *timestamp) { /* Copies 8 bytes long timestamp into "timestamp" buffer. - * Timestamp is Little-endian, 64-bit signed value representing the number of tenths of a microsecond since January 1, 1601. + * Timestamp is Little-endian, 64-bit signed value representing the + * number of tenths of a microsecond since January 1, 1601. */ UINTEGER64 timestamp_ull; @@ -151,16 +152,17 @@ unicodize(char *dst, const char *src) { dst[i++] = *src; dst[i++] = 0; - } - while (*src++); + } while (*src++); return i; } static void -add_security_buffer(int sb_offset, void *data, int length, unsigned char *msg_buf, int *msg_bufpos) +add_security_buffer(int sb_offset, void *data, int length, + unsigned char *msg_buf, int *msg_bufpos) { - /* Adds security buffer data to a message and sets security buffer's offset and length */ + /* Adds security buffer data to a message and sets security buffer's + * offset and length */ msg_buf[sb_offset] = (unsigned char)length; msg_buf[sb_offset + 2] = msg_buf[sb_offset]; msg_buf[sb_offset + 4] = (unsigned char)(*msg_bufpos & 0xff); @@ -187,7 +189,8 @@ ntlm_phase_1(const struct http_proxy_info *p, struct gc_arena *gc) } const char * -ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_arena *gc) +ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, + struct gc_arena *gc) { /* NTLM handshake * @@ -297,13 +300,16 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are ntlmv2_blob[0x18] = 0; /* Unknown, zero should work */ /* Add target information block to the blob */ - if (( *((long *)&buf2[0x14]) & 0x00800000) == 0x00800000) /* Check for Target Information block */ + + /* Check for Target Information block */ + if ((*((long *)&buf2[0x14]) & 0x00800000) == 0x00800000) { tib_len = buf2[0x28]; /* Get Target Information block size */ if (tib_len > 96) { tib_len = 96; } + { uint8_t *tib_ptr; uint8_t tib_pos = buf2[0x2c]; @@ -311,8 +317,10 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are { return NULL; } - tib_ptr = buf2 + tib_pos; /* Get Target Information block pointer */ - memcpy(&ntlmv2_blob[0x1c], tib_ptr, tib_len); /* Copy Target Information block into the blob */ + /* Get Target Information block pointer */ + tib_ptr = buf2 + tib_pos; + /* Copy Target Information block into the blob */ + memcpy(&ntlmv2_blob[0x1c], tib_ptr, tib_len); } } else @@ -320,7 +328,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are tib_len = 0; } - ntlmv2_blob[0x1c + tib_len] = 0; /* Unknown, zero works */ + /* Unknown, zero works */ + ntlmv2_blob[0x1c + tib_len] = 0; /* Get blob length */ ntlmv2_blob_size = 0x20 + tib_len; @@ -329,15 +338,18 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are memcpy(&ntlmv2_response[8], challenge, 8); /* hmac-md5 */ - gen_hmac_md5(&ntlmv2_response[8], ntlmv2_blob_size + 8, ntlmv2_hash, MD5_DIGEST_LENGTH, ntlmv2_hmacmd5); - - /* Add hmac-md5 result to the blob */ - memcpy(ntlmv2_response, ntlmv2_hmacmd5, MD5_DIGEST_LENGTH); /* Note: This overwrites challenge previously written at ntlmv2_response[8..15] */ + gen_hmac_md5(&ntlmv2_response[8], ntlmv2_blob_size + 8, ntlmv2_hash, + MD5_DIGEST_LENGTH, ntlmv2_hmacmd5); + /* Add hmac-md5 result to the blob. + * Note: This overwrites challenge previously written at + * ntlmv2_response[8..15] */ + memcpy(ntlmv2_response, ntlmv2_hmacmd5, MD5_DIGEST_LENGTH); } - else /* Generate NTLM response */ + else /* Generate NTLM response */ { - unsigned char key1[DES_KEY_LENGTH], key2[DES_KEY_LENGTH], key3[DES_KEY_LENGTH]; + unsigned char key1[DES_KEY_LENGTH], key2[DES_KEY_LENGTH]; + unsigned char key3[DES_KEY_LENGTH]; create_des_keys(md4_hash, key1); cipher_des_encrypt_ecb(key1, challenge, ntlm_response); @@ -346,7 +358,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are cipher_des_encrypt_ecb(key2, challenge, &ntlm_response[DES_KEY_LENGTH]); create_des_keys(&md4_hash[2 * (DES_KEY_LENGTH - 1)], key3); - cipher_des_encrypt_ecb(key3, challenge, &ntlm_response[DES_KEY_LENGTH*2]); + cipher_des_encrypt_ecb(key3, challenge, + &ntlm_response[DES_KEY_LENGTH * 2]); } @@ -357,7 +370,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are if (ntlmv2_enabled) /* NTLMv2 response */ { - add_security_buffer(0x14, ntlmv2_response, ntlmv2_blob_size + 16, phase3, &phase3_bufpos); + add_security_buffer(0x14, ntlmv2_response, ntlmv2_blob_size + 16, + phase3, &phase3_bufpos); } else /* NTLM response */ { @@ -365,12 +379,13 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are } /* username in ascii */ - add_security_buffer(0x24, username, strlen(username), phase3, &phase3_bufpos); + add_security_buffer(0x24, username, strlen(username), phase3, + &phase3_bufpos); - /* Set domain. If is empty, default domain will be used (i.e. proxy's domain) */ + /* Set domain. If is empty, default domain will be used + * (i.e. proxy's domain) */ add_security_buffer(0x1c, domain, strlen(domain), phase3, &phase3_bufpos); - /* other security buffers will be empty */ phase3[0x10] = phase3_bufpos; /* lm not used */ phase3[0x30] = phase3_bufpos; /* no workstation name supplied */ @@ -380,7 +395,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are phase3[0x3c] = 0x02; /* negotiate oem */ phase3[0x3d] = 0x02; /* negotiate ntlm */ - return ((const char *)make_base64_string2((unsigned char *)phase3, phase3_bufpos, gc)); + return ((const char *)make_base64_string2((unsigned char *)phase3, + phase3_bufpos, gc)); } #else /* if NTLM */ From bb23eca847c8edac9c3979b7f35468b74db00459 Mon Sep 17 00:00:00 2001 From: Arne Schwabe Date: Sun, 23 Jul 2017 18:45:36 +0200 Subject: [PATCH 23/31] Print ec bit details, refuse management-external-key if key is not RSA V2: Print also curve details, add missing ifdef V3: Goto err instead of using M_FATAL, format fixes, use EC_GROUP_get_curve_name + OBJ_nid2sn instead of ECPKParameters_print, add compat headers for 1.0.2 V4: Formatting changes and change M_ERR to M_WARN Acked-by: Steffan Karger Message-Id: <1500828336-30314-1-git-send-email-arne@rfc2549.org> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15124.html Signed-off-by: David Sommerseth --- configure.ac | 2 ++ src/openvpn/openssl_compat.h | 32 ++++++++++++++++++++++++++++++++ src/openvpn/ssl_openssl.c | 29 +++++++++++++++++++++++++++-- 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 39d992c0..f6eeb40d 100644 --- a/configure.ac +++ b/configure.ac @@ -934,6 +934,7 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then EVP_PKEY_id \ EVP_PKEY_get0_RSA \ EVP_PKEY_get0_DSA \ + EVP_PKEY_get0_EC_KEY \ RSA_set_flags \ RSA_bits \ RSA_get0_key \ @@ -949,6 +950,7 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then RSA_meth_set_init \ RSA_meth_set_finish \ RSA_meth_set0_app_data \ + EC_GROUP_order_bits ] ) diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h index 36f68b01..70b19aea 100644 --- a/src/openvpn/openssl_compat.h +++ b/src/openvpn/openssl_compat.h @@ -244,6 +244,20 @@ EVP_PKEY_get0_RSA(EVP_PKEY *pkey) } #endif +#if !defined(HAVE_EVP_PKEY_GET0_EC_KEY) && !defined(OPENSSL_NO_EC) +/** + * Get the EC_KEY object of a public key + * + * @param pkey Public key object + * @return The underlying EC_KEY object + */ +static inline EC_KEY * +EVP_PKEY_get0_EC_KEY(EVP_PKEY *pkey) +{ + return pkey ? pkey->pkey.ec : NULL; +} +#endif + #if !defined(HAVE_EVP_PKEY_ID) /** * Get the PKEY type @@ -610,6 +624,24 @@ RSA_meth_set0_app_data(RSA_METHOD *meth, void *app_data) } #endif +#if !defined(HAVE_EC_GROUP_ORDER_BITS) && !defined(OPENSSL_NO_EC) +/** + * Gets the number of bits of the order of an EC_GROUP + * + * @param group EC_GROUP object + * @return number of bits of group order. + */ +static inline int +EC_GROUP_order_bits(const EC_GROUP *group) +{ + BIGNUM* order = BN_new(); + EC_GROUP_get_order(group, order, NULL); + int bits = BN_num_bits(order); + BN_free(order); + return bits; +} +#endif + /* SSLeay symbols have been renamed in OpenSSL 1.1 */ #if !defined(RSA_F_RSA_OSSL_PRIVATE_ENCRYPT) #define RSA_F_RSA_OSSL_PRIVATE_ENCRYPT RSA_F_RSA_EAY_PRIVATE_ENCRYPT diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 11f4a567..189a8470 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -1077,6 +1077,13 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx, ASSERT(pkey); /* NULL before SSL_CTX_use_certificate() is called */ pub_rsa = EVP_PKEY_get0_RSA(pkey); + /* Certificate might not be RSA but DSA or EC */ + if (!pub_rsa) + { + crypto_msg(M_WARN, "management-external-key requires a RSA certificate"); + goto err; + } + /* initialize RSA object */ const BIGNUM *n = NULL; const BIGNUM *e = NULL; @@ -1683,18 +1690,36 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix) EVP_PKEY *pkey = X509_get_pubkey(cert); if (pkey != NULL) { - if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA && EVP_PKEY_get0_RSA(pkey) != NULL) + if ((EVP_PKEY_id(pkey) == EVP_PKEY_RSA) && (EVP_PKEY_get0_RSA(pkey) != NULL)) { RSA *rsa = EVP_PKEY_get0_RSA(pkey); openvpn_snprintf(s2, sizeof(s2), ", %d bit RSA", RSA_bits(rsa)); } - else if (EVP_PKEY_id(pkey) == EVP_PKEY_DSA && EVP_PKEY_get0_DSA(pkey) != NULL) + else if ((EVP_PKEY_id(pkey) == EVP_PKEY_DSA) && (EVP_PKEY_get0_DSA(pkey) != NULL)) { DSA *dsa = EVP_PKEY_get0_DSA(pkey); openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA", DSA_bits(dsa)); } +#ifndef OPENSSL_NO_EC + else if ((EVP_PKEY_id(pkey) == EVP_PKEY_EC) && (EVP_PKEY_get0_EC_KEY(pkey) != NULL)) + { + EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey); + const EC_GROUP *group = EC_KEY_get0_group(ec); + const char* curve; + + int nid = EC_GROUP_get_curve_name(group); + if (nid == 0 || (curve = OBJ_nid2sn(nid)) == NULL) + { + curve = "Error getting curve name"; + } + + openvpn_snprintf(s2, sizeof(s2), ", %d bit EC, curve: %s", + EC_GROUP_order_bits(group), curve); + + } +#endif EVP_PKEY_free(pkey); } X509_free(cert); From 59e7e9fce8de6ea90d13baeaede83adc0b594e22 Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Tue, 25 Jul 2017 15:03:14 +0200 Subject: [PATCH 24/31] contrib: Remove keychain-mcd code After the security audits performed by Cryptography Engineering the spring of 2017 [1], there were several concerns about the contrib code for the macOS keychain support. After more careful review of this code base, it was considered to be in such a bad shape that it will need a massive overhaul. There were more issues than what the security audit revealed. It was attempted several times to get in touch with the contributor of this code; with no response at all [2]. There has however been some discussions with the Tunnelblick project [3]. There is one person there willing to go through this and improve the situation. The main Tunnelblick maintainer is also willing to include the improved code to their project instead of having this as a contrib code in the upstream OpenVPN project. So this patch just removes the code which we will no longer ship as part of OpenVPN - and the Tunnelblick project will take over the responsibility for this code base on their own. And since this code base is purely macOS specific, this seems to be a far better place for this code to reside. Signed-off-by: David Sommerseth [1] [2] [3] Acked-by: Jonathan K. Bullard Message-Id: <20170725130314.12919-1-davids@openvpn.net> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15130.html Signed-off-by: David Sommerseth --- contrib/keychain-mcd/Makefile | 13 - contrib/keychain-mcd/cert_data.c | 866 ---------------------------- contrib/keychain-mcd/cert_data.h | 50 -- contrib/keychain-mcd/common_osx.c | 100 ---- contrib/keychain-mcd/common_osx.h | 38 -- contrib/keychain-mcd/crypto_osx.c | 79 --- contrib/keychain-mcd/crypto_osx.h | 44 -- contrib/keychain-mcd/keychain-mcd.8 | 161 ------ contrib/keychain-mcd/main.c | 310 ---------- 9 files changed, 1661 deletions(-) delete mode 100644 contrib/keychain-mcd/Makefile delete mode 100644 contrib/keychain-mcd/cert_data.c delete mode 100644 contrib/keychain-mcd/cert_data.h delete mode 100644 contrib/keychain-mcd/common_osx.c delete mode 100644 contrib/keychain-mcd/common_osx.h delete mode 100644 contrib/keychain-mcd/crypto_osx.c delete mode 100644 contrib/keychain-mcd/crypto_osx.h delete mode 100644 contrib/keychain-mcd/keychain-mcd.8 delete mode 100644 contrib/keychain-mcd/main.c diff --git a/contrib/keychain-mcd/Makefile b/contrib/keychain-mcd/Makefile deleted file mode 100644 index c6431df1..00000000 --- a/contrib/keychain-mcd/Makefile +++ /dev/null @@ -1,13 +0,0 @@ -CFILES = cert_data.c common_osx.c crypto_osx.c main.c -OFILES = $(CFILES:.c=.o) ../../src/openvpn/base64.o -prog = keychain-mcd - -CC = gcc -CFLAGS = -Wall -LDFLAGS = -framework CoreFoundation -framework Security -framework CoreServices - -$(prog): $(OFILES) - $(CC) $(LDFLAGS) $(OFILES) -o $(prog) - -%.o: %.c - $(CC) $(CFLAGS) -c $< -o $@ diff --git a/contrib/keychain-mcd/cert_data.c b/contrib/keychain-mcd/cert_data.c deleted file mode 100644 index c04f68ec..00000000 --- a/contrib/keychain-mcd/cert_data.c +++ /dev/null @@ -1,866 +0,0 @@ -/* - * OpenVPN -- An application to securely tunnel IP networks - * over a single UDP port, with support for SSL/TLS-based - * session authentication and key exchange, - * packet encryption, packet authentication, and - * packet compression. - * - * Copyright (C) 2010 Brian Raderman - * Copyright (C) 2013-2015 Vasily Kulikov - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 - * as published by the Free Software Foundation. - * - * 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. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - */ - - -#include "cert_data.h" -#include -#include - -#include "common_osx.h" -#include "crypto_osx.h" -#include - -CFStringRef kCertDataSubjectName = CFSTR("subject"), - kCertDataIssuerName = CFSTR("issuer"), - kCertDataSha1Name = CFSTR("SHA1"), - kCertDataMd5Name = CFSTR("MD5"), - kCertDataSerialName = CFSTR("serial"), - kCertNameFwdSlash = CFSTR("/"), - kCertNameEquals = CFSTR("="); -CFStringRef kCertNameOrganization = CFSTR("o"), - kCertNameOrganizationalUnit = CFSTR("ou"), - kCertNameCountry = CFSTR("c"), - kCertNameLocality = CFSTR("l"), - kCertNameState = CFSTR("st"), - kCertNameCommonName = CFSTR("cn"), - kCertNameEmail = CFSTR("e"); -CFStringRef kStringSpace = CFSTR(" "), - kStringEmpty = CFSTR(""); - -typedef struct _CertName -{ - CFArrayRef countryName, organization, organizationalUnit, commonName, description, emailAddress, - stateName, localityName; -} CertName, *CertNameRef; - -typedef struct _DescData -{ - CFStringRef name, value; -} DescData, *DescDataRef; - -void destroyDescData(DescDataRef pData); - -CertNameRef -createCertName() -{ - CertNameRef pCertName = (CertNameRef)malloc(sizeof(CertName)); - pCertName->countryName = CFArrayCreateMutable(NULL, 0, &kCFTypeArrayCallBacks); - pCertName->organization = CFArrayCreateMutable(NULL, 0, &kCFTypeArrayCallBacks); - pCertName->organizationalUnit = CFArrayCreateMutable(NULL, 0, &kCFTypeArrayCallBacks); - pCertName->commonName = CFArrayCreateMutable(NULL, 0, &kCFTypeArrayCallBacks); - pCertName->description = CFArrayCreateMutable(NULL, 0, &kCFTypeArrayCallBacks); - pCertName->emailAddress = CFArrayCreateMutable(NULL, 0, &kCFTypeArrayCallBacks); - pCertName->stateName = CFArrayCreateMutable(NULL, 0, &kCFTypeArrayCallBacks); - pCertName->localityName = CFArrayCreateMutable(NULL, 0, &kCFTypeArrayCallBacks); - return pCertName; -} - -void -destroyCertName(CertNameRef pCertName) -{ - if (!pCertName) - { - return; - } - - CFRelease(pCertName->countryName); - CFRelease(pCertName->organization); - CFRelease(pCertName->organizationalUnit); - CFRelease(pCertName->commonName); - CFRelease(pCertName->description); - CFRelease(pCertName->emailAddress); - CFRelease(pCertName->stateName); - CFRelease(pCertName->localityName); - free(pCertName); -} - -bool -CFStringRefCmpCString(CFStringRef cfstr, const char *str) -{ - CFStringRef tmp = CFStringCreateWithCStringNoCopy(NULL, str, kCFStringEncodingUTF8, kCFAllocatorNull); - CFComparisonResult cresult = CFStringCompare(cfstr, tmp, 0); - bool result = cresult == kCFCompareEqualTo; - CFRelease(tmp); - return result; -} - -CFDateRef -GetDateFieldFromCertificate(SecCertificateRef certificate, CFTypeRef oid) -{ - const void *keys[] = { oid }; - CFDictionaryRef dict = NULL; - CFErrorRef error; - CFDateRef date = NULL; - - CFArrayRef keySelection = CFArrayCreate(NULL, keys, sizeof(keys)/sizeof(keys[0]), &kCFTypeArrayCallBacks); - dict = SecCertificateCopyValues(certificate, keySelection, &error); - if (dict == NULL) - { - printErrorMsg("GetDateFieldFromCertificate: SecCertificateCopyValues", error); - goto release_ks; - } - CFDictionaryRef vals = dict ? CFDictionaryGetValue(dict, oid) : NULL; - CFNumberRef vals2 = vals ? CFDictionaryGetValue(vals, kSecPropertyKeyValue) : NULL; - if (vals2 == NULL) - { - goto release_dict; - } - - CFAbsoluteTime validityNotBefore; - if (CFNumberGetValue(vals2, kCFNumberDoubleType, &validityNotBefore)) - { - date = CFDateCreate(kCFAllocatorDefault,validityNotBefore); - } - -release_dict: - CFRelease(dict); -release_ks: - CFRelease(keySelection); - return date; -} - -CFArrayRef -GetFieldsFromCertificate(SecCertificateRef certificate, CFTypeRef oid) -{ - CFMutableArrayRef fields = CFArrayCreateMutable(NULL, 0, NULL); - CertNameRef pCertName = createCertName(); - const void *keys[] = { oid, }; - CFDictionaryRef dict; - CFErrorRef error; - - CFArrayRef keySelection = CFArrayCreate(NULL, keys, 1, NULL); - - dict = SecCertificateCopyValues(certificate, keySelection, &error); - if (dict == NULL) - { - printErrorMsg("GetFieldsFromCertificate: SecCertificateCopyValues", error); - CFRelease(keySelection); - CFRelease(fields); - destroyCertName(pCertName); - return NULL; - } - CFDictionaryRef vals = CFDictionaryGetValue(dict, oid); - CFArrayRef vals2 = vals ? CFDictionaryGetValue(vals, kSecPropertyKeyValue) : NULL; - if (vals2) - { - for (int i = 0; i < CFArrayGetCount(vals2); i++) { - CFDictionaryRef subDict = CFArrayGetValueAtIndex(vals2, i); - CFStringRef label = CFDictionaryGetValue(subDict, kSecPropertyKeyLabel); - CFStringRef value = CFDictionaryGetValue(subDict, kSecPropertyKeyValue); - - if (CFStringCompare(label, kSecOIDEmailAddress, 0) == kCFCompareEqualTo) - { - CFArrayAppendValue((CFMutableArrayRef)pCertName->emailAddress, value); - } - else if (CFStringCompare(label, kSecOIDCountryName, 0) == kCFCompareEqualTo) - { - CFArrayAppendValue((CFMutableArrayRef)pCertName->countryName, value); - } - else if (CFStringCompare(label, kSecOIDOrganizationName, 0) == kCFCompareEqualTo) - { - CFArrayAppendValue((CFMutableArrayRef)pCertName->organization, value); - } - else if (CFStringCompare(label, kSecOIDOrganizationalUnitName, 0) == kCFCompareEqualTo) - { - CFArrayAppendValue((CFMutableArrayRef)pCertName->organizationalUnit, value); - } - else if (CFStringCompare(label, kSecOIDCommonName, 0) == kCFCompareEqualTo) - { - CFArrayAppendValue((CFMutableArrayRef)pCertName->commonName, value); - } - else if (CFStringCompare(label, kSecOIDDescription, 0) == kCFCompareEqualTo) - { - CFArrayAppendValue((CFMutableArrayRef)pCertName->description, value); - } - else if (CFStringCompare(label, kSecOIDStateProvinceName, 0) == kCFCompareEqualTo) - { - CFArrayAppendValue((CFMutableArrayRef)pCertName->stateName, value); - } - else if (CFStringCompare(label, kSecOIDLocalityName, 0) == kCFCompareEqualTo) - { - CFArrayAppendValue((CFMutableArrayRef)pCertName->localityName, value); - } - } - CFArrayAppendValue(fields, pCertName); - } - - CFRelease(dict); - CFRelease(keySelection); - return fields; -} - -CertDataRef -createCertDataFromCertificate(SecCertificateRef certificate) -{ - CertDataRef pCertData = (CertDataRef)malloc(sizeof(CertData)); - pCertData->subject = GetFieldsFromCertificate(certificate, kSecOIDX509V1SubjectName); - pCertData->issuer = GetFieldsFromCertificate(certificate, kSecOIDX509V1IssuerName); - - CFDataRef data = SecCertificateCopyData(certificate); - if (data == NULL) - { - warnx("SecCertificateCopyData() returned NULL"); - destroyCertData(pCertData); - return NULL; - } - - unsigned char sha1[CC_SHA1_DIGEST_LENGTH]; - CC_SHA1(CFDataGetBytePtr(data), CFDataGetLength(data), sha1); - pCertData->sha1 = createHexString(sha1, CC_SHA1_DIGEST_LENGTH); - - unsigned char md5[CC_MD5_DIGEST_LENGTH]; - CC_MD5(CFDataGetBytePtr(data), CFDataGetLength(data), md5); - pCertData->md5 = createHexString((unsigned char *)md5, CC_MD5_DIGEST_LENGTH); - - CFDataRef serial = SecCertificateCopySerialNumber(certificate, NULL); - pCertData->serial = createHexString((unsigned char *)CFDataGetBytePtr(serial), CFDataGetLength(serial)); - CFRelease(serial); - - return pCertData; -} - -CFStringRef -stringFromRange(const char *cstring, CFRange range) -{ - CFStringRef str = CFStringCreateWithBytes(NULL, (uint8 *)&cstring[range.location], range.length, kCFStringEncodingUTF8, false); - CFMutableStringRef mutableStr = CFStringCreateMutableCopy(NULL, 0, str); - CFStringTrimWhitespace(mutableStr); - CFRelease(str); - return mutableStr; -} - -DescDataRef -createDescData(const char *description, CFRange nameRange, CFRange valueRange) -{ - DescDataRef pRetVal = (DescDataRef)malloc(sizeof(DescData)); - - memset(pRetVal, 0, sizeof(DescData)); - - if (nameRange.length > 0) - { - pRetVal->name = stringFromRange(description, nameRange); - } - - if (valueRange.length > 0) - { - pRetVal->value = stringFromRange(description, valueRange); - } - -#if 0 - fprintf(stderr, "name = '%s', value = '%s'\n", - CFStringGetCStringPtr(pRetVal->name, kCFStringEncodingUTF8), - CFStringGetCStringPtr(pRetVal->value, kCFStringEncodingUTF8)); -#endif - return pRetVal; -} - -void -destroyDescData(DescDataRef pData) -{ - if (pData->name) - { - CFRelease(pData->name); - } - - if (pData->value) - { - CFRelease(pData->value); - } - - free(pData); -} - -CFArrayRef -createDescDataPairs(const char *description) -{ - int numChars = strlen(description); - CFRange nameRange, valueRange; - DescDataRef pData; - CFMutableArrayRef retVal = CFArrayCreateMutable(NULL, 0, NULL); - - int i = 0; - - nameRange = CFRangeMake(0, 0); - valueRange = CFRangeMake(0, 0); - bool bInValue = false; - - while (i < numChars) - { - if (!bInValue && (description[i] != ':')) - { - nameRange.length++; - } - else if (bInValue && (description[i] != ':')) - { - valueRange.length++; - } - else if (!bInValue) - { - bInValue = true; - valueRange.location = i + 1; - valueRange.length = 0; - } - else /*(bInValue) */ - { - bInValue = false; - while (description[i] != ' ') - { - valueRange.length--; - i--; - } - - pData = createDescData(description, nameRange, valueRange); - CFArrayAppendValue(retVal, pData); - - nameRange.location = i + 1; - nameRange.length = 0; - } - - i++; - } - - pData = createDescData(description, nameRange, valueRange); - CFArrayAppendValue(retVal, pData); - return retVal; -} - -void -arrayDestroyDescData(const void *val, void *context) -{ - DescDataRef pData = (DescDataRef) val; - destroyDescData(pData); -} - - -int -parseNameComponent(CFStringRef dn, CFStringRef *pName, CFStringRef *pValue) -{ - CFArrayRef nameStrings = CFStringCreateArrayBySeparatingStrings(NULL, dn, kCertNameEquals); - - *pName = *pValue = NULL; - - if (CFArrayGetCount(nameStrings) != 2) - { - return 0; - } - - CFMutableStringRef str; - - str = CFStringCreateMutableCopy(NULL, 0, CFArrayGetValueAtIndex(nameStrings, 0)); - CFStringTrimWhitespace(str); - *pName = str; - - str = CFStringCreateMutableCopy(NULL, 0, CFArrayGetValueAtIndex(nameStrings, 1)); - CFStringTrimWhitespace(str); - *pValue = str; - - CFRelease(nameStrings); - return 1; -} - -int -tryAppendSingleCertField(CertNameRef pCertName, CFArrayRef where, CFStringRef key, - CFStringRef name, CFStringRef value) -{ - if (CFStringCompareWithOptions(name, key, CFRangeMake(0, CFStringGetLength(name)), kCFCompareCaseInsensitive) - == kCFCompareEqualTo) - { - CFArrayAppendValue((CFMutableArrayRef)where, value); - return 1; - } - return 0; -} - -int -appendCertField(CertNameRef pCert, CFStringRef name, CFStringRef value) -{ - struct { - CFArrayRef field; - CFStringRef key; - } fields[] = { - { pCert->organization, kCertNameOrganization}, - { pCert->organizationalUnit, kCertNameOrganizationalUnit}, - { pCert->countryName, kCertNameCountry}, - { pCert->localityName, kCertNameLocality}, - { pCert->stateName, kCertNameState}, - { pCert->commonName, kCertNameCommonName}, - { pCert->emailAddress, kCertNameEmail}, - }; - int i; - int ret = 0; - - for (i = 0; iname || !pDescData->value) - { - return 0; - } - - if (CFStringCompareWithOptions(pDescData->name, kCertDataSubjectName, CFRangeMake(0, CFStringGetLength(pDescData->name)), kCFCompareCaseInsensitive) == kCFCompareEqualTo) - { - ret = parseCertName(pDescData->value, (CFMutableArrayRef)pCertData->subject); - } - else if (CFStringCompareWithOptions(pDescData->name, kCertDataIssuerName, CFRangeMake(0, CFStringGetLength(pDescData->name)), kCFCompareCaseInsensitive) == kCFCompareEqualTo) - { - ret = parseCertName(pDescData->value, (CFMutableArrayRef)pCertData->issuer); - } - else if (CFStringCompareWithOptions(pDescData->name, kCertDataSha1Name, CFRangeMake(0, CFStringGetLength(pDescData->name)), kCFCompareCaseInsensitive) == kCFCompareEqualTo) - { - pCertData->sha1 = CFRetain(pDescData->value); - } - else if (CFStringCompareWithOptions(pDescData->name, kCertDataMd5Name, CFRangeMake(0, CFStringGetLength(pDescData->name)), kCFCompareCaseInsensitive) == kCFCompareEqualTo) - { - pCertData->md5 = CFRetain(pDescData->value); - } - else if (CFStringCompareWithOptions(pDescData->name, kCertDataSerialName, CFRangeMake(0, CFStringGetLength(pDescData->name)), kCFCompareCaseInsensitive) == kCFCompareEqualTo) - { - pCertData->serial = CFRetain(pDescData->value); - } - else - { - return 0; - } - - return ret; -} - -CertDataRef -createCertDataFromString(const char *description) -{ - CertDataRef pCertData = (CertDataRef)malloc(sizeof(CertData)); - pCertData->subject = CFArrayCreateMutable(NULL, 0, NULL); - pCertData->issuer = CFArrayCreateMutable(NULL, 0, NULL); - pCertData->sha1 = NULL; - pCertData->md5 = NULL; - pCertData->serial = NULL; - - CFArrayRef pairs = createDescDataPairs(description); - for (int i = 0; isubject) - { - CFArrayApplyFunction(pCertData->subject, CFRangeMake(0, CFArrayGetCount(pCertData->subject)), arrayDestroyCertName, NULL); - CFRelease(pCertData->subject); - } - - if (pCertData->issuer) - { - CFArrayApplyFunction(pCertData->issuer, CFRangeMake(0, CFArrayGetCount(pCertData->issuer)), arrayDestroyCertName, NULL); - CFRelease(pCertData->issuer); - } - - if (pCertData->sha1) - { - CFRelease(pCertData->sha1); - } - - if (pCertData->md5) - { - CFRelease(pCertData->md5); - } - - if (pCertData->serial) - { - CFRelease(pCertData->serial); - } - - free(pCertData); -} - -bool -stringArrayMatchesTemplate(CFArrayRef strings, CFArrayRef templateArray) -{ - int templateCount, stringCount, i; - - templateCount = CFArrayGetCount(templateArray); - - if (templateCount > 0) - { - stringCount = CFArrayGetCount(strings); - if (stringCount != templateCount) - { - return false; - } - - for (i = 0; i < stringCount; i++) - { - CFStringRef str, template; - - template = (CFStringRef)CFArrayGetValueAtIndex(templateArray, i); - str = (CFStringRef)CFArrayGetValueAtIndex(strings, i); - - if (CFStringCompareWithOptions(template, str, CFRangeMake(0, CFStringGetLength(template)), kCFCompareCaseInsensitive) != kCFCompareEqualTo) - { - return false; - } - } - } - - return true; - -} - -bool -certNameMatchesTemplate(CertNameRef pCertName, CertNameRef pTemplate) -{ - if (!stringArrayMatchesTemplate(pCertName->countryName, pTemplate->countryName)) - { - return false; - } - else if (!stringArrayMatchesTemplate(pCertName->organization, pTemplate->organization)) - { - return false; - } - else if (!stringArrayMatchesTemplate(pCertName->organizationalUnit, pTemplate->organizationalUnit)) - { - return false; - } - else if (!stringArrayMatchesTemplate(pCertName->commonName, pTemplate->commonName)) - { - return false; - } - else if (!stringArrayMatchesTemplate(pCertName->emailAddress, pTemplate->emailAddress)) - { - return false; - } - else if (!stringArrayMatchesTemplate(pCertName->stateName, pTemplate->stateName)) - { - return false; - } - else if (!stringArrayMatchesTemplate(pCertName->localityName, pTemplate->localityName)) - { - return false; - } - else - { - return true; - } -} - -bool -certNameArrayMatchesTemplate(CFArrayRef certNameArray, CFArrayRef templateArray) -{ - int templateCount, certCount, i; - - templateCount = CFArrayGetCount(templateArray); - - if (templateCount > 0) - { - certCount = CFArrayGetCount(certNameArray); - if (certCount != templateCount) - { - return false; - } - - for (i = 0; i < certCount; i++) - { - CertNameRef pName, pTemplateName; - - pTemplateName = (CertNameRef)CFArrayGetValueAtIndex(templateArray, i); - pName = (CertNameRef)CFArrayGetValueAtIndex(certNameArray, i); - - if (!certNameMatchesTemplate(pName, pTemplateName)) - { - return false; - } - } - } - - return true; -} - -bool -hexStringMatchesTemplate(CFStringRef str, CFStringRef template) -{ - if (template) - { - if (!str) - { - return false; - } - - CFMutableStringRef strMutable, templateMutable; - - strMutable = CFStringCreateMutableCopy(NULL, 0, str); - templateMutable = CFStringCreateMutableCopy(NULL, 0, template); - - CFStringFindAndReplace(strMutable, kStringSpace, kStringEmpty, CFRangeMake(0, CFStringGetLength(strMutable)), 0); - CFStringFindAndReplace(templateMutable, kStringSpace, kStringEmpty, CFRangeMake(0, CFStringGetLength(templateMutable)), 0); - - CFComparisonResult result = CFStringCompareWithOptions(templateMutable, strMutable, CFRangeMake(0, CFStringGetLength(templateMutable)), kCFCompareCaseInsensitive); - - CFRelease(strMutable); - CFRelease(templateMutable); - - if (result != kCFCompareEqualTo) - { - return false; - } - } - - return true; -} - -bool -certDataMatchesTemplate(CertDataRef pCertData, CertDataRef pTemplate) -{ - if (!certNameArrayMatchesTemplate(pCertData->subject, pTemplate->subject)) - { - return false; - } - - if (!certNameArrayMatchesTemplate(pCertData->issuer, pTemplate->issuer)) - { - return false; - } - - if (!hexStringMatchesTemplate(pCertData->sha1, pTemplate->sha1)) - { - return false; - } - - if (!hexStringMatchesTemplate(pCertData->md5, pTemplate->md5)) - { - return false; - } - - if (!hexStringMatchesTemplate(pCertData->serial, pTemplate->serial)) - { - return false; - } - - return true; -} - -bool -certExpired(SecCertificateRef certificate) -{ - bool result; - CFDateRef notAfter = GetDateFieldFromCertificate(certificate, kSecOIDX509V1ValidityNotAfter); - CFDateRef notBefore = GetDateFieldFromCertificate(certificate, kSecOIDX509V1ValidityNotBefore); - CFDateRef now = CFDateCreate(kCFAllocatorDefault, CFAbsoluteTimeGetCurrent()); - - if (!notAfter || !notBefore || !now) - { - warnx("GetDateFieldFromCertificate() returned NULL"); - result = true; - } - else - { - if (CFDateCompare(notBefore, now, NULL) != kCFCompareLessThan - || CFDateCompare(now, notAfter, NULL) != kCFCompareLessThan) - { - result = true; - } - else - { - result = false; - } - } - - CFRelease(notAfter); - CFRelease(notBefore); - CFRelease(now); - return result; -} - -SecIdentityRef -findIdentity(CertDataRef pCertDataTemplate) -{ - const void *keys[] = { - kSecClass, - kSecReturnRef, - kSecMatchLimit - }; - const void *values[] = { - kSecClassIdentity, - kCFBooleanTrue, - kSecMatchLimitAll - }; - CFArrayRef result = NULL; - - CFDictionaryRef query = CFDictionaryCreate(NULL, keys, values, - sizeof(keys) / sizeof(*keys), - &kCFTypeDictionaryKeyCallBacks, - &kCFTypeDictionaryValueCallBacks); - OSStatus status = SecItemCopyMatching(query, (CFTypeRef *)&result); - CFRelease(query); - if (status != noErr) - { - warnx("No identities in keychain found"); - return NULL; - } - - SecIdentityRef bestIdentity = NULL; - CFDateRef bestNotBeforeDate = NULL; - - for (int i = 0; i - * Copyright (C) 2013-2015 Vasily Kulikov - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 - * as published by the Free Software Foundation. - * - * 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. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - */ -#ifndef __cert_data_h__ -#define __cert_data_h__ - -#include -#include - -typedef struct _CertData -{ - CFArrayRef subject; - CFArrayRef issuer; - CFStringRef serial; - CFStringRef md5, sha1; -} CertData, *CertDataRef; - -CertDataRef createCertDataFromCertificate(SecCertificateRef certificate); - -CertDataRef createCertDataFromString(const char *description); - -void destroyCertData(CertDataRef pCertData); - -bool certDataMatchesTemplate(CertDataRef pCertData, CertDataRef pTemplate); - -void printCertData(CertDataRef pCertData); - -SecIdentityRef findIdentity(CertDataRef pCertDataTemplate); - -#endif /* ifndef __cert_data_h__ */ diff --git a/contrib/keychain-mcd/common_osx.c b/contrib/keychain-mcd/common_osx.c deleted file mode 100644 index f8178149..00000000 --- a/contrib/keychain-mcd/common_osx.c +++ /dev/null @@ -1,100 +0,0 @@ -/* - * OpenVPN -- An application to securely tunnel IP networks - * over a single UDP port, with support for SSL/TLS-based - * session authentication and key exchange, - * packet encryption, packet authentication, and - * packet compression. - * - * Copyright (C) 2010 Brian Raderman - * Copyright (C) 2013-2015 Vasily Kulikov - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 - * as published by the Free Software Foundation. - * - * 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. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - */ - -/* - #include "config.h" - #include "syshead.h" - #include "common.h" - #include "buffer.h" - #include "error.h" - */ - -#include "common_osx.h" -#include - -void -printCFString(CFStringRef str) -{ - CFIndex bufferLength = CFStringGetLength(str) + 1; - char *pBuffer = (char *)malloc(sizeof(char) * bufferLength); - CFStringGetCString(str, pBuffer, bufferLength, kCFStringEncodingUTF8); - warnx("%s\n", pBuffer); - free(pBuffer); -} - -char * -cfstringToCstr(CFStringRef str) -{ - CFIndex bufferLength = CFStringGetLength(str) + 1; - char *pBuffer = (char *)malloc(sizeof(char) * bufferLength); - CFStringGetCString(str, pBuffer, bufferLength, kCFStringEncodingUTF8); - return pBuffer; -} - -void -appendHexChar(CFMutableStringRef str, unsigned char halfByte) -{ - if (halfByte < 10) - { - CFStringAppendFormat(str, NULL, CFSTR("%d"), halfByte); - } - else - { - char tmp[2] = {'A'+halfByte-10, 0}; - CFStringAppendCString(str, tmp, kCFStringEncodingUTF8); - } -} - -CFStringRef -createHexString(unsigned char *pData, int length) -{ - unsigned char byte, low, high; - int i; - CFMutableStringRef str = CFStringCreateMutable(NULL, 0); - - for (i = 0; i < length; i++) - { - byte = pData[i]; - low = byte & 0x0F; - high = (byte >> 4); - - appendHexChar(str, high); - appendHexChar(str, low); - - if (i != (length - 1)) - { - CFStringAppendCString(str, " ", kCFStringEncodingUTF8); - } - } - - return str; -} - -void -printHex(unsigned char *pData, int length) -{ - CFStringRef hexStr = createHexString(pData, length); - printCFString(hexStr); - CFRelease(hexStr); -} diff --git a/contrib/keychain-mcd/common_osx.h b/contrib/keychain-mcd/common_osx.h deleted file mode 100644 index d37e059e..00000000 --- a/contrib/keychain-mcd/common_osx.h +++ /dev/null @@ -1,38 +0,0 @@ -/* - * OpenVPN -- An application to securely tunnel IP networks - * over a single UDP port, with support for SSL/TLS-based - * session authentication and key exchange, - * packet encryption, packet authentication, and - * packet compression. - * - * Copyright (C) 2010 Brian Raderman - * Copyright (C) 2013-2015 Vasily Kulikov - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 - * as published by the Free Software Foundation. - * - * 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. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - */ - -#ifndef __common_osx_h__ -#define __common_osx_h__ - -#include - -void printCFString(CFStringRef str); - -char *cfstringToCstr(CFStringRef str); - -CFStringRef createHexString(unsigned char *pData, int length); - -void printHex(unsigned char *pData, int length); - -#endif /*__Common_osx_h__ */ diff --git a/contrib/keychain-mcd/crypto_osx.c b/contrib/keychain-mcd/crypto_osx.c deleted file mode 100644 index 27ac4f5b..00000000 --- a/contrib/keychain-mcd/crypto_osx.c +++ /dev/null @@ -1,79 +0,0 @@ -/* - * OpenVPN -- An application to securely tunnel IP networks - * over a single UDP port, with support for SSL/TLS-based - * session authentication and key exchange, - * packet encryption, packet authentication, and - * packet compression. - * - * Copyright (C) 2010 Brian Raderman - * Copyright (C) 2013-2015 Vasily Kulikov - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 - * as published by the Free Software Foundation. - * - * 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. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - */ - - -#include -#include -#include - -#include "crypto_osx.h" -#include - -void -printErrorMsg(const char *func, CFErrorRef error) -{ - CFStringRef desc = CFErrorCopyDescription(error); - warnx("%s failed: %s", func, CFStringGetCStringPtr(desc, kCFStringEncodingUTF8)); - CFRelease(desc); -} - -void -printErrorStatusMsg(const char *func, OSStatus status) -{ - CFStringRef error; - error = SecCopyErrorMessageString(status, NULL); - if (error) - { - warnx("%s failed: %s", func, CFStringGetCStringPtr(error, kCFStringEncodingUTF8)); - CFRelease(error); - } - else - { - warnx("%s failed: %X", func, (int)status); - } -} - -void -signData(SecIdentityRef identity, const uint8_t *from, int flen, uint8_t *to, size_t *tlen) -{ - SecKeyRef privateKey = NULL; - OSStatus status; - - status = SecIdentityCopyPrivateKey(identity, &privateKey); - if (status != noErr) - { - printErrorStatusMsg("signData: SecIdentityCopyPrivateKey", status); - *tlen = 0; - return; - } - - status = SecKeyRawSign(privateKey, kSecPaddingPKCS1, from, flen, to, tlen); - CFRelease(privateKey); - if (status != noErr) - { - printErrorStatusMsg("signData: SecKeyRawSign", status); - *tlen = 0; - return; - } -} diff --git a/contrib/keychain-mcd/crypto_osx.h b/contrib/keychain-mcd/crypto_osx.h deleted file mode 100644 index 9f4b3f9d..00000000 --- a/contrib/keychain-mcd/crypto_osx.h +++ /dev/null @@ -1,44 +0,0 @@ -/* - * OpenVPN -- An application to securely tunnel IP networks - * over a single UDP port, with support for SSL/TLS-based - * session authentication and key exchange, - * packet encryption, packet authentication, and - * packet compression. - * - * Copyright (C) 2010 Brian Raderman - * Copyright (C) 2013-2015 Vasily Kulikov - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 - * as published by the Free Software Foundation. - * - * 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. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - */ - -#ifndef __crypto_osx_h__ -#define __crypto_osx_h__ - -#include -#include - -extern OSStatus SecKeyRawSign( - SecKeyRef key, - SecPadding padding, - const uint8_t *dataToSign, - size_t dataToSignLen, - uint8_t *sig, - size_t *sigLen - ); - -void signData(SecIdentityRef identity, const uint8_t *from, int flen, uint8_t *to, size_t *tlen); - -void printErrorMsg(const char *func, CFErrorRef error); - -#endif /*__crypto_osx_h__ */ diff --git a/contrib/keychain-mcd/keychain-mcd.8 b/contrib/keychain-mcd/keychain-mcd.8 deleted file mode 100644 index 676b1646..00000000 --- a/contrib/keychain-mcd/keychain-mcd.8 +++ /dev/null @@ -1,161 +0,0 @@ -.TH keychain-mcd 8 -.SH NAME - -keychain-mcd \- Mac OS X Keychain management daemon for OpenVPN - -.SH SYNOPSIS - -.B keychain-mcd -.I identity-template management-server-ip management-server-port -[ -.I password-file -] - -.SH DESCRIPTION - -.B keychain-mcd -is Mac OS X Keychain management daemon for OpenVPN. -It loads the certificate and private key from the Mac OSX Keychain (Mac OSX Only). -.B keychain-mcd -connects to OpenVPN via management interface and handles -certificate and private key commands (namely -.B NEED-CERTIFICATE -and -.B RSA-SIGN -commands). - -.B keychain-mcd -makes it possible to use any smart card supported by Mac OSX using the tokend interface, but also any -kind of certificate, residing in the Keychain, where you have access to -the private key. This option has been tested on the client side with an Aladdin eToken -on Mac OSX Leopard and with software certificates stored in the Keychain on Mac OS X. - -Note that Mac OS X might need to present the user with an authentication GUI when the Keychain -is accessed by keychain-mcd. - -Use -.B keychain-mcd -along with -.B --management-external-key -and/or -.B --management-external-cert -passed to -.B openvpn. - -.SH OPTIONS - -.TP -.BR identity-template - -A select string which is used to choose a keychain identity from -Mac OS X Keychain or -.I auto -if the identity template is passed from openvpn. - -\fBSubject\fR, \fBIssuer\fR, \fBSerial\fR, \fBSHA1\fR, \fBMD5\fR selectors can be used. - -To select a certificate based on a string search in the -certificate's subject and/or issuer: - -.nf - -"SUBJECT:c=US/o=Apple Inc./ou=me.com/cn=username ISSUER:c=US/o=Apple Computer, Inc./ou=Apple Computer Certificate Authority/cn=Apple .Mac Certificate Authority" - -.fi - -.I "Distinguished Name Component Abbreviations:" -.br -o = organization -.br -ou = organizational unit -.br -c = country -.br -l = locality -.br -st = state -.br -cn = common name -.br -e = email -.br - -All of the distinguished name components are optional, although you do need to specify at least one of them. You can -add spaces around the '/' and '=' characters, e.g. "SUBJECT: c = US / o = Apple Inc.". You do not need to specify -both the subject and the issuer, one or the other will work fine. -The identity searching algorithm will return the -certificate it finds that matches all of the criteria you have specified. -If there are several certificates matching all of the criteria then the youngest certificate is returned -(i.e. with the greater "not before" validity field). -You can also include the MD5 and/or SHA1 thumbprints and/or serial number -along with the subject and issuer. - -To select a certificate based on certificate's MD5 or SHA1 thumbprint: - -.nf -"SHA1: 30 F7 3A 7A B7 73 2A 98 54 33 4A A7 00 6F 6E AC EC D1 EF 02" - -"MD5: D5 F5 11 F1 38 EB 5F 4D CF 23 B6 94 E8 33 D8 B5" -.fi - -Again, you can include both the SHA1 and the MD5 thumbprints, but you can also use just one of them. -The thumbprint hex strings can easily be copy-and-pasted from the OSX Keychain Access GUI in the Applications/Utilities folder. -The hex string comparison is not case sensitive. - -To select a certificate based on certificate's serial number: - -"Serial: 3E 9B 6F 02 00 00 00 01 1F 20" - -If -.BR identity-template -equals to -.I auto -then the actual identity template is -obtained from argument of NEED-CERTIFICATE notification of openvpn. -In this case the argument of NEED-CERTIFICATE must begin with 'macosx-keychain:' prefix -and the rest of it must contain the actual identity template in the format described above. - - -.TP -.BR management-server-ip -OpenVPN management IP to connect to. -Both IPv4 and IPv6 addresses can be used. - -.TP -.BR management-server-port -OpenVPN management port to connect to. -Use -.B unix -for -.I management-server-port -and socket path for -.I management-server-ip -to connect to a local unix socket. - -.TP -.BR password-file - -Password file containing the management password on first line. -The password will be used to connect to -.B openvpn -management interface. - -Pass -.I password-file -to -.B keychain-mcd -if -.I pw-file -was specified in -.B --management -option to -.B openvpn. - - -.SH AUTHOR - -Vasily Kulikov - -.SH "SEE ALSO" - -.BR openvpn (8) diff --git a/contrib/keychain-mcd/main.c b/contrib/keychain-mcd/main.c deleted file mode 100644 index c1d091ea..00000000 --- a/contrib/keychain-mcd/main.c +++ /dev/null @@ -1,310 +0,0 @@ -/* - * OpenVPN -- An application to securely tunnel IP networks - * over a single UDP port, with support for SSL/TLS-based - * session authentication and key exchange, - * packet encryption, packet authentication, and - * packet compression. - * - * Copyright (C) 2015 Vasily Kulikov - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 - * as published by the Free Software Foundation. - * - * 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. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - */ - - -#include -#include -#include -#include -#include -#include - -#include -#include - -#include "cert_data.h" -#include "crypto_osx.h" -#include "../../src/openvpn/base64.h" - - -SecIdentityRef -template_to_identity(const char *template) -{ - SecIdentityRef identity; - CertDataRef pCertDataTemplate = createCertDataFromString(template); - if (pCertDataTemplate == NULL) - { - errx(1, "Bad certificate template"); - } - identity = findIdentity(pCertDataTemplate); - if (identity == NULL) - { - errx(1, "No such identify"); - } - fprintf(stderr, "Identity found\n"); - destroyCertData(pCertDataTemplate); - return identity; -} - -int -connect_to_management_server(const char *ip, const char *port) -{ - int fd; - struct sockaddr_un addr_un; - struct sockaddr *addr; - size_t addr_len; - - if (strcmp(port, "unix") == 0) - { - addr = (struct sockaddr *)&addr_un; - addr_len = sizeof(addr_un); - - addr_un.sun_family = AF_UNIX; - strncpy(addr_un.sun_path, ip, sizeof(addr_un.sun_path)); - fd = socket(AF_UNIX, SOCK_STREAM, 0); - } - else - { - int rv; - struct addrinfo *result; - struct addrinfo hints; - - memset(&hints, 0, sizeof(hints)); - hints.ai_family = AF_UNSPEC; - hints.ai_socktype = SOCK_STREAM; - - rv = getaddrinfo(ip, port, &hints, &result); - if (rv < 0) - { - errx(1, "getaddrinfo: %s", gai_strerror(rv)); - } - if (result == NULL) - { - errx(1, "getaddrinfo returned 0 addressed"); - } - - /* Use the first found address */ - fd = socket(result->ai_family, result->ai_socktype, result->ai_protocol); - addr = result->ai_addr; - addr_len = result->ai_addrlen; - } - if (fd < 0) - { - err(1, "socket"); - } - - if (connect(fd, addr, addr_len) < 0) - { - err(1, "connect"); - } - - return fd; -} - -int -is_prefix(const char *s, const char *prefix) -{ - return strncmp(s, prefix, strlen(prefix)) == 0; -} - -void -handle_rsasign(FILE *man_file, SecIdentityRef identity, const char *input) -{ - const char *input_b64 = strchr(input, ':') + 1; - char *input_binary; - int input_len; - char *output_binary; - size_t output_len; - char *output_b64; - - input_len = strlen(input_b64)*8/6 + 4; - input_binary = malloc(input_len); - input_len = openvpn_base64_decode(input_b64, input_binary, input_len); - if (input_len < 0) - { - errx(1, "openvpn_base64_decode: overflow"); - } - - output_len = 1024; - output_binary = malloc(output_len); - signData(identity, (const uint8_t *)input_binary, input_len, (uint8_t *)output_binary, &output_len); - if (output_len == 0) - { - errx(1, "handle_rsasign: failed to sign data"); - } - - openvpn_base64_encode(output_binary, output_len, &output_b64); - fprintf(man_file, "rsa-sig\n%s\nEND\n", output_b64); - free(output_b64); - free(input_binary); - free(output_binary); - - fprintf(stderr, "Handled RSA_SIGN command\n"); -} - -void -handle_needcertificate(FILE *man_file, SecIdentityRef identity) -{ - OSStatus status; - SecCertificateRef certificate = NULL; - CFDataRef data; - const unsigned char *cert; - size_t cert_len; - char *result_b64, *tmp_b64; - - status = SecIdentityCopyCertificate(identity, &certificate); - if (status != noErr) - { - const char *msg = GetMacOSStatusErrorString(status); - err(1, "SecIdentityCopyCertificate() failed: %s", msg); - } - - data = SecCertificateCopyData(certificate); - if (data == NULL) - { - err(1, "SecCertificateCopyData() returned NULL"); - } - - cert = CFDataGetBytePtr(data); - cert_len = CFDataGetLength(data); - - openvpn_base64_encode(cert, cert_len, &result_b64); -#if 0 - fprintf(stderr, "certificate %s\n", result_b64); -#endif - - fprintf(man_file, "certificate\n"); - fprintf(man_file, "-----BEGIN CERTIFICATE-----\n"); - tmp_b64 = result_b64; - while (strlen(tmp_b64) > 64) { - fprintf(man_file, "%.64s\n", tmp_b64); - tmp_b64 += 64; - } - if (*tmp_b64) - { - fprintf(man_file, "%s\n", tmp_b64); - } - fprintf(man_file, "-----END CERTIFICATE-----\n"); - fprintf(man_file, "END\n"); - - free(result_b64); - CFRelease(data); - CFRelease(certificate); - - fprintf(stderr, "Handled NEED 'cert' command\n"); -} - -void -management_loop(SecIdentityRef identity, int man_fd, const char *password) -{ - char *buffer = NULL; - size_t buffer_len = 0; - FILE *man = fdopen(man_fd, "w+"); - if (man == 0) - { - err(1, "fdopen"); - } - - if (password) - { - fprintf(man, "%s\n", password); - } - - while (1) { - if (getline(&buffer, &buffer_len, man) < 0) - { - err(1, "getline"); - } -#if 0 - fprintf(stderr, "M: %s", buffer); -#endif - - if (is_prefix(buffer, ">RSA_SIGN:")) - { - handle_rsasign(man, identity, buffer); - } - if (is_prefix(buffer, ">NEED-CERTIFICATE")) - { - if (!identity) - { - const char prefix[] = ">NEED-CERTIFICATE:macosx-keychain:"; - if (!is_prefix(buffer, prefix)) - { - errx(1, "No identity template is passed via command line and " \ - "NEED-CERTIFICATE management interface command " \ - "misses 'macosx-keychain' prefix."); - } - identity = template_to_identity(buffer+strlen(prefix)); - } - handle_needcertificate(man, identity); - } - if (is_prefix(buffer, ">FATAL")) - { - fprintf(stderr, "Fatal message from OpenVPN: %s\n", buffer+7); - } - if (is_prefix(buffer, ">INFO")) - { - fprintf(stderr, "INFO message from OpenVPN: %s\n", buffer+6); - } - } -} - -char * -read_password(const char *fname) -{ - char *password = NULL; - FILE *pwf = fopen(fname, "r"); - size_t n = 0; - - if (pwf == NULL) - { - errx(1, "fopen(%s) failed", fname); - } - if (getline(&password, &n, pwf) < 0) - { - err(1, "getline"); - } - fclose(pwf); - return password; -} - -int -main(int argc, char *argv[]) -{ - if (argc < 4) - { - err(1, "usage: %s []", argv[0]); - } - - char *identity_template = argv[1]; - char *s_ip = argv[2]; - char *s_port = argv[3]; - char *password = NULL; - int man_fd; - - if (argc > 4) - { - char *s_pw_file = argv[4]; - password = read_password(s_pw_file); - } - - SecIdentityRef identity = NULL; - if (strcmp(identity_template, "auto")) - { - identity = template_to_identity(identity_template); - } - man_fd = connect_to_management_server(s_ip, s_port); - fprintf(stderr, "Successfully connected to openvpn\n"); - - management_loop(identity, man_fd, password); -} From e74e3a4db891b3ace0a96461c597d86e87be06f0 Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Tue, 25 Jul 2017 17:07:23 +0200 Subject: [PATCH 25/31] cleanup: Move init_random_seed() to where it is being used The init_random_seed() function is only used by the init_static() in init.c. As this function was pretty basic and it is only being called once, it was merged into init_static() instead of keeping it as a separate function. (I agree that calling functions often makes the code more readable, but I would rather see that as a part of cleaning up the whole init_static() function - in fact when moving all "unit tests" in init_static() to cmocka, it will not be too bad in the end.) Signed-off-by: David Sommerseth Acked-by: Steffan Karger Message-Id: <20170725150723.14919-1-davids@openvpn.net> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15136.html Signed-off-by: David Sommerseth --- src/openvpn/init.c | 17 +++++++++++++++-- src/openvpn/misc.c | 19 ------------------- src/openvpn/misc.h | 3 --- 3 files changed, 15 insertions(+), 24 deletions(-) diff --git a/src/openvpn/init.c b/src/openvpn/init.c index bc3b81e3..860df774 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -610,6 +610,7 @@ init_port_share(struct context *c) #endif /* if PORT_SHARE */ + bool init_static(void) { @@ -619,8 +620,20 @@ init_static(void) crypto_init_dmalloc(); #endif - init_random_seed(); /* init random() function, only used as - * source for weak random numbers */ + + /* + * Initialize random number seed. random() is only used + * when "weak" random numbers are acceptable. + * SSL library routines are always used when cryptographically + * strong random numbers are required. + */ + struct timeval tv; + if (!gettimeofday(&tv, NULL)) + { + const unsigned int seed = (unsigned int) tv.tv_sec ^ tv.tv_usec; + srandom(seed); + } + error_reset(); /* initialize error.c */ reset_check_status(); /* initialize status check code in socket.c */ diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index 8a76bba8..aff1bb2e 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -404,25 +404,6 @@ openvpn_popen(const struct argv *a, const struct env_set *es) -/* - * Initialize random number seed. random() is only used - * when "weak" random numbers are acceptable. - * OpenSSL routines are always used when cryptographically - * strong random numbers are required. - */ - -void -init_random_seed(void) -{ - struct timeval tv; - - if (!gettimeofday(&tv, NULL)) - { - const unsigned int seed = (unsigned int) tv.tv_sec ^ tv.tv_usec; - srandom(seed); - } -} - /* * Set environmental variable (int or string). * diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index 734e679c..a7aa7622 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -100,9 +100,6 @@ void set_std_files_to_null(bool stdin_only); extern int inetd_socket_descriptor; void save_inetd_socket_descriptor(void); -/* init random() function, only used as source for weak random numbers, when !ENABLE_CRYPTO */ -void init_random_seed(void); - /* set/delete environmental variable */ void setenv_str_ex(struct env_set *es, const char *name, From 4a9306255cf0e1cc056e66ed4fa0f2e687c137f6 Mon Sep 17 00:00:00 2001 From: Ilya Shipitsin Date: Mon, 7 Aug 2017 18:23:00 +0500 Subject: [PATCH 26/31] travis-ci: update openssl to 1.0.2l, update mbedtls to 2.5.1 Acked-by: Steffan Karger Message-Id: <20170807132301.22759-2-chipitsine@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15171.html Signed-off-by: David Sommerseth --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index db90e03a..7d842b10 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,10 +15,10 @@ env: - TAP_WINDOWS_VERSION=9.21.2 - LZO_VERSION=2.10 - PKCS11_HELPER_VERSION=1.11 - - MBEDTLS_VERSION="2.4.0" + - MBEDTLS_VERSION="2.5.1" - MBEDTLS_CFLAGS="-I${PREFIX}/include" - MBEDTLS_LIBS="-L${PREFIX}/lib -lmbedtls -lmbedx509 -lmbedcrypto" - - OPENSSL_VERSION="1.0.2k" + - OPENSSL_VERSION="1.0.2l" - OPENSSL_CFLAGS="-I${PREFIX}/include" - OPENSSL_LIBS="-L${PREFIX}/lib -lssl -lcrypto" From 28dba48541f5b212c7510ab3b0776dc39044502a Mon Sep 17 00:00:00 2001 From: Ilya Shipitsin Date: Wed, 9 Aug 2017 13:12:19 +0500 Subject: [PATCH 27/31] travis-ci: update pkcs11-helper to 1.22 use pkcs11-helper from https://github.com/OpenSC/pkcs11-helper/ to match build process used in windows installer build Signed-off-by: Ilya Shipitsin Acked-by: Steffan Karger Message-Id: <20170809081219.10367-1-chipitsine@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15187.html Signed-off-by: David Sommerseth --- .travis.yml | 2 +- .travis/build-deps.sh | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 7d842b10..0b531529 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,7 +14,7 @@ env: - PREFIX="${HOME}/opt" - TAP_WINDOWS_VERSION=9.21.2 - LZO_VERSION=2.10 - - PKCS11_HELPER_VERSION=1.11 + - PKCS11_HELPER_VERSION=1.22 - MBEDTLS_VERSION="2.5.1" - MBEDTLS_CFLAGS="-I${PREFIX}/include" - MBEDTLS_LIBS="-L${PREFIX}/lib -lmbedtls -lmbedx509 -lmbedcrypto" diff --git a/.travis/build-deps.sh b/.travis/build-deps.sh index 9cc18584..e787abab 100755 --- a/.travis/build-deps.sh +++ b/.travis/build-deps.sh @@ -35,7 +35,7 @@ build_lzo () { download_pkcs11_helper () { if [ ! -f "pkcs11-helper-${PKCS11_HELPER_VERSION}.tar.bz2" ]; then wget -P download-cache/ \ - "http://downloads.sourceforge.net/project/opensc/pkcs11-helper/pkcs11-helper-${PKCS11_HELPER_VERSION}.tar.bz2" + "https://github.com/OpenSC/pkcs11-helper/releases/download/pkcs11-helper-${PKCS11_HELPER_VERSION}/pkcs11-helper-${PKCS11_HELPER_VERSION}.tar.bz2" fi } @@ -46,7 +46,11 @@ build_pkcs11_helper () { cd "pkcs11-helper-${PKCS11_HELPER_VERSION}" ./configure --host=${CHOST} --program-prefix='' --libdir=${PREFIX}/lib \ - --prefix=${PREFIX} --build=x86_64-pc-linux-gnu --disable-crypto-engine-gnutls --disable-crypto-engine-nss + --prefix=${PREFIX} --build=x86_64-pc-linux-gnu \ + --disable-crypto-engine-gnutls \ + --disable-crypto-engine-nss \ + --disable-crypto-engine-polarssl \ + --disable-crypto-engine-mbedtls make all install ) echo "${PKCS11_HELPER_VERSION}" > "${PREFIX}/.pkcs11_helper-version" From 5b004f99d069fe0238aacbb0b3288872a4d7ae17 Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Wed, 9 Aug 2017 15:42:37 +0800 Subject: [PATCH 28/31] OpenSSL: remove unreachable call to SSL_CTX_get0_privatekey() In tls_ctx_load_ecdh_params() the SSL_CTX_get0_privatekey() function is invoked only when "OPENSSL_VERSION_NUMBER >= 0x10002000L" and curve_name is NULL. However, under the very same conditions the code flow will lead to an earlier return, thus never reaching the invocation of SSL_CTX_get0_privatekey(). Restructure the surrounding code in order to make the if/else block a bit easier to read and get rid of the unreachable invocation. Signed-off-by: Antonio Quartulli Acked-by: Steffan Karger Message-Id: <20170809074237.31291-1-a@unstable.cc> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15186.html Signed-off-by: David Sommerseth --- src/openvpn/ssl_openssl.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 189a8470..79947e37 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -484,15 +484,7 @@ tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, const char *curve_name /* Generate a new ECDH key for each SSL session (for non-ephemeral ECDH) */ SSL_CTX_set_options(ctx->ctx, SSL_OP_SINGLE_ECDH_USE); -#if OPENSSL_VERSION_NUMBER >= 0x10002000L - /* OpenSSL 1.0.2 and newer can automatically handle ECDH parameter loading */ - if (NULL == curve_name) - { - SSL_CTX_set_ecdh_auto(ctx->ctx, 1); - return; - } -#endif - /* For older OpenSSL, we'll have to do the parameter loading on our own */ + if (curve_name != NULL) { /* Use user supplied curve if given */ @@ -501,14 +493,17 @@ tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, const char *curve_name } else { - /* Extract curve from key */ +#if OPENSSL_VERSION_NUMBER >= 0x10002000L + /* OpenSSL 1.0.2 and newer can automatically handle ECDH parameter + * loading */ + SSL_CTX_set_ecdh_auto(ctx->ctx, 1); + return; +#else + /* For older OpenSSL we have to extract the curve from key on our own */ EC_KEY *eckey = NULL; const EC_GROUP *ecgrp = NULL; EVP_PKEY *pkey = NULL; -#if OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(LIBRESSL_VERSION_NUMBER) - pkey = SSL_CTX_get0_privatekey(ctx->ctx); -#else /* Little hack to get private key ref from SSL_CTX, yay OpenSSL... */ SSL *ssl = SSL_new(ctx->ctx); if (!ssl) @@ -517,7 +512,6 @@ tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, const char *curve_name } pkey = SSL_get_privatekey(ssl); SSL_free(ssl); -#endif msg(D_TLS_DEBUG, "Extracting ECDH curve from private key"); @@ -526,6 +520,7 @@ tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, const char *curve_name { nid = EC_GROUP_get_curve_name(ecgrp); } +#endif } /* Translate NID back to name , just for kicks */ From e2a0cad46e8f98399387c334fec912b7bb7097fc Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Fri, 11 Aug 2017 17:07:40 +0800 Subject: [PATCH 29/31] make function declarations C99 compliant In the attempt of adhering to the C99 standard as much as possible, ensure that all the function declarations with no parameter contain the "void" keyword[1]. Defects identified with sparse[2]. [1] ISO/IEC 9899:1999 spec, TC3 - section 6.7.5.3 [1] https://sparse.wiki.kernel.org/index.php/Main_Page Signed-off-by: Antonio Quartulli Acked-by: Steffan Karger Message-Id: <20170811090744.31750-2-a@unstable.cc> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15203.html Signed-off-by: David Sommerseth --- src/openvpn/buffer.c | 2 +- src/openvpn/console.c | 2 +- src/openvpn/console.h | 8 ++++---- src/openvpn/console_builtin.c | 2 +- src/openvpn/console_systemd.c | 4 ++-- src/openvpn/crypto.c | 4 ++-- src/openvpn/crypto.h | 2 +- src/openvpn/crypto_mbedtls.c | 10 +++++----- src/openvpn/crypto_mbedtls.h | 4 ++-- src/openvpn/crypto_openssl.c | 6 +++--- src/openvpn/error.c | 6 +++--- src/openvpn/error.h | 2 +- src/openvpn/init.c | 2 +- src/openvpn/interval.h | 2 +- src/openvpn/manage.c | 2 +- src/openvpn/occ-inline.h | 2 +- src/openvpn/packet_id.c | 2 +- src/openvpn/packet_id.h | 2 +- src/openvpn/pkcs11.c | 6 +++--- src/openvpn/platform.c | 2 +- src/openvpn/ps.c | 2 +- src/openvpn/ssl.c | 4 ++-- src/openvpn/ssl_backend.h | 6 +++--- src/openvpn/ssl_mbedtls.c | 6 +++--- src/openvpn/ssl_openssl.c | 12 ++++++------ src/openvpn/win32.c | 6 +++--- src/openvpn/win32.h | 4 ++-- 27 files changed, 56 insertions(+), 56 deletions(-) diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index 87e27ec0..a63ce14a 100644 --- a/src/openvpn/buffer.c +++ b/src/openvpn/buffer.c @@ -180,7 +180,7 @@ buf_assign(struct buffer *dest, const struct buffer *src) } struct buffer -clear_buf() +clear_buf(void) { struct buffer buf; CLEAR(buf); diff --git a/src/openvpn/console.c b/src/openvpn/console.c index eb6944d0..7e170241 100644 --- a/src/openvpn/console.c +++ b/src/openvpn/console.c @@ -44,7 +44,7 @@ struct _query_user query_user[QUERY_USER_NUMSLOTS]; /* GLOBAL */ void -query_user_clear() +query_user_clear(void) { int i; diff --git a/src/openvpn/console.h b/src/openvpn/console.h index aa51e6f6..3f74e77e 100644 --- a/src/openvpn/console.h +++ b/src/openvpn/console.h @@ -46,7 +46,7 @@ extern struct _query_user query_user[]; /**< Global variable, declared in conso * Wipes all data put into all of the query_user structs * */ -void query_user_clear(); +void query_user_clear(void); /** @@ -72,7 +72,7 @@ void query_user_add(char *prompt, size_t prompt_len, * * @return True if executing all the defined steps completed successfully */ -bool query_user_exec_builtin(); +bool query_user_exec_builtin(void); #if defined(ENABLE_SYSTEMD) @@ -83,7 +83,7 @@ bool query_user_exec_builtin(); * * @return True if executing all the defined steps completed successfully */ -bool query_user_exec(); +bool query_user_exec(void); #else /* ENABLE_SYSTEMD not defined*/ /** @@ -92,7 +92,7 @@ bool query_user_exec(); * */ static bool -query_user_exec() +query_user_exec(void) { return query_user_exec_builtin(); } diff --git a/src/openvpn/console_builtin.c b/src/openvpn/console_builtin.c index 7b95da9d..f005ed74 100644 --- a/src/openvpn/console_builtin.c +++ b/src/openvpn/console_builtin.c @@ -267,7 +267,7 @@ get_console_input(const char *prompt, const bool echo, char *input, const int ca * */ bool -query_user_exec_builtin() +query_user_exec_builtin(void) { bool ret = true; /* Presume everything goes okay */ int i; diff --git a/src/openvpn/console_systemd.c b/src/openvpn/console_systemd.c index 8cee8c8e..e7a72ae3 100644 --- a/src/openvpn/console_systemd.c +++ b/src/openvpn/console_systemd.c @@ -41,7 +41,7 @@ */ static bool -check_systemd_running() +check_systemd_running(void) { struct stat c; @@ -95,7 +95,7 @@ get_console_input_systemd(const char *prompt, const bool echo, char *input, cons * */ bool -query_user_exec() +query_user_exec(void) { bool ret = true; /* Presume everything goes okay */ int i; diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index 78ca4197..cc3c7b63 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -1693,7 +1693,7 @@ static int nonce_secret_len = 0; /* GLOBAL */ /* Reset the nonce value, also done periodically to refresh entropy */ static void -prng_reset_nonce() +prng_reset_nonce(void) { const int size = md_kt_size(nonce_md) + nonce_secret_len; #if 1 /* Must be 1 for real usage */ @@ -1772,7 +1772,7 @@ prng_bytes(uint8_t *output, int len) /* an analogue to the random() function, but use prng_bytes */ long int -get_random() +get_random(void) { long int l; prng_bytes((unsigned char *)&l, sizeof(l)); diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h index fec2eea7..5dae383c 100644 --- a/src/openvpn/crypto.h +++ b/src/openvpn/crypto.h @@ -453,7 +453,7 @@ void prng_init(const char *md_name, const int nonce_secret_len_parm); */ void prng_bytes(uint8_t *output, int len); -void prng_uninit(); +void prng_uninit(void); void test_crypto(struct crypto_options *co, struct frame *f); diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c index 30b51a5f..f4d239bc 100644 --- a/src/openvpn/crypto_mbedtls.c +++ b/src/openvpn/crypto_mbedtls.c @@ -159,7 +159,7 @@ print_cipher(const cipher_kt_t *info) } void -show_available_ciphers() +show_available_ciphers(void) { const int *ciphers = mbedtls_cipher_list(); @@ -196,7 +196,7 @@ show_available_ciphers() } void -show_available_digests() +show_available_digests(void) { const int *digests = mbedtls_md_list(); @@ -223,7 +223,7 @@ show_available_digests() } void -show_available_engines() +show_available_engines(void) { printf("Sorry, mbed TLS hardware crypto engine functionality is not " "available\n"); @@ -243,7 +243,7 @@ show_available_engines() * entropy gathering function. */ mbedtls_ctr_drbg_context * -rand_ctx_get() +rand_ctx_get(void) { static mbedtls_entropy_context ec = {0}; static mbedtls_ctr_drbg_context cd_ctx = {0}; @@ -280,7 +280,7 @@ rand_ctx_get() #ifdef ENABLE_PREDICTION_RESISTANCE void -rand_ctx_enable_prediction_resistance() +rand_ctx_enable_prediction_resistance(void) { mbedtls_ctr_drbg_context *cd_ctx = rand_ctx_get(); diff --git a/src/openvpn/crypto_mbedtls.h b/src/openvpn/crypto_mbedtls.h index a434ce34..4417b924 100644 --- a/src/openvpn/crypto_mbedtls.h +++ b/src/openvpn/crypto_mbedtls.h @@ -85,13 +85,13 @@ typedef mbedtls_md_context_t hmac_ctx_t; * added. During initialisation, a personalisation string will be added based * on the time, the PID, and a pointer to the random context. */ -mbedtls_ctr_drbg_context *rand_ctx_get(); +mbedtls_ctr_drbg_context *rand_ctx_get(void); #ifdef ENABLE_PREDICTION_RESISTANCE /** * Enable prediction resistance on the random number generator. */ -void rand_ctx_enable_prediction_resistance(); +void rand_ctx_enable_prediction_resistance(void); #endif diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index 138455a4..0134e55d 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -280,7 +280,7 @@ print_cipher(const EVP_CIPHER *cipher) } void -show_available_ciphers() +show_available_ciphers(void) { int nid; size_t i; @@ -339,7 +339,7 @@ show_available_ciphers() } void -show_available_digests() +show_available_digests(void) { int nid; @@ -364,7 +364,7 @@ show_available_digests() } void -show_available_engines() +show_available_engines(void) { #if HAVE_OPENSSL_ENGINE /* Only defined for OpenSSL */ ENGINE *e; diff --git a/src/openvpn/error.c b/src/openvpn/error.c index 3817666b..04bf0da5 100644 --- a/src/openvpn/error.c +++ b/src/openvpn/error.c @@ -159,7 +159,7 @@ set_machine_readable_output(bool parsable) } void -error_reset() +error_reset(void) { use_syslog = std_redir = false; suppress_timestamps = false; @@ -480,7 +480,7 @@ open_syslog(const char *pgmname, bool stdio_to_null) } void -close_syslog() +close_syslog(void) { #if SYSLOG_CAPABILITY if (use_syslog) @@ -635,7 +635,7 @@ unsigned int x_cs_verbose_level; /* GLOBAL */ unsigned int x_cs_err_delay_ms; /* GLOBAL */ void -reset_check_status() +reset_check_status(void) { x_cs_info_level = 0; x_cs_verbose_level = 0; diff --git a/src/openvpn/error.h b/src/openvpn/error.h index 14ef7e65..023cec46 100644 --- a/src/openvpn/error.h +++ b/src/openvpn/error.h @@ -261,7 +261,7 @@ void msg_forked(void); void open_syslog(const char *pgmname, bool stdio_to_null); -void close_syslog(); +void close_syslog(void); /* log file output */ void redirect_stdout_stderr(const char *file, bool append); diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 860df774..f0fc2418 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -1917,7 +1917,7 @@ do_close_tun(struct context *c, bool force) } void -tun_abort() +tun_abort(void) { struct context *c = static_context; if (c) diff --git a/src/openvpn/interval.h b/src/openvpn/interval.h index 8095c0b9..dd5dfbc8 100644 --- a/src/openvpn/interval.h +++ b/src/openvpn/interval.h @@ -155,7 +155,7 @@ event_timeout_clear(struct event_timeout *et) } static inline struct event_timeout -event_timeout_clear_ret() +event_timeout_clear_ret(void) { struct event_timeout ret; event_timeout_clear(&ret); diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c index ff948240..3bbe972e 100644 --- a/src/openvpn/manage.c +++ b/src/openvpn/manage.c @@ -68,7 +68,7 @@ static void man_output_standalone(struct management *man, volatile int *signal_r static void man_reset_client_socket(struct management *man, const bool exiting); static void -man_help() +man_help(void) { msg(M_CLIENT, "Management Interface for %s", title_string); msg(M_CLIENT, "Commands:"); diff --git a/src/openvpn/occ-inline.h b/src/openvpn/occ-inline.h index 68e9098f..0fa8e5ba 100644 --- a/src/openvpn/occ-inline.h +++ b/src/openvpn/occ-inline.h @@ -31,7 +31,7 @@ */ static inline int -occ_reset_op() +occ_reset_op(void) { return -1; } diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c index 30ae8fbc..a3ff5722 100644 --- a/src/openvpn/packet_id.c +++ b/src/openvpn/packet_id.c @@ -643,7 +643,7 @@ packet_id_debug_print(int msglevel, #ifdef PID_TEST void -packet_id_interactive_test() +packet_id_interactive_test(void) { struct packet_id pid; struct packet_id_net pin; diff --git a/src/openvpn/packet_id.h b/src/openvpn/packet_id.h index a370936c..8509e590 100644 --- a/src/openvpn/packet_id.h +++ b/src/openvpn/packet_id.h @@ -299,7 +299,7 @@ packet_id_persist_save_obj(struct packet_id_persist *p, const struct packet_id * const char *packet_id_net_print(const struct packet_id_net *pin, bool print_timestamp, struct gc_arena *gc); #ifdef PID_TEST -void packet_id_interactive_test(); +void packet_id_interactive_test(void); #endif diff --git a/src/openvpn/pkcs11.c b/src/openvpn/pkcs11.c index 60418280..a0d0906a 100644 --- a/src/openvpn/pkcs11.c +++ b/src/openvpn/pkcs11.c @@ -356,7 +356,7 @@ cleanup: } void -pkcs11_terminate() +pkcs11_terminate(void) { dmsg( D_PKCS11_DEBUG, @@ -422,13 +422,13 @@ pkcs11_addProvider( } int -pkcs11_logout() +pkcs11_logout(void) { return pkcs11h_logout() == CKR_OK; } int -pkcs11_management_id_count() +pkcs11_management_id_count(void) { pkcs11h_certificate_id_list_t id_list = NULL; pkcs11h_certificate_id_list_t t = NULL; diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c index d936890e..e942ba94 100644 --- a/src/openvpn/platform.c +++ b/src/openvpn/platform.c @@ -173,7 +173,7 @@ platform_nice(int niceval) /* Get current PID */ unsigned int -platform_getpid() +platform_getpid(void) { #ifdef _WIN32 return (unsigned int) GetCurrentProcessId(); diff --git a/src/openvpn/ps.c b/src/openvpn/ps.c index c2b05cd9..45e24ded 100644 --- a/src/openvpn/ps.c +++ b/src/openvpn/ps.c @@ -172,7 +172,7 @@ send_control(const socket_descriptor_t fd, int code) } static int -cmsg_size() +cmsg_size(void) { return CMSG_SPACE(sizeof(socket_descriptor_t)); } diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index df232894..339baf7d 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -347,7 +347,7 @@ tls_init_control_channel_frame_parameters(const struct frame *data_channel_frame } void -init_ssl_lib() +init_ssl_lib(void) { tls_init_lib(); @@ -355,7 +355,7 @@ init_ssl_lib() } void -free_ssl_lib() +free_ssl_lib(void) { crypto_uninit_lib(); prng_uninit(); diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h index a738f0f4..aba5a4de 100644 --- a/src/openvpn/ssl_backend.h +++ b/src/openvpn/ssl_backend.h @@ -88,17 +88,17 @@ int pem_password_callback(char *buf, int size, int rwflag, void *u); * Perform any static initialisation necessary by the library. * Called on OpenVPN initialisation */ -void tls_init_lib(); +void tls_init_lib(void); /** * Free any global SSL library-specific data structures. */ -void tls_free_lib(); +void tls_free_lib(void); /** * Clear the underlying SSL library's error state. */ -void tls_clear_error(); +void tls_clear_error(void); /** * Parse a TLS version specifier diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index ef583e67..861d936d 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -63,17 +63,17 @@ #include void -tls_init_lib() +tls_init_lib(void) { } void -tls_free_lib() +tls_free_lib(void) { } void -tls_clear_error() +tls_clear_error(void) { } diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 79947e37..090a66e8 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -69,7 +69,7 @@ int mydata_index; /* GLOBAL */ void -tls_init_lib() +tls_init_lib(void) { SSL_library_init(); #ifndef ENABLE_SMALL @@ -82,7 +82,7 @@ tls_init_lib() } void -tls_free_lib() +tls_free_lib(void) { EVP_cleanup(); #ifndef ENABLE_SMALL @@ -91,7 +91,7 @@ tls_free_lib() } void -tls_clear_error() +tls_clear_error(void) { ERR_clear_error(); } @@ -1329,7 +1329,7 @@ static time_t biofp_last_open; /* GLOBAL */ static const int biofp_reopen_interval = 600; /* GLOBAL */ static void -close_biofp() +close_biofp(void) { if (biofp) { @@ -1339,7 +1339,7 @@ close_biofp() } static void -open_biofp() +open_biofp(void) { const time_t current = time(NULL); const pid_t pid = getpid(); @@ -1775,7 +1775,7 @@ show_available_tls_ciphers(const char *cipher_list) * in the OpenSSL library. */ void -show_available_curves() +show_available_curves(void) { #ifndef OPENSSL_NO_EC EC_builtin_curve *curves = NULL; diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c index d0b10bad..95fea5df 100644 --- a/src/openvpn/win32.c +++ b/src/openvpn/win32.c @@ -1235,7 +1235,7 @@ set_win_sys_path_via_env(struct env_set *es) const char * -win_get_tempdir() +win_get_tempdir(void) { static char tmpdir[MAX_PATH]; WCHAR wtmpdir[MAX_PATH]; @@ -1398,7 +1398,7 @@ win_wfp_uninit(const NET_IFINDEX index, const HANDLE msg_channel) } int -win32_version_info() +win32_version_info(void) { if (!IsWindowsXPOrGreater()) { @@ -1426,7 +1426,7 @@ win32_version_info() } bool -win32_is_64bit() +win32_is_64bit(void) { #if defined(_WIN64) return true; /* 64-bit programs run only on Win64 */ diff --git a/src/openvpn/win32.h b/src/openvpn/win32.h index 21a1021a..7fc57ccc 100644 --- a/src/openvpn/win32.h +++ b/src/openvpn/win32.h @@ -285,7 +285,7 @@ char *get_win_sys_path(void); void fork_to_self(const char *cmdline); /* Find temporary directory */ -const char *win_get_tempdir(); +const char *win_get_tempdir(void); /* Convert a string from UTF-8 to UCS-2 */ WCHAR *wide_string(const char *utf8, struct gc_arena *gc); @@ -299,7 +299,7 @@ bool win_wfp_uninit(const NET_IFINDEX index, const HANDLE msg_channel); #define WIN_7 2 #define WIN_8 3 -int win32_version_info(); +int win32_version_info(void); /* * String representation of Windows version number and name, see From 4158f46f6474447520ebc7440050411eb8be8cb9 Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Fri, 11 Aug 2017 17:07:43 +0800 Subject: [PATCH 30/31] remove unused functions Signed-off-by: Antonio Quartulli Acked-by: Steffan Karger Message-Id: <20170811090744.31750-5-a@unstable.cc> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15205.html Signed-off-by: David Sommerseth --- src/openvpn/misc.c | 23 ----------------------- src/openvpn/ssl.c | 10 ---------- src/openvpn/ssl_openssl.c | 6 ------ 3 files changed, 39 deletions(-) diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index aff1bb2e..0adb164e 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -429,29 +429,6 @@ construct_name_value(const char *name, const char *value, struct gc_arena *gc) return BSTR(&out); } -bool -deconstruct_name_value(const char *str, const char **name, const char **value, struct gc_arena *gc) -{ - char *cp; - - ASSERT(str); - ASSERT(name && value); - - *name = cp = string_alloc(str, gc); - *value = NULL; - - while ((*cp)) - { - if (*cp == '=' && !*value) - { - *cp = 0; - *value = cp + 1; - } - ++cp; - } - return *name && *value; -} - static bool env_string_equal(const char *s1, const char *s2) { diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 339baf7d..ff311d8e 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -2190,16 +2190,6 @@ read_string_alloc(struct buffer *buf) return str; } -void -read_string_discard(struct buffer *buf) -{ - char *data = read_string_alloc(buf); - if (data) - { - free(data); - } -} - /* * Handle the reading and writing of key data to and from * the TLS control channel (cleartext). diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 090a66e8..26b71743 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -803,12 +803,6 @@ tls_ctx_load_cert_file(struct tls_root_ctx *ctx, const char *cert_file, tls_ctx_load_cert_file_and_copy(ctx, cert_file, cert_file_inline, NULL); } -void -tls_ctx_free_cert_file(X509 *x509) -{ - X509_free(x509); -} - int tls_ctx_load_priv_file(struct tls_root_ctx *ctx, const char *priv_key_file, const char *priv_key_file_inline From 280150a02a117eb0cc9c34e69ebe9ec3f4ded0f4 Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Fri, 11 Aug 2017 17:07:44 +0800 Subject: [PATCH 31/31] use NULL instead of 0 when assigning pointers Signed-off-by: Antonio Quartulli Acked-by: Gert Doering Message-Id: <20170811090744.31750-6-a@unstable.cc> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15204.html Signed-off-by: David Sommerseth --- src/openvpn/ps.c | 2 +- src/openvpn/ssl_openssl.c | 2 +- src/openvpn/ssl_verify_openssl.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/openvpn/ps.c b/src/openvpn/ps.c index 45e24ded..5136a20c 100644 --- a/src/openvpn/ps.c +++ b/src/openvpn/ps.c @@ -922,7 +922,7 @@ port_share_open(const char *host, openvpn_close_socket(fd[1]); exit(0); - return 0; /* NOTREACHED */ + return NULL; /* NOTREACHED */ } error: diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 26b71743..597c62d8 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -705,7 +705,7 @@ tls_ctx_add_extra_certs(struct tls_root_ctx *ctx, BIO *bio) for (;; ) { cert = NULL; - if (!PEM_read_bio_X509(bio, &cert, 0, NULL)) /* takes ownership of cert */ + if (!PEM_read_bio_X509(bio, &cert, NULL, NULL)) /* takes ownership of cert */ { break; } diff --git a/src/openvpn/ssl_verify_openssl.c b/src/openvpn/ssl_verify_openssl.c index ea474955..2f3b10b9 100644 --- a/src/openvpn/ssl_verify_openssl.c +++ b/src/openvpn/ssl_verify_openssl.c @@ -202,8 +202,8 @@ extract_x509_field_ssl(X509_NAME *x509, const char *field_name, char *out, { int lastpos = -1; int tmp = -1; - X509_NAME_ENTRY *x509ne = 0; - ASN1_STRING *asn1 = 0; + X509_NAME_ENTRY *x509ne = NULL; + ASN1_STRING *asn1 = NULL; unsigned char *buf = NULL; ASN1_OBJECT *field_name_obj = OBJ_txt2obj(field_name, 0);