search

OpenVPN / openvpn

OpenVPN is an open source VPN daemon
http://openvpn.net
Other
10.25k stars 2.92k forks source link

Small inconveniences with the `peer-fingerprint` option #516

Closed vlk-charles closed 3 months ago

vlk-charles commented 3 months ago

Describe the bug The peer-fingerprint option logs a badly formatted line and the supplied fingerprint requires colons.

To Reproduce Fingerprint format error:

$ openvpn --remote example.com --dev tun --client --auth-user-pass --tls-exit --peer-fingerprint 9d898358c658068745fe6226163ed911f914486d0c8b204b8799758ad4aa3554
Options error: format error in hash fingerprint: 9d898358c658068745fe6226163ed911f914486d0c8b204b8799758ad4aa3554

Use a random wrong fingerprint to see the bad string:

$ openvpn --remote example.com --dev tun --client --auth-user-pass --tls-exit --peer-fingerprint 9d:89:83:58:c6:58:06:87:45:fe:62:26:16:3e:d9:11:f9:14:48:6d:0c:8b:20:4b:87:99:75:8a:d4:aa:35:54
[...]
2024-03-07 12:02:42 TLS Error: --tls-verify/--peer-fingerprintcertificate hash verification failed. (got fingerprint: 9a:26:3e:4e:a3:9c:73:af:1d:7e:1f:d1:6a:b8:8f:61:29:26:ed:a7:42:d0:37:f9:4d:0c:9c:20:fc:34:3e:da
[...]

Expected behavior Colons to be optional as they add no meaning and the verification error string to contain an extra space and closing parenthesis (or none at all) like this:

2024-03-07 12:02:42 TLS Error: --tls-verify/--peer-fingerprint certificate hash verification failed. (got fingerprint: 9a:26:3e:4e:a3:9c:73:af:1d:7e:1f:d1:6a:b8:8f:61:29:26:ed:a7:42:d0:37:f9:4d:0c:9c:20:fc:34:3e:da)

Version information

Additional context For example neither sha256sum or openssl dgst -sha256 use colons in their outputs.

cron2 commented 3 months ago

The fingerprint examples in doc/man-sections/example-fingerprint.rst suggest to use openssl x509 -fingerprint -sha256 -in server.crt -noout, which in my tests produces an output with colons just fine.

Our parser tries to err on the side of "being too strict", in general, thus the colons are not optional.

On the formatting of the text message - indeed, this needs to be fixed.

vlk-charles commented 3 months ago

I understand if it is safer to require colons. Ultimately it is the developers' decision. I just wanted to bring it to attention that it can be inconvenient for the user. I agree that most certificate-oriented tools do use colons.

But I also have another suggestion. The following warning is shown even when peer-fingerprint is in use:

WARNING: No server certificate verification method has been enabled.  See http://openvpn.net/howto.html#mitm for more info.

Isn't supplying the fingerprint a verification method in a way?