Re: [Openvpn-devel] [PATCH v2] Rework NCP compability logic and drop BF-CBC support by default
On 05/08/2020 21:25, Steffan Karger wrote:
Hi,
No full review yet, but this version does seem to address my previous
comments. Some minor nits I noticed on my first run through v2:
On 29-07-2020 13:38, Arne Schwabe wrote:
This reworks the NCP logic to be more strict about what is
considered an acceptable result of an NCP negotiation. It also
us to finally drop BF-CBC support by default.
All new behaviour is currently limited to server/client
mode with pull enabled. P2p mode without pull does not change.
New Server behaviour:
- when a client announces its supported ciphers through either
OCC or IV_CIPHER/IV_NCP we reject the client with a
AUTH_FAILED message if we have no common cipher.
- When a client does not announce any cipher in either
OCC or NCP we by reject it unless fallback-cipher is
specified in either ccd or config.
New client behaviour:
- When no cipher is pushed (or a cipher we refused to support)
and we also cannot support the server's server announced in
OCC we fail the connection and log why
- If fallback-cipher is specified we will in the case that
cipher is missing from occ use the fallback cipher instead
of failing the connection
Both client and server behaviour:
- We only announce --cipher xyz in occ if we are willing
to support that cipher.
It means that we only announce the fallback-cipher if
it is also contained in --data-ciphers
Compatiblity behaviour:
Compatiblity -> Compatibility
In 2.5 both client and server will automatically set
fallback-cipher xyz if --cipher xyz is in the config and
also add append the cipher to the end of data-ciphers.
We log a warn user about this and point to --data-ciphers and
--fallback-cipher. This also happens if the configuration
contains an explicit --cipher BF-CBC.
If --cipher is not set, we only warn that previous versions
allowed BF-CBC and point how to reenable BF-CBC. This might
break very few config where someone connects a very old
client to a 2.5 server but at some point we need to drop
the BF-CBC and those affected use will already have a the
scary SWEET32 warning in their logs.
In short: If --cipher is explicitly set 2.6 will work the same as
2.4 did. When --cipher is not set, BF-CBC support is dropped and
we warn about it.
Examples how breaking the default BF-CBC will be logged:
Client side:
- Client connecting to server that does not push cipher but
has --cipher in OCC
OPTIONS ERROR: failed to negotiate cipher with server. Add the server's
cipher ('BF-CBC') to --data-ciphers (currently 'AES-256-GCM:AES-128-CBC') if
you want to connect to this server.
- Client connecting to a server that does not support OCC:
OPTIONS ERROR: failed to negotioate cipher with server. Configure
--fallback-cipher if you want connect to this server.
negotioate -> *-)
Server Side:
- Server has a client only supporting BF-CBC connecting:
styx/IP PUSH: No common cipher between server and client. Server
data-ciphers:
'CHACHA20-POLY1305:AES-128-GCM:AES-256-GCM:AES-256-CBC:AES-128-CBC', client
supports cipher 'BF-CBC'.
- Client without OCC:
styx/IP PUSH:No NCP or OCC cipher data received from peer.
styx/IP Use --fallback-cipher with the cipher the client is using if you
want to allow the client to connect
In all reject cases on the client:
AUTH: Received control message: AUTH_FAILED,Data channel cipher negotiation
failed (no shared cipher)
Signed-off-by: Arne Schwabe
Patch V2: rename fallback-cipher to data-ciphers-fallback
add all corrections from Steffan
Ignore occ cipher for clients sending IV_CIPHERS
move client side ncp in its own function
do not print INSECURE cipher warning if BF-CBC is not allowed
Signed-off-by: Arne Schwabe
---
doc/man-sections/protocol-options.rst | 22 -
src/openvpn/crypto.c | 4 +-
src/openvpn/init.c| 18 ++--
src/openvpn/multi.c | 135 --
src/openvpn/options.c | 117 +-
src/openvpn/options.h | 2 +
src/openvpn/ssl.c | 17 ++--
src/openvpn/ssl_ncp.c | 82 +---
src/openvpn/ssl_ncp.h | 18 ++--
tests/unit_tests/openvpn/test_ncp.c | 26 +++--
10 files changed, 311 insertions(+), 130 deletions(-)
No other spelling or grammar to worry about.
diff --git a/doc/man-sections/protocol-options.rst
b/doc/man-sections/protocol-options.rst
index 051f1d32..69d3bc67 100644
--- a/doc/man-sections/protocol-options.rst
+++ b/doc/man-sections/protocol-options.rst
@@ -57,6 +57,9 @@ configured in a compatible way between both the local and
remote side.
http://www.cs.ucsd.edu/users/mihir/papers/hmac.html
--cipher alg
+ This option is deprecated for server-client mode and ``--data-ciphers``
+ or rarely `--data-ciphers-fallback`` should be used
Re: [Openvpn-devel] [PATCH v2] Rework NCP compability logic and drop BF-CBC support by default
Hi,
No full review yet, but this version does seem to address my previous
comments. Some minor nits I noticed on my first run through v2:
On 29-07-2020 13:38, Arne Schwabe wrote:
> This reworks the NCP logic to be more strict about what is
> considered an acceptable result of an NCP negotiation. It also
> us to finally drop BF-CBC support by default.
>
> All new behaviour is currently limited to server/client
> mode with pull enabled. P2p mode without pull does not change.
>
> New Server behaviour:
> - when a client announces its supported ciphers through either
> OCC or IV_CIPHER/IV_NCP we reject the client with a
> AUTH_FAILED message if we have no common cipher.
>
> - When a client does not announce any cipher in either
> OCC or NCP we by reject it unless fallback-cipher is
> specified in either ccd or config.
>
> New client behaviour:
> - When no cipher is pushed (or a cipher we refused to support)
> and we also cannot support the server's server announced in
> OCC we fail the connection and log why
>
> - If fallback-cipher is specified we will in the case that
> cipher is missing from occ use the fallback cipher instead
> of failing the connection
>
> Both client and server behaviour:
> - We only announce --cipher xyz in occ if we are willing
> to support that cipher.
>
> It means that we only announce the fallback-cipher if
> it is also contained in --data-ciphers
>
> Compatiblity behaviour:
>
> In 2.5 both client and server will automatically set
> fallback-cipher xyz if --cipher xyz is in the config and
> also add append the cipher to the end of data-ciphers.
>
> We log a warn user about this and point to --data-ciphers and
> --fallback-cipher. This also happens if the configuration
> contains an explicit --cipher BF-CBC.
>
> If --cipher is not set, we only warn that previous versions
> allowed BF-CBC and point how to reenable BF-CBC. This might
> break very few config where someone connects a very old
> client to a 2.5 server but at some point we need to drop
> the BF-CBC and those affected use will already have a the
> scary SWEET32 warning in their logs.
>
> In short: If --cipher is explicitly set 2.6 will work the same as
> 2.4 did. When --cipher is not set, BF-CBC support is dropped and
> we warn about it.
>
> Examples how breaking the default BF-CBC will be logged:
>
> Client side:
> - Client connecting to server that does not push cipher but
>has --cipher in OCC
>
> OPTIONS ERROR: failed to negotiate cipher with server. Add the server's
> cipher ('BF-CBC') to --data-ciphers (currently 'AES-256-GCM:AES-128-CBC') if
> you want to connect to this server.
>
> - Client connecting to a server that does not support OCC:
>
>OPTIONS ERROR: failed to negotioate cipher with server. Configure
> --fallback-cipher if you want connect to this server.
>
> Server Side:
>
> - Server has a client only supporting BF-CBC connecting:
>
> styx/IP PUSH: No common cipher between server and client. Server
> data-ciphers:
> 'CHACHA20-POLY1305:AES-128-GCM:AES-256-GCM:AES-256-CBC:AES-128-CBC', client
> supports cipher 'BF-CBC'.
>
> - Client without OCC:
>
>styx/IP PUSH:No NCP or OCC cipher data received from peer.
>styx/IP Use --fallback-cipher with the cipher the client is using if you
> want to allow the client to connect
>
> In all reject cases on the client:
>
>AUTH: Received control message: AUTH_FAILED,Data channel cipher
> negotiation failed (no shared cipher)
>
> Signed-off-by: Arne Schwabe
>
> Patch V2: rename fallback-cipher to data-ciphers-fallback
> add all corrections from Steffan
> Ignore occ cipher for clients sending IV_CIPHERS
> move client side ncp in its own function
> do not print INSECURE cipher warning if BF-CBC is not allowed
>
> Signed-off-by: Arne Schwabe
> ---
> doc/man-sections/protocol-options.rst | 22 -
> src/openvpn/crypto.c | 4 +-
> src/openvpn/init.c| 18 ++--
> src/openvpn/multi.c | 135 --
> src/openvpn/options.c | 117 +-
> src/openvpn/options.h | 2 +
> src/openvpn/ssl.c | 17 ++--
> src/openvpn/ssl_ncp.c | 82 +---
> src/openvpn/ssl_ncp.h | 18 ++--
> tests/unit_tests/openvpn/test_ncp.c | 26 +++--
> 10 files changed, 311 insertions(+), 130 deletions(-)
>
> diff --git a/doc/man-sections/protocol-options.rst
> b/doc/man-sections/protocol-options.rst
> index 051f1d32..69d3bc67 100644
> --- a/doc/man-sections/protocol-options.rst
> +++ b/doc/man-sections/protocol-options.rst
> @@ -57,6 +57,9 @@ configured in a compatible way between both the local and
> remote side.
>http://www.cs.ucsd.edu/users/mihir/papers/hmac.html
>
> --cipher alg
> + This option is deprecated for server-client mode and ``--data-ciphers``
> + or rarely
[Openvpn-devel] [PATCH applied] Re: Fix compilation with --disable-lzo and --disable-lz4
Acked-by: Gert Doering Thanks. Tested that it indeed fixes things, and that it is not necessary for 2.4 (compile-tested only). Added "trac #1308" to the commit message. Your patch has been applied to the master branch. commit dab34fdd0639c6de8c5ca759cca00b7e60da32f1 Author: Lev Stipakov Date: Wed Aug 5 06:25:48 2020 + Fix compilation with --disable-lzo and --disable-lz4 Signed-off-by: Lev Stipakov Acked-by: Gert Doering Message-Id: <20200805062548.38082-1-lstipa...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20637.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 applied] Re: Log serial number of revoked certificate
Hi, On Wed, Aug 05, 2020 at 05:27:45PM +0500, Vladislav Grishenko wrote: > Thank you. > I'd appreciate if patch could be applied to release/2.4 too, no changes are > required - related code is the same, just hunks offset in ssl_verify.c and > ssl_verify_openssl.c > I've tested 2.4.9 [git:release/2.4/7c428ca19a8df6b9+] with patch on sample > certificates, please refer log is below: OK. Done. Thanks :-) commit 4ee2c1cd877b2e99b41fd248bf853329af825188 (HEAD -> release/2.4) 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 applied] Re: Log serial number of revoked certificate
Hi, Gert Thank you. I'd appreciate if patch could be applied to release/2.4 too, no changes are required - related code is the same, just hunks offset in ssl_verify.c and ssl_verify_openssl.c I've tested 2.4.9 [git:release/2.4/7c428ca19a8df6b9+] with patch on sample certificates, please refer log is below: OpenSSL, --crl-verify sample-keys/ca.crl Wed Aug 5 17:18:49 2020 127.0.0.1:16001 VERIFY ERROR: depth=0, error=certificate revoked: C=KG, ST=NA, O=OpenVPN-TEST, CN=client-revoked, emailAddress=me@myhost.mydomain, serial=3 Wed Aug 5 17:18:49 2020 127.0.0.1:16001 OpenSSL: error:1417C086:SSL routines:tls_process_client_certificate:certificate verify failed Wed Aug 5 17:18:49 2020 127.0.0.1:16001 TLS_ERROR: BIO read tls_read_plaintext error mbedTLS, --crl-verify sample-keys/ca.crl Wed Aug 5 17:25:28 2020 127.0.0.1:16001 VERIFY OK: depth=1, C=KG, ST=NA, L=BISHKEK, O=OpenVPN-TEST, emailAddress=me@myhost.mydomain Wed Aug 5 17:25:28 2020 127.0.0.1:16001 VERIFY ERROR: depth=0, subject=C=KG, ST=NA, O=OpenVPN-TEST, CN=client-revoked, emailAddress=me@myhost.mydomain, serial=3: The certificate has been revoked (is on a CRL) Wed Aug 5 17:25:28 2020 127.0.0.1:16001 TLS_ERROR: read tls_read_plaintext error: X509 - Certificate verification failed, e.g. CRL, CA or signature check failed touch sample-keys/3, --crl-verify sample-keys/ dir Wed Aug 5 17:18:12 2020 127.0.0.1:16001 VERIFY OK: depth=1, C=KG, ST=NA, L=BISHKEK, O=OpenVPN-TEST, emailAddress=me@myhost.mydomain Wed Aug 5 17:18:12 2020 127.0.0.1:16001 VERIFY CRL: depth=0, C=KG, ST=NA, O=OpenVPN-TEST, CN=client-revoked, emailAddress=me@myhost.mydomain, serial=3 is revoked Wed Aug 5 17:18:12 2020 127.0.0.1:16001 OpenSSL: error:1417C086:SSL routines:tls_process_client_certificate:certificate verify failed Wed Aug 5 17:18:12 2020 127.0.0.1:16001 TLS_ERROR: BIO read tls_read_plaintext error -- Best Regards, Vladislav Grishenko -Original Message- From: Gert Doering Sent: Wednesday, August 5, 2020 4:55 PM To: Vladislav Grishenko Cc: openvpn-devel@lists.sourceforge.net Subject: [PATCH applied] Re: Log serial number of revoked certificate Your patch has been applied to the master branch. I have not done much testing, just a test run "make check" on an OpenSSL and mbedTLS build. I have not looked into applying it to "release/2.4" - if you think it's needed, let me know (or if it needs more work because the code has diverged too much, send a 2.4 patch) - thanks. commit 992e9cec40539a155afa9eae10502aa62f617965 Author: Vladislav Grishenko Date: Wed Aug 5 15:23:33 2020 +0500 Log serial number of revoked certificate Signed-off-by: Vladislav Grishenko Acked-by: Lev Stipakov Message-Id: <20200805102333.3109-1-themi...@yandex-team.ru> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20642.ht ml 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: Log serial number of revoked certificate
Your patch has been applied to the master branch. I have not done much testing, just a test run "make check" on an OpenSSL and mbedTLS build. I have not looked into applying it to "release/2.4" - if you think it's needed, let me know (or if it needs more work because the code has diverged too much, send a 2.4 patch) - thanks. commit 992e9cec40539a155afa9eae10502aa62f617965 Author: Vladislav Grishenko Date: Wed Aug 5 15:23:33 2020 +0500 Log serial number of revoked certificate Signed-off-by: Vladislav Grishenko Acked-by: Lev Stipakov Message-Id: <20200805102333.3109-1-themi...@yandex-team.ru> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20642.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 v2] Rework NCP compability logic and drop BF-CBC support by default
Hi, On Wed, Aug 05, 2020 at 08:43:18AM +0200, Gert Doering wrote: > Test run with "cipher bf-cbc" in all server configs next... For completeness, this works nicely: start client jobs... 22... Test sets succeeded: 1 2 3 4 6 8. Test sets failed: none. 23.small... Test sets succeeded: 1 2 3 4. Test sets failed: none. 23... Test sets succeeded: 1 1a 1b 1d 2 2a 2b 2c 2d 3 4 5 6 8 8a 9. Test sets failed: none. 24... Test sets succeeded: 1 1a 1b 1c 1d 1e 2 2a 2b 2c 2d 2e 3 4 4a 5 6 8 8a 9. Test sets failed: none. master... Test sets succeeded: 1 1a 1b 1c 1d 1e 2 2a 2b 2c 2d 2e 3 4 5 5a 5b 5c 5d 5v1 5v2 5v3 5w1 5w2 5w3 5w4 5x1 5x2 5x3 5x4 6 7 7x 8 8a 9 2f 4b. Test sets failed: none. so, if we break someone's existing server setup, the answer is "put 'fallback-cipher BF-CBC' into your config!" (or 'cipher BF-CBC' explicitly, so it's 2.4-compatible) Not sure how to tackle the "ccd/ push cipher is broken now with 2.4-NCP clients" part. I think this is useful functionality, but the current patch does not allow this "unless the client is already using the to-be- pushed cipher, or it's one of the two NCP=2 AEAD ciphers". Which makes it slightly less than useful... 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 v2] Log serial number of revoked certificate
Code looks good. Compiled and tested (openssl and revoked cert) on Ubuntu 20.04. Acked-by: Lev Stipakov ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] Log serial number of revoked certificate
As it appears commit 767e4c56becbfeea525e4695a810593f373883cd "Log
serial number of revoked certificate" hasn't survive refactoring
of CRL handling.
In most of situations admin of OpenVPN server needs to know which
particular certificate is used by client.
In the case when certificate is valid, environment variable can be
used for that but once it is revoked, no user scripts are invoked
so there is no way to get serial number, only subject is logged.
Let's log certificate serial in case it is revoked and additionally
log certificate depth & subject in crl-verify "dir" mode for better
consistency with crl file (non-dir) mode.
v2: log if serial is not availble, require it in crl-verify dir mode
Signed-off-by: Vladislav Grishenko
---
src/openvpn/ssl_verify.c | 14 +++---
src/openvpn/ssl_verify_mbedtls.c | 5 +++--
src/openvpn/ssl_verify_openssl.c | 5 +++--
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 844bc57d..97ccb93b 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -599,7 +599,8 @@ cleanup:
* check peer cert against CRL directory
*/
static result_t
-verify_check_crl_dir(const char *crl_dir, openvpn_x509_cert_t *cert)
+verify_check_crl_dir(const char *crl_dir, openvpn_x509_cert_t *cert,
+ const char *subject, int cert_depth)
{
result_t ret = FAILURE;
char fn[256];
@@ -607,6 +608,12 @@ verify_check_crl_dir(const char *crl_dir,
openvpn_x509_cert_t *cert)
struct gc_arena gc = gc_new();
char *serial = backend_x509_get_serial(cert, );
+if (!serial)
+{
+msg(D_HANDSHAKE, "VERIFY CRL: depth=%d, %s, serial number is not
available",
+cert_depth, subject);
+goto cleanup;
+}
if (!openvpn_snprintf(fn, sizeof(fn), "%s%c%s", crl_dir,
OS_SPECIFIC_DIRSEP, serial))
{
@@ -616,7 +623,8 @@ verify_check_crl_dir(const char *crl_dir,
openvpn_x509_cert_t *cert)
fd = platform_open(fn, O_RDONLY, 0);
if (fd >= 0)
{
-msg(D_HANDSHAKE, "VERIFY CRL: certificate serial number %s is
revoked", serial);
+msg(D_HANDSHAKE, "VERIFY CRL: depth=%d, %s, serial=%s is revoked",
+cert_depth, subject, serial);
goto cleanup;
}
@@ -758,7 +766,7 @@ verify_cert(struct tls_session *session,
openvpn_x509_cert_t *cert, int cert_dep
{
if (opt->ssl_flags & SSLF_CRL_VERIFY_DIR)
{
-if (SUCCESS != verify_check_crl_dir(opt->crl_file, cert))
+if (SUCCESS != verify_check_crl_dir(opt->crl_file, cert, subject,
cert_depth))
{
goto cleanup;
}
diff --git a/src/openvpn/ssl_verify_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c
index fd31bbbd..93891038 100644
--- a/src/openvpn/ssl_verify_mbedtls.c
+++ b/src/openvpn/ssl_verify_mbedtls.c
@@ -68,6 +68,7 @@ verify_callback(void *session_obj, mbedtls_x509_crt *cert,
int cert_depth,
int ret = 0;
char errstr[512] = { 0 };
char *subject = x509_get_subject(cert, );
+char *serial = backend_x509_get_serial(cert, );
ret = mbedtls_x509_crt_verify_info(errstr, sizeof(errstr)-1, "",
*flags);
if (ret <= 0 && !openvpn_snprintf(errstr, sizeof(errstr),
@@ -82,8 +83,8 @@ verify_callback(void *session_obj, mbedtls_x509_crt *cert,
int cert_depth,
if (subject)
{
-msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, subject=%s: %s",
-cert_depth, subject, errstr);
+msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, subject=%s, serial=%s:
%s",
+cert_depth, subject, serial ? serial : "",
errstr);
}
else
{
diff --git a/src/openvpn/ssl_verify_openssl.c b/src/openvpn/ssl_verify_openssl.c
index ff14db23..454efeec 100644
--- a/src/openvpn/ssl_verify_openssl.c
+++ b/src/openvpn/ssl_verify_openssl.c
@@ -71,6 +71,7 @@ verify_callback(int preverify_ok, X509_STORE_CTX *ctx)
{
/* get the X509 name */
char *subject = x509_get_subject(current_cert, );
+char *serial = backend_x509_get_serial(current_cert, );
if (!subject)
{
@@ -89,10 +90,10 @@ verify_callback(int preverify_ok, X509_STORE_CTX *ctx)
}
/* Remote site specified a certificate, but it's not correct */
-msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, error=%s: %s",
+msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, error=%s: %s, serial=%s",
X509_STORE_CTX_get_error_depth(ctx),
X509_verify_cert_error_string(X509_STORE_CTX_get_error(ctx)),
-subject);
+subject, serial ? serial : "");
ERR_clear_error();
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Skip existing interfaces on opening the first available utun on macOS
Hi, +ASSERT(snprintf(ifname, sizeof(ifname), "utun%d", utunnum)); Not sure about ASSERT here, because according to https://linux.die.net/man/3/snprintf > If an output error is encountered, a negative value is returned. which won't trigger assert. Otherwise looks good. Compiled and tested on macOS. Maybe Gert could remove ASSERT wrapping before commiting. Acked-by: Lev Stipakov ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Log serial number of revoked certificate
Hi, Lev Thanks for review, I'll make improvements in V2. -- Best Regards, Vladislav Grishenko -Original Message- From: Lev Stipakov Sent: Wednesday, August 5, 2020 1:29 PM To: Vladislav Grishenko Cc: openvpn-devel Subject: Re: [Openvpn-devel] [PATCH] Log serial number of revoked certificate Hi, Compiled and tested on Ubuntu 20.04, looks good. A few nit-picks: > +verify_check_crl_dir(const char *crl_dir, int cert_depth, > +openvpn_x509_cert_t *cert, char *subject) The last parameter could benefit from const to indicate that function is not going to modify it. > -msg(D_HANDSHAKE, "VERIFY CRL: certificate serial number %s is > revoked", serial); > +msg(D_HANDSHAKE, "VERIFY CRL: depth=%d, %s, serial=%s is revoked", > +cert_depth, subject, serial); Since you are modifying this line, could you add a NULL check to serial to and pass something like "" in this case? > +msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, subject=%s, > serial=%s: %s", > +cert_depth, subject, serial ? serial : "", errstr); I would use "" in NULL case, otherwise the error message becomes funny. > +msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, error=%s: %s, > + serial=%s", > X509_STORE_CTX_get_error_depth(ctx), > X509_verify_cert_error_string(X509_STORE_CTX_get_error(ctx)), > -subject); > +subject, serial ? serial : ""); Same as above. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Log serial number of revoked certificate
Hi, Compiled and tested on Ubuntu 20.04, looks good. A few nit-picks: > +verify_check_crl_dir(const char *crl_dir, int cert_depth, > openvpn_x509_cert_t *cert, char *subject) The last parameter could benefit from const to indicate that function is not going to modify it. > -msg(D_HANDSHAKE, "VERIFY CRL: certificate serial number %s is > revoked", serial); > +msg(D_HANDSHAKE, "VERIFY CRL: depth=%d, %s, serial=%s is revoked", > +cert_depth, subject, serial); Since you are modifying this line, could you add a NULL check to serial to and pass something like "" in this case? > +msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, subject=%s, > serial=%s: %s", > +cert_depth, subject, serial ? serial : "", errstr); I would use "" in NULL case, otherwise the error message becomes funny. > +msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, error=%s: %s, serial=%s", > X509_STORE_CTX_get_error_depth(ctx), > X509_verify_cert_error_string(X509_STORE_CTX_get_error(ctx)), > -subject); > +subject, serial ? serial : ""); Same as above. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2] Rework NCP compability logic and drop BF-CBC support by default
HI,
On Wed, Aug 05, 2020 at 12:20:54AM +0200, Arne Schwabe wrote:
> > Is that intentional?
>
> Yes. That is intentional. If you do not have any cipher option in the
> config, there is nowadays a very high change that you allow BF-CBC by
> "accident". I encountered this first-hand ("I do want to put as few
> option in a config as possible").
OK, I can see that line of reasoning. This needs to be put very
prominently into the release notes.
Updating on server test success - with the core dump fix, but still
*no* "--cipher" in the server configs, I get
22...
Test sets succeeded: 8.
Test sets failed: 1 2 3 4 6.
23.small...
Test sets succeeded: none.
Test sets failed: 1 2 3 4.
23...
Test sets succeeded: 8 8a 9.
Test sets failed: 1 1a 1b 1d 2 2a 2b 2c 2d 3 4 5 6.
24...
Test sets succeeded: 1 1a 1b 1c 1d 1e 2 2b 2d 2e 3 5 6 8 8a 9.
Test sets failed: 2a 2c 4 4a.
master...
Test sets succeeded: 1 1a 1b 1c 1d 1e 2 2b 2c 2d 2e 3 5 5a 5b 5c 5d 5v1 5v2 5v3
5w1 5w2 5w3 5w4 5x1 5x2 5x3 5x4 6 7 7x 8 8a 9 2f.
Test sets failed: 2a 4 4b.
so the 2.2/2.3-small/2.3 failures are expected.
2a and 2c (for 2.4) is expected, because that's "--ncp-disable"
4 got broken somewhat accidently - this is tap tests, relying on a
secondary client to be connected to ping "across the tap". That client
is using pushed ccd ciphers, which fails in interesting ways
Aug 5 08:39:33 gentoo tap-udp-p2mp[2418]:
freebsd-74-amd64/2001:608:0:814::f000:3 PUSH: No common cipher between server
and client. Server data-ciphers: 'CAMELLIA-128-CBC', client supported ciphers
'AES-256-GCM:AES-128-GCM'
but this error message is misleading - the client has
ncp-ciphers CAMELLIA-128-CBC:AES-256-GCM:AES-128-GCM
in its config, but it's a 2.4 client so cannot signal this to the master.
So we *will* break pushed ciphers to 2.4 client swith non-AEAD ciphers
here. Any idea how to tackle this?
Test run with "cipher bf-cbc" in all server configs next...
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] Fix compilation with --disable-lzo and --disable-lz4
struct compress_options is defined under USE_COMP, therefore
compilation fails when it is referenced without that define.
Since function show_compression_warning, which uses aforementioned
struct, is only called under USE_COMP, it is safe to wrap its definition
under USE_COMP, which fixes compilation issue.
Signed-off-by: Lev Stipakov
---
src/openvpn/options.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index bc256b18..1c246f4b 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5085,6 +5085,7 @@ set_user_script(struct options *options,
#endif
}
+#ifdef USE_COMP
static void
show_compression_warning(struct compress_options *info)
{
@@ -5103,6 +5104,7 @@ show_compression_warning(struct compress_options *info)
}
}
}
+#endif
static void
add_option(struct options *options,
--
2.25.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
