[Openvpn-devel] [Patch v2] Fix message for too long tls-crypt-v2 metadata
The current code only checks if the base64-encoded metadata is at most
980 character. However, that can encode up to 735 bytes of data, while
only up to 733 bytes are allowed. When passing 734 or 735 bytes, openvpn
prints a misleading error message saying that the base64 cannot be
decoded.
This patch checks the decoded length to show an accurate error message.
v2: Remove now-unused macro and fix an off-by-one error.
Signed-off-by: Max Fillinger
---
src/openvpn/base64.h| 4
src/openvpn/tls_crypt.c | 18 +++---
src/openvpn/tls_crypt.h | 2 --
3 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/src/openvpn/base64.h b/src/openvpn/base64.h
index f49860fc..7b4224a5 100644
--- a/src/openvpn/base64.h
+++ b/src/openvpn/base64.h
@@ -38,6 +38,10 @@
#define OPENVPN_BASE64_LENGTH(binary_length) \
8 * binary_length) / 6) + 3) & ~3)
+/** Compute the maximal number of bytes encoded in a base64 string. */
+#define OPENVPN_BASE64_DECODED_LENGTH(base64_length) \
+((base64_length / 4) * 3)
+
int openvpn_base64_encode(const void *data, int size, char **str);
int openvpn_base64_decode(const char *str, void *data, int size);
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 2fc79111..1e461fcf 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -627,15 +627,11 @@ tls_crypt_v2_write_client_key_file(const char *filename,
}
ASSERT(buf_write(, client_key.keys, sizeof(client_key.keys)));
-struct buffer metadata = alloc_buf_gc(TLS_CRYPT_V2_MAX_METADATA_LEN, );
+struct buffer metadata;
if (b64_metadata)
{
-if (TLS_CRYPT_V2_MAX_B64_METADATA_LEN < strlen(b64_metadata))
-{
-msg(M_FATAL,
-"ERROR: metadata too long (%d bytes, max %u bytes)",
-(int)strlen(b64_metadata), TLS_CRYPT_V2_MAX_B64_METADATA_LEN);
-}
+size_t b64_length = strlen(b64_metadata);
+metadata = alloc_buf_gc(OPENVPN_BASE64_DECODED_LENGTH(b64_length) + 1,
);
ASSERT(buf_write(, _CRYPT_METADATA_TYPE_USER, 1));
int decoded_len = openvpn_base64_decode(b64_metadata, BEND(),
BCAP());
@@ -644,10 +640,18 @@ tls_crypt_v2_write_client_key_file(const char *filename,
msg(M_FATAL, "ERROR: failed to base64 decode provided metadata");
goto cleanup;
}
+if (decoded_len > TLS_CRYPT_V2_MAX_METADATA_LEN - 1)
+{
+msg(M_FATAL,
+"ERROR: metadata too long (%d bytes, max %u bytes)",
+decoded_len, TLS_CRYPT_V2_MAX_METADATA_LEN - 1);
+goto cleanup;
+}
ASSERT(buf_inc_len(, decoded_len));
}
else
{
+metadata = alloc_buf_gc(1 + sizeof(int64_t), );
int64_t timestamp = htonll((uint64_t)now);
ASSERT(buf_write(, _CRYPT_METADATA_TYPE_TIMESTAMP, 1));
ASSERT(buf_write(, , sizeof(timestamp)));
diff --git a/src/openvpn/tls_crypt.h b/src/openvpn/tls_crypt.h
index 928ff547..d5c73752 100644
--- a/src/openvpn/tls_crypt.h
+++ b/src/openvpn/tls_crypt.h
@@ -101,8 +101,6 @@
#define TLS_CRYPT_V2_MAX_METADATA_LEN (unsigned)(TLS_CRYPT_V2_MAX_WKC_LEN \
-
(TLS_CRYPT_V2_CLIENT_KEY_LEN + TLS_CRYPT_V2_TAG_SIZE \
+ sizeof(uint16_t)))
-#define TLS_CRYPT_V2_MAX_B64_METADATA_LEN \
-OPENVPN_BASE64_LENGTH(TLS_CRYPT_V2_MAX_METADATA_LEN - 1)
/**
* Initialize a key_ctx_bi structure for use with --tls-crypt.
--
2.30.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 2/2] Fix message for too long tls-crypt-v2 metadata
The current code only checks if the base64-encoded metadata is at most
980 characters. However, that can encode up to 735 bytes of data, while
only up to 733 bytes are allowed. When passing 734 or 735 bytes, openvpn
prints a misleading error message saying that the base64 cannot be
decoded.
This patch checks the decoded length to show an accurate error message.
Signed-off-by: Max Fillinger
---
src/openvpn/base64.h| 4
src/openvpn/tls_crypt.c | 18 +++---
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/src/openvpn/base64.h b/src/openvpn/base64.h
index f49860fc..7b4224a5 100644
--- a/src/openvpn/base64.h
+++ b/src/openvpn/base64.h
@@ -38,6 +38,10 @@
#define OPENVPN_BASE64_LENGTH(binary_length) \
8 * binary_length) / 6) + 3) & ~3)
+/** Compute the maximal number of bytes encoded in a base64 string. */
+#define OPENVPN_BASE64_DECODED_LENGTH(base64_length) \
+((base64_length / 4) * 3)
+
int openvpn_base64_encode(const void *data, int size, char **str);
int openvpn_base64_decode(const char *str, void *data, int size);
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 2fc79111..5d247b84 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -627,15 +627,11 @@ tls_crypt_v2_write_client_key_file(const char *filename,
}
ASSERT(buf_write(, client_key.keys, sizeof(client_key.keys)));
-struct buffer metadata = alloc_buf_gc(TLS_CRYPT_V2_MAX_METADATA_LEN, );
+struct buffer metadata;
if (b64_metadata)
{
-if (TLS_CRYPT_V2_MAX_B64_METADATA_LEN < strlen(b64_metadata))
-{
-msg(M_FATAL,
-"ERROR: metadata too long (%d bytes, max %u bytes)",
-(int)strlen(b64_metadata), TLS_CRYPT_V2_MAX_B64_METADATA_LEN);
-}
+size_t b64_length = strlen(b64_metadata);
+metadata = alloc_buf_gc(OPENVPN_BASE64_DECODED_LENGTH(b64_length) + 1,
);
ASSERT(buf_write(, _CRYPT_METADATA_TYPE_USER, 1));
int decoded_len = openvpn_base64_decode(b64_metadata, BEND(),
BCAP());
@@ -644,10 +640,18 @@ tls_crypt_v2_write_client_key_file(const char *filename,
msg(M_FATAL, "ERROR: failed to base64 decode provided metadata");
goto cleanup;
}
+if (decoded_len > TLS_CRYPT_V2_MAX_METADATA_LEN)
+{
+msg(M_FATAL,
+"ERROR: metadata too long (%d bytes, max %u bytes)",
+decoded_len, TLS_CRYPT_V2_MAX_METADATA_LEN - 1);
+goto cleanup;
+}
ASSERT(buf_inc_len(, decoded_len));
}
else
{
+metadata = alloc_buf_gc(1 + sizeof(int64_t), );
int64_t timestamp = htonll((uint64_t)now);
ASSERT(buf_write(, _CRYPT_METADATA_TYPE_TIMESTAMP, 1));
ASSERT(buf_write(, , sizeof(timestamp)));
--
2.30.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 1/2] Correct tls-crypt-v2 metadata length in man page
The manual page claims that the client metadata can be up to 735 bytes (encoded as upt to 980 characters base64), but the actual maximum length is 733 bytes which is also encoded as 980 characters in base64. Signed-off-by: Max Fillinger --- doc/man-sections/encryption-options.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/man-sections/encryption-options.rst b/doc/man-sections/encryption-options.rst index ee34f14e..abc73d90 100644 --- a/doc/man-sections/encryption-options.rst +++ b/doc/man-sections/encryption-options.rst @@ -104,7 +104,8 @@ Generating key material If supplied, include the supplied ``metadata`` in the wrapped client key. This metadata must be supplied in base64-encoded form. The -metadata must be at most 735 bytes long (980 bytes in base64). +metadata must be at most 733 bytes long (980 characters in base64, though +note that 980 base64 characters can encode more than 733 bytes). If no metadata is supplied, OpenVPN will use a 64-bit unix timestamp representing the current time in UTC, encoded in network order, as -- 2.30.2 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Check if pkcs11_cert is NULL before freeing it
When running openvpn --show-tls with mbedtls, it showed a null pointer
error at the end because of this.
Signed-off-by: Max Fillinger
---
src/openvpn/ssl_mbedtls.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index ea06cf70..aa55a1a0 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -165,7 +165,10 @@ tls_ctx_free(struct tls_root_ctx *ctx)
free(ctx->crl);
#if defined(ENABLE_PKCS11)
-pkcs11h_certificate_freeCertificate(ctx->pkcs11_cert);
+if (ctx->pkcs11_cert)
+{
+pkcs11h_certificate_freeCertificate(ctx->pkcs11_cert);
+}
#endif
free(ctx->allowed_ciphers);
--
2.30.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [Patch v2 1/2] Update openssl_compat.h for newer LibreSSL
LibreSSL has added some of the functions that are defined here. However, we still need RSA_F_RSA_OSSL_PRIVATE_ENCRYPT. v2: Change ifdef condition for RSA_F_RSA_OSSL_PRIVATE_ENCRYPT. v3: Don't break WolfSSL. Signed-off-by: Max Fillinger --- src/openvpn/openssl_compat.h | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h index b3ee94f1..9d89bd0a 100644 --- a/src/openvpn/openssl_compat.h +++ b/src/openvpn/openssl_compat.h @@ -51,8 +51,8 @@ #define SSL_CTX_set1_groups SSL_CTX_set1_curves #endif -/* Functionality missing in LibreSSL and OpenSSL 1.0.2 */ -#if (OPENSSL_VERSION_NUMBER < 0x1010L || defined(LIBRESSL_VERSION_NUMBER)) && !defined(ENABLE_CRYPTO_WOLFSSL) +/* Functionality missing in LibreSSL before 3.5 and OpenSSL 1.0.2 */ +#if (OPENSSL_VERSION_NUMBER < 0x1010L || (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x305fL)) && !defined(ENABLE_CRYPTO_WOLFSSL) /** * Destroy a X509 object * @@ -68,11 +68,13 @@ X509_OBJECT_free(X509_OBJECT *obj) } } -#define RSA_F_RSA_OSSL_PRIVATE_ENCRYPT RSA_F_RSA_EAY_PRIVATE_ENCRYPT #define EVP_CTRL_AEAD_SET_TAGEVP_CTRL_GCM_SET_TAG #define EVP_CTRL_AEAD_GET_TAGEVP_CTRL_GCM_GET_TAG #endif +#if (OPENSSL_VERSION_NUMBER < 0x1010L || defined(LIBRESSL_VERSION_NUMBER)) && !defined(ENABLE_CRYPTO_WOLFSSL) +#define RSA_F_RSA_OSSL_PRIVATE_ENCRYPT RSA_F_RSA_EAY_PRIVATE_ENCRYPT +#endif /* Functionality missing in 1.0.2 */ #if OPENSSL_VERSION_NUMBER < 0x1010L && !defined(ENABLE_CRYPTO_WOLFSSL) -- 2.30.2 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2 1/2] Update openssl_compat.h for newer LibreSSL
LibreSSL has added some of the functions that are defined here. However, we still need RSA_F_RSA_OSSL_PRIVATE_ENCRYPT. v2: Change ifdef condition for RSA_F_RSA_OSSL_PRIVATE_ENCRYPT. Signed-off-by: Max Fillinger --- src/openvpn/openssl_compat.h | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h index b3ee94f1..c78d2229 100644 --- a/src/openvpn/openssl_compat.h +++ b/src/openvpn/openssl_compat.h @@ -51,8 +51,8 @@ #define SSL_CTX_set1_groups SSL_CTX_set1_curves #endif -/* Functionality missing in LibreSSL and OpenSSL 1.0.2 */ -#if (OPENSSL_VERSION_NUMBER < 0x1010L || defined(LIBRESSL_VERSION_NUMBER)) && !defined(ENABLE_CRYPTO_WOLFSSL) +/* Functionality missing in LibreSSL before 3.5 and OpenSSL 1.0.2 */ +#if (OPENSSL_VERSION_NUMBER < 0x1010L || (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x305fL)) && !defined(ENABLE_CRYPTO_WOLFSSL) /** * Destroy a X509 object * @@ -68,11 +68,13 @@ X509_OBJECT_free(X509_OBJECT *obj) } } -#define RSA_F_RSA_OSSL_PRIVATE_ENCRYPT RSA_F_RSA_EAY_PRIVATE_ENCRYPT #define EVP_CTRL_AEAD_SET_TAGEVP_CTRL_GCM_SET_TAG #define EVP_CTRL_AEAD_GET_TAGEVP_CTRL_GCM_GET_TAG #endif +#if OPENSSL_VERSION_NUMBER < 0x1010L || defined(LIBRESSL_VERSION_NUMBER) +#define RSA_F_RSA_OSSL_PRIVATE_ENCRYPT RSA_F_RSA_EAY_PRIVATE_ENCRYPT +#endif /* Functionality missing in 1.0.2 */ #if OPENSSL_VERSION_NUMBER < 0x1010L && !defined(ENABLE_CRYPTO_WOLFSSL) -- 2.30.2 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 2/2] Handle EVP_MD_CTX as an opaque struct
Building OpenVPN on the latest OpenBSD snapshot failed because EVP_MD_CTX
is an opaque struct in LibreSSL now. Therefore, call md_ctx_new() instead
of declaring them on the stack. When they're not on the stack anymore, we
don't have to call EVP_MD_CTX_init() anymore, but we need to call
EVP_MD_CTX_free() instead of cleanup.
Signed-off-by: Max Fillinger
---
src/openvpn/crypto_openssl.c | 38 ++--
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 5cd09e33..5c86268d 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -1492,7 +1492,7 @@ tls1_P_hash(const EVP_MD *md, const unsigned char *sec,
{
int chunk;
size_t j;
-EVP_MD_CTX ctx, ctx_tmp, ctx_init;
+EVP_MD_CTX *ctx, *ctx_tmp, *ctx_init;
EVP_PKEY *mac_key;
unsigned char A1[EVP_MAX_MD_SIZE];
size_t A1_len = EVP_MAX_MD_SIZE;
@@ -1501,28 +1501,28 @@ tls1_P_hash(const EVP_MD *md, const unsigned char *sec,
chunk = EVP_MD_size(md);
OPENSSL_assert(chunk >= 0);
-EVP_MD_CTX_init();
-EVP_MD_CTX_init(_tmp);
-EVP_MD_CTX_init(_init);
-EVP_MD_CTX_set_flags(_init, EVP_MD_CTX_FLAG_NON_FIPS_ALLOW);
+ctx = md_ctx_new();
+ctx_tmp = md_ctx_new();
+ctx_init = md_ctx_new();
+EVP_MD_CTX_set_flags(ctx_init, EVP_MD_CTX_FLAG_NON_FIPS_ALLOW);
mac_key = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL, sec, sec_len);
if (!mac_key)
{
goto err;
}
-if (!EVP_DigestSignInit(_init, NULL, md, NULL, mac_key))
+if (!EVP_DigestSignInit(ctx_init, NULL, md, NULL, mac_key))
{
goto err;
}
-if (!EVP_MD_CTX_copy_ex(, _init))
+if (!EVP_MD_CTX_copy_ex(ctx, ctx_init))
{
goto err;
}
-if (!EVP_DigestSignUpdate(, seed, seed_len))
+if (!EVP_DigestSignUpdate(ctx, seed, seed_len))
{
goto err;
}
-if (!EVP_DigestSignFinal(, A1, _len))
+if (!EVP_DigestSignFinal(ctx, A1, _len))
{
goto err;
}
@@ -1530,19 +1530,19 @@ tls1_P_hash(const EVP_MD *md, const unsigned char *sec,
for (;; )
{
/* Reinit mac contexts */
-if (!EVP_MD_CTX_copy_ex(, _init))
+if (!EVP_MD_CTX_copy_ex(ctx, ctx_init))
{
goto err;
}
-if (!EVP_DigestSignUpdate(, A1, A1_len))
+if (!EVP_DigestSignUpdate(ctx, A1, A1_len))
{
goto err;
}
-if (olen > chunk && !EVP_MD_CTX_copy_ex(_tmp, ))
+if (olen > chunk && !EVP_MD_CTX_copy_ex(ctx_tmp, ctx))
{
goto err;
}
-if (!EVP_DigestSignUpdate(, seed, seed_len))
+if (!EVP_DigestSignUpdate(ctx, seed, seed_len))
{
goto err;
}
@@ -1550,14 +1550,14 @@ tls1_P_hash(const EVP_MD *md, const unsigned char *sec,
if (olen > chunk)
{
j = olen;
-if (!EVP_DigestSignFinal(, out, ))
+if (!EVP_DigestSignFinal(ctx, out, ))
{
goto err;
}
out += j;
olen -= j;
/* calc the next A1 value */
-if (!EVP_DigestSignFinal(_tmp, A1, _len))
+if (!EVP_DigestSignFinal(ctx_tmp, A1, _len))
{
goto err;
}
@@ -1566,7 +1566,7 @@ tls1_P_hash(const EVP_MD *md, const unsigned char *sec,
{
A1_len = EVP_MAX_MD_SIZE;
/* last one */
-if (!EVP_DigestSignFinal(, A1, _len))
+if (!EVP_DigestSignFinal(ctx, A1, _len))
{
goto err;
}
@@ -1577,9 +1577,9 @@ tls1_P_hash(const EVP_MD *md, const unsigned char *sec,
ret = true;
err:
EVP_PKEY_free(mac_key);
-EVP_MD_CTX_cleanup();
-EVP_MD_CTX_cleanup(_tmp);
-EVP_MD_CTX_cleanup(_init);
+EVP_MD_CTX_free(ctx);
+EVP_MD_CTX_free(ctx_tmp);
+EVP_MD_CTX_free(ctx_init);
OPENSSL_cleanse(A1, sizeof(A1));
return ret;
}
--
2.30.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 1/2] Update openssl_compat.h for newer LibreSSL
LibreSSL has added some of the functions that are defined here. However, we still need RSA_F_RSA_OSSL_PRIVATE_ENCRYPT. Signed-off-by: Max Fillinger --- src/openvpn/openssl_compat.h | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h index b3ee94f1..38eb760b 100644 --- a/src/openvpn/openssl_compat.h +++ b/src/openvpn/openssl_compat.h @@ -51,8 +51,8 @@ #define SSL_CTX_set1_groups SSL_CTX_set1_curves #endif -/* Functionality missing in LibreSSL and OpenSSL 1.0.2 */ -#if (OPENSSL_VERSION_NUMBER < 0x1010L || defined(LIBRESSL_VERSION_NUMBER)) && !defined(ENABLE_CRYPTO_WOLFSSL) +/* Functionality missing in LibreSSL before 3.5 and OpenSSL 1.0.2 */ +#if (OPENSSL_VERSION_NUMBER < 0x1010L || (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x305fL)) && !defined(ENABLE_CRYPTO_WOLFSSL) /** * Destroy a X509 object * @@ -68,11 +68,13 @@ X509_OBJECT_free(X509_OBJECT *obj) } } -#define RSA_F_RSA_OSSL_PRIVATE_ENCRYPT RSA_F_RSA_EAY_PRIVATE_ENCRYPT #define EVP_CTRL_AEAD_SET_TAGEVP_CTRL_GCM_SET_TAG #define EVP_CTRL_AEAD_GET_TAGEVP_CTRL_GCM_GET_TAG #endif +#if !defined(RSA_F_RSA_OSSL_PRIVATE_ENCRYPT) +#define RSA_F_RSA_OSSL_PRIVATE_ENCRYPT RSA_F_RSA_EAY_PRIVATE_ENCRYPT +#endif /* Functionality missing in 1.0.2 */ #if OPENSSL_VERSION_NUMBER < 0x1010L && !defined(ENABLE_CRYPTO_WOLFSSL) -- 2.30.2 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v6] Don't "undo" ifconfig on exit if it wasn't done
When running with --ifconfig-noexec, OpenVPN does not execute ifconfig,
but on exit, it still tries to "undo" the configuration it would have
done. This patch fixes it by extracting an undo_ifconfig() function from
close_tun(). The undo function is called before close_tun(), but only if
--ifconfig-noexec isn't set. This is symmetric to how open_tun() and
do_ifconfig() are used.
v2: Fix tabs-vs-spaces.
v3: Fix another style mistake.
v4: Move undo_ifconfig{4,6}() out of #ifdef TARGET_LINUX.
v5: Keep ctx argument in close_tun().
v6: Fix bug in non-Linux non-Windows version of undo_ifconfig_ipv6
Signed-off-by: Max Fillinger
---
src/openvpn/init.c | 4 ++
src/openvpn/tun.c | 159 +++--
src/openvpn/tun.h | 8 +++
3 files changed, 95 insertions(+), 76 deletions(-)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index b6705921..ee800880 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1913,6 +1913,10 @@ do_close_tun_simple(struct context *c)
msg(D_CLOSE, "Closing TUN/TAP interface");
if (c->c1.tuntap)
{
+if (!c->options.ifconfig_noexec)
+{
+undo_ifconfig(c->c1.tuntap, >net_ctx);
+}
close_tun(c->c1.tuntap, >net_ctx);
c->c1.tuntap = NULL;
}
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index f3152a52..2df36c74 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -1604,6 +1604,89 @@ do_ifconfig(struct tuntap *tt, const char *ifname, int
tun_mtu,
net_ctx_free(ctx);
}
+static void
+undo_ifconfig_ipv4(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+{
+#if defined(TARGET_LINUX)
+int netbits = netmask_to_netbits2(tt->remote_netmask);
+
+if (is_tun_p2p(tt))
+{
+if (net_addr_ptp_v4_del(ctx, tt->actual_name, >local,
+>remote_netmask) < 0)
+{
+msg(M_WARN, "Linux can't del IP from iface %s",
+tt->actual_name);
+}
+}
+else
+{
+if (net_addr_v4_del(ctx, tt->actual_name, >local, netbits) < 0)
+{
+msg(M_WARN, "Linux can't del IP from iface %s",
+tt->actual_name);
+}
+}
+#elif !defined(_WIN32) /* if !defined(TARGET_LINUX) && !defined(_WIN32) */
+struct argv argv = argv_new();
+
+argv_printf(, "%s %s 0.0.0.0", IFCONFIG_PATH, tt->actual_name);
+
+argv_msg(M_INFO, );
+openvpn_execve_check(, NULL, 0, "Generic ip addr del failed");
+
+argv_free();
+#endif /* if defined(TARGET_LINUX) */
+ /* Empty for _WIN32. */
+}
+
+static void
+undo_ifconfig_ipv6(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+{
+#if defined(TARGET_LINUX)
+if (net_addr_v6_del(ctx, tt->actual_name, >local_ipv6,
+tt->netbits_ipv6) < 0)
+{
+msg(M_WARN, "Linux can't del IPv6 from iface %s", tt->actual_name);
+}
+#elif !defined(_WIN32) /* if !defined(TARGET_LINUX) && !defined(_WIN32) */
+struct gc_arena gc = gc_new();
+const char *ifconfig_ipv6_local = print_in6_addr(tt->local_ipv6, 0, );
+struct argv argv = argv_new();
+
+argv_printf(, "%s %s del %s/%d", IFCONFIG_PATH, tt->actual_name,
+ifconfig_ipv6_local, tt->netbits_ipv6);
+
+argv_msg(M_INFO, );
+openvpn_execve_check(, NULL, 0, "Generic ip -6 addr del failed");
+
+argv_free();
+gc_free();
+#endif /* if defined(TARGET_LINUX) */
+ /* Empty for _WIN32. */
+}
+
+void
+undo_ifconfig(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+{
+if (tt->type != DEV_TYPE_NULL)
+{
+if (tt->did_ifconfig_setup)
+{
+undo_ifconfig_ipv4(tt, ctx);
+}
+
+if (tt->did_ifconfig_ipv6_setup)
+{
+undo_ifconfig_ipv6(tt, ctx);
+}
+
+/* release resources potentially allocated during undo */
+net_ctx_reset(ctx);
+}
+
+}
+
static void
clear_tuntap(struct tuntap *tuntap)
{
@@ -2213,87 +2296,11 @@ tuncfg(const char *dev, const char *dev_type, const
char *dev_node,
#endif /* ENABLE_FEATURE_TUN_PERSIST */
-static void
-undo_ifconfig_ipv4(struct tuntap *tt, openvpn_net_ctx_t *ctx)
-{
-#if defined(TARGET_LINUX)
-int netbits = netmask_to_netbits2(tt->remote_netmask);
-
-if (is_tun_p2p(tt))
-{
-if (net_addr_ptp_v4_del(ctx, tt->actual_name, >local,
->remote_netmask) < 0)
-{
-msg(M_WARN, "Linux can't del IP from iface %s",
-tt->actual_name);
-}
-}
-else
-{
-if (net_addr_v4_del(ctx, tt->actual_name, >local, netbits) < 0)
-{
-msg(M_WARN, "Linux can't del IP from iface %s",
-tt->actual_name);
-}
-}
-#else /* ifndef TARGET_LINU
[Openvpn-devel] [Patch v5] Don't "undo" ifconfig on exit if it wasn't done
When running with --ifconfig-noexec, OpenVPN does not execute ifconfig,
but on exit, it still tries to "undo" the configuration it would have
done. This patch fixes it by extracting an undo_ifconfig() function from
close_tun(). The undo function is called before close_tun(), but only if
--ifconfig-noexec isn't set. This is symmetric to how open_tun() and
do_ifconfig() are used.
v2: Fix tabs-vs-spaces.
v3: Fix another style mistake.
v4: Move undo_ifconfig{4,6}() out of #ifdef TARGET_LINUX.
v5: Keep ctx argument in close_tun().
Signed-off-by: Max Fillinger
---
src/openvpn/init.c | 4 ++
src/openvpn/tun.c | 161 +++--
src/openvpn/tun.h | 8 +++
3 files changed, 96 insertions(+), 77 deletions(-)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index c9d05c31..f5ca2be8 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1873,6 +1873,10 @@ do_close_tun_simple(struct context *c)
msg(D_CLOSE, "Closing TUN/TAP interface");
if (c->c1.tuntap)
{
+if (!c->options.ifconfig_noexec)
+{
+undo_ifconfig(c->c1.tuntap, >net_ctx);
+}
close_tun(c->c1.tuntap, >net_ctx);
c->c1.tuntap = NULL;
}
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 5e7b8c49..feee83ef 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -1605,6 +1605,89 @@ do_ifconfig(struct tuntap *tt, const char *ifname, int
tun_mtu,
net_ctx_free(ctx);
}
+static void
+undo_ifconfig_ipv4(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+{
+#if defined(TARGET_LINUX)
+int netbits = netmask_to_netbits2(tt->remote_netmask);
+
+if (is_tun_p2p(tt))
+{
+if (net_addr_ptp_v4_del(ctx, tt->actual_name, >local,
+>remote_netmask) < 0)
+{
+msg(M_WARN, "Linux can't del IP from iface %s",
+tt->actual_name);
+}
+}
+else
+{
+if (net_addr_v4_del(ctx, tt->actual_name, >local, netbits) < 0)
+{
+msg(M_WARN, "Linux can't del IP from iface %s",
+tt->actual_name);
+}
+}
+#elif !defined(_WIN32) /* if !defined(TARGET_LINUX) && !defined(_WIN32) */
+struct argv argv = argv_new();
+
+argv_printf(, "%s %s 0.0.0.0", IFCONFIG_PATH, tt->actual_name);
+
+argv_msg(M_INFO, );
+openvpn_execve_check(, NULL, 0, "Generic ip addr del failed");
+
+argv_free();
+#endif /* ifdef TARGET_LINUX */
+ /* Empty for _WIN32. */
+}
+
+static void
+undo_ifconfig_ipv6(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+{
+#if defined(TARGET_LINUX)
+if (net_addr_v6_del(ctx, tt->actual_name, >local_ipv6,
+tt->netbits_ipv6) < 0)
+{
+msg(M_WARN, "Linux can't del IPv6 from iface %s", tt->actual_name);
+}
+#elif !defined(_WIN32) /* if !defined(TARGET_LINUX) && !defined(_WIN32) */
+struct gc_arena gc = gc_new();
+const char *ifconfig_ipv6_local = print_in6_addr(tt->local_ipv6, 0, gc);
+struct argv argv = argv_new();
+
+argv_printf(, "%s %s del %s/%d", IFCONFIG_PATH, tt->actual_name,
+ifconfig_ipv6_local, tt->netbits_ipv6);
+
+argv_msg(M_INFO, );
+openvpn_execve_check(, NULL, 0, "Linux ip -6 addr del failed");
+
+argv_free();
+gc_free();
+#endif /* ifdef TARGET_LINUX */
+ /* Empty for _WIN32. */
+}
+
+void
+undo_ifconfig(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+{
+if (tt->type != DEV_TYPE_NULL)
+{
+if (tt->did_ifconfig_setup)
+{
+undo_ifconfig_ipv4(tt, ctx);
+}
+
+if (tt->did_ifconfig_ipv6_setup)
+{
+undo_ifconfig_ipv6(tt, ctx);
+}
+
+/* release resources potentially allocated during undo */
+net_ctx_reset(ctx);
+}
+
+}
+
static void
clear_tuntap(struct tuntap *tuntap)
{
@@ -1846,7 +1929,7 @@ open_tun_generic(const char *dev, const char *dev_type,
const char *dev_node,
msg(M_INFO, "DCO device %s opened", tunname);
}
else
-#endif
+#endif /* if defined(TARGET_LINUX) */
{
if (!dynamic_opened)
{
@@ -2176,87 +2259,11 @@ tuncfg(const char *dev, const char *dev_type, const
char *dev_node,
#endif /* ENABLE_FEATURE_TUN_PERSIST */
-static void
-undo_ifconfig_ipv4(struct tuntap *tt, openvpn_net_ctx_t *ctx)
-{
-#if defined(TARGET_LINUX)
-int netbits = netmask_to_netbits2(tt->remote_netmask);
-
-if (is_tun_p2p(tt))
-{
-if (net_addr_ptp_v4_del(ctx, tt->actual_name, >local,
->remote_netmask) < 0)
-{
-msg(M_WARN, "Linux can't del IP from iface %s",
-tt->actual_name);
-}
-}
-else
-{
-if (net_addr_
[Openvpn-devel] [PATCH v4] Don't "undo" ifconfig on exit if it wasn't done
When running with --ifconfig-noexec, OpenVPN does not execute ifconfig,
but on exit, it still tries to "undo" the configuration it would have
done. This patch fixes it by extracting an undo_ifconfig() function from
close_tun(). The undo function is called before close_tun(), but only if
--ifconfig-noexec isn't set. This is symmetric to how open_tun() and
do_ifconfig() are used.
This change also allows us to drop the second argument from close_tun().
v2: Fix tabs-vs-spaces.
v3: Fix another style mistake.
v4: Move undo_ifconfig{4,6}() out of #ifdef TARGET_LINUX.
Signed-off-by: Max Fillinger
---
src/openvpn/init.c | 9 ++-
src/openvpn/tun.c | 187 +++--
src/openvpn/tun.h | 13 +++-
3 files changed, 113 insertions(+), 96 deletions(-)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index b0c62a85..949cdad0 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1072,8 +1072,7 @@ do_persist_tuntap(const struct options *options,
openvpn_net_ctx_t *ctx)
#ifdef ENABLE_FEATURE_TUN_PERSIST
tuncfg(options->dev, options->dev_type, options->dev_node,
options->persist_mode,
- options->username, options->groupname, >tuntap_options,
- ctx);
+ options->username, options->groupname,
>tuntap_options);
if (options->persist_mode && options->lladdr)
{
set_lladdr(ctx, options->dev, options->lladdr, NULL);
@@ -1858,7 +1857,11 @@ do_close_tun_simple(struct context *c)
msg(D_CLOSE, "Closing TUN/TAP interface");
if (c->c1.tuntap)
{
-close_tun(c->c1.tuntap, >net_ctx);
+if (!c->options.ifconfig_noexec)
+{
+undo_ifconfig(c->c1.tuntap, >net_ctx);
+}
+close_tun(c->c1.tuntap);
c->c1.tuntap = NULL;
}
c->c1.tuntap_owned = false;
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index e12f0369..b863e9c9 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -1605,6 +1605,89 @@ do_ifconfig(struct tuntap *tt, const char *ifname, int
tun_mtu,
net_ctx_free(ctx);
}
+static void
+undo_ifconfig_ipv4(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+{
+#if defined(TARGET_LINUX)
+int netbits = netmask_to_netbits2(tt->remote_netmask);
+
+if (is_tun_p2p(tt))
+{
+if (net_addr_ptp_v4_del(ctx, tt->actual_name, >local,
+>remote_netmask) < 0)
+{
+msg(M_WARN, "Linux can't del IP from iface %s",
+tt->actual_name);
+}
+}
+else
+{
+if (net_addr_v4_del(ctx, tt->actual_name, >local, netbits) < 0)
+{
+msg(M_WARN, "Linux can't del IP from iface %s",
+tt->actual_name);
+}
+}
+#elif !defined(_WIN32) /* if !defined(TARGET_LINUX) && !defined(_WIN32) */
+struct argv argv = argv_new();
+
+argv_printf(, "%s %s 0.0.0.0", IFCONFIG_PATH, tt->actual_name);
+
+argv_msg(M_INFO, );
+openvpn_execve_check(, NULL, 0, "Generic ip addr del failed");
+
+argv_free();
+#endif /* ifdef TARGET_LINUX */
+ /* Empty for _WIN32. */
+}
+
+static void
+undo_ifconfig_ipv6(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+{
+#if defined(TARGET_LINUX)
+if (net_addr_v6_del(ctx, tt->actual_name, >local_ipv6,
+tt->netbits_ipv6) < 0)
+{
+msg(M_WARN, "Linux can't del IPv6 from iface %s", tt->actual_name);
+}
+#elif !defined(_WIN32) /* if !defined(TARGET_LINUX) && !defined(_WIN32) */
+struct gc_arena gc = gc_new();
+const char *ifconfig_ipv6_local = print_in6_addr(tt->local_ipv6, 0, gc);
+struct argv argv = argv_new();
+
+argv_printf(, "%s %s del %s/%d", IFCONFIG_PATH, tt->actual_name,
+ifconfig_ipv6_local, tt->netbits_ipv6);
+
+argv_msg(M_INFO, );
+openvpn_execve_check(, NULL, 0, "Linux ip -6 addr del failed");
+
+argv_free();
+gc_free();
+#endif /* ifdef TARGET_LINUX */
+ /* Empty for _WIN32. */
+}
+
+void
+undo_ifconfig(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+{
+if (tt->type != DEV_TYPE_NULL)
+{
+if (tt->did_ifconfig_setup)
+{
+undo_ifconfig_ipv4(tt, ctx);
+}
+
+if (tt->did_ifconfig_ipv6_setup)
+{
+undo_ifconfig_ipv6(tt, ctx);
+}
+
+/* release resources potentially allocated during undo */
+net_ctx_reset(ctx);
+}
+
+}
+
static void
clear_tuntap(struct tuntap *tuntap)
{
@@ -1910,7 +1993,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
}
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -2073,7 +2156,7 @@ open_tun(const
[Openvpn-devel] [PATCH v3] Don't "undo" ifconfig on exit if it wasn't done
When running with --ifconfig-noexec, OpenVPN does not execute ifconfig,
but on exit, it still tries to "undo" the configuration it would have
done. This patch fixes it by extracting an undo_ifconfig() function from
close_tun(). The undo function is called before close_tun(), but only if
--ifconfig-noexec isn't set. This is symmetric to how open_tun() and
do_ifconfig() are used.
This change also allows us to drop the second argument from close_tun().
v2: Fix tabs-vs-spaces.
v3: Fix another style mistake.
Signed-off-by: Max Fillinger
---
src/openvpn/init.c | 6 +-
src/openvpn/tun.c | 37 +
src/openvpn/tun.h | 10 +-
3 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index b0c62a85..e52a084e 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1858,7 +1858,11 @@ do_close_tun_simple(struct context *c)
msg(D_CLOSE, "Closing TUN/TAP interface");
if (c->c1.tuntap)
{
-close_tun(c->c1.tuntap, >net_ctx);
+if (!c->options.ifconfig_noexec)
+{
+undo_ifconfig(c->c1.tuntap, >net_ctx);
+}
+close_tun(c->c1.tuntap);
c->c1.tuntap = NULL;
}
c->c1.tuntap_owned = false;
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index e12f0369..015bf46d 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -1910,7 +1910,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
}
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -2073,7 +2073,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
void
tuncfg(const char *dev, const char *dev_type, const char *dev_node,
int persist_mode, const char *username, const char *groupname,
- const struct tuntap_options *options, openvpn_net_ctx_t *ctx)
+ const struct tuntap_options *options)
{
struct tuntap *tt;
@@ -2112,7 +2112,7 @@ tuncfg(const char *dev, const char *dev_type, const char
*dev_node,
msg(M_ERR, "Cannot ioctl TUNSETGROUP(%s) %s", groupname, dev);
}
}
-close_tun(tt, ctx);
+close_tun(tt);
msg(M_INFO, "Persist state set to: %s", (persist_mode ? "ON" : "OFF"));
}
@@ -2179,10 +2179,8 @@ undo_ifconfig_ipv6(struct tuntap *tt, openvpn_net_ctx_t
*ctx)
}
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+undo_ifconfig(struct tuntap *tt, openvpn_net_ctx_t *ctx)
{
-ASSERT(tt);
-
if (tt->type != DEV_TYPE_NULL)
{
if (tt->did_ifconfig_setup)
@@ -2199,6 +2197,13 @@ close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
net_ctx_reset(ctx);
}
+}
+
+void
+close_tun(struct tuntap *tt)
+{
+ASSERT(tt);
+
close_tun_generic(tt);
free(tt);
}
@@ -2513,7 +2518,7 @@ solaris_close_tun(struct tuntap *tt)
* Close TUN device.
*/
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -2546,7 +2551,7 @@ solaris_error_close(struct tuntap *tt, const struct
env_set *es,
argv_msg(M_INFO, );
openvpn_execve_check(, es, 0, "Solaris ifconfig unplumb failed");
-close_tun(tt, NULL);
+close_tun(tt);
msg(M_FATAL, "Solaris ifconfig failed");
argv_free();
}
@@ -2609,7 +2614,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
*/
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -2695,7 +2700,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
* need to be explicitly destroyed
*/
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -2836,7 +2841,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
* we need to call "ifconfig ... destroy" for cleanup
*/
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -2952,7 +2957,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
}
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -3218,7 +3223,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
}
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -3366,7 +3371,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
/* tap devices need to be manually destroyed on AIX
*/
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -6704,7 +6709,7 @@ netsh_delete_address_d
[Openvpn-devel] [PATCH v2] Don't "undo" ifconfig on exit if it wasn't done
When running with --ifconfig-noexec, OpenVPN does not execute ifconfig,
but on exit, it still tries to "undo" the configuration it would have
done. This patch fixes it by extracting an undo_ifconfig() function from
close_tun(). The undo function is called before close_tun(), but only if
--ifconfig-noexec isn't set. This is symmetric to how open_tun() and
do_ifconfig() are used.
This change also allows us to drop the second argument from close_tun().
v2: Fix tabs-vs-spaces.
Signed-off-by: Max Fillinger
---
src/openvpn/init.c | 5 -
src/openvpn/tun.c | 37 +
src/openvpn/tun.h | 10 +-
3 files changed, 34 insertions(+), 18 deletions(-)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index b0c62a85..87f825c2 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1858,7 +1858,10 @@ do_close_tun_simple(struct context *c)
msg(D_CLOSE, "Closing TUN/TAP interface");
if (c->c1.tuntap)
{
-close_tun(c->c1.tuntap, >net_ctx);
+if (!c->options.ifconfig_noexec) {
+undo_ifconfig(c->c1.tuntap, >net_ctx);
+}
+close_tun(c->c1.tuntap);
c->c1.tuntap = NULL;
}
c->c1.tuntap_owned = false;
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index e12f0369..015bf46d 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -1910,7 +1910,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
}
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -2073,7 +2073,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
void
tuncfg(const char *dev, const char *dev_type, const char *dev_node,
int persist_mode, const char *username, const char *groupname,
- const struct tuntap_options *options, openvpn_net_ctx_t *ctx)
+ const struct tuntap_options *options)
{
struct tuntap *tt;
@@ -2112,7 +2112,7 @@ tuncfg(const char *dev, const char *dev_type, const char
*dev_node,
msg(M_ERR, "Cannot ioctl TUNSETGROUP(%s) %s", groupname, dev);
}
}
-close_tun(tt, ctx);
+close_tun(tt);
msg(M_INFO, "Persist state set to: %s", (persist_mode ? "ON" : "OFF"));
}
@@ -2179,10 +2179,8 @@ undo_ifconfig_ipv6(struct tuntap *tt, openvpn_net_ctx_t
*ctx)
}
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+undo_ifconfig(struct tuntap *tt, openvpn_net_ctx_t *ctx)
{
-ASSERT(tt);
-
if (tt->type != DEV_TYPE_NULL)
{
if (tt->did_ifconfig_setup)
@@ -2199,6 +2197,13 @@ close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
net_ctx_reset(ctx);
}
+}
+
+void
+close_tun(struct tuntap *tt)
+{
+ASSERT(tt);
+
close_tun_generic(tt);
free(tt);
}
@@ -2513,7 +2518,7 @@ solaris_close_tun(struct tuntap *tt)
* Close TUN device.
*/
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -2546,7 +2551,7 @@ solaris_error_close(struct tuntap *tt, const struct
env_set *es,
argv_msg(M_INFO, );
openvpn_execve_check(, es, 0, "Solaris ifconfig unplumb failed");
-close_tun(tt, NULL);
+close_tun(tt);
msg(M_FATAL, "Solaris ifconfig failed");
argv_free();
}
@@ -2609,7 +2614,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
*/
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -2695,7 +2700,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
* need to be explicitly destroyed
*/
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -2836,7 +2841,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
* we need to call "ifconfig ... destroy" for cleanup
*/
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -2952,7 +2957,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
}
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -3218,7 +3223,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
}
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -3366,7 +3371,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
/* tap devices need to be manually destroyed on AIX
*/
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -6704,7 +6709,7 @@ netsh_delete_address_dns(const struct tuntap *tt, bo
[Openvpn-devel] [PATCH] Don't "undo" ifconfig on exit if it wasn't done
When running with --ifconfig-noexec, OpenVPN does not execute ifconfig,
but on exit, it still tries to "undo" the configuration it would have
done. This patch fixes it by extracting an undo_ifconfig() function from
close_tun(). The undo function is called before close_tun(), but only if
--ifconfig-noexec isn't set. This is symmetric to how open_tun() and
do_ifconfig() are used.
This change also allows us to drop the second argument from close_tun().
Signed-off-by: Max Fillinger
---
src/openvpn/init.c | 5 -
src/openvpn/tun.c | 37 +
src/openvpn/tun.h | 10 +-
3 files changed, 34 insertions(+), 18 deletions(-)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index b0c62a85..429f2cee 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1858,7 +1858,10 @@ do_close_tun_simple(struct context *c)
msg(D_CLOSE, "Closing TUN/TAP interface");
if (c->c1.tuntap)
{
-close_tun(c->c1.tuntap, >net_ctx);
+ if (!c->options.ifconfig_noexec) {
+ undo_ifconfig(c->c1.tuntap, >net_ctx);
+ }
+close_tun(c->c1.tuntap);
c->c1.tuntap = NULL;
}
c->c1.tuntap_owned = false;
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index e12f0369..015bf46d 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -1910,7 +1910,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
}
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -2073,7 +2073,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
void
tuncfg(const char *dev, const char *dev_type, const char *dev_node,
int persist_mode, const char *username, const char *groupname,
- const struct tuntap_options *options, openvpn_net_ctx_t *ctx)
+ const struct tuntap_options *options)
{
struct tuntap *tt;
@@ -2112,7 +2112,7 @@ tuncfg(const char *dev, const char *dev_type, const char
*dev_node,
msg(M_ERR, "Cannot ioctl TUNSETGROUP(%s) %s", groupname, dev);
}
}
-close_tun(tt, ctx);
+close_tun(tt);
msg(M_INFO, "Persist state set to: %s", (persist_mode ? "ON" : "OFF"));
}
@@ -2179,10 +2179,8 @@ undo_ifconfig_ipv6(struct tuntap *tt, openvpn_net_ctx_t
*ctx)
}
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+undo_ifconfig(struct tuntap *tt, openvpn_net_ctx_t *ctx)
{
-ASSERT(tt);
-
if (tt->type != DEV_TYPE_NULL)
{
if (tt->did_ifconfig_setup)
@@ -2199,6 +2197,13 @@ close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
net_ctx_reset(ctx);
}
+}
+
+void
+close_tun(struct tuntap *tt)
+{
+ASSERT(tt);
+
close_tun_generic(tt);
free(tt);
}
@@ -2513,7 +2518,7 @@ solaris_close_tun(struct tuntap *tt)
* Close TUN device.
*/
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -2546,7 +2551,7 @@ solaris_error_close(struct tuntap *tt, const struct
env_set *es,
argv_msg(M_INFO, );
openvpn_execve_check(, es, 0, "Solaris ifconfig unplumb failed");
-close_tun(tt, NULL);
+close_tun(tt);
msg(M_FATAL, "Solaris ifconfig failed");
argv_free();
}
@@ -2609,7 +2614,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
*/
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -2695,7 +2700,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
* need to be explicitly destroyed
*/
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -2836,7 +2841,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
* we need to call "ifconfig ... destroy" for cleanup
*/
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -2952,7 +2957,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
}
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -3218,7 +3223,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
}
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -3366,7 +3371,7 @@ open_tun(const char *dev, const char *dev_type, const
char *dev_node, struct tun
/* tap devices need to be manually destroyed on AIX
*/
void
-close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
+close_tun(struct tuntap *tt)
{
ASSERT(tt);
@@ -6704,7 +6709,7 @@ netsh_delete_address_dns(const struct tuntap *tt, bool
ipv6, struct gc_arena *gc
}
[Openvpn-devel] [PATCH v3] Add warning about mbed TLS licensing problem
Signed-off-by: Max Fillinger --- README.mbedtls | 18 ++ 1 file changed, 18 insertions(+) diff --git a/README.mbedtls b/README.mbedtls index 4875822d..d3466fa9 100644 --- a/README.mbedtls +++ b/README.mbedtls @@ -11,6 +11,24 @@ This version depends on mbed TLS 2.0 (and requires at least 2.0.0). * +Warning: + +As of mbed TLS 2.17, it can be licensed *only* under the Apache v2.0 license. +That license is incompatible with OpenVPN's GPLv2. + +If you wish to distribute OpenVPN linked with mbed TLS, there are two options: + + * Ensure that your case falls under the system library exception in GPLv2, or + + * Use an earlier version of mbed TLS. Version 2.16.12 is the last release + that may be licensed under GPLv2. Unfortunately, this version is + unsupported and won't receive any more updates. + +If nothing changes about the license situation, mbed TLS support may be +deprecated in a future release of OpenVPN. + +* + Due to limitations in the mbed TLS library, the following features are missing in the mbed TLS version of OpenVPN: -- 2.20.1 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [Patch v2] Add warning about mbed TLS licensing problem
Signed-off-by: Max Fillinger --- README.mbedtls | 17 + 1 file changed, 17 insertions(+) diff --git a/README.mbedtls b/README.mbedtls index 4875822d..062ae470 100644 --- a/README.mbedtls +++ b/README.mbedtls @@ -11,6 +11,23 @@ This version depends on mbed TLS 2.0 (and requires at least 2.0.0). * +Warning: + +As of version 2.17, mbed TLS can be licensed *only* under the Apache v2.0 +license. That license is incompatible with OpenVPN's GPLv2. + +If you wish to distribute OpenVPN linked with mbed TLS, there are two options: + + * Ensure that your case falls under the system library exception in GPLv2, or + + * Use an earlier version of mbed TLS. Version 2.16.12 is the last release + that may be licensed under GPLv2. Unfortunately, this version is + unsupported and won't receive any more updates. + +Support for mbed TLS is likely to be removed in OpenVPN 2.7. + +* + Due to limitations in the mbed TLS library, the following features are missing in the mbed TLS version of OpenVPN: -- 2.20.1 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Add warning about mbed TLS licensing problem
Signed-off-by: Max Fillinger --- README.mbedtls | 17 + 1 file changed, 17 insertions(+) diff --git a/README.mbedtls b/README.mbedtls index 4875822d..b5604bb8 100644 --- a/README.mbedtls +++ b/README.mbedtls @@ -11,6 +11,23 @@ This version depends on mbed TLS 2.0 (and requires at least 2.0.0). * +Warning: + +As of version 2.17, mbed TLS can be licensed *only* under the Apache v2.0 +license. That license is incompatible with OpenVPN's GPLv2. + +If you wish to distribute OpenVPN linked with mbed TLS, there are two options: + + * Ensure that your case falls under the system library exception in GPLv2, or + + * Use an earlier version of mbed TLS. Version 2.16.12 is the last release + that may be licensed under GPLv2. Unfortunately, this version is + unsupported and won't receive any more updates. + +Support for mbed TLS is likely to be removed in OpenVPN 2.17. + +* + Due to limitations in the mbed TLS library, the following features are missing in the mbed TLS version of OpenVPN: -- 2.20.1 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 2.5] Define have_blowfish variable in ncp unit tests
The previous commit was backported from master and needs this variable
to exist.
Signed-off-by: Max Fillinger
---
tests/unit_tests/openvpn/test_ncp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/unit_tests/openvpn/test_ncp.c
b/tests/unit_tests/openvpn/test_ncp.c
index 8da76c03..bcafd232 100644
--- a/tests/unit_tests/openvpn/test_ncp.c
+++ b/tests/unit_tests/openvpn/test_ncp.c
@@ -49,6 +49,7 @@ test_check_ncp_ciphers_list(void **state)
{
struct gc_arena gc = gc_new();
bool have_chacha = cipher_kt_get("CHACHA20-POLY1305");
+bool have_blowfish = cipher_kt_get("BF-CBC");
assert_string_equal(mutate_ncp_cipher_list("none", ), "none");
assert_string_equal(mutate_ncp_cipher_list("AES-256-GCM:none", ),
@@ -56,7 +57,7 @@ test_check_ncp_ciphers_list(void **state)
assert_string_equal(mutate_ncp_cipher_list(aes_ciphers, ), aes_ciphers);
-if (have_chacha)
+if (have_chacha && have_blowfish)
{
assert_string_equal(mutate_ncp_cipher_list(bf_chacha, ), bf_chacha);
assert_string_equal(mutate_ncp_cipher_list("BF-CBC:CHACHA20-POLY1305",
),
--
2.11.0
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Don't use BF-CBC in unit tests if we don't have it
Signed-off-by: Max Fillinger
---
tests/unit_tests/openvpn/test_ncp.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tests/unit_tests/openvpn/test_ncp.c
b/tests/unit_tests/openvpn/test_ncp.c
index 6702133a..f4c28ffd 100644
--- a/tests/unit_tests/openvpn/test_ncp.c
+++ b/tests/unit_tests/openvpn/test_ncp.c
@@ -120,8 +120,11 @@ test_check_ncp_ciphers_list(void **state)
assert_string_equal(mutate_ncp_cipher_list("id-aes128-GCM:id-aes256-GCM",
),
"AES-128-GCM:AES-256-GCM");
#else
-assert_string_equal(mutate_ncp_cipher_list("BLOWFISH-CBC",
- ), "BF-CBC");
+if (have_blowfish)
+{
+assert_string_equal(mutate_ncp_cipher_list("BLOWFISH-CBC",
+ ), "BF-CBC");
+}
#endif
gc_free();
}
--
2.11.0
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Don't "undo" ifconfig when given --ifconfig-noexec
When running with --ifconfig-noexec on Linux, OpenVPN may still delete
the ip address from the tun interface on exit, because it tries to undo
the ifconfig that did not actually happen.
This commit reintroduces the did_ifconfig member to struct tuntap so
that we can check if ifconfig was actually done before trying to undo
it. It's behind an #ifdef because it's only used on Linux, and that was
the reason why it was removed before.
Signed-off-by: Max Fillinger
---
src/openvpn/tun.c | 6 +-
src/openvpn/tun.h | 3 +++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 75d5eaf7..32e739fc 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -1601,6 +1601,10 @@ do_ifconfig(struct tuntap *tt, const char *ifname, int
tun_mtu,
do_ifconfig_ipv6(tt, ifname, tun_mtu, es, ctx);
}
+#ifdef TARGET_LINUX
+tt->did_ifconfig = true;
+#endif
+
/* release resources potentially allocated during interface setup */
net_ctx_free(ctx);
}
@@ -2190,7 +2194,7 @@ close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
{
ASSERT(tt);
-if (tt->type != DEV_TYPE_NULL)
+if (tt->type != DEV_TYPE_NULL && tt->did_ifconfig)
{
if (tt->did_ifconfig_setup)
{
diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index aa1e47b5..1f579e34 100644
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -162,6 +162,9 @@ struct tuntap
bool did_ifconfig_setup;
bool did_ifconfig_ipv6_setup;
+#ifdef TARGET_LINUX
+bool did_ifconfig;
+#endif
bool persistent_if; /* if existed before, keep on program end */
--
2.20.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Remove unused havege.h header
This header was removed in mbedtls 3. Luckily, we weren't actually using it, it seems. Signed-off-by: Max Fillinger --- src/openvpn/crypto_mbedtls.c | 1 - src/openvpn/ssl_mbedtls.c| 2 -- 2 files changed, 3 deletions(-) diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c index 2f7f00d1..72e19d23 100644 --- a/src/openvpn/crypto_mbedtls.c +++ b/src/openvpn/crypto_mbedtls.c @@ -50,7 +50,6 @@ #include #include #include -#include #include #include diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index e7c45c09..1cb27aaa 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -46,8 +46,6 @@ #include "pkcs11_backend.h" #include "ssl_common.h" -#include - #include "ssl_verify_mbedtls.h" #include #include -- 2.11.0 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Completely remove DES checks
On 07/11/2021 13:29, Arne Schwabe wrote: The patch removes checking for weak keys and making DES just like any other CBC cipher and not doing extra checks for this. It basically removes the special treatment of DES. After this, do we have any DES functionality left in OpenVPN? If so, we should remove it. After this patch, no special handling for DES anymore. YOu can still use DES but it is handled like any other cipher, e.g. BF-CBC, AES-CBC Arne I think the point is that if we stop checking weak keys, we should rip out DES support completely. (I'd be in favor, but I'm not deep enough into it to know what the fallout would be.) My view is, if someone's doing DES, they're not caring about security, so the small risk of weak keys is acceptable. Basically, "all DES keys are weak keys." ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Completely remove DES checks
On 07/11/2021 10:01, Arne Schwabe wrote: We already removed the check in d67658fee for OpenSSL 3.0. This removes the checks entirely for all crypto libraries. Signed-off-by: Arne Schwabe Acked-by: Max Fillinger Looks good to me! Compiled and ran --test-crypto for DES/DES3, with mbedtls and OpenSSL 3. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v4] [OSSL 3.0] Implement DES ECB encrypt via EVP_CIPHER api
On 29/10/2021 13:11, Arne Schwabe wrote: Even though DES is super outdated and also NTLM is super outdated, eliminating the warnings for OpenSSL 3.0 is still a step in the right direction and using the correct APIs. We cheat a bit by using 3DES instead of DES to avoid needing legacy provider for DES encryption for now. Patch v4: add unit test, use 3DES to avoid legacy provider for now Signed-off-by: Arne Schwabe Acked-by: Max Fillinger Looks good to me, and the unit tests succeed with OpenSSL 1.1.1 and 3. Small nitpick that can be fixed at compile time: +if (!EVP_EncryptInit_ex(ctx, EVP_des_ede3_ecb(), NULL, key3, 0)) The last argument for this function is "const unsigned char *iv", so this should be NULL instead of 0. (Passing NULL here is correct because ECB mode doesn't need an initialization vector.) ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v4] Add insecure tls-cert-profile options
On 29/10/2021 13:24, Arne Schwabe wrote: The recent deprecation of SHA1 certificates in OpenSSL 3.0 makes it necessary to reallow them in certain deployments. Currently this works by using the hack of using tls-cipher "DEFAULT:@SECLEVEL=0". Add insecure as option to tls-cert-profile to allow setting a seclevel of 0. Patch v4: fix default accidentially changed to insecure Signed-off-by: Arne Schwabe Acked-by: Max Fillinger With OpenSSL 3, OpenVPN accepts certs signed with SHA1 if and only if "--tls-cert-profile insecure" is used. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 19/21] Add insecure tls-cert-profile options
On 19/10/2021 20:31, Arne Schwabe wrote:
The recent deprecation of SHA1 certificates in OpenSSL 3.0 makes it necessary
to reallow them in certain deployments. Currently this works by using the
hack of using tls-cipher "DEFAULT:@SECLEVEL=0". Add insecure as option to
tls-cert-profile to allow setting a seclevel of 0.
Signed-off-by: Arne Schwabe
This makes insecure the default mode:
+if (!profile || 0 == strcmp(profile, "insecure"))
+{
+SSL_CTX_set_security_level(ctx->ctx, 0);
+}
+else if (!profile || 0 == strcmp(profile, "legacy"))
{
SSL_CTX_set_security_level(ctx->ctx, 1);
}
When we don't pass a tls-cert-profile option, profile is NULL here, so
it takes the first branch.
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 18/21] Fix error when BF-CBC is not available
On 19/10/2021 20:31, Arne Schwabe wrote: Through the multiple iteration of allowing OpenVPN to run without BF-CBC we accidentially made a regression and still required BF-CBC. This patch fixes the code path and restores its intended function. Signed-off-by: Arne Schwabe Acked-by: Max Fillinger This fixes running with --mode server when BF-CBC is unavailable. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 14/21] [OSSL 3.0] Use TYPE_do_all_provided function for listing cipher/digest
On 19/10/2021 20:31, Arne Schwabe wrote:
With OpenSSL 3.0 the use of nid values is deprecated and new algorithms
do not even have NID values anymore.
This also works nicely with providers now:
openvpn --provider legacy:default --show-ciphers
shows more ciphers (e.g. BF-CBC) than just
openvpn --show-ciphers
when compiled with OpenSSL 3.0
Signed-off-by: Arne Schwabe
Looks good, and the tests work with OpenSSL 3 and OpenSSL 1.1.1 when I
also apply the "Do not allow CTS ciphers" patch.
One nitpick:
+struct collect_ciphers {
+/* If we ever exceed this, we must be more selective */
+const EVP_CIPHER *list[1000];
+size_t num;
+};
+
+static void collect_ciphers(EVP_CIPHER *cipher, void *list)
+{
+struct collect_ciphers* cipher_list = list;
+if (cipher_list->num ==
(sizeof(cipher_list->list)/sizeof(*cipher_list->list)))
+{
+msg(M_WARN, "WARNING: Too many ciphers, not showing all");
+return;
+}
I think it would be more readable to use a const (or a #define) for the
length of the cipher list array, instead of using sizeof.
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 15/21] [OSSL 3.0] Do not allow CTS ciphers
On 26/10/2021 17:27, Max Fillinger wrote:
On 19/10/2021 20:31, Arne Schwabe wrote:
We do not support CTS algorithms (cipher text stealing) algorithms.
Signed-off-by: Arne Schwabe
---
 src/openvpn/crypto_openssl.c | 3 +++
 1 file changed, 3 insertions(+)
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index ab552efab..ac8287440 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -760,6 +760,9 @@ cipher_kt_mode_cbc(const cipher_kt_t *cipher)
 {
 return cipher && cipher_kt_mode(cipher) == OPENVPN_MODE_CBC
/* Exclude AEAD cipher modes, they require a different
API */
+#ifdef EVP_CIPH_FLAG_CTS
+Â Â && !(EVP_CIPHER_flags(cipher) & EVP_CIPH_FLAG_CTS)
+#endif
&& !(EVP_CIPHER_flags(cipher) & EVP_CIPH_FLAG_AEAD_CIPHER);
 }
Together with the previous patch, this makes the tests work. One thing
I'm unsure about is that this check is only done for CBC mode.
Cipher-text stealing can be used in *any* block cipher mode (even CTR,
though that would be amazingly pointless).
I compiled OpenVPN with support for OFB and CFB modes and didn't see any
CTS in the --show-ciphers output. But do we know for sure that there's
no supported version or configuration of OpenSSL that uses cipher-text
stealing in non-CBC modes?
Disregard that. I keep forgetting how OFB and CFB work. They don't need
any padding so ciphertext stealing would be pointless here.
Acked-by: Max Fillinger
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 15/21] [OSSL 3.0] Do not allow CTS ciphers
On 19/10/2021 20:31, Arne Schwabe wrote:
We do not support CTS algorithms (cipher text stealing) algorithms.
Signed-off-by: Arne Schwabe
---
src/openvpn/crypto_openssl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index ab552efab..ac8287440 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -760,6 +760,9 @@ cipher_kt_mode_cbc(const cipher_kt_t *cipher)
{
return cipher && cipher_kt_mode(cipher) == OPENVPN_MODE_CBC
/* Exclude AEAD cipher modes, they require a different API */
+#ifdef EVP_CIPH_FLAG_CTS
+ && !(EVP_CIPHER_flags(cipher) & EVP_CIPH_FLAG_CTS)
+#endif
&& !(EVP_CIPHER_flags(cipher) & EVP_CIPH_FLAG_AEAD_CIPHER);
}
Together with the previous patch, this makes the tests work. One thing
I'm unsure about is that this check is only done for CBC mode.
Cipher-text stealing can be used in *any* block cipher mode (even CTR,
though that would be amazingly pointless).
I compiled OpenVPN with support for OFB and CFB modes and didn't see any
CTS in the --show-ciphers output. But do we know for sure that there's
no supported version or configuration of OpenSSL that uses cipher-text
stealing in non-CBC modes?
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 13/21] [OSSL 3.0] Remove dependency on BF-CBC existance from test_ncp
On 19/10/2021 20:31, Arne Schwabe wrote:
The test_check_ncp_ciphers_list test assumed that BF-CBC is always
available, which is no longer the case with OpenSSL 3.0. Rewrite the
test to not rely on BF-CBC to be available.
Unit tests are working now. I've got some style nitpicks that I think
can be fixed during commit:
bool have_chacha = cipher_kt_get("CHACHA20-POLY1305");
+bool have_blowfish= cipher_kt_get("BF-CBC");
Add space before =
-if (have_chacha)
+if(have_chacha)
+{
+assert_string_equal(mutate_ncp_cipher_list(aes_chacha, ),
aes_chacha);
+}
Add space before (
Acked-by: Max Fillinger
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 11/21] [OSSL 3.0] USe EVP_MD_get0_name instead EV_MD_name
On 19/10/2021 20:31, Arne Schwabe wrote: Use the new name for the function as it indicates with get0 the ownership of the returned value Signed-off-by: Arne Schwabe Acked-by: Max Fillinger Looks good to me. Typo: "USe" ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 10/21] [OSSL 3.0] Replace EVP_get_cipherbyname with EVP_CIPHER_fetch
On 19/10/2021 20:31, Arne Schwabe wrote: In OpenSSL 3.0 EVP_get_cipherbyname return a non NULL algorithm even if the algorithm is not avaialble with the currently available provider. Luckily EVP_get_cipherbyname can be used here as drop in replacement and returns only non NULL if the algorithm is actually currently supported. Signed-off-by: Arne Schwabe Acked-by: Max Fillinger Looks good to me! Some small errors in the commit message: "return a non Null algorithm": Should be "may return", I think. second "EVP_get_cipherbyname" should be "EVP_CIPHER_fetch". ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Don't manually free DH params in OpenSSL 3
When the EVP_PKEY object with the Diffie-Hellman parameters is passed to SSL_CTX_set0_tmp_dh_pkey, it does not create a copy but stores the pointer in the SSL_CTX. Therefore, we should not free it. The EVP_PKEY will be freed automatically when we free the SSL_CTX. Signed-off-by: Max Fillinger --- src/openvpn/ssl_openssl.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 2414fc5e..6f2d6d57 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -685,8 +685,6 @@ tls_ctx_load_dh_params(struct tls_root_ctx *ctx, const char *dh_file, msg(D_TLS_DEBUG_LOW, "Diffie-Hellman initialized with %d bit key", 8 * EVP_PKEY_get_size(dh)); - -EVP_PKEY_free(dh); #else DH *dh = PEM_read_bio_DHparams(bio, NULL, NULL, NULL); BIO_free(bio); -- 2.11.0 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 09/21] Refactor early initialisation and uninitialisation into methods
On 19/10/2021 20:31, Arne Schwabe wrote: This put the early initialisation and uninitialisation that needs to happen between option parsing and post processing into small methods. Signed-off-by: Arne Schwabe Acked-by: Max Fillinger It's easy to see that this does not change the behavior. Compiled and tested with OpenSSL 3 only because it doesn't actually touch any OpenSSL code. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 08/21] [OSSL 3.0] Use EVP_PKEY_get_group_name to query group name
On 19/10/2021 20:31, Arne Schwabe wrote:
EC_Key methods are deprecated in OpenSSL 3.0. Use
EVP_PKEY_get_group_name instead to query the EC group name from an
EVP_PKEY and add a compatibility function for older OpenSSL versions.
Signed-off-by: Arne Schwabe
---
src/openvpn/openssl_compat.h | 42
src/openvpn/ssl_openssl.c| 14 ++--
2 files changed, 50 insertions(+), 6 deletions(-)
diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index ce8e2b360..dda47d76c 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -718,4 +718,46 @@ SSL_CTX_set_max_proto_version(SSL_CTX *ctx, long
tls_ver_max)
return 1;
}
#endif /* if OPENSSL_VERSION_NUMBER < 0x1010L &&
!defined(ENABLE_CRYPTO_WOLFSSL) */
+
+/* Functionality missing in 1.1.1 */
+#if OPENSSL_VERSION_NUMBER < 0x3000L && !defined(OPENSSL_NO_EC)
+
+/* Note that this is not a perfect emulation of the new function but
+ * is good enough for our case of printing certificate details during
+ * handshake */
+static inline
+int EVP_PKEY_get_group_name(EVP_PKEY *pkey, char *gname, size_t gname_sz,
+size_t *gname_len)
+{
+const EC_KEY* ec = EVP_PKEY_get0_EC_KEY(pkey);
+if (ec == NULL)
+{
+return 0;
+}
+const EC_GROUP* group = EC_KEY_get0_group(ec);
+int nid = EC_GROUP_get_curve_name(group);
+
+if (nid == 0)
+{
+return 0;
+}
+const char *curve = OBJ_nid2sn(nid);
The old code also has a curve == NULL check. Is that not necessary here?
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 07/21] [OSSL 3.0] Remove DES key fixup code
On 19/10/2021 20:31, Arne Schwabe wrote: This code mainly sets the parity bits in the DES keys. As mbed TLS and OpenSSL already ignore these bits in the DES key and since DES is deprecated, remove this special DES code that is not even needed by the libraries. Signed-off-by: Arne Schwabe Acked-by: Max Fillinger Makes sense, why should we care about the parity bits when no-one else does? Compiled and ran --test-crypto for DES/DES3 with OpenSSL 3.1.0, 1.1.1 and mbedtls 2.26. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 06/21] [OSSL 3.0] Deprecate --ecdh-curve with OpenSSL 3.0 and adjust mbed TLS message
On 19/10/2021 20:31, Arne Schwabe wrote: OpenSSL 3.0 deprecates SSL_CTX_set_tmp_ecdh() in favour of SSL_CTX_set1_groups(3). We already support the SSL_CTX_set1_groups using the --tls-groups. Adjust both mbed TLS and OpenSSL 3.0 to say that --ecdh-curve is ingored and --tls-groups should be used. Signed-off-by: Arne Schwabe Acked-by: Max Fillinger Not much to say here. It compiles and I can see the warning when I use the option. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 05/21] [OSSL 3.0] Use EVP_PKEY based API for loading DH keys
On 19/10/2021 20:31, Arne Schwabe wrote: OpenSSL 3.0 replaces the DH API with a generic EVP_KEY based API to load DH parameters. Signed-off-by: Arne Schwabe Acked-by: Max Fillinger Looked at the patch, compiled with OpenSSL 3.1.0, tested that I can get a server and client to talk to each other on localhost. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 04/21] [OSSL 3.0] Remove DES check with OpenSSL 3.0
On 19/10/2021 20:31, Arne Schwabe wrote:
DES is very deprecated and accidently getting on the of the 16 insecure
keys that OpenSSL checks is extremely unlikely so we no longer use the
deprecated functions without replacement in OpenSSL 3.0.
Signed-off-by: Arne Schwabe
---
src/openvpn/crypto_openssl.c | 8
1 file changed, 8 insertions(+)
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 021698f12..8db2ddd09 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -521,6 +521,11 @@ key_des_num_cblocks(const EVP_CIPHER *kt)
bool
key_des_check(uint8_t *key, int key_len, int ndc)
{
+#if OPENSSL_VERSION_NUMBER < 0x3000L
+/* DES is deprecated and the method to even check the keys is deprecated
+ * in OpenSSL 3.0. Instead of checking for the 16 weak/semi-weak keys
+ * we just accept them in OpenSSL 3.0 since the risk of randomly getting
+ * these is pretty weak */
int i;
struct buffer b;
@@ -553,6 +558,9 @@ key_des_check(uint8_t *key, int key_len, int ndc)
err:
ERR_clear_error();
return false;
+#else
+return true;
+#endif
}
void
The code change looks good to me. Dropping the check is no loss because
*all* DES keys are weak keys.
The comment probably makes more sense in the #else branch, or before the
version check. Since I'm already nitpicking, it should be "the risk is
low", not "weak".
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 02/21] [OSSL 3.0] Add --with-openssl-engine autoconf option (auto|yes|no)
On 19/10/2021 20:31, Arne Schwabe wrote:
This allows to select engine support at configure time. For OpenSSL 1.1 the
default is not changed and we detect if engine support is available.
Engine support is deprecated in OpenSSL 3.0 and for OpenSSL 3.0 the default
is to disable engine support as engine support is deprecated and generates
compiler warnings which in turn also break -Werror.
By using --with-openssl-engine=no or --with-openssl-engine=yes engine support
can be forced on or off. If it is enabled but not detected an error will be
thown.
This commit cleans up the configurelogic a bit and removes the ENGINE_cleanup
checks as we can just assume that it will be also available as macro or function
if the other engine functions are available. Before the cleanup we would only
check for the existance of engine.h if ENGINE_cleanup was not found.
Signed-off-by: Arne Schwabe
Looks good to me. My one nitpick is that the part below uses a mix of
spaces and tabs for indentation. But the entire file doesn't seem
terribly consistent about that.
+if test "${with_openssl_engine}" = "auto"; then
+AC_COMPILE_IFELSE(
+ [AC_LANG_PROGRAM(
+ [[
+#include
+ ]],
+ [[
+/* Version encoding: MNNFFPPS - see opensslv.h for details */
+#if OPENSSL_VERSION_NUMBER >= 0x3000L
+#error Engine supported disabled by default in OpenSSL 3.0+
+#endif
+ ]]
+ )],
+ [have_openssl_engine="yes"],
+ [have_openssl_engine="no"]
+)
+if test "${have_openssl_engine}" = "yes"; then
+AC_CHECK_FUNCS(
+[ \
+ENGINE_load_builtin_engines \
+ENGINE_register_all_complete \
+],
+,
+[have_openssl_engine="no"; break]
+)
+fi
+else
+have_openssl_engine="${with_openssl_engine}"
+if test "${have_openssl_engine}" = "yes"; then
+AC_CHECK_FUNCS(
+[ \
+ENGINE_load_builtin_engines \
+ENGINE_register_all_complete \
+],
+,
+[AC_MSG_ERROR([OpenSSL engine support not found])]
+)
+fi
+fi
if test "${have_openssl_engine}" = "yes"; then
AC_DEFINE([HAVE_OPENSSL_ENGINE], [1], [OpenSSL engine support
available])
fi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 01/21] [OSSL 3.0] Use new EVP_MAC API for HMAC implementation
On 19/10/2021 20:31, Arne Schwabe wrote: The old API is deprecated in OpenSSL 3.0 and the new API does not yet exist in OpenSSL 1.1. Emulating the new API would be more complex than just having two implementations. So this switches to a new hmac implementation for OpenSSL 3.0. Unfortunately the new API does not have an easy to reset an HMAC, so we need to keep the key around to emulate a reset functionality. Signed-off-by: Arne Schwabe Acked-by: Max Fillinger Looked at the code, compiled with OpenSSL 3.1.0 and 1.1.1, and ran --test-crypto for both. Small typo in commit message: "does not have an easy to reset", probably should be "easy way to reset". ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 03/21] [OSSL 3.0] Implement DES ECB encrypt via EVP_CIPHER api
On 19/10/2021 20:31, Arne Schwabe wrote: +if (!EVP_EncryptInit_ex(ctx, EVP_bf_ecb(), NULL, key, 0)) EVP_bf_ecb() is the Blowfish cipher, not DES. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2] mbedtls: do not define mbedtls_ctr_drbg_update_ret when not needed
On 12/08/2021 10:53, Antonio Quartulli wrote: The mbedtls_ctr_drbg_update_ret() function was backported to various older branches, including 2.14 and 2.7. To aqvoid making the if guard too complex, let's detect if this function exist at configure time. All versions not having this function, will use our compat code. Cc: Max Fillinger Signed-off-by: Antonio Quartulli Thanks again for cleaning up my mess! Compile-tested with mbedtls versions 2.27.0 2.26.0 2.25.0 2.16.11 2.15.1 2.14.1 2.14.0 2.12.0 2.7.19 2.7.0 All good! (Typo: "aqvoid" in the commit message, but that can be fixed when committing it, right?) Acked-by: Max Fillinger ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Replace deprecated mbedtls DRBG update function
The function mbedtls_ctr_drbg_update is deprecated as of mbedtls 2.16
and is superseded by mbedtls_ctr_drbg_update_ret, which returns an error
code. This commit replaces the call to the deprecated function with the
new one and logs a warning in case of an error.
For older versions of mbedtls, we add a compatibility function that runs
mbedtls_ctr_drbg_update and returns 0.
Signed-off-by: Max Fillinger
---
src/openvpn/ssl_mbedtls.c | 20 +++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 265ea36f..1853335e 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -62,6 +62,21 @@
#include
#include
+/**
+ * Compatibility: mbedtls_ctr_drbg_update was deprecated in mbedtls 2.16 and
+ * replaced with mbedtls_ctr_drbg_update_ret, which returns an error code.
+ * For older versions, we call mbedtls_ctr_drbg_update and return 0 (success).
+ */
+#if MBEDTLS_VERSION_NUMBER < 0x0210
+static int mbedtls_ctr_drbg_update_ret(mbedtls_ctr_drbg_context *ctx,
+ const unsigned char *additional,
+ size_t add_len)
+{
+mbedtls_ctr_drbg_update(ctx, additional, add_len);
+return 0;
+}
+#endif
+
static const mbedtls_x509_crt_profile openvpn_x509_crt_profile_legacy =
{
/* Hashes from SHA-1 and above */
@@ -950,7 +965,10 @@ tls_ctx_personalise_random(struct tls_root_ctx *ctx)
if (0 != memcmp(old_sha256_hash, sha256_hash, sizeof(sha256_hash)))
{
-mbedtls_ctr_drbg_update(cd_ctx, sha256_hash, 32);
+if (!mbed_ok(mbedtls_ctr_drbg_update_ret(cd_ctx, sha256_hash, 32)))
+{
+msg(M_WARN, "WARNING: failed to personalise random, could not
update CTR_DRBG");
+}
memcpy(old_sha256_hash, sha256_hash, sizeof(old_sha256_hash));
}
}
--
2.11.0
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Update Fox e-mail address in copyright notices
Replace open...@fox-it.com with open...@foxcrypto.com. Signed-off-by: Max Fillinger --- doc/doxygen/Makefile.am| 2 +- doc/doxygen/doc_compression.h | 2 +- doc/doxygen/doc_control_processor.h| 2 +- doc/doxygen/doc_control_tls.h | 2 +- doc/doxygen/doc_data_control.h | 2 +- doc/doxygen/doc_data_crypto.h | 2 +- doc/doxygen/doc_eventloop.h| 2 +- doc/doxygen/doc_external_multiplexer.h | 2 +- doc/doxygen/doc_fragmentation.h| 2 +- doc/doxygen/doc_internal_multiplexer.h | 2 +- doc/doxygen/doc_key_generation.h | 2 +- doc/doxygen/doc_mainpage.h | 2 +- doc/doxygen/doc_memory_management.h| 2 +- doc/doxygen/doc_protocol_overview.h| 2 +- doc/doxygen/doc_reliable.h | 2 +- doc/doxygen/doc_tunnel_state.h | 2 +- src/openvpn/crypto.c | 2 +- src/openvpn/crypto.h | 2 +- src/openvpn/crypto_backend.h | 2 +- src/openvpn/crypto_mbedtls.c | 2 +- src/openvpn/crypto_mbedtls.h | 2 +- src/openvpn/crypto_openssl.c | 2 +- src/openvpn/crypto_openssl.h | 2 +- src/openvpn/openssl_compat.h | 2 +- src/openvpn/pkcs11_backend.h | 2 +- src/openvpn/pkcs11_mbedtls.c | 2 +- src/openvpn/pkcs11_openssl.c | 2 +- src/openvpn/ssl.c | 2 +- src/openvpn/ssl.h | 2 +- src/openvpn/ssl_backend.h | 2 +- src/openvpn/ssl_common.h | 2 +- src/openvpn/ssl_mbedtls.c | 2 +- src/openvpn/ssl_mbedtls.h | 2 +- src/openvpn/ssl_ncp.c | 2 +- src/openvpn/ssl_ncp.h | 2 +- src/openvpn/ssl_openssl.c | 2 +- src/openvpn/ssl_openssl.h | 2 +- src/openvpn/ssl_verify.c | 2 +- src/openvpn/ssl_verify.h | 2 +- src/openvpn/ssl_verify_backend.h | 2 +- src/openvpn/ssl_verify_mbedtls.c | 2 +- src/openvpn/ssl_verify_mbedtls.h | 2 +- src/openvpn/ssl_verify_openssl.c | 2 +- src/openvpn/ssl_verify_openssl.h | 2 +- src/openvpn/tls_crypt.c| 2 +- src/openvpn/tls_crypt.h| 2 +- tests/unit_tests/openvpn/mock_get_random.c | 2 +- tests/unit_tests/openvpn/mock_msg.c| 2 +- tests/unit_tests/openvpn/mock_msg.h| 2 +- tests/unit_tests/openvpn/test_auth_token.c | 2 +- tests/unit_tests/openvpn/test_buffer.c | 2 +- tests/unit_tests/openvpn/test_crypto.c | 2 +- tests/unit_tests/openvpn/test_packet_id.c | 2 +- tests/unit_tests/openvpn/test_tls_crypt.c | 2 +- 54 files changed, 54 insertions(+), 54 deletions(-) diff --git a/doc/doxygen/Makefile.am b/doc/doxygen/Makefile.am index fe0eafc4..82d909dd 100644 --- a/doc/doxygen/Makefile.am +++ b/doc/doxygen/Makefile.am @@ -5,7 +5,7 @@ # packet encryption, packet authentication, and # packet compression. # -# Copyright (C) 2017-2021 Fox-IT B.V. +# Copyright (C) 2017-2021 Fox-IT B.V. # MAINTAINERCLEANFILES = \ diff --git a/doc/doxygen/doc_compression.h b/doc/doxygen/doc_compression.h index a6180913..f67eba35 100644 --- a/doc/doxygen/doc_compression.h +++ b/doc/doxygen/doc_compression.h @@ -5,7 +5,7 @@ * packet encryption, packet authentication, and * packet compression. * - * Copyright (C) 2010-2021 Fox Crypto B.V. + * Copyright (C) 2010-2021 Fox Crypto B.V. * * * This program is free software; you can redistribute it and/or modify diff --git a/doc/doxygen/doc_control_processor.h b/doc/doxygen/doc_control_processor.h index 58124789..3e9c5921 100644 --- a/doc/doxygen/doc_control_processor.h +++ b/doc/doxygen/doc_control_processor.h @@ -5,7 +5,7 @@ * packet encryption, packet authentication, and * packet compression. * - * Copyright (C) 2010-2021 Fox Crypto B.V. + * Copyright (C) 2010-2021 Fox Crypto B.V. * * * This program is free software; you can redistribute it and/or modify diff --git a/doc/doxygen/doc_control_tls.h b/doc/doxygen/doc_control_tls.h index e6988a1c..7ff5e996 100644 --- a/doc/doxygen/doc_control_tls.h +++ b/doc/doxygen/doc_control_tls.h @@ -5,7 +5,7 @@ * packet encryption, packet authentication, and * packet compression. * - * Copyright (C) 2010-2021 Fox Crypto B.V. + * Copyright (C) 2010-2021 Fox Crypto B.V. * * * This program is free software; you can redistribute it and/or modify diff --git a/doc/doxygen/doc_data_control.h b/doc/doxygen/doc_data_control.h index 66d8c54e..ccb0117a 100644 --- a/doc/doxygen/doc_data_control.h +++ b/doc/doxygen/doc_data_control.h @@ -5,7 +5,7 @@ * packet encryption, packet authentication
[Openvpn-devel] [PATCH v3 2/2] Abort if CRL file can't be stat-ed in ssl_init
Now that the path for the CRL file is handled correctly when using
chroot, there's no good reason for the file to be inaccessible during
ssl_init().
This commit ensures that the CRL file is accessed successfully at least
once, which fixes a bug where the mbedtls version of OpenVPN wouldn't
use a reloaded CRL if it initially failed to access the file.
Signed-off-by: Max Fillinger
---
src/openvpn/ssl.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 1e0e6170..6ce1d079 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -559,7 +559,15 @@ tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const
char *crl_file,
}
else if (platform_stat(crl_file, _stat) < 0)
{
-msg(M_WARN, "WARNING: Failed to stat CRL file, not (re)loading CRL.");
+/* If crl_last_mtime is zero, the CRL file has not been read before. */
+if (ssl_ctx->crl_last_mtime == 0)
+{
+msg(M_FATAL, "ERROR: Failed to stat CRL file during
initialization, exiting.");
+}
+else
+{
+msg(M_WARN, "WARNING: Failed to stat CRL file, not reloading
CRL.");
+}
return;
}
--
2.11.0
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v3 1/2] In init_ssl, open the correct CRL path pre-chroot
When using the chroot option, the init_ssl function can be called before
entering the chroot or, when OpenVPN receives a SIGHUP, afterwards. This
commit ensures that OpenVPN tries to open the correct path for the CRL
file in either situation.
This commit does not address key and certificate files. For these, the
--persist-key option should be used.
Signed-off-by: Max Fillinger
---
src/openvpn/init.c| 2 +-
src/openvpn/misc.c| 11 +++
src/openvpn/misc.h| 6 ++
src/openvpn/options.c | 8 +---
src/openvpn/ssl.c | 21 +++--
src/openvpn/ssl.h | 2 +-
6 files changed, 39 insertions(+), 11 deletions(-)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 28d183aa..f18e054a 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2702,7 +2702,7 @@ do_init_crypto_tls_c1(struct context *c)
* Initialize the OpenSSL library's global
* SSL context.
*/
-init_ssl(options, &(c->c1.ks.ssl_ctx));
+init_ssl(options, &(c->c1.ks.ssl_ctx), c->c0 &&
c->c0->uid_gid_chroot_set);
if (!tls_ctx_initialised(>c1.ks.ssl_ctx))
{
switch (auth_retry_get())
diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index feaefb3b..650daa0c 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -767,3 +767,14 @@ get_num_elements(const char *string, char delimiter)
return element_count;
}
+
+struct buffer
+prepend_dir(const char *dir, const char *path, struct gc_arena *gc)
+{
+size_t len = strlen(dir) + strlen(PATH_SEPARATOR_STR) + strlen(path) + 1;
+struct buffer combined_path = alloc_buf_gc(len, gc);
+buf_printf(_path, "%s%s%s", dir, PATH_SEPARATOR_STR, path);
+ASSERT(combined_path.len > 0);
+
+return combined_path;
+}
diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h
index 9b018eb5..d9005353 100644
--- a/src/openvpn/misc.h
+++ b/src/openvpn/misc.h
@@ -190,4 +190,10 @@ void output_peer_info_env(struct env_set *es, const char
*peer_info);
int
get_num_elements(const char *string, char delimiter);
+/**
+ * Prepend a directory to a path.
+ */
+struct buffer
+prepend_dir(const char *dir, const char *path, struct gc_arena *gc);
+
#endif /* ifndef MISC_H */
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 9e61b1e0..9ce12a2b 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3317,14 +3317,8 @@ check_file_access_chroot(const char *chroot, const int
type, const char *file, c
{
struct gc_arena gc = gc_new();
struct buffer chroot_file;
-int len = 0;
-
-/* Build up a new full path including chroot directory */
-len = strlen(chroot) + strlen(PATH_SEPARATOR_STR) + strlen(file) + 1;
-chroot_file = alloc_buf_gc(len, );
-buf_printf(_file, "%s%s%s", chroot, PATH_SEPARATOR_STR, file);
-ASSERT(chroot_file.len > 0);
+chroot_file = prepend_dir(chroot, file, );
ret = check_file_access(type, BSTR(_file), mode, opt);
gc_free();
}
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index d8662d00..1e0e6170 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -584,7 +584,7 @@ tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char
*crl_file,
* All files are in PEM format.
*/
void
-init_ssl(const struct options *options, struct tls_root_ctx *new_ctx)
+init_ssl(const struct options *options, struct tls_root_ctx *new_ctx, bool
in_chroot)
{
ASSERT(NULL != new_ctx);
@@ -702,7 +702,24 @@ init_ssl(const struct options *options, struct
tls_root_ctx *new_ctx)
/* Read CRL */
if (options->crl_file && !(options->ssl_flags & SSLF_CRL_VERIFY_DIR))
{
-tls_ctx_reload_crl(new_ctx, options->crl_file,
options->crl_file_inline);
+/* If we're running with the chroot option, we may run init_ssl()
before
+ * and after chroot-ing. We can use the crl_file path as-is if we're
+ * not going to chroot, or if we already are inside the chroot.
+ *
+ * If we're going to chroot later, we need to prefix the path of the
+ * chroot directory to crl_file.
+ */
+if (!options->chroot_dir || in_chroot || options->crl_file_inline)
+{
+tls_ctx_reload_crl(new_ctx, options->crl_file,
options->crl_file_inline);
+}
+else
+{
+struct gc_arena gc = gc_new();
+struct buffer crl_file_buf = prepend_dir(options->chroot_dir,
options->crl_file, );
+tls_ctx_reload_crl(new_ctx, BSTR(_file_buf),
options->crl_file_inline);
+gc_free();
+}
}
/* Once keys and cert are loaded, load ECDH parameters */
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 300a70d3..45ebe720 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -159,7 +159,7 @@ void free_ssl_lib(void);
* Build m
[Openvpn-devel] [PATCH] Fix build with mbedtls w/o SSL renegotiation support
In mbedtls, support for SSL renegotiation can be disabled at
compile-time. However, OpenVPN cannot be built with such a library
because it calls mbedtls_ssl_conf_renegotiation() to disable this
feature at runtime. This function doesn't exist when mbedtls was built
without support for SSL renegotiation.
This commit fixes the build by ifdef'ing out the function call when
mbedtls was built without support for SSL renegotiation.
Signed-off-by: Max Fillinger
---
src/openvpn/ssl_mbedtls.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 8917fb18..7e2f0f5d 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -1086,10 +1086,13 @@ key_state_ssl_init(struct key_state_ssl *ks_ssl,
{
mbedtls_ssl_conf_curves(ks_ssl->ssl_config, ssl_ctx->groups);
}
-/* Disable TLS renegotiations. OpenVPN's renegotiation creates new SSL
- * session and does not depend on this feature. And TLS renegotiations have
- * been problematic in the past */
+
+/* Disable TLS renegotiations if the mbedtls library supports that feature.
+ * OpenVPN's renegotiation creates new SSL sessions and does not depend on
+ * this feature and TLS renegotiations have been problematic in the past.
*/
+#if defined(MBEDTLS_SSL_RENEGOTIATION)
mbedtls_ssl_conf_renegotiation(ks_ssl->ssl_config,
MBEDTLS_SSL_RENEGOTIATION_DISABLED);
+#endif /* MBEDTLS_SSL_RENEGOTIATION */
/* Disable record splitting (for now). OpenVPN assumes records are sent
* unfragmented, and changing that will require thorough review and
--
2.11.0
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2 2/2] Abort if CRL file can't be stat-ed in init_ssl
Now that the path for the CRL file is handled correctly when using
chroot, there's no good reason for the file to be inaccessible during
init_ssl().
This commit ensures that the CRL file is accessed successfully at least
once, which fixes a bug where the mbedtls version of OpenVPN wouldn't
use a reloaded CRL if it initially failed to access the file.
In tls_process(), we stick with the previous behavior of logging a
warning and keeping the old CRL to ensure that the CRL file can be
updated on-the-fly.
Signed-off-by: Max Fillinger
---
src/openvpn/ssl.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 8782b140..a42b639c 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -539,10 +539,14 @@ tls_version_parse(const char *vstr, const char *extra)
* @param crl_file The file name to load the CRL from, or
* "[[INLINE]]" in the case of inline files.
* @param crl_inlineA string containing the CRL
+ * @param abort_on_err If this is true, OpenVPN exits if it can't stat
+ * the CRL file. If it is false, it will instead
+ * log a warning and continue using the previously
+ * loaded CRL.
*/
static void
tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char *crl_file,
- bool crl_file_inline)
+ bool crl_file_inline, bool abort_on_err)
{
/* if something goes wrong with stat(), we'll store 0 as mtime */
platform_stat_t crl_stat = {0};
@@ -559,7 +563,14 @@ tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const
char *crl_file,
}
else if (platform_stat(crl_file, _stat) < 0)
{
-msg(M_WARN, "WARNING: Failed to stat CRL file, not (re)loading CRL.");
+if (abort_on_err)
+{
+msg(M_FATAL, "ERROR: Failed to stat CRL file during init_ssl,
exiting.");
+}
+else
+{
+msg(M_WARN, "WARNING: Failed to stat CRL file, not (re)loading
CRL.");
+}
return;
}
@@ -710,13 +721,13 @@ init_ssl(const struct options *options, struct
tls_root_ctx *new_ctx, bool in_ch
* chroot directory to crl_file. */
if (options->chroot_dir == NULL || in_chroot ||
options->crl_file_inline)
{
-tls_ctx_reload_crl(new_ctx, options->crl_file,
options->crl_file_inline);
+tls_ctx_reload_crl(new_ctx, options->crl_file,
options->crl_file_inline, true);
}
else
{
struct gc_arena gc = gc_new();
struct buffer crl_file_buf = prepend_chroot(options->chroot_dir,
options->crl_file, );
-tls_ctx_reload_crl(new_ctx, BSTR(_file_buf),
options->crl_file_inline);
+tls_ctx_reload_crl(new_ctx, BSTR(_file_buf),
options->crl_file_inline, true);
gc_free();
}
}
@@ -2752,7 +2763,7 @@ tls_process(struct tls_multi *multi,
&& !(session->opt->ssl_flags & SSLF_CRL_VERIFY_DIR))
{
tls_ctx_reload_crl(>opt->ssl_ctx,
- session->opt->crl_file,
session->opt->crl_file_inline);
+ session->opt->crl_file,
session->opt->crl_file_inline, false);
}
/* New connection, remove any old X509 env variables */
--
2.11.0
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2 1/2] In init_ssl, open the correct CRL path pre-chroot
When using the chroot option, the init_ssl function can be called before
entering the chroot or, when OpenVPN receives a SIGHUP, afterwards. This
commit ensures that OpenVPN tries to open the correct path for the CRL
file in either situations.
This commit does not address key and certificate files. For these, the
--persist-key option should be used.
Signed-off-by: Max Fillinger
---
src/openvpn/init.c| 3 ++-
src/openvpn/misc.c| 11 +++
src/openvpn/misc.h| 7 +++
src/openvpn/options.c | 8 +---
src/openvpn/ssl.c | 20 ++--
src/openvpn/ssl.h | 2 +-
6 files changed, 40 insertions(+), 11 deletions(-)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 28d183aa..ba38e548 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2702,7 +2702,8 @@ do_init_crypto_tls_c1(struct context *c)
* Initialize the OpenSSL library's global
* SSL context.
*/
-init_ssl(options, &(c->c1.ks.ssl_ctx));
+bool in_chroot = (c->c0 != NULL) ? c->c0->uid_gid_chroot_set : false;
+init_ssl(options, &(c->c1.ks.ssl_ctx), in_chroot);
if (!tls_ctx_initialised(>c1.ks.ssl_ctx))
{
switch (auth_retry_get())
diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index feaefb3b..a64803a7 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -767,3 +767,14 @@ get_num_elements(const char *string, char delimiter)
return element_count;
}
+
+struct buffer
+prepend_chroot(const char *chroot_dir, const char *path, struct gc_arena *gc)
+{
+size_t len = strlen(chroot_dir) + strlen(PATH_SEPARATOR_STR) +
strlen(path) + 1;
+struct buffer combined_path = alloc_buf_gc(len, gc);
+buf_printf(_path, "%s%s%s", chroot_dir, PATH_SEPARATOR_STR, path);
+ASSERT(combined_path.len > 0);
+
+return combined_path;
+}
diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h
index 9b018eb5..9f352754 100644
--- a/src/openvpn/misc.h
+++ b/src/openvpn/misc.h
@@ -190,4 +190,11 @@ void output_peer_info_env(struct env_set *es, const char
*peer_info);
int
get_num_elements(const char *string, char delimiter);
+/**
+ * Prepend the chroot directory to a path. Used when we need to access
+ * a file in the chroot directory before we have chroot-ed.
+ */
+struct buffer
+prepend_chroot(const char *chroot_dir, const char *path, struct gc_arena *gc);
+
#endif /* ifndef MISC_H */
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 9e61b1e0..d6ba2938 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3317,14 +3317,8 @@ check_file_access_chroot(const char *chroot, const int
type, const char *file, c
{
struct gc_arena gc = gc_new();
struct buffer chroot_file;
-int len = 0;
-
-/* Build up a new full path including chroot directory */
-len = strlen(chroot) + strlen(PATH_SEPARATOR_STR) + strlen(file) + 1;
-chroot_file = alloc_buf_gc(len, );
-buf_printf(_file, "%s%s%s", chroot, PATH_SEPARATOR_STR, file);
-ASSERT(chroot_file.len > 0);
+chroot_file = prepend_chroot(chroot, file, );
ret = check_file_access(type, BSTR(_file), mode, opt);
gc_free();
}
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index d8662d00..8782b140 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -584,7 +584,7 @@ tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char
*crl_file,
* All files are in PEM format.
*/
void
-init_ssl(const struct options *options, struct tls_root_ctx *new_ctx)
+init_ssl(const struct options *options, struct tls_root_ctx *new_ctx, bool
in_chroot)
{
ASSERT(NULL != new_ctx);
@@ -702,7 +702,23 @@ init_ssl(const struct options *options, struct
tls_root_ctx *new_ctx)
/* Read CRL */
if (options->crl_file && !(options->ssl_flags & SSLF_CRL_VERIFY_DIR))
{
-tls_ctx_reload_crl(new_ctx, options->crl_file,
options->crl_file_inline);
+/* If we're running with the chroot option, we may run init_ssl()
before
+ * and after chroot-ing. We can use the crl_file path as-is if we're
+ * not going to chroot, or if we already are inside the chroot.
+ *
+ * If we're going to chroot later, we need to prefix the path of the
+ * chroot directory to crl_file. */
+if (options->chroot_dir == NULL || in_chroot ||
options->crl_file_inline)
+{
+tls_ctx_reload_crl(new_ctx, options->crl_file,
options->crl_file_inline);
+}
+else
+{
+struct gc_arena gc = gc_new();
+struct buffer crl_file_buf = prepend_chroot(options->chroot_dir,
options->crl_file, );
+tls_ctx_reload_crl(new_ctx, BSTR(_file_buf),
options->crl_file_inline);
+gc_free();
+}
}
/* Once keys and cert are loaded, load ECDH parameters */
dif
[Openvpn-devel] [PATCH v2 0/2] CRL reloading and chroot with mbedtls
After a lot of discussion on IRC on Friday, here's a new attempt at fixing the mbedtls certificate reloading issue. To sum up the background: Compumatica discovered the following pair of bugs in OpenVPN-NL, which are also present in stock OpenVPN with mbedtls. 1) With mbedtls, if the CRL file can't be accessed during init_ssl(), OpenVPN will read the file in tls_process() when it becomes available later, but it will not actually use it. This situation is likely to happen when running in a chroot because of the second bug. 2) OpenVPN attempts to read the CRL file in init_ssl() before chroot-ing and tries to access the path outside of the chroot directory. For example, let's say we have the CRL file in "/chroot/crl.pem", and we run OpenVPN with "--chroot /chroot/" and "--crl-verify /crl.pem". During option validation, OpenVPN will check that "/chroot/crl.pem" exists. Pre-chroot, it will try to access "/crl.pem", which fails. Post-chroot, it opens the file. Bug 2) is present in OpenVPN with OpenSSL, too, but OpenSSL actually uses the reloaded CRL from tls_process(), so the only consequence is a warning message in the logs. The first patch fixes bug 2) by prefixing the path to the chroot directory to the CRL file when we're running init_ssl() pre-chroot. By itself, this makes it much more difficult to trigger bug 1). The second patch makes OpenVPN abort in init_ssl() if the CRL file cannot be accessed. Now that the path is handled correctly pre- and post-chroot, there is no good reason why accessing it should fail. This fixes bug 1). Max Fillinger (2): In init_ssl, open the correct CRL path pre-chroot Abort if CRL file can't be stat-ed in init_ssl src/openvpn/init.c| 3 ++- src/openvpn/misc.c| 11 +++ src/openvpn/misc.h| 7 +++ src/openvpn/options.c | 8 +--- src/openvpn/ssl.c | 37 - src/openvpn/ssl.h | 2 +- 6 files changed, 54 insertions(+), 14 deletions(-) -- 2.11.0 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 0/1] CRL issues with mbedtls
This patch fixes the bug I wrote about earlier[0] where the mbedtls version of OpenVPN might not properly reload a CRL when running in a chroot. I've submitted a somewhat hacky patch for it[1]. While looking into it further, I also noticed another unrelated problem: The mbedtls documentation states that the config struct for a mbedtls_ssl_context is not supposed to be modified after calling mbedtls_ssl_setup(). However, the config contains a pointer to the CRL, and we're currently modifying the CRL in place when we reload it. I figured that by reworking the way CRLs are handled, I could fix the CRL reloading bug in a less hacky manner and also make sure that we don't modify the configs of active mbedtls_ssl_contexts. [0] https://sourceforge.net/p/openvpn/mailman/message/37254045/ [1] https://sourceforge.net/p/openvpn/mailman/message/37254048/ Max Fillinger (1): Rework mbedtls CRL handling src/openvpn/ssl.c| 8 +++ src/openvpn/ssl_mbedtls.c| 103 ++- src/openvpn/ssl_mbedtls.h| 25 +- src/openvpn/ssl_verify_mbedtls.c | 2 +- 4 files changed, 125 insertions(+), 13 deletions(-) -- 2.11.0 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 1/1] Rework mbedtls CRL handling
This commit fixes the following two issues:
The config belonging to a mbedtls_ssl_ctx struct is not supposed to be
changed after mbedtls_ssl_setup() has been called. Previously, we
modified the CRL structure in place when a new CRL was loaded, but a
pointer to this struct appears in configs that are currently in use.
This commit uses reference counting to ensure that CRLs remain in memory
while the mbedtls_ssl_ctx struct and config remain.
The other issue fixed by this commit is that, if OpenVPN failed to read
the CRL in init_ssl(), but succeeded in reading it later, it would not
actually use that CRL.
---
src/openvpn/ssl.c| 8 +++
src/openvpn/ssl_mbedtls.c| 103 ++-
src/openvpn/ssl_mbedtls.h| 25 +-
src/openvpn/ssl_verify_mbedtls.c | 2 +-
4 files changed, 125 insertions(+), 13 deletions(-)
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index d8662d00..0f2cb62d 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -2737,6 +2737,14 @@ tls_process(struct tls_multi *multi,
{
tls_ctx_reload_crl(>opt->ssl_ctx,
session->opt->crl_file,
session->opt->crl_file_inline);
+
+#ifdef ENABLE_CRYPTO_MBEDTLS
+/* With mbedtls, the new CRL doesn't end up in the key_state
+ * automatically, so we need to put it there and recreate the
+ * mbedtls_ssl_context. */
+update_key_state_crl(>ks_ssl, >opt->ssl_ctx);
+#endif
+
}
/* New connection, remove any old X509 env variables */
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 8917fb18..87a4b8df 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -105,6 +105,45 @@ tls_clear_error(void)
{
}
+/* Functions to handle reference counting for CRLs.
+ *
+ * The config of an mbedtls_ssl_context is not supposed to be changed
+ * after mbedtls_ssl_setup() has been called. Therefore, we need to
+ * leave previous CRLs in memory. They will be freed when they are no
+ * longer needed.
+ */
+static struct refcounted_crl *
+init_refcounted_crl(void)
+{
+struct refcounted_crl *ref_crl;
+ALLOC_OBJ(ref_crl, struct refcounted_crl);
+mbedtls_x509_crl_init(_crl->crl);
+ref_crl->refcount = 1;
+
+return ref_crl;
+}
+
+static struct refcounted_crl *
+get_refcounted_crl(struct refcounted_crl *ref_crl)
+{
+ASSERT(ref_crl->refcount < SIZE_MAX);
+ref_crl->refcount += 1;
+return ref_crl;
+}
+
+static void
+release_refcounted_crl(struct refcounted_crl *ref_crl)
+{
+if (ref_crl != NULL) {
+ASSERT(ref_crl->refcount > 0);
+if (--ref_crl->refcount == 0)
+{
+mbedtls_x509_crl_free(_crl->crl);
+free(ref_crl);
+}
+}
+}
+
void
tls_ctx_server_new(struct tls_root_ctx *ctx)
{
@@ -149,8 +188,7 @@ tls_ctx_free(struct tls_root_ctx *ctx)
mbedtls_dhm_free(ctx->dhm_ctx);
free(ctx->dhm_ctx);
-mbedtls_x509_crl_free(ctx->crl);
-free(ctx->crl);
+release_refcounted_crl(ctx->ref_crl);
#if defined(ENABLE_PKCS11)
pkcs11h_certificate_freeCertificate(ctx->pkcs11_cert);
@@ -1013,15 +1051,12 @@ backend_tls_ctx_reload_crl(struct tls_root_ctx *ctx,
const char *crl_file,
{
ASSERT(crl_file);
-if (ctx->crl == NULL)
-{
-ALLOC_OBJ_CLEAR(ctx->crl, mbedtls_x509_crl);
-}
-mbedtls_x509_crl_free(ctx->crl);
+release_refcounted_crl(ctx->ref_crl);
+ctx->ref_crl = init_refcounted_crl();
if (crl_inline)
{
-if (!mbed_ok(mbedtls_x509_crl_parse(ctx->crl,
+if (!mbed_ok(mbedtls_x509_crl_parse(>ref_crl->crl,
(const unsigned char *)crl_file,
strlen(crl_file) + 1)))
{
@@ -1031,7 +1066,7 @@ backend_tls_ctx_reload_crl(struct tls_root_ctx *ctx,
const char *crl_file,
}
else
{
-if (!mbed_ok(mbedtls_x509_crl_parse_file(ctx->crl, crl_file)))
+if (!mbed_ok(mbedtls_x509_crl_parse_file(>ref_crl->crl,
crl_file)))
{
msg(M_WARN, "CRL: cannot read CRL from file %s", crl_file);
goto err;
@@ -1040,7 +1075,9 @@ backend_tls_ctx_reload_crl(struct tls_root_ctx *ctx,
const char *crl_file,
return;
err:
-mbedtls_x509_crl_free(ctx->crl);
+/* Leaving the CRL in the freed state causes all connection attempts to
+ * be rejected. */
+mbedtls_x509_crl_free(>ref_crl->crl);
}
void
@@ -1121,8 +1158,17 @@ key_state_ssl_init(struct key_state_ssl *ks_ssl,
}
mbedtls_ssl_conf_verify(ks_ssl->ssl_config, verify_callback, session);
+if (ssl_ctx->ref_crl)
+{
+ks_ssl->ref_crl = get_refcounted_crl(ssl_ctx->ref_crl);
+}
+else
+{
+ks_ssl->ref_crl = NULL;
+}
+
/* TODO: mbed TLS does not currently support sending the CA chain to the
client */
[Openvpn-devel] [PATCH] Change CTR DRBG update function call to new mbedtls 2.16.0 API
From: Uipko Berghuis
In mbedtls 2.16.0 mbedtls_ctr_drbg_update() changed to
mbedtls_ctr_drbg_update_ret(). Change the function name and handle
the new return value error code.
---
src/openvpn/ssl_mbedtls.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 5d7af351..56e9f045 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -950,7 +950,10 @@ tls_ctx_personalise_random(struct tls_root_ctx *ctx)
if (0 != memcmp(old_sha256_hash, sha256_hash, sizeof(sha256_hash)))
{
-mbedtls_ctr_drbg_update(cd_ctx, sha256_hash, 32);
+if (!mbed_ok(mbedtls_ctr_drbg_update_ret(cd_ctx, sha256_hash, 32)))
+{
+msg(M_WARN, "WARNING: failed to personalise random, could not
update CTR_DRBG");
+}
memcpy(old_sha256_hash, sha256_hash, sizeof(old_sha256_hash));
}
}
--
2.11.0
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 1/1] Let mbedtls_ssl_configs find reloaded CRLs
From: Maximilian Fillinger
If the CRL file cannot be read during initialization, a NULL pointer is
passed to the mbedtls_ssl_config in key_state_ssl_init(). Then, if the
CRL file is successfully read later, the config won't have a pointer to
it. Therefore, the CRL won't actually take effect.
This commit fixes the bug by creating an empty CRL if crl-verify is in
the config, but the file cannot be read during initialization. That way,
we can always give the mbedtls_ssl_config struct a pointer to the
location where the CRL will be if it is successfully read later.
This commit also fixes an additional issue:
When a CRL file is present but cannot be parsed, OpenVPN rejects all
incoming connections. When the CRL file cannot be stat'ed, OpenVPN keeps
using the previous CRL. This commit makes it so that OpenVPN rejects
connections in both situations.
---
src/openvpn/ssl.c | 11 +++
src/openvpn/ssl_mbedtls.c | 13 +
src/openvpn/ssl_mbedtls.h | 3 +++
3 files changed, 27 insertions(+)
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 08222b5e..f20771d7 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -559,6 +559,17 @@ tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const
char *crl_file,
else if (platform_stat(crl_file, _stat) < 0)
{
msg(M_WARN, "WARNING: Failed to stat CRL file, not (re)loading CRL.");
+
+#ifdef ENABLE_CRYPTO_MBEDTLS
+/* Store an empty CRL in ssl_ctx. This is so that we can give the
+ * mbedtls_ssl_configs in the key_states a pointer to the location
where
+ * the CRL will be if we successfully reload the file later.
+ *
+ * Storing an empty CRL also causes all connection attempts to be
rejected
+ * until an actual CRL is loaded. */
+make_empty_crl(ssl_ctx);
+#endif
+
return;
}
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 4626e983..5d7af351 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -1044,6 +1044,19 @@ err:
}
void
+make_empty_crl(struct tls_root_ctx *ctx)
+{
+if (ctx->crl == NULL)
+{
+ALLOC_OBJ_CLEAR(ctx->crl, mbedtls_x509_crl);
+}
+else
+{
+mbedtls_x509_crl_free(ctx->crl);
+}
+}
+
+void
key_state_ssl_init(struct key_state_ssl *ks_ssl,
const struct tls_root_ctx *ssl_ctx, bool is_server,
struct tls_session *session)
diff --git a/src/openvpn/ssl_mbedtls.h b/src/openvpn/ssl_mbedtls.h
index ff64e17c..579e3c8e 100644
--- a/src/openvpn/ssl_mbedtls.h
+++ b/src/openvpn/ssl_mbedtls.h
@@ -144,4 +144,7 @@ int tls_ctx_use_external_signing_func(struct tls_root_ctx
*ctx,
external_sign_func sign_func,
void *sign_ctx);
+void make_empty_crl(struct tls_root_ctx *);
+
+
#endif /* SSL_MBEDTLS_H_ */
--
2.11.0
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 0/1] CRL reloading issue with mbedTLS and chroot
There is an issue with the mbedTLS version of OpenVPN where a CRL file wouldn't be used when running in a chroot. This is due to a combination of two bugs found by Adam Lukosek at Compumatica. 1) With mbedTLS, if the CRL file can't be opened during initialization, OpenVPN will read the file when it becomes available later, but it will not actually use it. This situation is likely to happen when running in a chroot because... 2) When a file needs to be opened both pre- and post-chroot, OpenVPN will try to open the path outside of the chroot directory. For example, let's say we have the CRL file in "/chroot/crl.pem", and we run OpenVPN with "--chroot /chroot/" and "--crl-verify /crl.pem". During option validation, OpenVPN will check that "/chroot/crl.pem" exists. Pre-chroot, it will try to open "/crl.pem", which fails. Post-chroot, it opens the file. In my patch, I fix issue 1). I don't find it very elegant, but the only other solution I saw was to add function arguments to ssl_backend.h functions that are only relevant for mbedTLS. I'd be happy to hear other suggestions! I think that issue 2) should be addressed as well. Reloading CRLs does work with OpenSSL, but there's still a possibility that OpenVPN reads different files pre- and post-chroot. (E.g., imagine a user updates "/crl.pem" but forgets "/chroot/crl.pem".) I would like to always prefix the chroot-path when opening a file that needs to be available post-chroot during init. However, this might break some existing setups. Does that seem like a good idea to you? Finally, I noticed a behavior that seems inconsistent to me: When OpenVPN cannot parse a CRL file, it will reject all connection attempts. When it cannot stat a CRL file, it will continue without a CRL. Is there a reason why it has to work that way? I saw this behavior with both mbedTLS and OpenSSL. My patch accidentally fixed it for mbedTLS. Maximilian Fillinger (1): Let mbedtls_ssl_configs find reloaded CRLs src/openvpn/ssl.c | 11 +++ src/openvpn/ssl_mbedtls.c | 13 + src/openvpn/ssl_mbedtls.h | 3 +++ 3 files changed, 27 insertions(+) -- 2.11.0 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 1/1] reliable: retransmit if 3 follow-up ACKs are received
From: Steffan Karger
To improve the control channel performance under packet loss conditions,
add a more aggressive retransmit policy similar to what many TCP
implementations do: retransmit a packet if the ACK timeout expires (like
we already do), *or* if three ACKs for follow-up packets are received.
The rationale behind this is that if follow-up packets *are* received, the
connection is apparently functional and we should be able to retransmit
immediately. This significantly improves performance for connections with
low (up to a few percent) packet loss.
---
src/openvpn/reliable.c | 20 +---
src/openvpn/reliable.h | 7 +++
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/src/openvpn/reliable.c b/src/openvpn/reliable.c
index 6c1f2da1..15b90fbe 100644
--- a/src/openvpn/reliable.c
+++ b/src/openvpn/reliable.c
@@ -382,7 +382,14 @@ reliable_send_purge(struct reliable *rel, const struct
reliable_ack *ack)
}
#endif
e->active = false;
-break;
+}
+else if (e->active && e->packet_id < pid)
+{
+/* We have received an ACK for a packet with a higher PID.
Either
+ * we have received ACKs out of or order or the packet has been
+ * lost. We count the number of ACKs to determine if we should
+ * resend it early. */
+e->n_acks++;
}
}
}
@@ -555,7 +562,7 @@ reliable_can_send(const struct reliable *rel)
if (e->active)
{
++n_active;
-if (now >= e->next_try)
+if (now >= e->next_try || e->n_acks >= N_ACK_RETRANSMIT)
{
++n_current;
}
@@ -581,7 +588,12 @@ reliable_send(struct reliable *rel, int *opcode)
for (i = 0; i < rel->size; ++i)
{
struct reliable_entry *e = >array[i];
-if (e->active && local_now >= e->next_try)
+
+/* If N_ACK_RETRANSMIT later packets have received ACKs, we assume
+ * that the packet was lost and resend it even if the timeout has
+ * not expired yet. */
+if (e->active
+&& (e->n_acks >= N_ACK_RETRANSMIT || local_now >= e->next_try))
{
if (!best || reliable_pid_min(e->packet_id, best->packet_id))
{
@@ -599,6 +611,7 @@ reliable_send(struct reliable *rel, int *opcode)
/* constant timeout, no backoff */
best->next_try = local_now + best->timeout;
#endif
+best->n_acks = 0;
*opcode = best->opcode;
dmsg(D_REL_DEBUG, "ACK reliable_send ID " packet_id_format " (size=%d
to=%d)",
(packet_id_print_type)best->packet_id, best->buf.len,
@@ -686,6 +699,7 @@ reliable_mark_active_incoming(struct reliable *rel, struct
buffer *buf,
e->opcode = opcode;
e->next_try = 0;
e->timeout = 0;
+e->n_acks = 0;
dmsg(D_REL_DEBUG, "ACK mark active incoming ID " packet_id_format,
(packet_id_print_type)e->packet_id);
return;
}
diff --git a/src/openvpn/reliable.h b/src/openvpn/reliable.h
index a84d4290..97e6dce7 100644
--- a/src/openvpn/reliable.h
+++ b/src/openvpn/reliable.h
@@ -52,6 +52,10 @@
* the reliability layer for one VPN
* tunnel in one direction can store. */
+#define N_ACK_RETRANSMIT 3 /**< We retry sending a packet early if
+ * this many later packets have been
+ * ACKed. */
+
/**
* The acknowledgment structure in which packet IDs are stored for later
* acknowledgment.
@@ -72,6 +76,9 @@ struct reliable_entry
interval_t timeout;
time_t next_try;
packet_id_type packet_id;
+size_t n_acks; /* Number of acks received for packets with higher PID.
+ * Used for fast retransmission when there were at least
+ * N_ACK_RETRANSMIT. */
int opcode;
struct buffer buf;
};
--
2.11.0
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 0/1] reliable: retransmit if 3 follow-up ACKs are received
This is my second attempt at sending this patch, this time without mixing up commit message and cover letter, and from an account that (I hope) doesn't hate mailing lists. This patch changes reliable_send() to resend a packet if at least three later packets have been ACKed. This improves performance when there are small amounts of packet loss. The patch was originally written by Steffan Karger for OpenVPN-NL. I added some comments as suggested by Arne Schwabe. Steffan Karger (1): reliable: retransmit if 3 follow-up ACKs are received src/openvpn/reliable.c | 20 +--- src/openvpn/reliable.h | 7 +++ 2 files changed, 24 insertions(+), 3 deletions(-) -- 2.11.0 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Wipe Socks5 credentials after use
Plaintext authentication is not exactly high security, but we might as
well memzero the credentials before leaving the function.
---
src/openvpn/socks.c | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/src/openvpn/socks.c b/src/openvpn/socks.c
index 36df7470..add7a6d4 100644
--- a/src/openvpn/socks.c
+++ b/src/openvpn/socks.c
@@ -104,12 +104,13 @@ socks_username_password_auth(struct socks_proxy_info *p,
const int timeout_sec = 5;
struct user_pass creds;
ssize_t size;
+bool ret = false;
creds.defined = 0;
if (!get_user_pass(, p->authfile, UP_TYPE_SOCKS,
GET_USER_PASS_MANAGEMENT))
{
msg(M_NONFATAL, "SOCKS failed to get username/password.");
-return false;
+goto cleanup;
}
if ( (strlen(creds.username) > 255) || (strlen(creds.password) > 255) )
@@ -117,7 +118,7 @@ socks_username_password_auth(struct socks_proxy_info *p,
msg(M_NONFATAL,
"SOCKS username and/or password exceeds 255 characters. "
"Authentication not possible.");
-return false;
+goto cleanup;
}
openvpn_snprintf(to_send, sizeof(to_send), "\x01%c%s%c%s", (int)
strlen(creds.username),
creds.username, (int) strlen(creds.password),
creds.password);
@@ -126,7 +127,7 @@ socks_username_password_auth(struct socks_proxy_info *p,
if (size != strlen(to_send))
{
msg(D_LINK_ERRORS | M_ERRNO, "socks_username_password_auth: TCP port
write failed on send()");
-return false;
+goto cleanup;
}
while (len < 2)
@@ -147,21 +148,21 @@ socks_username_password_auth(struct socks_proxy_info *p,
get_signal(signal_received);
if (*signal_received)
{
-return false;
+goto cleanup;
}
/* timeout? */
if (status == 0)
{
msg(D_LINK_ERRORS | M_ERRNO, "socks_username_password_auth: TCP
port read timeout expired");
-return false;
+goto cleanup;
}
/* error */
if (status < 0)
{
msg(D_LINK_ERRORS | M_ERRNO, "socks_username_password_auth: TCP
port read failed on select()");
-return false;
+goto cleanup;
}
/* read single char */
@@ -171,7 +172,7 @@ socks_username_password_auth(struct socks_proxy_info *p,
if (size != 1)
{
msg(D_LINK_ERRORS | M_ERRNO, "socks_username_password_auth: TCP
port read failed on recv()");
-return false;
+goto cleanup;
}
/* store char in buffer */
@@ -182,10 +183,14 @@ socks_username_password_auth(struct socks_proxy_info *p,
if (buf[0] != 5 && buf[1] != 0)
{
msg(D_LINK_ERRORS, "socks_username_password_auth: server refused the
authentication");
-return false;
+goto cleanup;
}
-return true;
+ret = true;
+cleanup:
+secure_memzero(, sizeof(creds));
+secure_memzero(to_send, sizeof(to_send));
+return ret;
}
static bool
--
2.29.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
