[Openvpn-devel] [PATCH] man: correct a --redirection-gateway option flag
Replace "servers" with "peers" in the description of the --redirection-gateway option flag local. --- doc/openvpn.8 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/openvpn.8 b/doc/openvpn.8 index 084c5415..e94ab760 100644 --- a/doc/openvpn.8 +++ b/doc/openvpn.8 @@ -1184,7 +1184,7 @@ Option flags: .B local \-\- Add the .B local -flag if both OpenVPN servers are directly connected via a common subnet, +flag if both OpenVPN peers are directly connected via a common subnet, such as with wireless. The .B local flag will cause step -- 2.19.0 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2 2/2] List ChaCha20-Poly1305 as stream cipher
As Antonio pointed out, "8-bit block cipher" is a bit funny. So teach
print_cipher() to print such cipher as "stream cipher".
Because I didn't want to write the same code twice, I decided to merge the
two print_cipher() implementations into one shared function. That should
make it easier to keep both backends consistent.
Signed-off-by: Steffan Karger
---
v2: introduce this patch
src/openvpn/crypto.c | 27 +++
src/openvpn/crypto.h | 3 +++
src/openvpn/crypto_mbedtls.c | 23 ++-
src/openvpn/crypto_mbedtls.h | 4
src/openvpn/crypto_openssl.c | 15 ---
src/openvpn/crypto_openssl.h | 4
6 files changed, 40 insertions(+), 36 deletions(-)
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 6d34acd7..e81399b7 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -1769,6 +1769,33 @@ get_random(void)
return l;
}
+void
+print_cipher(const cipher_kt_t *cipher)
+{
+const char *var_key_size = cipher_kt_var_key_size(cipher) ?
+" by default" : "";
+
+printf("%s (%d bit key%s, ",
+ translate_cipher_name_to_openvpn(cipher_kt_name(cipher)),
+ cipher_kt_key_size(cipher) * 8, var_key_size);
+
+if (cipher_kt_block_size(cipher) == 1)
+{
+printf("stream cipher");
+}
+else
+{
+printf("%d bit block", cipher_kt_block_size(cipher) * 8);
+}
+
+if (!cipher_kt_mode_cbc(cipher))
+{
+printf(", TLS client/server mode only");
+}
+
+printf(")\n");
+}
+
static const cipher_name_pair *
get_cipher_name_pair(const char *cipher_name)
{
diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index f4b3dca3..cf87cb49 100644
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -465,6 +465,9 @@ void prng_uninit(void);
/* an analogue to the random() function, but use prng_bytes */
long int get_random(void);
+/** Print a cipher list entry */
+void print_cipher(const cipher_kt_t *cipher);
+
void test_crypto(struct crypto_options *co, struct frame *f);
diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c
index 0c39eccc..54ac1893 100644
--- a/src/openvpn/crypto_mbedtls.c
+++ b/src/openvpn/crypto_mbedtls.c
@@ -140,26 +140,6 @@ const cipher_name_pair cipher_name_translation_table[] = {
const size_t cipher_name_translation_table_count =
sizeof(cipher_name_translation_table) /
sizeof(*cipher_name_translation_table);
-static void
-print_cipher(const cipher_kt_t *info)
-{
-if (info && (cipher_kt_mode_cbc(info)
-#ifdef HAVE_AEAD_CIPHER_MODES
- || cipher_kt_mode_aead(info)
-#endif
- ))
-{
-const char *ssl_only = cipher_kt_mode_cbc(info) ?
- "" : ", TLS client/server mode only";
-const char *var_key_size = info->flags &
MBEDTLS_CIPHER_VARIABLE_KEY_LEN ?
- " by default" : "";
-
-printf("%s (%d bit key%s, %d bit block%s)\n",
- cipher_kt_name(info), cipher_kt_key_size(info) * 8,
var_key_size,
- cipher_kt_block_size(info) * 8, ssl_only);
-}
-}
-
void
show_available_ciphers(void)
{
@@ -175,7 +155,8 @@ show_available_ciphers(void)
while (*ciphers != 0)
{
const cipher_kt_t *info = mbedtls_cipher_info_from_type(*ciphers);
-if (info && !cipher_kt_insecure(info))
+if (info && !cipher_kt_insecure(info)
+&& (cipher_kt_mode_aead(info) || cipher_kt_mode_cbc(info)))
{
print_cipher(info);
}
diff --git a/src/openvpn/crypto_mbedtls.h b/src/openvpn/crypto_mbedtls.h
index 452b06ed..81b542bc 100644
--- a/src/openvpn/crypto_mbedtls.h
+++ b/src/openvpn/crypto_mbedtls.h
@@ -146,5 +146,9 @@ mbed_log_func_line_lite(unsigned int flags, int errval,
#define mbed_ok(errval) \
mbed_log_func_line_lite(D_CRYPT_ERRORS, errval, __func__, __LINE__)
+static inline bool cipher_kt_var_key_size(const cipher_kt_t *cipher)
+{
+return cipher->flags & MBEDTLS_CIPHER_VARIABLE_KEY_LEN;
+}
#endif /* CRYPTO_MBEDTLS_H_ */
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 1c0fae86..7989127b 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -265,21 +265,6 @@ cipher_name_cmp(const void *a, const void *b)
return strcmp(cipher_name_a, cipher_name_b);
}
-static void
-print_cipher(const EVP_CIPHER *cipher)
-{
-const char *var_key_size =
-(EVP_CIPHER_flags(cipher) & EVP_CIPH_VARIABLE_LENGTH) ?
-" by default" : "";
-const char *ssl_only = cipher_kt_mode_cbc(cipher) ?
- "" : ", TLS client/server mode only";
-
-printf("%s (%d bit key%s, %d bit block%s)\n",
- translate_cipher_name_to_openvpn(EVP_CIPHER_name(cipher)),
- EVP_CIPHER_key_length(cipher) * 8, var_key_size,
- cipher_kt_block_size(cipher) * 8, ssl_only);
-}
-
void
show_available_ciphers(void)
{
[Openvpn-devel] [PATCH] Add support for OpenSSL TLS 1.3 when using management-external-key
For TLS 1.0 to 1.2 OpenSSL calls us and requires a PKCS1 padded
response, for TLS 1.3 it requires to an unpadded response. Since we
can PCKS1 pad an unpadded response, we prefer to always query for
an unpadded response from the management interface and add the PCKS1
padding ourselves when needed.
This patch adds an 'unpadded' parameter to the management-external-key
option to signal that it is uses the new unpadded API. Since we cannot
support TLS 1.3 without unpadded queries we disable TLS 1.3 otherwise.
We also do the same for cryptoapi since it uses the same API.
Using the management api client version instead might seem like the
more logical way but since we only now that version very late,
it would extra logic and complexity to deal with this asynchronous
behaviour .
---
doc/management-notes.txt | 7 -
src/openvpn/manage.h | 9 ---
src/openvpn/options.c | 54 ++-
src/openvpn/ssl_openssl.c | 26 ++-
4 files changed, 83 insertions(+), 13 deletions(-)
diff --git a/doc/management-notes.txt b/doc/management-notes.txt
index 17645c1d..7e61ff50 100644
--- a/doc/management-notes.txt
+++ b/doc/management-notes.txt
@@ -832,7 +832,12 @@ END
Base 64 encoded output of RSA_private_encrypt for RSA or ECDSA_sign()
for EC using OpenSSL or mbedtls_pk_sign() using mbed TLS will provide a
-correct signature.
+correct signature. With the 'nopadding' argument to the
+external-management-interface the interface expects unpadded signatures
+(RSA_NO_PADDING in OpenSSL). When the 'nopadding' keyword is missing the
+interfaces expects PKCS1 padded signatures for RSA keys (RSA_PKCS1_PADDING).
+EC signatures are always unpadded. To support TLS 1.3 using unpadded
+signatures is required.
This capability is intended to allow the use of arbitrary cryptographic
service providers with OpenVPN via the management interface.
diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h
index ff143fc1..03d46456 100644
--- a/src/openvpn/manage.h
+++ b/src/openvpn/manage.h
@@ -348,11 +348,12 @@ struct management *management_init(void);
#define MF_UNIX_SOCK (1<<8)
#ifdef MANAGMENT_EXTERNAL_KEY
#define MF_EXTERNAL_KEY(1<<9)
+#define MF_EXTERNAL_KEY_NOPADDING (1<<10)
#endif
-#define MF_UP_DOWN (1<<10)
-#define MF_QUERY_REMOTE (1<<11)
-#define MF_QUERY_PROXY (1<<12)
-#define MF_EXTERNAL_CERT(1<<13)
+#define MF_UP_DOWN (1<<11)
+#define MF_QUERY_REMOTE (1<<12)
+#define MF_QUERY_PROXY (1<<13)
+#define MF_EXTERNAL_CERT(1<<14)
bool management_open(struct management *man,
const char *addr,
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 891468bd..52ec6582 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3024,6 +3024,34 @@ options_postprocess_verify(const struct options *o)
options_postprocess_verify_ce(o, >ce);
}
}
+#if defined(ENABLE_CRYPTOAPI) || (defined(ENABLE_CRYPTO_OPENSSL) &&
defined(MANAGMENT_EXTERNAL_KEY))
+static void
+disable_tls13_if_avilable(struct options *o, const char* msg)
+{
+#if OPENSSL_VERSION_NUMBER >= 0x1010L
+const int tls_version_max =
+(o->ssl_flags >> SSLF_TLS_VERSION_MAX_SHIFT) &
+SSLF_TLS_VERSION_MAX_MASK;
+
+/*
+ * The library we are *linked* against is OpenSSL 1.1.1 and therefore
support TLS 1.3
+ * this need to be a runtime version check since we can be compiled
against 1.1.0 and
+ * then the library can be upgraded to 1.1.1
+ */
+if (OpenSSL_version_num() > 0x1010100fL &&
+(tls_version_max == TLS_VER_UNSPEC || tls_version_max > TLS_VER_1_2))
+{
+msg(M_WARN, "%s Setting maximum TLS version to 1.2 ", msg);
+o->ssl_flags &= ~(SSLF_TLS_VERSION_MAX_MASK <<
+
SSLF_TLS_VERSION_MAX_SHIFT);
+o->ssl_flags |= (TLS_VER_1_1 << SSLF_TLS_VERSION_MAX_SHIFT);
+
+}
+#else
+return;
+#endif
+}
+#endif
static void
options_postprocess_mutate(struct options *o)
@@ -3105,6 +3133,26 @@ options_postprocess_mutate(struct options *o)
}
#endif
+#if defined(ENABLE_CRYPTO_MBEDTLS) && defined(MANAGMENT_EXTERNAL_KEY)
+if (o->management_flags & MF_EXTERNAL_KEY_NOPADDING)
+{
+msg(M_FATAL, "mbed TLS does not support the 'nopadding' argument for
the --management-external-key option");
+}
+#endif
+
+#if defined(ENABLE_CRYPTOAPI)
+if (o->cryptoapi_cert)
+{
+disable_tls13_if_avilable(o, "Warning: cryptapicert used.");
+}
+#endif
+#if defined(ENABLE_CRYPTO_OPENSSL)
+if ((o->management_flags & MF_EXTERNAL_KEY) && !(o->management_flags &
MF_EXTERNAL_KEY_NOPADDING))
+{
+disable_tls13_if_avilable(o, "Warning: Using management-external-key "
+ "without nopadding option.");
+}
+#endif
#if P2MP
/*
* Save certain parms before modifying options via --pull
@@ -5178,9 +5226,13 @@
[Openvpn-devel] [PATCH v4] Add support for tls-ciphersuites for TLS 1.3
OpenSSL 1.1.1 introduces a separate list for TLS 1.3 ciphers. As these
interfaces are meant to be user facing or not exposed at all and we
expose the tls-cipher interface, we should also expose tls-cipherlist.
Combining both settings into tls-cipher would add a lot of glue logic
that needs to be maintained and is error prone. On top of that, users
should not set either settings unless absolutely required.
OpenSSL's own s_client/s_server also expose both settings and I believe
most other software will too:
-cipher val Specify TLSv1.2 and below cipher list to be used
-ciphersuites val Specify TLSv1.3 ciphersuites to be used
For mbed TLS only the future can tell if we will see a combined or also
two separate lists.
---
doc/openvpn.8 | 20 +++---
src/openvpn/options.c | 7 +
src/openvpn/options.h | 1 +
src/openvpn/ssl.c | 3 ++-
src/openvpn/ssl_backend.h | 13 -
src/openvpn/ssl_mbedtls.c | 13 +
src/openvpn/ssl_openssl.c | 56 +++
7 files changed, 108 insertions(+), 5 deletions(-)
diff --git a/doc/openvpn.8 b/doc/openvpn.8
index feec0b6e..226d9a98 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -5001,11 +5001,13 @@ determines the derivation of the tunnel session keys.
.\"*
.TP
.B \-\-tls\-cipher l
+.TQ
+.B \-\-tls\-ciphersuites l
A list
.B l
of allowable TLS ciphers delimited by a colon (":").
-This setting can be used to ensure that certain cipher suites are used (or
+These setting can be used to ensure that certain cipher suites are used (or
not used) for the TLS connection. OpenVPN uses TLS to secure the control
channel, over which the keys that are used to protect the actual VPN traffic
are exchanged.
@@ -5014,20 +5016,32 @@ The supplied list of ciphers is (after potential
OpenSSL/IANA name translation)
simply supplied to the crypto library. Please see the OpenSSL and/or mbed TLS
documentation for details on the cipher list interpretation.
+For OpenSSL, the
+.B \-\-tls-cipher
+is used for TLS 1.2 and below. For TLS 1.3 and up, the
+.B \-\-tls\-ciphersuites
+setting is used. mbed TLS has no TLS 1.3 support yet and only the
+.B \-\-tls-cipher
+setting is used.
+
Use
.B \-\-show\-tls
to see a list of TLS ciphers supported by your crypto library.
Warning!
.B \-\-tls\-cipher
-is an expert feature, which \- if used correcly \- can improve the security of
-your VPN connection. But it is also easy to unwittingly use it to carefully
+and
+.B \-\-tls\-ciphersuites
+are expert features, which \- if used correcly \- can improve the security of
+your VPN connection. But it is also easy to unwittingly use them to carefully
align a gun with your foot, or just break your connection. Use with care!
The default for \-\-tls\-cipher is to use mbed TLS's default cipher list
when using mbed TLS or
"DEFAULT:!EXP:!LOW:!MEDIUM:!kDH:!kECDH:!DSS:!PSK:!SRP:!kRSA" when using
OpenSSL.
+
+The default for \-\-tls\-ciphersuites is to use the crypto library's default.
.\"*
.TP
.B \-\-tls\-cert\-profile profile
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 95ebe3b3..891468bd 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1763,6 +1763,7 @@ show_settings(const struct options *o)
SHOW_STR(cryptoapi_cert);
#endif
SHOW_STR(cipher_list);
+SHOW_STR(cipher_list_tls13);
SHOW_STR(tls_cert_profile);
SHOW_STR(tls_verify);
SHOW_STR(tls_export_cert);
@@ -2757,6 +2758,7 @@ options_postprocess_verify_ce(const struct options
*options, const struct connec
MUST_BE_UNDEF(pkcs12_file);
#endif
MUST_BE_UNDEF(cipher_list);
+MUST_BE_UNDEF(cipher_list_tls13);
MUST_BE_UNDEF(tls_cert_profile);
MUST_BE_UNDEF(tls_verify);
MUST_BE_UNDEF(tls_export_cert);
@@ -7954,6 +7956,11 @@ add_option(struct options *options,
VERIFY_PERMISSION(OPT_P_GENERAL);
options->tls_cert_profile = p[1];
}
+else if (streq(p[0], "tls-ciphersuites") && p[1] && !p[2])
+{
+VERIFY_PERMISSION(OPT_P_GENERAL);
+options->cipher_list_tls13 = p[1];
+}
else if (streq(p[0], "crl-verify") && p[1] && ((p[2] && streq(p[2], "dir"))
|| (p[2] && streq(p[1],
INLINE_FILE_TAG) ) || !p[2]) && !p[3])
{
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 4c3bc4fb..3e7ef4f8 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -508,6 +508,7 @@ struct options
const char *priv_key_file;
const char *pkcs12_file;
const char *cipher_list;
+const char *cipher_list_tls13;
const char *tls_cert_profile;
const char *ecdh_curve;
const char *tls_verify;
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index fd12af05..adb0f3c8 100644
--- a/src/openvpn/ssl.c
+++
Re: [Openvpn-devel] [PATCH] Add support for CHACHA20-POLY1305 in the data channel
Hi, On 07-10-18 22:18, Antonio Quartulli wrote: > am I missing something? > 2.12.0 is the first release where my grep command prints something. No, you're absolutely right. I was misinterpreting my gitk. I'll tackle this in v2. -Steffan ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3] Add support for tls-ciphersuites for TLS 1.3
Hi,
Looks mostly good now. One final nit:
On 07-10-18 22:59, Arne Schwabe wrote:
> OpenSSL 1.1.1 introduces a separate list for TLS 1.3 ciphers. As these
> interfaces are meant to be user facing or not exposed at all and we
> expose the tls-cipher interface, we should also expose tls-cipherlist.
>
> Combining both settings into tls-cipher would add a lot of glue logic
> that needs to be maintained and is error prone. On top of that, users
> should not set either settings unless absolutely required.
>
> OpenSSL's own s_client/s_server also expose both settings and I believe
> most other software will too:
>
> -cipher val Specify TLSv1.2 and below cipher list to be used
> -ciphersuites val Specify TLSv1.3 ciphersuites to be used
>
> For mbed TLS only the future can tell if we will see a combined or also
> two separate lists.
> ---
> doc/openvpn.8 | 20 +++---
> src/openvpn/options.c | 7 +
> src/openvpn/options.h | 1 +
> src/openvpn/ssl.c | 3 ++-
> src/openvpn/ssl_backend.h | 13 -
> src/openvpn/ssl_mbedtls.c | 13 +
> src/openvpn/ssl_openssl.c | 56 +++
> 7 files changed, 108 insertions(+), 5 deletions(-)
>
> diff --git a/doc/openvpn.8 b/doc/openvpn.8
> index feec0b6e..226d9a98 100644
> --- a/doc/openvpn.8
> +++ b/doc/openvpn.8
> @@ -5001,11 +5001,13 @@ determines the derivation of the tunnel session keys.
> .\"*
> .TP
> .B \-\-tls\-cipher l
> +.TQ
> +.B \-\-tls\-ciphersuites l
> A list
> .B l
> of allowable TLS ciphers delimited by a colon (":").
>
> -This setting can be used to ensure that certain cipher suites are used (or
> +These setting can be used to ensure that certain cipher suites are used (or
> not used) for the TLS connection. OpenVPN uses TLS to secure the control
> channel, over which the keys that are used to protect the actual VPN traffic
> are exchanged.
> @@ -5014,20 +5016,32 @@ The supplied list of ciphers is (after potential
> OpenSSL/IANA name translation)
> simply supplied to the crypto library. Please see the OpenSSL and/or mbed
> TLS
> documentation for details on the cipher list interpretation.
>
> +For OpenSSL, the
> +.B \-\-tls-cipher
> +is used for TLS 1.2 and below. For TLS 1.3 and up, the
> +.B \-\-tls\-ciphersuites
> +setting is used. mbed TLS has no TLS 1.3 support yet and only the
> +.B \-\-tls-cipher
> +setting is used.
> +
> Use
> .B \-\-show\-tls
> to see a list of TLS ciphers supported by your crypto library.
>
> Warning!
> .B \-\-tls\-cipher
> -is an expert feature, which \- if used correcly \- can improve the security
> of
> -your VPN connection. But it is also easy to unwittingly use it to carefully
> +and
> +.B \-\-tls\-ciphersuites
> +are expert features, which \- if used correcly \- can improve the security of
> +your VPN connection. But it is also easy to unwittingly use them to
> carefully
> align a gun with your foot, or just break your connection. Use with care!
>
> The default for \-\-tls\-cipher is to use mbed TLS's default cipher list
> when using mbed TLS or
> "DEFAULT:!EXP:!LOW:!MEDIUM:!kDH:!kECDH:!DSS:!PSK:!SRP:!kRSA" when using
> OpenSSL.
> +
> +The default for \-\-tls\-ciphersuites is to use the crypto library's default.
> .\"*
> .TP
> .B \-\-tls\-cert\-profile profile
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 95ebe3b3..891468bd 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -1763,6 +1763,7 @@ show_settings(const struct options *o)
> SHOW_STR(cryptoapi_cert);
> #endif
> SHOW_STR(cipher_list);
> +SHOW_STR(cipher_list_tls13);
> SHOW_STR(tls_cert_profile);
> SHOW_STR(tls_verify);
> SHOW_STR(tls_export_cert);
> @@ -2757,6 +2758,7 @@ options_postprocess_verify_ce(const struct options
> *options, const struct connec
> MUST_BE_UNDEF(pkcs12_file);
> #endif
> MUST_BE_UNDEF(cipher_list);
> +MUST_BE_UNDEF(cipher_list_tls13);
> MUST_BE_UNDEF(tls_cert_profile);
> MUST_BE_UNDEF(tls_verify);
> MUST_BE_UNDEF(tls_export_cert);
> @@ -7954,6 +7956,11 @@ add_option(struct options *options,
> VERIFY_PERMISSION(OPT_P_GENERAL);
> options->tls_cert_profile = p[1];
> }
> +else if (streq(p[0], "tls-ciphersuites") && p[1] && !p[2])
> +{
> +VERIFY_PERMISSION(OPT_P_GENERAL);
> +options->cipher_list_tls13 = p[1];
> +}
> else if (streq(p[0], "crl-verify") && p[1] && ((p[2] && streq(p[2],
> "dir"))
> || (p[2] && streq(p[1],
> INLINE_FILE_TAG) ) || !p[2]) && !p[3])
> {
> diff --git a/src/openvpn/options.h b/src/openvpn/options.h
> index 4c3bc4fb..3e7ef4f8 100644
> --- a/src/openvpn/options.h
> +++ b/src/openvpn/options.h
> @@ -508,6 +508,7 @@ struct options
> const
[Openvpn-devel] [PATCH v3] Add support for tls-ciphersuites for TLS 1.3
OpenSSL 1.1.1 introduces a separate list for TLS 1.3 ciphers. As these
interfaces are meant to be user facing or not exposed at all and we
expose the tls-cipher interface, we should also expose tls-cipherlist.
Combining both settings into tls-cipher would add a lot of glue logic
that needs to be maintained and is error prone. On top of that, users
should not set either settings unless absolutely required.
OpenSSL's own s_client/s_server also expose both settings and I believe
most other software will too:
-cipher val Specify TLSv1.2 and below cipher list to be used
-ciphersuites val Specify TLSv1.3 ciphersuites to be used
For mbed TLS only the future can tell if we will see a combined or also
two separate lists.
---
doc/openvpn.8 | 20 +++---
src/openvpn/options.c | 7 +
src/openvpn/options.h | 1 +
src/openvpn/ssl.c | 3 ++-
src/openvpn/ssl_backend.h | 13 -
src/openvpn/ssl_mbedtls.c | 13 +
src/openvpn/ssl_openssl.c | 56 +++
7 files changed, 108 insertions(+), 5 deletions(-)
diff --git a/doc/openvpn.8 b/doc/openvpn.8
index feec0b6e..226d9a98 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -5001,11 +5001,13 @@ determines the derivation of the tunnel session keys.
.\"*
.TP
.B \-\-tls\-cipher l
+.TQ
+.B \-\-tls\-ciphersuites l
A list
.B l
of allowable TLS ciphers delimited by a colon (":").
-This setting can be used to ensure that certain cipher suites are used (or
+These setting can be used to ensure that certain cipher suites are used (or
not used) for the TLS connection. OpenVPN uses TLS to secure the control
channel, over which the keys that are used to protect the actual VPN traffic
are exchanged.
@@ -5014,20 +5016,32 @@ The supplied list of ciphers is (after potential
OpenSSL/IANA name translation)
simply supplied to the crypto library. Please see the OpenSSL and/or mbed TLS
documentation for details on the cipher list interpretation.
+For OpenSSL, the
+.B \-\-tls-cipher
+is used for TLS 1.2 and below. For TLS 1.3 and up, the
+.B \-\-tls\-ciphersuites
+setting is used. mbed TLS has no TLS 1.3 support yet and only the
+.B \-\-tls-cipher
+setting is used.
+
Use
.B \-\-show\-tls
to see a list of TLS ciphers supported by your crypto library.
Warning!
.B \-\-tls\-cipher
-is an expert feature, which \- if used correcly \- can improve the security of
-your VPN connection. But it is also easy to unwittingly use it to carefully
+and
+.B \-\-tls\-ciphersuites
+are expert features, which \- if used correcly \- can improve the security of
+your VPN connection. But it is also easy to unwittingly use them to carefully
align a gun with your foot, or just break your connection. Use with care!
The default for \-\-tls\-cipher is to use mbed TLS's default cipher list
when using mbed TLS or
"DEFAULT:!EXP:!LOW:!MEDIUM:!kDH:!kECDH:!DSS:!PSK:!SRP:!kRSA" when using
OpenSSL.
+
+The default for \-\-tls\-ciphersuites is to use the crypto library's default.
.\"*
.TP
.B \-\-tls\-cert\-profile profile
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 95ebe3b3..891468bd 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1763,6 +1763,7 @@ show_settings(const struct options *o)
SHOW_STR(cryptoapi_cert);
#endif
SHOW_STR(cipher_list);
+SHOW_STR(cipher_list_tls13);
SHOW_STR(tls_cert_profile);
SHOW_STR(tls_verify);
SHOW_STR(tls_export_cert);
@@ -2757,6 +2758,7 @@ options_postprocess_verify_ce(const struct options
*options, const struct connec
MUST_BE_UNDEF(pkcs12_file);
#endif
MUST_BE_UNDEF(cipher_list);
+MUST_BE_UNDEF(cipher_list_tls13);
MUST_BE_UNDEF(tls_cert_profile);
MUST_BE_UNDEF(tls_verify);
MUST_BE_UNDEF(tls_export_cert);
@@ -7954,6 +7956,11 @@ add_option(struct options *options,
VERIFY_PERMISSION(OPT_P_GENERAL);
options->tls_cert_profile = p[1];
}
+else if (streq(p[0], "tls-ciphersuites") && p[1] && !p[2])
+{
+VERIFY_PERMISSION(OPT_P_GENERAL);
+options->cipher_list_tls13 = p[1];
+}
else if (streq(p[0], "crl-verify") && p[1] && ((p[2] && streq(p[2], "dir"))
|| (p[2] && streq(p[1],
INLINE_FILE_TAG) ) || !p[2]) && !p[3])
{
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 4c3bc4fb..3e7ef4f8 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -508,6 +508,7 @@ struct options
const char *priv_key_file;
const char *pkcs12_file;
const char *cipher_list;
+const char *cipher_list_tls13;
const char *tls_cert_profile;
const char *ecdh_curve;
const char *tls_verify;
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index e5e4aac2..616c2696 100644
--- a/src/openvpn/ssl.c
+++
Re: [Openvpn-devel] [PATCH] Add support for CHACHA20-POLY1305 in the data channel
Hi, On 08/10/18 02:13, Steffan Karger wrote: > On 07-10-18 15:36, Antonio Quartulli wrote: >> On 07/10/18 21:28, Antonio Quartulli wrote: +#if defined(MBEDTLS_CHACHAPOLY_C) && (MBEDTLS_VERSION_NUMBER >= 0x020C) >>> >>> Why do we need the dual condition? Isn't MBEDTLS_CHACHAPOLY_C enough to >>> know mbedTLS has what we need? Or you feel like we have to demand a >>> specific mbedTLS version when using ChaCha20? >> >> Just to reinforce my argument: ChaCha20-Poly1305 was introduced with >> ce66d5e8e (in mbedTLS upstream) and a quick search shows: >> >> mbedtls $ git tag --contains ce66d5e8e >> mbedtls-2.12.0 >> mbedtls-2.13.0 >> mbedtls-2.13.1 >> >> Given that 2.12 is 0x020C, I am fairly sure there is no reason to >> have both conditions in the if guard. >> But I might still be missing something. >> >> (p.s. I am arguing simply because I want to avoid more version-based >> ifdef that may become new nightmares) > > Which is a good cause, but that unfortunately won't work here. That is > because the actual implementation was introduced in dca3a5d8, [CUT] mbedtls $ git tag --contains dca3a5d8 mbedtls-2.12.0 mbedtls-2.13.0 mbedtls-2.13.1 To be safe I checked out 2.9.0 and grepped the define: mbedtls $ git checkout mbedtls-2.9.0 Note: checking out 'mbedtls-2.9.0'. You are in 'detached HEAD' state. You can look around, make experimental changes and commit them, and you can discard any commits you make in this state without impacting any branches by performing another checkout. If you want to create a new branch to retain commits you create, you may do so (now or later) by using -b with the checkout command again. Example: git checkout -b HEAD is now at 070e35647 Merge remote-tracking branch 'upstream-restricted/pr/481' into development-restricted mbedtls $ grep -rn CHACHAPOLY_C mbedtls $ am I missing something? 2.12.0 is the first release where my grep command prints something. Cheers, -- Antonio Quartulli signature.asc Description: OpenPGP digital signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Don't print OCC warnings about 'key-method', 'keydir' and 'tls-auth'
Acked-by: Gert Doering I have not actually bothered to find a constellation that *would* trigger an OCC warning (how?) but it compiles and passes tests, and the code looks reasonable. With added msg() calls I verified that it actually ignores the options this time. I have taken the liberty to remove the space after "tun-ipv6" to make it actually match (no arguments, so no space) and learned something new about the treasures in our sources (strprefix())... :-) Your patch has been applied to the master and release/2.4 branch (long-term compatibility and not very intrusive). commit 3baae9ba52187166b7d0b05901732666477a2acb (master) commit 266178b7b6280d5f32403ba8780ddbb517e3ac6a (release/2.4) Author: Steffan Karger Date: Sun Oct 7 19:52:15 2018 +0200 Don't print OCC warnings about 'key-method', 'keydir' and 'tls-auth' Signed-off-by: Steffan Karger Acked-by: Gert Doering Message-Id: <20181007175215.25009-1-stef...@karger.me> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17618.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Implement "status 4" (JSON) for management interface
On 10/7/18 9:10 AM, Gert Doering wrote: > Hi, Hi Gert, > - most voices argued for "do not require an external library" >(for *creation*). Not sure if we managed to convince David in >the end, though :-) In the meantime I had some "great" experience with manually generating JSON in another project. That was a terrible mistake ;-) So I would not recommend following this path any longer. Maybe not even JSON, but something typed and/or even binary. > ... but then it just died off, and the patch is sitting in patchwork > (where I moved it to "changes requested" just now). > > Is there interest in moving forward, or shall we just let it go? I'm not really interested in it anymore, and I brought it up ;-) So if no one else is interested, maybe better let it go... Cheers, François signature.asc Description: OpenPGP digital signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] Don't print OCC warnings about 'key-method', 'keydir' and 'tls-auth'
From: Steffan Karger
Like 'proto', a mismatch in key-method, keydir or tls-auth would fail
before we ever get to the point where we can print this warning.
This prepares for removing these from the occ string later on, but also
prepares for tls-crypt-v2, which allows a server to support tls-auth and
tls-crypt-v2 connections in parallel. Such a server will send 'keydir'
and 'tls-auth' in the occ string. This change removes the spurious
warnings about that in the client log.
Signed-off-by: Steffan Karger
---
v2: use strprefix instead of strcmp, and add tun-ipv6.
src/openvpn/options.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 2199af53..45c5ea64 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3788,11 +3788,15 @@ options_warning_safe_scan2(const int msglevel,
const char *b1_name,
const char *b2_name)
{
-/* we will stop sending 'proto xxx' in OCC in a future version
- * (because it's not useful), and to reduce questions when
- * interoperating, we start not-printing a warning about it today
+/* We will stop sending 'key-method', 'keydir', 'proto' and 'tls-auth' in
+ * OCC in a future version (because it's not useful). To reduce questions
+ * when interoperating, we no longer printing a warning about it.
*/
-if (strncmp(p1, "proto ", 6) == 0)
+if (strprefix(p1, "key-method ")
+|| strprefix(p1, "keydir ")
+|| strprefix(p1, "proto ")
+|| strprefix(p1, "tls-auth ")
+|| strprefix(p1, "tun-ipv6 "))
{
return;
}
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v8] convert *_inline attributes to bool
Carrying around the INLINE_TAG is not really efficient,
because it requires a strcmp() to be performed every
time we want to understand if the data is stored inline
or not.
Convert all the *_inline attributes to bool to make the
logic easier and checks more efficient.
Signed-off-by: Antonio Quartulli
---
Changes from v7:
- rebased on top of latest master (68e0b9db)
Changes from v6:
- rebased on top of latest master
Changes from v5:
- fix function invocation alignment in
options.c:options_postprocess_filechecks()
- fix typ0 in function invocation in options.c:options_postprocess_filechecks()
- fix doxygen comment for function tls_ctx_reload_crl() in ssl.c
Changes from v4:
- remove newline accidentally added in v4
Changes from v3:
- some code style adjustment in options.c:check_inline_file()
- move print_if_inline() from misc.c to crypto.c and rename it to
print_key_filename()
- make comment of check_file_access_inline() and
check_file_access_chroot_inline() doxygen compliant
- remove *is_inline argument in check_inline_file() and use its
return value instead
- move declarations of is_inline to narrower scope in options.c
- move return type of plugin_option_list_add() to its own line
Changes from v2:
- fix indentation in ssl_openssl.c
- do not attempt to push inline'd options
- do not attempt to parse inline'd plugin
- introduce check_file_access_inline() and check_file_access_chroot_inline()
- introduce OPT_P_INLINE to specify when an option is allowed to
be inline. Options not having this permission will fail to be
parsed if is_inline is true
Changes from v1:
- remove the INLINE_TAG from the options parsing logic at all. Now a
boolean variable is passed around
- add print_if_inline() helper function (to misc.c/h) to make sure we
never print the inline data, but only the INLINE tag. Such function
checks also for NULL pointers
- make sure print_if_inline() is always used when printing possibly
inline data
- remove the INLINE_TAG from the options parsing logic at all. Now a
boolean variable is passed around
- fix alignment error in comment
- remove CHKACC_INLINE from check_file_access() logic: this function
is now not invoked at all in case of inline data
src/openvpn/crypto.c | 39 +++--
src/openvpn/crypto.h | 17 ++-
src/openvpn/misc.c| 6 +-
src/openvpn/misc.h| 3 +-
src/openvpn/options.c | 300 +-
src/openvpn/options.h | 25 ++--
src/openvpn/plugin.c | 5 +-
src/openvpn/plugin.h | 3 +-
src/openvpn/push.c| 2 +-
src/openvpn/push.h| 3 +-
src/openvpn/ssl.c | 4 +-
src/openvpn/ssl_backend.h | 55 +++
src/openvpn/ssl_common.h | 2 +-
src/openvpn/ssl_mbedtls.c | 61
src/openvpn/ssl_openssl.c | 85 ++-
src/openvpn/tls_crypt.c | 2 +-
src/openvpn/tls_crypt.h | 9 +-
17 files changed, 344 insertions(+), 277 deletions(-)
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index c8d95f3b..dc5b8f1b 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -1174,25 +1174,25 @@ test_crypto(struct crypto_options *co, struct frame
*frame)
void
crypto_read_openvpn_key(const struct key_type *key_type,
-struct key_ctx_bi *ctx, const char *key_file, const
char *key_inline,
-const int key_direction, const char *key_name, const
char *opt_name)
+struct key_ctx_bi *ctx, const char *key_file,
+bool key_inline, const int key_direction,
+const char *key_name, const char *opt_name)
{
struct key2 key2;
struct key_direction_state kds;
+unsigned int flags = RKF_MUST_SUCCEED;
if (key_inline)
{
-read_key_file(, key_inline, RKF_MUST_SUCCEED|RKF_INLINE);
-}
-else
-{
-read_key_file(, key_file, RKF_MUST_SUCCEED);
+flags |= RKF_INLINE;
}
+read_key_file(, key_file, flags);
if (key2.n != 2)
{
msg(M_ERR, "File '%s' does not have OpenVPN Static Key format. Using "
-"free-form passphrase file is not supported anymore.", key_file);
+"free-form passphrase file is not supported anymore.",
+print_key_filename(key_file, key_inline));
}
/* check for and fix highly unlikely key problems */
@@ -1226,7 +1226,6 @@ read_key_file(struct key2 *key2, const char *file, const
unsigned int flags)
struct buffer in;
int size;
uint8_t hex_byte[3] = {0, 0, 0};
-const char *error_filename = file;
/* parse info */
const unsigned char *cp;
@@ -1264,7 +1263,6 @@ read_key_file(struct key2 *key2, const char *file, const
unsigned int flags)
{
size = strlen(file) + 1;
buf_set_read(, (const uint8_t *)file, size);
-error_filename = INLINE_FILE_TAG;
}
else /* 'file' is a filename which refers to a file containing the ascii
key */
Re: [Openvpn-devel] [PATCH] Add support for CHACHA20-POLY1305 in the data channel
Hi,
On 07/10/18 21:28, Antonio Quartulli wrote:
> Hi,
>
> On 07/10/18 15:34, Steffan Karger wrote:
>> We explicitly only supported GCM as a valid AEAD mode, change that to also
>> allow ChaCha20-Poly1305 as an AEAD cipher. That works nicely with our new
>> (GCM) data channel format, because is has the same 96-bit IV.
> ^^ it
>
>>
>> Note that we need some tricks to not treat the cipher as insecure, because
>> we used to only look at the block size of a cipher to determine if find a
>> cipher insecure. But ChaCha20-Poly1305 is a stream cipher, which essentially
>> has a 'block size' of 1 byte and is reported as such. So, special-case this
>> cipher to be in the list of secure ciphers.
>
> I am so excited we are finally including ChaCha20-Poly1305 support! :)
> Thanks!
>
>>
>> Signed-off-by: Steffan Karger
>> ---
>> Changes.rst | 10 ++
>> src/openvpn/crypto.c | 2 +-
>> src/openvpn/crypto_backend.h | 5 +
>> src/openvpn/crypto_mbedtls.c | 21 ++---
>> src/openvpn/crypto_openssl.c | 33 -
>> src/openvpn/ssl.c| 2 +-
>> 6 files changed, 63 insertions(+), 10 deletions(-)
>>
>> diff --git a/Changes.rst b/Changes.rst
>> index a6090cf5..70ce2e10 100644
>> --- a/Changes.rst
>> +++ b/Changes.rst
>> @@ -1,3 +1,13 @@
>> +Overview of changes in 2.5
>> +==
>> +
>> +New features
>> +
>> +ChaCha20-Poly1305 cipher support
>> +Added support for using the ChaCha20-Poly1305 cipher in the OpenVPN data
>> +channel.
>> +
>> +
>> Overview of changes in 2.4
>> ==
>>
>> diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
>> index c8d95f3b..6d34acd7 100644
>> --- a/src/openvpn/crypto.c
>> +++ b/src/openvpn/crypto.c
>> @@ -841,7 +841,7 @@ init_key_ctx(struct key_ctx *ctx, const struct key *key,
>> dmsg(D_CRYPTO_DEBUG, "%s: CIPHER block_size=%d iv_size=%d",
>> prefix, cipher_kt_block_size(kt->cipher),
>> cipher_kt_iv_size(kt->cipher));
>> -if (cipher_kt_block_size(kt->cipher) < 128/8)
>> +if (cipher_kt_insecure(kt->cipher))
>> {
>> msg(M_WARN, "WARNING: INSECURE cipher with block size less than
>> 128"
>> " bit (%d bit). This allows attacks like SWEET32.
>> Mitigate by "
>> diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h
>> index f917c8d7..38b2c175 100644
>> --- a/src/openvpn/crypto_backend.h
>> +++ b/src/openvpn/crypto_backend.h
>> @@ -284,6 +284,11 @@ int cipher_kt_block_size(const cipher_kt_t *cipher_kt);
>> */
>> int cipher_kt_tag_size(const cipher_kt_t *cipher_kt);
>>
>> +/**
>> + * Returns true if we consider this cipher to be insecure.
>> + */
>> +bool cipher_kt_insecure(const cipher_kt_t *cipher);
>> +
>> /**
>> * Returns the mode that the cipher runs in.
>> *
>> diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c
>> index 82f4e574..a5a096b1 100644
>> --- a/src/openvpn/crypto_mbedtls.c
>> +++ b/src/openvpn/crypto_mbedtls.c
>> @@ -51,6 +51,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>>
>> @@ -175,7 +176,7 @@ show_available_ciphers(void)
>> while (*ciphers != 0)
>> {
>> const cipher_kt_t *info = mbedtls_cipher_info_from_type(*ciphers);
>> -if (info && cipher_kt_block_size(info) >= 128/8)
>> +if (info && !cipher_kt_insecure(info))
>> {
>> print_cipher(info);
>> }
>> @@ -188,7 +189,7 @@ show_available_ciphers(void)
>> while (*ciphers != 0)
>> {
>> const cipher_kt_t *info = mbedtls_cipher_info_from_type(*ciphers);
>> -if (info && cipher_kt_block_size(info) < 128/8)
>> +if (info && cipher_kt_insecure(info))
>> {
>> print_cipher(info);
>> }
>> @@ -550,6 +551,16 @@ cipher_kt_tag_size(const mbedtls_cipher_info_t
>> *cipher_kt)
>> return 0;
>> }
>>
>> +bool
>> +cipher_kt_insecure(const mbedtls_cipher_info_t *cipher_kt)
>> +{
>> +return !(cipher_kt_block_size(cipher_kt) >= 128/8
>
> Since you are touching the line above (you moved it), please put spaces
> around '/'.
>
>> +#if defined(MBEDTLS_CHACHAPOLY_C) && (MBEDTLS_VERSION_NUMBER >= 0x020C)
>
> Why do we need the dual condition? Isn't MBEDTLS_CHACHAPOLY_C enough to
> know mbedTLS has what we need? Or you feel like we have to demand a
> specific mbedTLS version when using ChaCha20?
>
>> + || cipher_kt->type == MBEDTLS_CIPHER_CHACHA20_POLY1305
>> +#endif
>> + );
>> +}
>> +
>> int
>> cipher_kt_mode(const mbedtls_cipher_info_t *cipher_kt)
>> {
>> @@ -573,7 +584,11 @@ cipher_kt_mode_ofb_cfb(const cipher_kt_t *cipher)
>> bool
>> cipher_kt_mode_aead(const cipher_kt_t *cipher)
>> {
>> -return cipher && cipher_kt_mode(cipher) == OPENVPN_MODE_GCM;
>> +return cipher && (cipher_kt_mode(cipher) ==
Re: [Openvpn-devel] [PATCH] Add support for CHACHA20-POLY1305 in the data channel
Hi, On 07/10/18 21:28, Antonio Quartulli wrote: >> +#if defined(MBEDTLS_CHACHAPOLY_C) && (MBEDTLS_VERSION_NUMBER >= 0x020C) > > Why do we need the dual condition? Isn't MBEDTLS_CHACHAPOLY_C enough to > know mbedTLS has what we need? Or you feel like we have to demand a > specific mbedTLS version when using ChaCha20? Just to reinforce my argument: ChaCha20-Poly1305 was introduced with ce66d5e8e (in mbedTLS upstream) and a quick search shows: mbedtls $ git tag --contains ce66d5e8e mbedtls-2.12.0 mbedtls-2.13.0 mbedtls-2.13.1 Given that 2.12 is 0x020C, I am fairly sure there is no reason to have both conditions in the if guard. But I might still be missing something. (p.s. I am arguing simply because I want to avoid more version-based ifdef that may become new nightmares) Cheers, -- Antonio Quartulli signature.asc Description: OpenPGP digital signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Add support for CHACHA20-POLY1305 in the data channel
Hi,
On 07/10/18 15:34, Steffan Karger wrote:
> We explicitly only supported GCM as a valid AEAD mode, change that to also
> allow ChaCha20-Poly1305 as an AEAD cipher. That works nicely with our new
> (GCM) data channel format, because is has the same 96-bit IV.
^^ it
>
> Note that we need some tricks to not treat the cipher as insecure, because
> we used to only look at the block size of a cipher to determine if find a
> cipher insecure. But ChaCha20-Poly1305 is a stream cipher, which essentially
> has a 'block size' of 1 byte and is reported as such. So, special-case this
> cipher to be in the list of secure ciphers.
I am so excited we are finally including ChaCha20-Poly1305 support! :)
Thanks!
>
> Signed-off-by: Steffan Karger
> ---
> Changes.rst | 10 ++
> src/openvpn/crypto.c | 2 +-
> src/openvpn/crypto_backend.h | 5 +
> src/openvpn/crypto_mbedtls.c | 21 ++---
> src/openvpn/crypto_openssl.c | 33 -
> src/openvpn/ssl.c| 2 +-
> 6 files changed, 63 insertions(+), 10 deletions(-)
>
> diff --git a/Changes.rst b/Changes.rst
> index a6090cf5..70ce2e10 100644
> --- a/Changes.rst
> +++ b/Changes.rst
> @@ -1,3 +1,13 @@
> +Overview of changes in 2.5
> +==
> +
> +New features
> +
> +ChaCha20-Poly1305 cipher support
> +Added support for using the ChaCha20-Poly1305 cipher in the OpenVPN data
> +channel.
> +
> +
> Overview of changes in 2.4
> ==
>
> diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
> index c8d95f3b..6d34acd7 100644
> --- a/src/openvpn/crypto.c
> +++ b/src/openvpn/crypto.c
> @@ -841,7 +841,7 @@ init_key_ctx(struct key_ctx *ctx, const struct key *key,
> dmsg(D_CRYPTO_DEBUG, "%s: CIPHER block_size=%d iv_size=%d",
> prefix, cipher_kt_block_size(kt->cipher),
> cipher_kt_iv_size(kt->cipher));
> -if (cipher_kt_block_size(kt->cipher) < 128/8)
> +if (cipher_kt_insecure(kt->cipher))
> {
> msg(M_WARN, "WARNING: INSECURE cipher with block size less than
> 128"
> " bit (%d bit). This allows attacks like SWEET32. Mitigate
> by "
> diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h
> index f917c8d7..38b2c175 100644
> --- a/src/openvpn/crypto_backend.h
> +++ b/src/openvpn/crypto_backend.h
> @@ -284,6 +284,11 @@ int cipher_kt_block_size(const cipher_kt_t *cipher_kt);
> */
> int cipher_kt_tag_size(const cipher_kt_t *cipher_kt);
>
> +/**
> + * Returns true if we consider this cipher to be insecure.
> + */
> +bool cipher_kt_insecure(const cipher_kt_t *cipher);
> +
> /**
> * Returns the mode that the cipher runs in.
> *
> diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c
> index 82f4e574..a5a096b1 100644
> --- a/src/openvpn/crypto_mbedtls.c
> +++ b/src/openvpn/crypto_mbedtls.c
> @@ -51,6 +51,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -175,7 +176,7 @@ show_available_ciphers(void)
> while (*ciphers != 0)
> {
> const cipher_kt_t *info = mbedtls_cipher_info_from_type(*ciphers);
> -if (info && cipher_kt_block_size(info) >= 128/8)
> +if (info && !cipher_kt_insecure(info))
> {
> print_cipher(info);
> }
> @@ -188,7 +189,7 @@ show_available_ciphers(void)
> while (*ciphers != 0)
> {
> const cipher_kt_t *info = mbedtls_cipher_info_from_type(*ciphers);
> -if (info && cipher_kt_block_size(info) < 128/8)
> +if (info && cipher_kt_insecure(info))
> {
> print_cipher(info);
> }
> @@ -550,6 +551,16 @@ cipher_kt_tag_size(const mbedtls_cipher_info_t
> *cipher_kt)
> return 0;
> }
>
> +bool
> +cipher_kt_insecure(const mbedtls_cipher_info_t *cipher_kt)
> +{
> +return !(cipher_kt_block_size(cipher_kt) >= 128/8
Since you are touching the line above (you moved it), please put spaces
around '/'.
> +#if defined(MBEDTLS_CHACHAPOLY_C) && (MBEDTLS_VERSION_NUMBER >= 0x020C)
Why do we need the dual condition? Isn't MBEDTLS_CHACHAPOLY_C enough to
know mbedTLS has what we need? Or you feel like we have to demand a
specific mbedTLS version when using ChaCha20?
> + || cipher_kt->type == MBEDTLS_CIPHER_CHACHA20_POLY1305
> +#endif
> + );
> +}
> +
> int
> cipher_kt_mode(const mbedtls_cipher_info_t *cipher_kt)
> {
> @@ -573,7 +584,11 @@ cipher_kt_mode_ofb_cfb(const cipher_kt_t *cipher)
> bool
> cipher_kt_mode_aead(const cipher_kt_t *cipher)
> {
> -return cipher && cipher_kt_mode(cipher) == OPENVPN_MODE_GCM;
> +return cipher && (cipher_kt_mode(cipher) == OPENVPN_MODE_GCM
> +#if defined(MBEDTLS_CHACHAPOLY_C) && (MBEDTLS_VERSION_NUMBER >= 0x020C)
ditto
> + || cipher_kt_mode(cipher) == MBEDTLS_MODE_CHACHAPOLY
> +#endif
> +
Re: [Openvpn-devel] [PATCH] Don't print OCC warnings about 'key-method', 'keydir' and 'tls-auth'
Hi,
On Wed, Aug 08, 2018 at 11:58:47AM +0200, Steffan Karger wrote:
> Like 'proto', a mismatch in key-method, keydir or tls-auth would fail
> before we ever get to the point where we can print this warning.
>
> This prepares for removing these from the occ string later on, but also
> prepares for tls-crypt-v2, which allows a server to support tls-auth and
> tls-crypt-v2 connections in parallel. Such a server will send 'keydir'
> and 'tls-auth' in the occ string. This change removes the spurious
> warnings about that in the client log.
>
> Signed-off-by: Steffan Karger
ACK on the feature, NAK on the implementation...
> @@ -3790,11 +3790,14 @@ options_warning_safe_scan2(const int msglevel,
> const char *b1_name,
> const char *b2_name)
> {
> -/* we will stop sending 'proto xxx' in OCC in a future version
> - * (because it's not useful), and to reduce questions when
> - * interoperating, we start not-printing a warning about it today
> +/* We will stop sending 'key-method', 'keydir', 'proto' and 'tls-auth' in
> + * OCC in a future version (because it's not useful). To reduce
> questions
> + * when interoperating, we no longer printing a warning about it.
> */
> -if (strncmp(p1, "proto ", 6) == 0)
> +if (strcmp(p1, "key-method ") == 0
> +|| strcmp(p1, "keydir ") == 0
> +|| strcmp(p1, "proto ") == 0
> +|| strcmp(p1, "tls-auth ") == 0)
> {
... because this will actually not match if p1 is "proto TCPv4_SERVER" or
"key-method 2" (string in p1 being longer than "proto ", it will only
match with strncmp() ).
I wasn't sure why the original code had a strncmp() here, so I instrumented
a build to print "p1"...
Sun Oct 7 14:57:21 2018 OCC: 'proto TCPv4_SERVER'
Sun Oct 7 14:57:21 2018 OCC: 'key-method 2'
Sun Oct 7 14:57:21 2018 OCC: 'cipher BF-CBC'
Sun Oct 7 14:57:21 2018 OCC: 'auth SHA1'
Sun Oct 7 14:57:21 2018 OCC: 'keysize 128'
... and no match found...
While at it... could you ignore tun-ipv6 in v2, please? :-)
Sun Oct 7 14:57:21 2018 WARNING: 'tun-ipv6' is present in remote config but
missing in local config, remote='tun-ipv6'
thanks,
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany g...@greenie.muc.de
signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Simplify --genkey option syntax
Looks reasonable :-) - I've not tested it, just stared at code and ran a test-compile. Your patch has been applied to the master branch. commit d818bfedfc7a433a3a5dbd6ce8e9b957802a21b2 Author: Steffan Karger Date: Fri Oct 5 17:00:32 2018 +0200 Simplify --genkey option syntax Signed-off-by: Steffan Karger Acked-by: Antonio Quartulli Message-Id: <20181005150032.16541-1-stef...@karger.me> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17574.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Simplify --genkey option syntax
Hi, On 05/10/18 23:00, Steffan Karger wrote: > Instead of requiring users to do "--genkey --secret new.key", allow > them to just do "--genkey new.key". This has hit me often enough that I > decided to write a patch for it. Also, the upcoming tls-crypt-v2-genkey > uses a similar syntax and Antonio suggested we should make them consistent. > > The documentation is updated to no longer mention the old syntax, but it is > still supported so people who are used to the old syntax can still use it. > > Signed-off-by: Steffan Karger I totally agree that having the "--genkey file.key" syntax makes the command much more intuitive (I also hit this every time). The patch looks good and it does what it says. People used to the old format will still be happy as it is still supported. (Maybe at some point we can get rid of it) Acked-by: Antonio Quartulli Tested-by: Antonio Quartulli -- Antonio Quartulli signature.asc Description: OpenPGP digital signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: openvpnserv: clarify return values type
Your patch has been applied to the master branch. (Test built on ubuntu1604, and a quick stare-at-code, but if Selva says it's good, that is good enough :-) ) commit ab9193c32b19e42a1847bd4f6cf967ea0e3293c8 Author: Lev Stipakov Date: Wed Oct 3 20:21:21 2018 +0300 openvpnserv: clarify return values type Signed-off-by: Lev Stipakov Acked-by: Selva Nair Message-Id: <1538587281-3209-1-git-send-email-lstipa...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17532.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Fix use-after-free in tls_ctx_use_management_external_key
Your patch has been applied to the master branch. commit 68e0b9db253ff0437047d6a5377eeec6002873f8 Author: Steffan Karger Date: Sun Oct 7 12:00:32 2018 +0200 Fix use-after-free in tls_ctx_use_management_external_key Signed-off-by: Steffan Karger Acked-by: Arne Schwabe Message-Id: <20181007100032.17060-1-stef...@karger.me> URL: https://www.mail-archive.com/search?l=mid=20181007100032.17060-1-stef...@karger.me Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Fix use-after-free in tls_ctx_use_management_external_key
Am 07.10.18 um 13:00 schrieb Steffan Karger: > Commit 98bfeeb4 changed our openssl backend implementation of > tls_ctx_use_management_external_key() to no longer use > tls_ctx_load_cert_file_and_copy(), but still free'd 'cert'. Which it no > longer should do. Credits go to Arne for spotting the issue (even though > it was missed during the review). > > The offending commit is only recently applied to the master branch, so was > never part of a OpenVPN release. For that reason I did not do full impact > analysis. > ACK. I already tested this on my on side. Acked-By: Arne Schwabe Arne ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Fix use-after-free in tls_ctx_use_management_external_key
Commit 98bfeeb4 changed our openssl backend implementation of
tls_ctx_use_management_external_key() to no longer use
tls_ctx_load_cert_file_and_copy(), but still free'd 'cert'. Which it no
longer should do. Credits go to Arne for spotting the issue (even though
it was missed during the review).
The offending commit is only recently applied to the master branch, so was
never part of a OpenVPN release. For that reason I did not do full impact
analysis.
Signed-off-by: Steffan Karger
---
src/openvpn/ssl_openssl.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index fe4db604..7532773e 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -1291,7 +1291,6 @@ tls_ctx_use_management_external_key(struct tls_root_ctx
*ctx)
/* get the public key */
EVP_PKEY *pkey = X509_get0_pubkey(cert);
ASSERT(pkey); /* NULL before SSL_CTX_use_certificate() is called */
-X509_free(cert);
if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA)
{
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] windows build warnings
Hi,
our windows build seems to have grown a few new warnings with some
of the recent refactoring...?
tun.c: In function 'ipconfig_register_dns':
tun.c:4870:10: warning: variable 'status' set but not used
[-Wunused-but-set-variable]
bool status;
^
tun.c: In function 'service_enable_dhcp':
tun.c:5202:11: warning: unused variable 'len' [-Wunused-variable]
DWORD len;
^
win32.c: In function 'win32_pause':
win32.c:688:13: warning: variable 'status' set but not used [-Wunused-but-set-va
riable]
int status;
^
can I interest someone to run a windows build, and go for the low-hanging
fruit?
("unused variable" in #ifdef _WIN32 code is likely fairly straightforward,
signed/unsigned pointer stuff might not be worth jumping through hoops
because fixing MinGW might make the warning reoccur on the MSVC side...)
Gert Doering
--
have you enabled IPv6 on something today...?
SpaceNet AG Vorstand: Sebastian v. Bomhard, Michael Emmer
Joseph-Dollinger-Bogen 14Aufsichtsratsvors.: A. Grundner-Culemann
D-80807 Muenchen HRB: 136055 (AG Muenchen)
Tel: +49 (0)89/32356-444 USt-IdNr.: DE813185279
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Signed/unsigned warnings of MSVC resolved
Acked-by: Gert Doering
I went back and read the discussion we had on v1 of this patch, so we
already agreed on this changes (or we let Simon convince us :)) - we just
asked for a v2 because parts of v1 ("return -err") got fixed in parallel
already.
Test compiled on ubuntu1604/mingw.
Your patch has been applied to the master branch.
commit f755c992915b25acd114ef98e61dd9eae7ff57fe
Author: Simon Rozman
Date: Fri Apr 13 14:47:56 2018 +0200
Signed/unsigned warnings of MSVC resolved
Acked-by: Gert Doering
Message-Id: <20180413124756.5756-1-si...@rozman.si>
URL:
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16756.html
Signed-off-by: Gert Doering
--
kind regards,
Gert Doering
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Do not assume that SSL_CTX_get/set_min/max_proto_version are macros
Hi, On Sun, Mar 04, 2018 at 12:44:02PM -0500, selva.n...@gmail.com wrote: > From: Selva Nair > > Openssl docs do not explicitly state these to be macros although they > are currently defined as such. Use AC_CHECK_DECLS to test for these so that > both function and macro forms could be detected. > > Signed-off-by: Selva Nair > --- > Though not meant as a fixup for libressl, as a side effect > this also makes 2.4.5 build with newer libressl versions. > (built on freebsd 11 using libressl 2.6.4 while testing patch 238) > Notes: (i) libressl defines only the set functions and neither > are macros. So get functions will get used from the compat layer. So, going through open patches in patchwork I came to this one. Reading through the thread, I'm not sure what the final outcome on the patch and the problem was? LibreSSL seems to be fixed (thanks, Jeremie :-) ) and if we build with OpenSSL 1.1.0g+, all is good? Phrased differently: does anything need to be done with this patch or should I click on "superceded" in patchwork now? gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Add support for CHACHA20-POLY1305 in the data channel
We explicitly only supported GCM as a valid AEAD mode, change that to also
allow ChaCha20-Poly1305 as an AEAD cipher. That works nicely with our new
(GCM) data channel format, because is has the same 96-bit IV.
Note that we need some tricks to not treat the cipher as insecure, because
we used to only look at the block size of a cipher to determine if find a
cipher insecure. But ChaCha20-Poly1305 is a stream cipher, which essentially
has a 'block size' of 1 byte and is reported as such. So, special-case this
cipher to be in the list of secure ciphers.
Signed-off-by: Steffan Karger
---
Changes.rst | 10 ++
src/openvpn/crypto.c | 2 +-
src/openvpn/crypto_backend.h | 5 +
src/openvpn/crypto_mbedtls.c | 21 ++---
src/openvpn/crypto_openssl.c | 33 -
src/openvpn/ssl.c| 2 +-
6 files changed, 63 insertions(+), 10 deletions(-)
diff --git a/Changes.rst b/Changes.rst
index a6090cf5..70ce2e10 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -1,3 +1,13 @@
+Overview of changes in 2.5
+==
+
+New features
+
+ChaCha20-Poly1305 cipher support
+Added support for using the ChaCha20-Poly1305 cipher in the OpenVPN data
+channel.
+
+
Overview of changes in 2.4
==
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index c8d95f3b..6d34acd7 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -841,7 +841,7 @@ init_key_ctx(struct key_ctx *ctx, const struct key *key,
dmsg(D_CRYPTO_DEBUG, "%s: CIPHER block_size=%d iv_size=%d",
prefix, cipher_kt_block_size(kt->cipher),
cipher_kt_iv_size(kt->cipher));
-if (cipher_kt_block_size(kt->cipher) < 128/8)
+if (cipher_kt_insecure(kt->cipher))
{
msg(M_WARN, "WARNING: INSECURE cipher with block size less than
128"
" bit (%d bit). This allows attacks like SWEET32. Mitigate
by "
diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h
index f917c8d7..38b2c175 100644
--- a/src/openvpn/crypto_backend.h
+++ b/src/openvpn/crypto_backend.h
@@ -284,6 +284,11 @@ int cipher_kt_block_size(const cipher_kt_t *cipher_kt);
*/
int cipher_kt_tag_size(const cipher_kt_t *cipher_kt);
+/**
+ * Returns true if we consider this cipher to be insecure.
+ */
+bool cipher_kt_insecure(const cipher_kt_t *cipher);
+
/**
* Returns the mode that the cipher runs in.
*
diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c
index 82f4e574..a5a096b1 100644
--- a/src/openvpn/crypto_mbedtls.c
+++ b/src/openvpn/crypto_mbedtls.c
@@ -51,6 +51,7 @@
#include
#include
#include
+#include
#include
@@ -175,7 +176,7 @@ show_available_ciphers(void)
while (*ciphers != 0)
{
const cipher_kt_t *info = mbedtls_cipher_info_from_type(*ciphers);
-if (info && cipher_kt_block_size(info) >= 128/8)
+if (info && !cipher_kt_insecure(info))
{
print_cipher(info);
}
@@ -188,7 +189,7 @@ show_available_ciphers(void)
while (*ciphers != 0)
{
const cipher_kt_t *info = mbedtls_cipher_info_from_type(*ciphers);
-if (info && cipher_kt_block_size(info) < 128/8)
+if (info && cipher_kt_insecure(info))
{
print_cipher(info);
}
@@ -550,6 +551,16 @@ cipher_kt_tag_size(const mbedtls_cipher_info_t *cipher_kt)
return 0;
}
+bool
+cipher_kt_insecure(const mbedtls_cipher_info_t *cipher_kt)
+{
+return !(cipher_kt_block_size(cipher_kt) >= 128/8
+#if defined(MBEDTLS_CHACHAPOLY_C) && (MBEDTLS_VERSION_NUMBER >= 0x020C)
+ || cipher_kt->type == MBEDTLS_CIPHER_CHACHA20_POLY1305
+#endif
+ );
+}
+
int
cipher_kt_mode(const mbedtls_cipher_info_t *cipher_kt)
{
@@ -573,7 +584,11 @@ cipher_kt_mode_ofb_cfb(const cipher_kt_t *cipher)
bool
cipher_kt_mode_aead(const cipher_kt_t *cipher)
{
-return cipher && cipher_kt_mode(cipher) == OPENVPN_MODE_GCM;
+return cipher && (cipher_kt_mode(cipher) == OPENVPN_MODE_GCM
+#if defined(MBEDTLS_CHACHAPOLY_C) && (MBEDTLS_VERSION_NUMBER >= 0x020C)
+ || cipher_kt_mode(cipher) == MBEDTLS_MODE_CHACHAPOLY
+#endif
+ );
}
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 9ec2048d..0004be9e 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -245,6 +245,7 @@ const cipher_name_pair cipher_name_translation_table[] = {
{ "AES-128-GCM", "id-aes128-GCM" },
{ "AES-192-GCM", "id-aes192-GCM" },
{ "AES-256-GCM", "id-aes256-GCM" },
+{ "CHACHA20-POLY1305", "ChaCha20-Poly1305" },
};
const size_t cipher_name_translation_table_count =
sizeof(cipher_name_translation_table) /
sizeof(*cipher_name_translation_table);
@@ -321,7 +322,7 @@ show_available_ciphers(void)
qsort(cipher_list, num_ciphers, sizeof(*cipher_list),
Re: [Openvpn-devel] [PATCH v7] convert *_inline attributes to bool
Hi, On Sat, Dec 02, 2017 at 11:21:44PM +0800, Antonio Quartulli wrote: > Carrying around the INLINE_TAG is not really efficient, > because it requires a strcmp() to be performed every > time we want to understand if the data is stored inline > or not. > > Convert all the *_inline attributes to bool to make the > logic easier and checks more efficient. > > Signed-off-by: Antonio Quartulli As discussed with Antonio right now, this got left in the cold for too long and much of the surrounding code got changed - so it does not apply anymore. (Also, it was missing an ACK...) Can I have a rebased version & an ACK on that, please? ;-) gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCHv2 5/7] re-implement argv_printf_*()
Hi,
I'm going through open patches in patchwork, and noticed the argv stuff
- I remembered that David had reviewed it, but going through the mails
now I can see that we have no ACK yet, because there are changes needed.
On Mon, Nov 13, 2017 at 12:49:27PM +0100, David Sommerseth wrote:
> Thanks a lot for putting a renewed effort into this. I've reviewed
> this and compile tested with GCC 4.8.5, 6.3.1 and 7.2.1 - as there
> are some warnings appearing; I'll come back to them in a bit.
[..]
> > +argv_printf_arglist(struct argv *a, const char *format, va_list arglist)
> > +{
> > +struct gc_arena gc = gc_new();
> > +const char *delim = 035;
>
> argv.c:217:25: warning: initialization makes pointer from integer without a
> cast [-Wint-conversion]
> const char *delim = 035;
>
> argv.c:227:34: warning: passing argument 2 of ???argv_prep_format??? makes
> integer from pointer without a cast [-Wint-conversion]
> f = argv_prep_format(format, delim, , );
> ^
This really is a bug. It's declared to be a pointer, but it is used to
just transport "an integer value" (035). It is valid C, but the compiler
is correct in questioning our motives...
> Shouldn't *delim be just delim? I'd propose using:
>
>const char delim = 0x1D; /* ASCII group separator character */
>
> This also kills all the warnings above.
Yes.
Heiko, can you do a v3 round of the remaining argv patches, taking David's
comments into account? Then we can get it into master and finish this
work...
thanks!
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany g...@greenie.muc.de
signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Implement "status 4" (JSON) for management interface
Hi, On Fri, Nov 10, 2017 at 05:39:46PM +0100, François Kooman wrote: > Parsing CSV can be cumbersome in depending software, so instead > offer JSON as well using "status 4". > > The "status 4" command uses the same keys/values as "status 2" > and "status 3", just formatted as JSON. So... we had a nice and long discussion about this last year, which I think can be wrapped like this - JSON is nice because good parser support exist on the receiving end today - human readability is a plus for manually verifying output - if we do JSON, we should do it for management *and* status file - most voices argued for "do not require an external library" (for *creation*). Not sure if we managed to convince David in the end, though :-) ... but then it just died off, and the patch is sitting in patchwork (where I moved it to "changes requested" just now). Is there interest in moving forward, or shall we just let it go? gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
