Opened 5 years ago

Closed 4 years ago

#1140 closed Bug / Defect (fixed)

mssfix appears to be broken with NCP

Reported by: stipa Owned by: stipa
Priority: major Milestone:
Component: Generic / unclassified Version:
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords:
Cc: Gert Döring, Steffan Karger

Description

Steps to reproduce:

  • OpenVPN 2.4
  • fragment 1300/mssfix to client/server configs
  • establish connection, start downloading large file (or speedtest.net)
  • run "sudo tcpdump port 1194" on a server

Actual results:

  • TCP packets are fragmented:
15:23:36.578336 IP xxx.openvpn > yyy.openvpn: UDP, length 652
15:23:36.578441 IP xxx.openvpn > yyy.openvpn: UDP, length 648
15:23:36.578525 IP xxx.openvpn > yyy.openvpn: UDP, length 652
15:23:36.578591 IP xxx.openvpn > yyy.openvpn: UDP, length 648

Expected results:

  • TCP packets should not be fragmented. Below result of tcpdump with

"ncp-disable" added to server config:

15:27:00.099286 IP xxx.openvpn > yyy.openvpn: UDP, length 1216
15:27:00.101603 IP xxx.openvpn > yyy.openvpn: UDP, length 1216
15:27:00.101729 IP xxx.openvpn > yyy.openvpn: UDP, length 1216
15:27:00.104219 IP xxx.openvpn > yyy.openvpn: UDP, length 1216

From mss_fixup_dowork():

NCP on: MSS: 1460 -> 1207
NCP off: MSS: 1460 -> 1135

Attachments (1)

0001-Fix-broken-fragment-mssfix-with-NCP.patch (4.9 KB) - added by stipa 5 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 5 years ago by stipa

The problem seems to be caused by fragment_frame holding
incorrect "extra_frame" value. Initially it is based on
max crypto overhead.

After NCP, we adjust extra_frame value in frame and use it in
mss_fixup_dowork(). However resulting packet size exceeds
fragment size, which is calculated with incorrect
fragment_frame->extra_frame. Because fragment size is
exceeded, and we perform unneeded fragmentation.

As a fix, fragment_frame->frame_extra should be also updated on NCP negotiation.

Last edited 5 years ago by stipa (previous) (diff)

comment:2 Changed 5 years ago by stipa

Cc: Gert Döring Steffan Karger added

comment:3 Changed 5 years ago by tct

cc

comment:4 Changed 4 years ago by stipa

Server config:

port 1194
proto udp
dev tun
ca /etc/openvpn/ca.crt
cert /etc/openvpn/server.crt
key /etc/openvpn/server.key
dh /etc/openvpn/dh2048.pem
topology subnet
server 10.8.0.0 255.255.255.0
verb 4
keepalive 30 180
fragment 1300
mssfix

Client config:

client
proto udp
dev tun
fragment 1300
mssfix
remote server 1194
<ca>
xxx
</ca>
<cert>
xxx
</cert>
<key>
xxx
</key>
verb 4

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

Resolution: fixed
Status: assignedclosed

Thanks for the details.

I think this can now be closed :-)

Release/2.4 has

commit 3bd91cd0e68762b861c57cf37f144d8a11704e9d (HEAD -> release/2.4, stable/release/2.4, mattock/release/2.4, gitlab/release/2.4, github/release/2.4)
Author: Lev Stipakov <lev@…>
Date: Wed Oct 30 14:44:59 2019 +0200

Fix broken fragmentation logic when using NCP


This is the 2.4 backport of master patch (commit d22ba6b).

and master has

commit d22ba6b2c551fa83d23b5cf668e08a08fde446bc
Author: Lev Stipakov <lev@…>
Date: Mon Jan 21 22:04:54 2019 +0200

Fix broken fragment/mssfix with NCP

Note: See TracTickets for help on using tickets.