Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

T5148: Fix OpenVPN plugin dir variable #1945

Merged
merged 1 commit into from
Apr 10, 2023
Merged

Conversation

sever-sever
Copy link
Member

Change Summary

Jinja2 template uses {{ plugin_dir }} that it gets from the interface-openvpn.py variable plugin_dir but the correct var should be as part of 'openvpn' dictionary i.e. openvpn['plugin_dir']

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Component(s) name

Proposed changes

How to test

OpenVPN configuration:

set interfaces openvpn vtun10 description 'Server foo'
set interfaces openvpn vtun10 encryption cipher 'aes256'
set interfaces openvpn vtun10 hash 'sha512'
set interfaces openvpn vtun10 local-host '203.0.113.1'
set interfaces openvpn vtun10 local-port '1194'
set interfaces openvpn vtun10 mode 'server'
set interfaces openvpn vtun10 persistent-tunnel
set interfaces openvpn vtun10 protocol 'udp'
set interfaces openvpn vtun10 server client client1 ip '10.10.0.10'
set interfaces openvpn vtun10 server domain-name 'vyos.net'
set interfaces openvpn vtun10 server max-connections '250'
set interfaces openvpn vtun10 server mfa totp
set interfaces openvpn vtun10 server name-server '172.16.254.30'
set interfaces openvpn vtun10 server subnet '10.10.0.0/24'
set interfaces openvpn vtun10 server topology 'subnet'
set interfaces openvpn vtun10 tls ca-certificate 'ca'
set interfaces openvpn vtun10 tls certificate 'cert'
set interfaces openvpn vtun10 tls dh-params 'dh'
set interfaces openvpn vtun10 tls tls-version-min '1.0'

Before fix we get incorrect plugin_path

plugin "/openvpn-otp.so" "otp_secrets=/config/auth/openvpn/vtun10-otp-secrets otp_slop=180 totp_t0=0 totp_step=30 totp_digits=6 password_is_cr=1"

And OpenVPN cannot start:

Apr 10 13:17:38 r14 openvpn-vtun10[13114]: PLUGIN_INIT: could not load plugin shared object /openvpn-otp.so: /openvpn-otp.so: cannot open shared object file: No such file or directory: No such file or directory (errno=2)
Apr 10 13:17:38 r14 openvpn-vtun10[13114]: Exiting due to fatal error
Apr 10 13:17:38 r14 systemd[1]: openvpn@vtun10.service: Main process exited, code=exited, status=1/FAILURE
Apr 10 13:17:38 r14 systemd[1]: openvpn@vtun10.service: Failed with result 'exit-code'.
Apr 10 13:17:38 r14 systemd[1]: Failed to start openvpn@vtun10.service - OpenVPN connection to vtun10

After fix:

plugin "/usr/lib/openvpn/openvpn-otp.so" "otp_secrets=/config/auth/openvpn/vtun10-otp-secrets otp_slop=180 totp_t0=0 totp_step=30 totp_digits=6 password_is_cr=1"

Show interfaces

vyos@r14# run show interfaces openvpn 
Codes: S - State, L - Link, u - Up, D - Down, A - Admin Down
Interface        IP Address                        S/L  Description
---------        ----------                        ---  -----------
vtun10           10.10.0.1/24                      u/u  Server foo

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

Jinja2 template uses {{ plugin_dir }} that it gets from the
interface-openvpn.py variable 'plugin_dir' but the correct var
should be as part of 'openvpn' dictionary i.e. openvpn['plugin_dir']
@vyosbot vyosbot requested a review from a team April 10, 2023 10:42
@vyosbot vyosbot requested review from dmbaturin, sarthurdev, zdc, jestabro and c-po and removed request for a team April 10, 2023 10:42
Copy link
Member

@sarthurdev sarthurdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably have a smoketest for the OTP plugin too.

@dmbaturin dmbaturin merged commit 0ae6ad7 into vyos:current Apr 10, 2023
@dmbaturin
Copy link
Member

@sarthurdev Smoke tests for OTP is a good point indeed.

@sever-sever sever-sever deleted the T5148 branch April 10, 2023 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants