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

Small inconveniences with the peer-fingerprint option #516

Closed
vlk-charles opened this issue Mar 8, 2024 · 2 comments
Closed

Small inconveniences with the peer-fingerprint option #516

vlk-charles opened this issue Mar 8, 2024 · 2 comments
Labels

Comments

@vlk-charles
Copy link

vlk-charles commented Mar 8, 2024

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

  • OS: Fedora 39
  • OpenVPN version: 2.6.9 (-1.fc39.x86_64)

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

@cron2
Copy link
Contributor

cron2 commented Mar 8, 2024

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.

@cron2 cron2 added the bug label Mar 8, 2024
@vlk-charles
Copy link
Author

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?

cron2 pushed a commit that referenced this issue Mar 26, 2024
…fication

Github: fixes #516

Change-Id: Ia73d53002f4ba2658af18c17cce1b68f79de5781
Signed-off-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240326103853.494572-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28474.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 4b95656)
@cron2 cron2 closed this as completed in 4b95656 Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants