From b8356b60a6989f02d070c69d9626381264bdfadd Mon Sep 17 00:00:00 2001 From: "Quentin McGaw (desktop)" Date: Sat, 18 Sep 2021 16:09:21 +0000 Subject: [PATCH] Maint: use `OPENVPN_PORT` instead of `PORT` with retro-compatibility --- Dockerfile | 2 +- internal/configuration/ivpn.go | 7 +- internal/configuration/ivpn_test.go | 73 +++++++++++-------- internal/configuration/mullvad.go | 7 +- .../configuration/privateinternetaccess.go | 4 +- internal/configuration/provider.go | 40 +++++++--- internal/configuration/selection.go | 2 +- internal/configuration/windscribe.go | 8 +- 8 files changed, 89 insertions(+), 54 deletions(-) diff --git a/Dockerfile b/Dockerfile index e8917bc3..38976a24 100644 --- a/Dockerfile +++ b/Dockerfile @@ -84,7 +84,7 @@ ENV VPNSP=pia \ OPENVPN_IPV6=off \ OPENVPN_CUSTOM_CONFIG= \ OPENVPN_INTERFACE=tun0 \ - PORT= \ + OPENVPN_PORT= \ # Wireguard WIREGUARD_PRIVATE_KEY= \ WIREGUARD_PRESHARED_KEY= \ diff --git a/internal/configuration/ivpn.go b/internal/configuration/ivpn.go index 075b44aa..8d4084b9 100644 --- a/internal/configuration/ivpn.go +++ b/internal/configuration/ivpn.go @@ -50,8 +50,11 @@ func (settings *OpenVPNSelection) readIVPN(r reader) (err error) { return err } - settings.CustomPort, err = readOpenVPNCustomPort(r.env, settings.TCP, - []uint16{80, 443, 1443}, []uint16{53, 1194, 2049, 2050}) + settings.CustomPort, err = readOpenVPNCustomPort(r, openvpnPortValidation{ + tcp: settings.TCP, + allowedTCP: []uint16{80, 443, 1443}, + allowedUDP: []uint16{53, 1194, 2049, 2050}, + }) if err != nil { return err } diff --git a/internal/configuration/ivpn_test.go b/internal/configuration/ivpn_test.go index 567498c1..96574142 100644 --- a/internal/configuration/ivpn_test.go +++ b/internal/configuration/ivpn_test.go @@ -40,17 +40,18 @@ func Test_Provider_readIvpn(t *testing.T) { //nolint:gocognit } testCases := map[string]struct { - targetIP singleStringCall - countries sliceStringCall - cities sliceStringCall - isps sliceStringCall - hostnames sliceStringCall - protocol singleStringCall - ovpnPort portCall - wgPort portCall - wgOldPort portCall - settings Provider - err error + targetIP singleStringCall + countries sliceStringCall + cities sliceStringCall + isps sliceStringCall + hostnames sliceStringCall + protocol singleStringCall + ovpnPort portCall + ovpnOldPort portCall + wgPort portCall + wgOldPort portCall + settings Provider + err error }{ "target IP error": { targetIP: singleStringCall{call: true, value: "something", err: errDummy}, @@ -120,32 +121,34 @@ func Test_Provider_readIvpn(t *testing.T) { //nolint:gocognit settings: Provider{ Name: constants.Ivpn, }, - err: errors.New("environment variable PORT: dummy test error"), + err: errors.New("environment variable OPENVPN_PORT: dummy test error"), }, "wireguard custom port error": { - targetIP: singleStringCall{call: true}, - countries: sliceStringCall{call: true}, - cities: sliceStringCall{call: true}, - isps: sliceStringCall{call: true}, - hostnames: sliceStringCall{call: true}, - protocol: singleStringCall{call: true}, - ovpnPort: portCall{getCall: true, getValue: "0"}, - wgPort: portCall{getCall: true, getErr: errDummy}, + targetIP: singleStringCall{call: true}, + countries: sliceStringCall{call: true}, + cities: sliceStringCall{call: true}, + isps: sliceStringCall{call: true}, + hostnames: sliceStringCall{call: true}, + protocol: singleStringCall{call: true}, + ovpnPort: portCall{getCall: true, getValue: "0"}, + ovpnOldPort: portCall{getCall: true, getValue: "0"}, + wgPort: portCall{getCall: true, getErr: errDummy}, settings: Provider{ Name: constants.Ivpn, }, err: errors.New("environment variable WIREGUARD_ENDPOINT_PORT: dummy test error"), }, "default settings": { - targetIP: singleStringCall{call: true}, - countries: sliceStringCall{call: true}, - cities: sliceStringCall{call: true}, - isps: sliceStringCall{call: true}, - hostnames: sliceStringCall{call: true}, - protocol: singleStringCall{call: true}, - ovpnPort: portCall{getCall: true, getValue: "0"}, - wgPort: portCall{getCall: true, getValue: "0"}, - wgOldPort: portCall{getCall: true, getValue: "0"}, + targetIP: singleStringCall{call: true}, + countries: sliceStringCall{call: true}, + cities: sliceStringCall{call: true}, + isps: sliceStringCall{call: true}, + hostnames: sliceStringCall{call: true}, + protocol: singleStringCall{call: true}, + ovpnPort: portCall{getCall: true, getValue: "0"}, + ovpnOldPort: portCall{getCall: true, getValue: "0"}, + wgPort: portCall{getCall: true, getValue: "0"}, + wgOldPort: portCall{getCall: true, getValue: "0"}, settings: Provider{ Name: constants.Ivpn, }, @@ -218,13 +221,21 @@ func Test_Provider_readIvpn(t *testing.T) { //nolint:gocognit Return(testCase.protocol.value, testCase.protocol.err) } if testCase.ovpnPort.getCall { - env.EXPECT().Get("PORT", gomock.Any()). + env.EXPECT().Get("OPENVPN_PORT", gomock.Any()). Return(testCase.ovpnPort.getValue, testCase.ovpnPort.getErr) } if testCase.ovpnPort.portCall { - env.EXPECT().Port("PORT"). + env.EXPECT().Port("OPENVPN_PORT"). Return(testCase.ovpnPort.portValue, testCase.ovpnPort.portErr) } + if testCase.ovpnOldPort.getCall { + env.EXPECT().Get("PORT", gomock.Any()). + Return(testCase.ovpnOldPort.getValue, testCase.ovpnOldPort.getErr) + } + if testCase.ovpnOldPort.portCall { + env.EXPECT().Port("PORT"). + Return(testCase.ovpnOldPort.portValue, testCase.ovpnOldPort.portErr) + } if testCase.wgPort.getCall { env.EXPECT().Get("WIREGUARD_ENDPOINT_PORT", gomock.Any()). Return(testCase.wgPort.getValue, testCase.wgPort.getErr) diff --git a/internal/configuration/mullvad.go b/internal/configuration/mullvad.go index 3d2fc8b7..1c888bc7 100644 --- a/internal/configuration/mullvad.go +++ b/internal/configuration/mullvad.go @@ -55,8 +55,11 @@ func (settings *OpenVPNSelection) readMullvad(r reader) (err error) { return err } - settings.CustomPort, err = readOpenVPNCustomPort(r.env, settings.TCP, - []uint16{80, 443, 1401}, []uint16{53, 1194, 1195, 1196, 1197, 1300, 1301, 1302, 1303, 1400}) + settings.CustomPort, err = readOpenVPNCustomPort(r, openvpnPortValidation{ + tcp: settings.TCP, + allowedTCP: []uint16{80, 443, 1401}, + allowedUDP: []uint16{53, 1194, 1195, 1196, 1197, 1300, 1301, 1302, 1303, 1400}, + }) if err != nil { return err } diff --git a/internal/configuration/privateinternetaccess.go b/internal/configuration/privateinternetaccess.go index 4a7d3a07..cafaf193 100644 --- a/internal/configuration/privateinternetaccess.go +++ b/internal/configuration/privateinternetaccess.go @@ -53,9 +53,9 @@ func (settings *OpenVPNSelection) readPrivateInternetAccess(r reader) (err error return err } - settings.CustomPort, err = readPortOrZero(r.env, "PORT") + settings.CustomPort, err = readOpenVPNCustomPort(r, openvpnPortValidation{allAllowed: true}) if err != nil { - return fmt.Errorf("environment variable PORT: %w", err) + return err } return nil diff --git a/internal/configuration/provider.go b/internal/configuration/provider.go index 37966bd7..70713706 100644 --- a/internal/configuration/provider.go +++ b/internal/configuration/provider.go @@ -149,33 +149,49 @@ func readTargetIP(env params.Interface) (targetIP net.IP, err error) { return targetIP, nil } -func readOpenVPNCustomPort(env params.Interface, tcp bool, - allowedTCP, allowedUDP []uint16) (port uint16, err error) { - port, err = readPortOrZero(env, "PORT") +type openvpnPortValidation struct { + allAllowed bool + tcp bool + allowedTCP []uint16 + allowedUDP []uint16 +} + +func readOpenVPNCustomPort(r reader, validation openvpnPortValidation) ( + port uint16, err error) { + port, err = readPortOrZero(r.env, "OPENVPN_PORT") if err != nil { - return 0, fmt.Errorf("environment variable PORT: %w", err) + return 0, fmt.Errorf("environment variable OPENVPN_PORT: %w", err) } else if port == 0 { - return 0, nil + // Try using old variable name + port, err = readPortOrZero(r.env, "PORT") + if err != nil { + r.onRetroActive("PORT", "OPENVPN_PORT") + return 0, fmt.Errorf("environment variable PORT: %w", err) + } } - if tcp { - for i := range allowedTCP { - if allowedTCP[i] == port { + if port == 0 || validation.allAllowed { + return port, nil + } + + if validation.tcp { + for _, allowedPort := range validation.allowedTCP { + if port == allowedPort { return port, nil } } return 0, fmt.Errorf( "environment variable PORT: %w: port %d for TCP protocol, can only be one of %s", - ErrInvalidPort, port, portsToString(allowedTCP)) + ErrInvalidPort, port, portsToString(validation.allowedTCP)) } - for i := range allowedUDP { - if allowedUDP[i] == port { + for _, allowedPort := range validation.allowedUDP { + if port == allowedPort { return port, nil } } return 0, fmt.Errorf( "environment variable PORT: %w: port %d for UDP protocol, can only be one of %s", - ErrInvalidPort, port, portsToString(allowedUDP)) + ErrInvalidPort, port, portsToString(validation.allowedUDP)) } // note: set allowed to an empty slice to allow all valid ports diff --git a/internal/configuration/selection.go b/internal/configuration/selection.go index 67689344..0b6218da 100644 --- a/internal/configuration/selection.go +++ b/internal/configuration/selection.go @@ -142,7 +142,7 @@ func (settings *OpenVPNSelection) readProtocolAndPort(r reader) (err error) { return err } - settings.CustomPort, err = readPortOrZero(r.env, "PORT") + settings.CustomPort, err = readOpenVPNCustomPort(r, openvpnPortValidation{allAllowed: true}) if err != nil { return fmt.Errorf("environment variable PORT: %w", err) } diff --git a/internal/configuration/windscribe.go b/internal/configuration/windscribe.go index a4340e11..eb3f696d 100644 --- a/internal/configuration/windscribe.go +++ b/internal/configuration/windscribe.go @@ -46,9 +46,11 @@ func (settings *OpenVPNSelection) readWindscribe(r reader) (err error) { return err } - settings.CustomPort, err = readOpenVPNCustomPort(r.env, settings.TCP, - []uint16{21, 22, 80, 123, 143, 443, 587, 1194, 3306, 8080, 54783}, - []uint16{53, 80, 123, 443, 1194, 54783}) + settings.CustomPort, err = readOpenVPNCustomPort(r, openvpnPortValidation{ + tcp: settings.TCP, + allowedTCP: []uint16{21, 22, 80, 123, 143, 443, 587, 1194, 3306, 8080, 54783}, + allowedUDP: []uint16{53, 80, 123, 443, 1194, 54783}, + }) if err != nil { return err }