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

Update server.erb - fix proto for tcp client mode #349

Merged
merged 1 commit into from
Oct 4, 2019

Conversation

jimirocks
Copy link

Update server.erb - fix proto for tcp client mode so that it sets:
proto tcp
rather than
proto tcp-server

Update server.erb - fix proto for tcp client mode so that it sets:
proto tcp
rather than
proto tcp-server
@jimirocks
Copy link
Author

Rebased version of #230 I am afraid I am not skilled enough to add tests

@juniorsysadmin juniorsysadmin added bug Something isn't working tests-fail labels Aug 17, 2019
@jimirocks
Copy link
Author

@juniorsysadmin I have checked the failed build and it only says there is something wrong with the build config - no idea how could I fix it. Could somebody from maintainers take a look?

@Dan33l
Copy link
Member

Dan33l commented Sep 3, 2019

Hi @jimirocks i relaunched the job.

@Dan33l Dan33l removed the tests-fail label Sep 3, 2019
Copy link
Member

@Dan33l Dan33l left a comment

Choose a reason for hiding this comment

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

@jimirocks i suppose the value with directive proto changed but was previously correct when using tcp-server.
If so, do you know the version of openvpn when the value changed ? All supported OSes are with a superior version ?

@jimirocks
Copy link
Author

According to the doc https://openvpn.net/community-resources/reference-manual-for-openvpn-2-4/ the options tcp-server or tcp-client are valid only when setuping peer-to-peer. It's possible that for some time tcp-server has been accepted in plain client mode, but it never has been valid.

My version is

OpenVPN 2.4.4 x86_64-pc-linux-gnu [SSL (OpenSSL)] [LZO] [LZ4] [EPOLL] [PKCS11] [MH/PKTINFO] [AEAD] built on May 14 2019
library versions: OpenSSL 1.1.1  11 Sep 2018, LZO 2.08

Also note that the original #230 was pending here since 2017 - so in client mode the tcp-server is not correct at least for 2 years.

@Dan33l
Copy link
Member

Dan33l commented Sep 4, 2019

According to the doc https://openvpn.net/community-resources/reference-manual-for-openvpn-2-4/ the options tcp-server or tcp-client are valid only when setuping peer-to-peer.

If directive proto in the config file match with the option proto in the command line, the pointed doc does not even allow tcp as a value.
Quote:

–proto p
Use protocol p for communicating with remote host. p can be udp, tcp-client, or tcp-server.The default protocol is udp when –proto is not specified.

@Dan33l
Copy link
Member

Dan33l commented Sep 4, 2019

But in examples we have this:

If you want your OpenVPN server to listen on a TCP port instead of a UDP port, use:
proto tcp
instead of
proto udp
(If you want OpenVPN to listen on both a UDP and TCP port, you must run two separate OpenVPN instances).

@Dan33l
Copy link
Member

Dan33l commented Sep 4, 2019

During tests we already check that server is listening on tcp:
https://github.com/voxpupuli/puppet-openvpn/blob/master/spec/acceptance/openvpn_spec.rb#L87

And so, it looks that server really listen on tcp and not udp.

What is the reason you proposed the PR ? What issue did you face ?

@Dan33l
Copy link
Member

Dan33l commented Sep 4, 2019

proto with only tcp looks allowed in the code:
https://github.com/OpenVPN/openvpn/blob/master/src/openvpn/socket.h#L569

it is probably for convenience because we can have case where the code force to specify tcp-server or tcp-client:
https://github.com/OpenVPN/openvpn/blob/master/src/openvpn/options.c#L2007

@jimirocks
Copy link
Author

well, I use puppet-openvpn to generate this config

remote liberec.smarteon.cz
ca /etc/openvpn/liberec/keys/ca.crt
cert /etc/openvpn/liberec/keys/liberec.crt
key /etc/openvpn/liberec/keys/liberec.key
proto tcp-server
nobind
tls-client
group nogroup
user nobody
status /var/log/openvpn/liberec-status.log
dev tun0
route 192.168.2.0 255.255.255.0 192.168.87.1
topology net30
cipher AES-256-CBC
tls-cipher TLS-DHE-RSA-WITH-AES-256-GCM-SHA384:TLS-DHE-RSA-WITH-AES-256-CBC-SHA256:TLS-DHE-RSA-WITH-AES-128-GCM-SHA256:TLS-DHE-RSA-WITH-AES-128-CBC-SHA256

# Additional custom options
auth-user-pass /etc/openvpn/liberec/login
askpass /etc/openvpn/liberec/keys/liberec.pass
script-security 2
auth-nocache    

The openvpn doesn't start unless I replace tcp-server with tcp

Here is my puppet code (including workaroudn for the issue):

openvpn::server { $connect_home:
		remote         => [ $connect_host ],
		proto          => 'tcp',
		tls_client     => true,
		nobind         => true,
		compression    => '',
		local          => '',
		ns_cert_type   => false,
		route		   => [ "192.168.2.0 255.255.255.0 192.168.87.1" ],
		custom_options => {
			'auth-user-pass'  => $login_file,
			'askpass'         => $client_key_pass_file,
			'script-security' => 2,
			'auth-nocache'    => undef
		},
		require        => Class[$depends_on_class]
	}
	-> # see https://github.com/voxpupuli/puppet-openvpn/pull/230
	file_line { "workaround tcp proto issue":
		ensure => present,
		path   => "$openvpn_etc/$connect_home.conf",
		line   => 'proto tcp',
		match  => 'proto tcp-server'
	}
	->
	exec { "restart openvpn once more":
		command => "/bin/systemctl restart openvpn@$connect_home"
	}

@Dan33l
Copy link
Member

Dan33l commented Sep 4, 2019

Oh! On a server, a remote directive is used. And this is a client directive.
So probably, in case of a server used also as a client, the proto directive can not be either tcp-server or tcp-client but tcp.

We saw that openvpn code can force to specify tcp-server or tcp-client, can you set the value of directive proto by testing if remote is used of not ?

Probably others client options like nobind or tls-client should be also tested.

@jimirocks
Copy link
Author

Hi, sorry for inactivity cause of vacation.

Well, I am not really sure what are you asking me to try... could you be more specific in terms of code sample?

Copy link
Member

@Dan33l Dan33l left a comment

Choose a reason for hiding this comment

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

don't worry. It is not important. The PR LGTM.

@Dan33l Dan33l merged commit b8b8035 into voxpupuli:master Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants