Opened 6 years ago

Closed 4 years ago

#1050 closed Bug / Defect (fixed)

OpenVPN 2.4.5 openssl 1.1.0 client not able to verify to openssl 1.0.1e-fips server

Reported by: gizi Owned by: plaisthos
Priority: major Milestone: release 2.4.7
Component: Generic / unclassified Version: OpenVPN 2.4.5 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords: ssl handshake failure, bad signature
Cc:

Description

After upgrade from OpenVPN 2.4.4 to OpenVPN 2.4.5, Windows clients cannot connect to CentOS 6 server. I guess it is because of openssl 1.1.0 (client) and 1.0.1e-fips (server). Here is log report:
Apr 3 12:28:11 server openvpn[32166]: 192.168.1.173:5001 VERIFY OK: depth=1, C=CZ, ST=......, L=...., O=....., CN=......., emailAddress=.......
Apr 3 12:28:11 server openvpn[32166]: 192.168.1.173:5001 VERIFY OK: depth=0, C=CZ, ST=......, O=....., CN=......., emailAddress=.......
Apr 3 12:28:11 server openvpn[32166]: 192.168.1.173:5001 OpenSSL: error:0D07209B:asn1 encoding routines:ASN1_get_object:too long
Apr 3 12:28:11 server openvpn[32166]: 192.168.1.173:5001 OpenSSL: error:0D068066:asn1 encoding routines:ASN1_CHECK_TLEN:bad object header
Apr 3 12:28:11 server openvpn[32166]: 192.168.1.173:5001 OpenSSL: error:0D07803A:asn1 encoding routines:ASN1_ITEM_EX_D2I:nested asn1 error
Apr 3 12:28:11 server openvpn[32166]: 192.168.1.173:5001 OpenSSL: error:1408807B:SSL routines:SSL3_GET_CERT_VERIFY:bad signature
Apr 3 12:28:11 server openvpn[32166]: 192.168.1.173:5001 OpenSSL: error:140940E5:SSL routines:SSL3_READ_BYTES:ssl handshake failure
Apr 3 12:28:11 server openvpn[32166]: 192.168.1.173:5001 TLS_ERROR: BIO read tls_read_plaintext error
Apr 3 12:28:11 server openvpn[32166]: 192.168.1.173:5001 TLS Error: TLS object -> incoming plaintext read error
Apr 3 12:28:11 server openvpn[32166]: 192.168.1.173:5001 TLS Error: TLS handshake failed
Apr 3 12:28:11 server openvpn[32166]: 192.168.1.173:5001 SIGUSR1[soft,tls-error] received, client-instance restarting

Attachments (2)

vpn-pkcs11_report.log (7.8 KB) - added by gizi 6 years ago.
log after running debig executrable with pkcs11 connection
vpn_cryptoapicert.log (1.7 KB) - added by gizi 6 years ago.
log after running debug executrable with cryptoapicert connection

Download all attachments as: .zip

Change History (23)

comment:1 Changed 6 years ago by plaisthos

OpenSSL 1.1.0 is lot more strict in its ssl implementation. The server ceritifcate might not be accpeted by OpenSSL anymore. Try to do a manual openssl x509 -in ca.pem and openssl openssl x509 -in server.pem with openssl 1.1 to check if this is the case.

comment:2 Changed 6 years ago by gizi

I run openssl x509 -in ca.pem and openssl x509 -in server.crt with openssl 1.1 (installed with OpenVPN 2.4.5) and output were certificates without any error.
I probably found the problem. When client uses Safenet ikey2032 with certificate and private key in it, error occurs. When I installed client certificate and private key in Microsoft certificate store, everything works as before. So there is some problem between OpenVPN 2.4.5 and ikey2032 or Safenet Authentication Client (currently tested with old version 8.3 with Windows 10 - 1709).

Version 1, edited 6 years ago by gizi (previous) (next) (diff)

comment:3 Changed 6 years ago by Selva Nair

So it seems the signature generated by the token is either invalid or not correctly parsed by OpenSSL. With key in cert store the signature is created by Windows Crypto API (CNG) and that does work. This is indeed unexpected.

Is the ikey2032 accessed using pkcs11 or cryptoapicert? If the former, could you try using cryptoapicert (with key not manually loaded to the cert store)?

comment:4 Changed 6 years ago by gizi

I use cryptoapicert. Client ovpn.vpn config contains this line:
cryptoapicert "THUMB:73 40 7b 47 c3 37 5d a6 bf df eb 13 09 4a 50 1d 4c 88 95 1f"
It works many years, last with OpenVPN 2.4.4 and stopped with v 2.4.5. Last line from client vpn log is:
Signing hash using CNG: data size = 83

