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

Add a global option to always prompt for OTP #702

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

selvanair
Copy link
Collaborator

This is more like a quick solution for user requests such as #663.
This is built on top of #701, by adding a global option to always prompt for OTP and submit it concatenated with password. Submitting as a separate PR as I'm not fully convinced of the need for this.

The newly extended static-challenge config option (GUI support is in PR #701) that can optionally specify concatenation format should be preferred over this. Always prompting for OTP could confuse users when none are required. But the former will be available only in OpenVPN 2.7 while this is immediately usable.

@selvanair
Copy link
Collaborator Author

selvanair commented Sep 9, 2024

If you review this please ignore #701 as it's a subset of this.
Rebased on top of #701

Add a checkbox to the general settings menu
to always prompt for OTP when prompting for password
even if no static-challenge request is received from
the management interface.

The response is appeneded to the password using the
concatenation format and submitted to the management interface.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Signed-off-by: Selva Nair <selva.nair@gmail.com>
@selvanair
Copy link
Collaborator Author

Anyone thinks this is useful? After hearing that tunnelblick has had such a feature for years I'm more sympathetic to users request for this.

Copy link
Member

@lstipakov lstipakov left a comment

Choose a reason for hiding this comment

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

Looks good. I compiled it and tested that OTP prompt indeed appears if checkbox is set.

@lstipakov lstipakov merged commit c4d1139 into OpenVPN:master Sep 16, 2024
10 checks passed
@r4dius
Copy link

r4dius commented Nov 23, 2024

Hello, sorry to reopen this, but this approach only works when the OTP is added at the end of the password.

At work, all our VPN connections that use OTP require it to be added at the beginning of the password. I modified this by replacing the checkbox with radio buttons to allow a choice between "Append OTP," "Prepend OTP" (for my use case), and a third option, "Disable." Would you consider merging this?

By the way, I'm not sure if it's feasible, but it would be great if such an option could be configured on a per-VPN basis.

@selvanair
Copy link
Collaborator Author

At work, all our VPN connections that use OTP require it to be added at the beginning of the password.

This is why I was extremely reluctant in adding this option. Over the years I have seen reports of a number of ad-hoc implementations that send OTP appended to password, and we finally added support for it although I consider it a hack. I'm hearing about "prepending" for the first time, but I'm not surprised. Soon there will be some who use a ":" to separate the two and so on.

The correct approach is to do server side implementation correctly by following the static challenge protocol. If that is not an option, lobby for a new protocol in OpenVPN core and we will support it in the GUI. Right now the core supports the original static-challenge protocol and concatenation option. So does the GUI.

@r4dius
Copy link

r4dius commented Nov 24, 2024

Ahah, I'm no VPN expert, I thought both would be standard practice. I'd rather have both options than none 🤣

@cron2
Copy link
Contributor

cron2 commented Nov 24, 2024

Someone has to maintain all the different code paths for "options", and test them.

Much better is to adhere to a standard, and not invent new standards and require changes to pre-existing software to make that work.

@selvanair
Copy link
Collaborator Author

selvanair commented Nov 24, 2024

Ahah, I'm no VPN expert, I thought both would be standard practice. I'd rather have both options than none

Neither are standard practice but unnecessary hacks invented by server administrators.

Anyway, this menu item to prompt for OTP and concatenate is an interim solution until OpenVPN 2.7 is out. Once 2.7 is released, the correct way to use the concatenation option is to add --static-challenge directive to the client config file with flag set to 2 or 3 to indicate "concatenation" as the protocol. Adding new meaning to this setting that is not in the specifications is not acceptable. In fact, I think your patch would break --static-challenge option processing.

So, please follow the protocol as documented in management-notes.txt in OpenVPN 2 code base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants