I have stared at the code a bit, and it generally looks good (indent
fixed as instructed).

One observation:

 - in options_postprocess_cipher(), we now set "o->enable_ncp_fallback 
   = true", but *only* if a "cipher foo" is set in the config.  If not, we 
   set the cipher to o->ciphername = "BF-CBC", but disallow ncp-fallback
   - not sure if this is part of the crusade against BF-CBC "as default",
   but it looks a bit weird (we *set* BF-CBC as default, but do not want 
   to use it).


Testing this in the server side testbed yielded interesting problems
for the two p2p test cases, when connecting from a "pre P2P NCP" client
(master, but a few months old):

** "9" is "--client" to "--tls-server" **

With a "plain" config (no data-ciphers or data-cipher-fallback) the
connection works fine, both ends negotiate AES-256-GCM (= NCP works)

   P2P mode NCP negotiation result: TLS_export=0, DATA_v2=0, peer-id 0, 
cipher=AES-256-GCM
   Data Channel: using negotiated cipher 'AES-256-GCM'
   Data Channel MTU parms [ L:1468 D:1450 EF:-64 EB:392 ET:32 EL:3 AF:14/8 ]
   Outgoing Data Channel: Cipher 'AES-256-GCM' initialized with 256 bit key
   SENT CONTROL [cron2-freebsd-tc-amd64]: 'PUSH_REPLY,route 10.204.0.0 
255.255.0.0 10.204.9.1,route-ipv6 fd00:abcd:204::/48,cipher AES-256-GCM' 
(status=1)

.. but then:

   TCP/UDP packet too large on write to [AF_INET6]::ffff:194.97.140.21:36144 
(tried=1504,max=1468)

so the frame calculation is wrong, and pinging with large packets fails.

If I force the server to "no AES" (data-ciphers BF-CBC), it works:

   P2P mode NCP negotiation result: TLS_export=0, DATA_v2=0, peer-id 0, 
cipher=BF-CBC
   Outgoing Data Channel: Cipher 'BF-CBC' initialized with 128 bit key
   SENT CONTROL [cron2-freebsd-tc-amd64]: 'PUSH_REPLY,route 10.204.0.0 
255.255.0.0 10.204.9.1,route-ipv6 fd00:abcd:204::/48,cipher BF-CBC' (status=1)

Notably it's not telling me anything about "using negotiated cipher",
and also nothing about the MTU params.


** "9a" is "--tls-client" (no -pull) to "--tls-server" **

Using the config "without --data-ciphers or --data-cipher-fallback",
incoming connections fail:

    P2P mode NCP negotiation result: TLS_export=0, DATA_v2=0, peer-id 0, 
cipher=(not negotiated, fallback-cipher: none)
    TLS Error: local/remote TLS keys are out of sync: 
[AF_INET6]2001:608:0:814::f000:21:36664 (received key id: 0, known key ids:  
[key#0 state=S_ACTIVE auth=KS_AUTH_TRUE id=0 sid=9daa0f59 9d7bf877] [key#1 
state=S_UNDEF auth=KS_AUTH_FALSE id=0 sid=00000000 00000000] [key#2 
state=S_UNDEF auth=KS_AUTH_FALSE id=0 sid=00000000 00000000]
    ERROR: failed to negotiate cipher with peer and --data-ciphers-fallback not 
enabled. No usable data channel cipher
    ERROR: Failed to apply P2P negotiated protocol options
    TLS Error: local/remote TLS keys are out of sync: 
[AF_INET6]2001:608:0:814::f000:21:36664 (received key id: 0, known key ids:  
[key#0 state=S_ACTIVE auth=KS_AUTH_TRUE id=0 sid=9daa0f59 9d7bf877] [key#1 
state=S_UNDEF auth=KS_AUTH_FALSE id=0 sid=00000000 00000000] [key#2 
state=S_UNDEF auth=KS_AUTH_FALSE id=0 sid=00000000 00000000])

but the server does not actively terminate the connection - so we'll
see "TLS Error" errors scroll through, masking the actual error.

Adding "data-ciphers-fallback BF-CBC" makes this test instance well-behaved
(though there still is *one* TLS error):

    P2P mode NCP negotiation result: TLS_export=0, DATA_v2=0, peer-id 0, 
cipher=(not negotiated, fallback-cipher: none)
    TLS Error: local/remote TLS keys are out of sync: 
[AF_INET6]2001:608:0:814::f000:21:47925 (received key id: 0, known key ids:  
[key#0 state=S_ACTIVE auth=KS_AUTH_TRUE id=0 sid=bfa5ccf1 c5c5e901] [key#1 
state=S_UNDEF auth=KS_AUTH_FALSE id=0 sid=00000000 00000000] [key#2 
state=S_UNDEF auth=KS_AUTH_FALSE id=0 sid=00000000 00000000])
    Outgoing Data Channel: Cipher 'BF-CBC' initialized with 128 bit key

("Fallback-cipher: none" looks a bit bogus, though... this might be
related to BF-CBC not being initialized early, or something)


The first case definitely needs fixing (I have more failing "frame" test
cases...), the second case could need some polishing - abort the session,
so it's clear "*this* is the error".


Connecting to the same instances from a client with P2P NCP yielded
different results (unsurprisingly) :-)