I checked using pkcs11 and it works (but it is not so simple to configure, if you do tens of clients every year).
Instead of cryptoapicert line, now clients have these two lines in config:
pkcs11-providers eTPKCS11.dll
pkcs11-id 'pkcs11:model=Model%20330;token=somename;manufacturer=%a9Rainbow%20Tech.;serial=XXXXXXXX;id=%XX%XXX%XX%XX%XX%XXX'

eTPKCS11.dll comes with Safenet client sw and lies in windows default path.
pkcs11-id 'id' gets using command:
openvpn.exe --show-pkcs11-ids eTPKCS11.dll

So bug is still unresolved. cryptoapicert now works with certificates in windows store but not with certificates in hw ikey2032 (tested also with ikey4000).

Last edited 6 years ago by gizi (previous) (diff)

comment:5 Changed 6 years ago by Selva Nair

The main relevant change in 2.4.5 is that cryptoapicert now uses the newer CNG API (replacement for older CAPI). That also means TLS 1.2 is now supported with cryptoapicert and in fact your client is negotiating 1.2 with SHA512 as the hash for the signature (data size = 83 bytes). In 2.4.4 it would have used TLS 1.1 and the old-style MD5+SHA1 hash.

The OpenSSL error on the server shows the signature could not be correctly parsed. This could be a bug in OpenVPN or something wrong with the token's support for TLS 1.2.

Please check whether the pkcs11 connection that works uses TLS 1.1 or 1.2 (logs will show this). I've made a debug version that prints out the signature and checks it client-side. If you run it on the client with verb=4 and send me the results I can take a look.

A test executable is included here: https://github.com/selvanair/openvpn/releases/tag/cng-debug

This is based on one added commit over 2.4.5 as in here: https://github.com/selvanair/openvpn/commits/cng-debug

Changed 6 years ago by gizi

Attachment: vpn-pkcs11_report.log added

log after running debig executrable with pkcs11 connection

Changed 6 years ago by gizi

Attachment: vpn_cryptoapicert.log added

log after running debug executrable with cryptoapicert connection

comment:6 Changed 6 years ago by Selva Nair

Thanks for testing. The token is truncating the data to 36 bytes (that's the size of the old style MD5+SHA1 hash used in TLS 1.1 and earlier) even though the passed in data is 83 bytes (64 bytes of SHA512 hash + 19 bytes for encoding digest-info). The token is definitely able to handle the larger data as that's exactly what pkcs11 would pass to it and works correctly.

But Windows CNG talks to the token using its minidriver. I'm not sure whether this is a bug in the token's minidriver or my misunderstanding of the docs.

Anyway, I did some more tests and made a patched version that works around this. Please test the executable included here: https://github.com/selvanair/openvpn/releases/tag/cng-fix

Note: A quick fix for 2.4.5 release is to set --tls-version-max 1.1 in the client. That's will make it revert to the 2.4.4 behaviour.


Some technical details:

The OpenSSL callback for signature that we use passes in the hash encoded with its OID so we call NCryptSignHash() with hash alogorithm set to NULL. The NULL is required to tell Windows that the OID is already added. This works with keys in the cert store. However, it seems token minidrivers interpret the NULL hash to imply that the data is a pre TLS 1.2 SSL3_SHAMD5 hash (MD5+SHA1) which should be 36 bytes. Silent truncation of data is common with many token drivers so no error is generated.

Based on my reading of Windows smart card minidriver docs, its hard to say whether this is a bug or not.

The above patched version avoids this issue by passing the raw hash and name of the hash algorithm to NCryptSignHash(). In this case PKCS #1 encoding of the hash OID is done by CNG (or the minidriver for tokens). This should work better.

comment:7 Changed 6 years ago by gizi

I tested openvpn-cng-fix.exe version and it works correctly with cryptoapicert. If you want log details please let me know. Thank you for quick solution.

comment:8 Changed 6 years ago by Selva Nair

Glad to know it works. Thanks.

comment:9 Changed 6 years ago by JJK

The executable does not work "out of the box" for me. The warning I get is:

Fri Apr 13 15:02:53 2018 us=322245 WARNING: cryptoapicert: private key is in a legacy store. Restricting TLS version to 1.1

This is with a Safenet eToken and a config

cryptoapicert "SUBJ:Jan Just Keijser"
tls-version-min 1.2

What am I doing wrong, or is this simply not supported with these tokens?

comment:10 Changed 6 years ago by gizi

I tested with Safenet ikey2032 (very old key) on Windows 10 (1709) with Safenet Authentication Client v8.3. Key and cert generated on CentOS 7 and inported from p12 to ikey. Config patameters are:
cryptoapicert "THUMB:xx xx xx ....."

I get no warnings and it uses TLSv1.2

comment:11 Changed 6 years ago by JJK

Update: I needed to upgrade my Safenet client to v9 and now the eToken is working as well.

comment:12 Changed 6 years ago by Selva Nair

I tested with Safenet ikey2032 (very old key) on Windows 10 (1709) with Safenet Authentication Client v8.3. Key and cert generated on CentOS 7 and inported from p12 to ikey. Config patameters are:
cryptoapicert "THUMB:xx xx xx ....."

Yes, CNG support (and hence TLS 1.2) needs newer versions of the safenet token driver. With some old versions you will get the "TLS version downgrade" warning but would still work if TLS 1.1 is acceptable -- in fact a graceful fallback to 1.1 was my main concern and the only case I had tested with hardware tokens when CNG support was added to 2.4.5.

Can anyone test this using a token with a different driver than safenet? I wonder whether this issue with 2.4.5 shows up only with some token drivers.

BTW, make the key inside the token (as non-exportable), generate a CSR on Windows and issue the certificate in CentOS (where your CA is) would be a better workflow. Its a bit of work to set this up on Windows but it does work. Windows native certreq tool should also be usable though I could not get openssl to correctly parse the resulting CSR (did not try hard though..)

Last edited 6 years ago by Selva Nair (previous) (diff)

comment:13 Changed 6 years ago by Selva Nair

@gizi Could you please test the exe included here https://github.com/selvanair/openvpn-gui/releases ?

I went for a simpler approach: do not abort on timeout but keep retrying unless the process or service itself triggers a startup error. Or the user can abort by pressing disconnect.

Things to test: (i) situations that require a long timeout works (ii) an early abort will not leave openvpn.exe running.

comment:14 Changed 6 years ago by Selva Nair

Sorry, wrong ticket :)

@gizi Could you please test the exe included here https://github.com/selvanair/openvpn-gui/releases ?

I went for a simpler approach: do not abort on timeout but keep retrying unless the process or service itself triggers a startup error. Or the user can abort by pressing disconnect.
Things to test: (i) situations that require a long timeout works (ii) an early abort will not leave openvpn.exe running.

comment:15 Changed 6 years ago by gizi

Do you plan to include the fix in next release 2.4.7? Today I tested 2.4.6 and the fix is not included.

comment:16 Changed 6 years ago by Selva Nair

Milestone: release 2.4.6release 2.4.7

Sorry for the delay. I just submitted the patch for review. https://patchwork.openvpn.net/patch/329/

Hopefully we can get it into the next version.

comment:17 Changed 5 years ago by Gert Döring

Sorry that this took so long. It's in the code base now, but only master "for now"

commit 6b495dc4c5cfc118091ddc9c19330b3c9e3e3dff
Author: Selva Nair <selva.nair@…>
Date: Thu Apr 26 10:24:24 2018 -0400

Pass the hash without the DigestInfo? header to NCryptSignHash()


Fixes Trac #1050
Acked-by: Steffan Karger <steffan.karger@…>
Message-Id: <1524752664-27946-1-git-send-email-selva.nair@…>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16840.html

the patch does not apply to 2.4, at least not "straight forward" - it seems some prerequisite patch is missing which I applied only to master.

So, as this is a bug that affects 2.4 users, I'm happy to pull in what is needed but would appreciate if someone can point me at the proper commits in master.

comment:18 Changed 5 years ago by Gert Döring

commit 163fa9c1e79bddc07c7dd47e8c313016b482314f (HEAD -> release/2.4)
Author: Selva Nair <selva.nair@…>
Date: Fri Oct 5 20:08:15 2018 -0400

Pass the hash without the DigestInfo? header to NCryptSignHash()

patch has been merged into 2.4 now (it just conflicted in context, so no prerequisites needed). Thanks again, Selva.

2.4.7 release is likely going to happen "soonish"

comment:19 Changed 4 years ago by Gert Döring

Owner: set to plaisthos
Status: newassigned

comment:20 Changed 4 years ago by Gert Döring

If I'm reading this right this should have been closed long ago, since the change was merged to 2.4.7 - right?

comment:21 Changed 4 years ago by plaisthos

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.