Hi, On Wed, Jul 29, 2020 at 01:38:35PM +0200, 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.
I think the goals are good, but there are two corner cases in the
code that are not quite right yet
[ RUN ] test_extract_client_ciphers
[ OK ] test_extract_client_ciphers
[ RUN ] test_poor_man
[ ERROR ] --- Test failed with exception: Segmentation fault(11)
[ FAILED ] test_poor_man
[ RUN ] test_ncp_best
[ OK ] test_ncp_best
[==========] 4 test(s) run.
[ PASSED ] 3 test(s).
[ FAILED ] 1 test(s), listed below:
[ FAILED ] test_poor_man
1 FAILED TEST(S)
FAIL: ncp_testdriver
... and unfortunately, it also segfaults when a 2.2 client connects -
so, on my server test rig all openvpn processes are gone after 2.2
has tested...
2020-08-04 22:05:05 us=274264 194.97.140.21:43906 TLS: Initial packet from
[AF_INET6]::ffff:194.97.140.21:43906, sid=3c3a2afa adfd47fa
2020-08-04 22:05:05 us=390684 194.97.140.21:43906 VERIFY OK: depth=1, C=US,
ST=California, L=Pleasanton, O=OpenVPN community project, CN=OpenVPN community
project CA, emailAddress=sam...@openvpn.net
2020-08-04 22:05:05 us=390938 194.97.140.21:43906 VERIFY OK: depth=0, C=DE,
ST=Bavaria, L=Munich, O=OpenVPN community project,
CN=cron2-freebsd-tc-amd64-22, ??=Gert Doering, emailAddress=g...@greenie.muc.de
2020-08-04 22:05:05 us=604124 194.97.140.21:43906 Control Channel: TLSv1.0,
cipher TLS-ECDHE-RSA-WITH-AES-256-CBC-SHA, 2048 bit key
2020-08-04 22:05:05 us=604229 194.97.140.21:43906 [cron2-freebsd-tc-amd64-22]
Peer Connection Initiated with [AF_INET6]::ffff:194.97.140.21:43906
2020-08-04 22:05:05 us=604292 cron2-freebsd-tc-amd64-22/194.97.140.21:43906
MULTI_sva: pool returned IPv4=10.204.1.18, IPv6=fd00:abcd:204:1::1003
2020-08-04 22:05:05 us=604386 cron2-freebsd-tc-amd64-22/194.97.140.21:43906
mcccp1 (enter): ret=3, deferred=0
2020-08-04 22:05:05 us=604438 cron2-freebsd-tc-amd64-22/194.97.140.21:43906
MULTI: Learn: 10.204.1.18 -> cron2-freebsd-tc-amd64-22/194.97.140.21:43906
2020-08-04 22:05:05 us=604531 cron2-freebsd-tc-amd64-22/194.97.140.21:43906
MULTI: primary virtual IP for cron2-freebsd-tc-amd64-22/194.97.140.21:43906:
10.204.1.18
2020-08-04 22:05:05 us=604614 cron2-freebsd-tc-amd64-22/194.97.140.21:43906
MULTI: Learn: fd00:abcd:204:1::1003 ->
cron2-freebsd-tc-amd64-22/194.97.140.21:43906
2020-08-04 22:05:05 us=604658 cron2-freebsd-tc-amd64-22/194.97.140.21:43906
MULTI: primary virtual IPv6 for cron2-freebsd-tc-amd64-22/194.97.140.21:43906:
fd00:abcd:204:1::1003
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7d9112d in ?? () from /lib64/libc.so.6
(gdb) where
#0 0x00007ffff7d9112d in ?? () from /lib64/libc.so.6
#1 0x00005555555d2299 in ncp_get_best_cipher (server_list=0x55555562bf78
"AES-256-GCM:AES-128-GCM", peer_info=peer_info@entry=0x0,
remote_cipher=0x55555566a800 "BF-CBC", gc=gc@entry=0x55555563de50) at
ssl_ncp.c:231
#2 0x000055555558ea38 in multi_client_set_protocol_options (c=0x55555563de50)
at multi.c:1833
#3 multi_client_connect_late_setup (option_types_found=<optimized out>,
mi=0x55555563dc90, m=0x7fffffffc410) at multi.c:2434
#4 multi_connection_established (mi=0x55555563dc90, m=0x7fffffffc410) at
multi.c:2672
#5 multi_process_post (m=m@entry=0x7fffffffc410, mi=0x55555563dc90,
flags=flags@entry=9) at multi.c:2989
#6 0x000055555558f802 in multi_process_incoming_link
(m=m@entry=0x7fffffffc410, instance=instance@entry=0x55555563dc90,
mpp_flags=mpp_flags@entry=9) at multi.c:3361
It seems to be failing on "peer_info" being "NULL" - this used to be
optional in 2.2, and we only made it "always send something" in 2.3 with
some of the more interesting IV_ variables. If I call the 2.2 client
with "--push-peer-info", it will no longer core dump the server, but
then fail with
2020-08-04 22:08:30 us=121232 cron2-freebsd-tc-amd64-22/194.97.140.21:10872
PUSH: No common cipher between server and client. Server data-ciphers:
'AES-256-GCM:AES-128-GCM', client supports cipher 'BF-CBC'
(as in the 2.3 case below)
The culprit is this code in ncp_get_best_cipher():
if (remote_cipher == NULL || strstr(peer_info, "IV_CIPHERS="))
- that's the only place where we do not check for "peer_info != NULL"
before looking into it.
I'm not exactly sure what the code *does*, TBH, but de-fusing the check
into
if (remote_cipher == NULL
|| (peer_info && strstr(peer_info, "IV_CIPHERS=") ))
makes it no longer crash (and also pass the unit test).
This patch also breaks connections from "default 2.3" clients, though in a
different way:
Aug 4 21:56:25 gentoo tun-udp-p2mp-112-mask[25184]:
cron2-freebsd-tc-amd64-23/194.97.140.21:48168 PUSH: No common cipher between
server and client. Server data-ciphers: 'AES-256-GCM:AES-128-GCM', client
supports cipher 'BF-CBC'
ug 4 21:56:27 gentoo tun-udp-p2mp-112-mask[25184]:
cron2-freebsd-tc-amd64-23/194.97.140.21:48168 SENT CONTROL
[cron2-freebsd-tc-amd64-23]: 'AUTH_FAILED,Data channel cipher negotiation
failed (no shared cipher)' (status=1)
this is a server that has *no* "--cipher" in its config, and a client
that has nothing either and no NCP - so it advertises "OCC cipher bf-cbc",
which is no longer accepted on the server.
Is that intentional?
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