** "9" is "--client" to "--tls-server" **

If the server is configured to "data-ciphers BF-CBC", this works.  If
AES is added to the cipher list, the client's IV_CIPHER= wins, and they
negotiate AES-256-GCM...

   peer info: IV_CIPHERS=AES-256-GCM:AES-128-GCM
   peer info: IV_PROTO=30
   P2P mode NCP negotiation result: TLS_export=0, DATA_v2=0, peer-id 0, 
cipher=AES-256-GCM
   Data Channel: using negotiated cipher 'AES-256-GCM'
   Data Channel MTU parms [ L:1504 D:1450 EF:-28 EB:398 ET:32 EL:3 ]

.. and it fails:

   TCP/UDP packet too large on write to [AF_INET6]2001:608:4::ce:c0f:51605 
(tried=1536,max=1504)

forcing the client to "--data-ciphers BF-CBC" makes them "negotiate"
BF-CBC, sidestepping the frame problem.

(I assume that having "--cipher AES-256-GCM" in both client and server
configs upfront would also solve this, but "having to add a cipher on
both sides to make NCP work" sounds like the wrong way forward)


** "9a" is "--tls-client" (no -pull) to "--tls-server" **

Given the changes to the server.conf ("data-ciphers BF-CBC") to work
around the "frame" problem, this now fails with

    peer info: IV_CIPHERS=BF-CBC
    peer info: IV_PROTO=42
    P2P mode NCP negotiation result: TLS_export=1, DATA_v2=1, peer-id 16371030, 
cipher=(not negotiated, fallback-cipher: none)
    ERROR: failed to negotiate cipher with peer and --data-ciphers-fallback not 
enabled. No usable data channel cipher
    ERROR: Failed to apply P2P negotiated protocol options

(but the client is not aborting!) - extending --data-ciphers makes it 
connect fine:

    peer info: IV_CIPHERS=BF-CBC:AES-256-GCM:AES-128-GCM
    P2P mode NCP negotiation result: TLS_export=1, DATA_v2=1, peer-id 14972877, 
cipher=AES-256-GCM

.. but then we are back into the "frame problem" land...

    TCP/UDP packet too large on write to 
[AF_INET6]2001:608:0:814::f000:11:51204 (tried=1539,max=1471)

So the only way to make this work is to restrict the client to BF-CBC,
as with the --client test case.


Your patch has been applied to the master branch.

commit 8c72d7981c32c43d1c7967a312bb439e13fc5b40
Author: Arne Schwabe
Date:   Wed Jul 28 14:30:50 2021 +0200

     Support NCP in pure P2P VPN setups

     Signed-off-by: Arne Schwabe <a...@rfc2549.org>
     Acked-by: Antonio Quartulli <anto...@openvpn.net>
     Message-Id: <20210728123050.564595-1-a...@rfc2549.org>
     URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22671.html
     Signed-off-by: Gert Doering <g...@greenie.muc.de>


--
kind regards,

Gert Doering



_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to