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>
Parse the flag in SC:<flag>,TEXT directive for static-challenge,
and enable the option to concatenate password and response
if indicated in flag.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Static challenge response and password are optionally
concatenated and submitted instead of using the SCRV1
protocol. The code is activated in the next commit.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
If an attacker with SeImeprsonatePrivilege manages to create a
namedpipe server with a name matching that used by the "Interactive
Service", the GUI connecting to it could allow the attacker to
impersonate the GUI user.
Fix by validating the service pipe by comparing the pid of the pipe
server with that of the "Interactive Service".
Note: GetNamedPipeServerProcessId() returns the pid of the process
that created the first instance of the pipe. So, this patch only
guards against a rogue pipe instance created before the service
has started. This has to work in combination with a patch for the
service that disallows creation of additional pipe instances when
the service is running.
CVE: CVE-2024-4877
Reported by: Zeze with TeamT5 <zeze7w@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Signed-off-by: Selva Nair <selva.nair@gmail.com>
The tray icon and its tip text get updated to the connecting state when starting
a new status window thread. This is not enough for persistent connections as these
can be restarted from the hold state which does not go through a new thread creation.
To see the bug, disconnect and reconnect a persistent connection and have it wait in the
connecting state. The tray icon colour does not change to yellow and the tray tip message
does not show the "Connecting to:" text.
Fix by ensuring that CheckAndSetTrayIcon() or SetTrayIcon() gets called whenever a state
change message is received.
Fixes issue #668
Signed-off-by: Selva Nair <selva.nair@gmail.com>
- Only "hot" user input -- i.e, freshly typed password starting
from an empty string, with keyboard focus still in the edit
box -- can be revealed.
In particular, prefilled passwod (from cached value) cannot be
revealed.
- Once keyboard focus moves out of the password edit box, the inpit has
to be deleted for the reveal feature to get re-enabled.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Modal dialogs opened within the connection thread may
hang around if the connection restarts. Intercept the state
change message and call EndDialog when that happens.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
- The wait function optionally calls IsDialogMessage() if a dialog
handle is specified. For other customizations the caller can
install a WH_MSGFILTER hook. The hook will get called with
nCode = MSGF_OVPN_WAIT and lParam = &msg.
- Use this in place of Sleep in main.c, scripts.c and PLAP dll.
Fixes#576
Signed-off-by: Selva Nair <selva.nair@gmail.com>
For all dialogs in a thread, set its status window in the same
thread as the owner.
Also set the owner of message boxes appropriately instead of
using NULL. This has the side effect of some of the modal message
popups blocking access to the status Window until dismissed.
Next:
Replace Sleep by a wait that pumps messages.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
- Automatic scroll sometimes does not appear to work as expected.
Add an explicit command to scroll to current selection.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
- Also call Kill the IDT_STOP_TIMER in StatusDialogFunc. Though timers
will get destroyed during Window destruction an explicit KillTimer
in case still running is the "right" thing to do.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
* Check return value of SetProp
- If SetProp() is unsuccessful, we'll crash later when GetProp()
returns null. Add a check, log the error and close the dialog.
We could abort here, but closing the current dialog and possibly the
corresponding connection, provides a chance for the user to fix the OOM
condition which is the most likely cause of SetProp() failure.
- In pkcs11.c if SetProp() fails just do not use bold font for
header instead of leaking the font resource.
Also correct a bad fixup in commit 80697ecae6: hfontProp was not set!
Github: FixesOpenVPN/openvpn-gui#577
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Currently we use an array of connection pointers which needs
to be reallocated when space runs out. But, that happens from
the main thread while the status thread may be referring to those
pointers. Its very hard to fence against possible invalid memory
access. Instead, use a list so that connection pointer never
changes once created.
The connection list is no longer recreated from scratch even when
no connections are active. This means configs added while GUI is
running will always appear at the bottom of the root group listing
until the GUI is restarted.
TODO: This behaviour could be improved by scanning through the groups to
graft new configs at the right branch in the config-group tree.
v2: removed unused references to SetMenuStatusById()
Signed-off-by: Selva Nair <selva.nair@gmail.com>
- GetTickcount() and current-time passed-in to the callback are
both 32 bit which wraps around every ~50 days. In the stop
condition compare the elasped ticks and timeout value instead
of end-time and now. The latter can wraparound in a long-running
process.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
When connected, the daemon now reports the state as
CONNECTED,ROUTE_ERROR on routing errors that would have been
reported as CONNECTED,SUCCESS in the past.
To not overly disrupt the current behaviour we treat CONNECTED,ROUTE_ERROR
almost the same was as CONNECTED,SUCCESS except that an error is logged and
the status window is popped up if not already open for all cases other
that CONNECTED,SUCCESS.
Further, the icons on the status window, is left at yellow
and the status text is set to "Connected with routing errors"
in case of ROUTE_ERROR.
Tray and menu icons will change to green. Leaving them yellow is not
persistent as we do not yet have a state variable in the GUI that distinguishes
between "successfully connected" and "connected with route errors".
TODO: re-work this CONNECTED state handling based on how critical
ROUTE_ERROR is in real use.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
If the prefix string includes ERROR or WARNING these messages
are now shown in colour (red or yellow) similar to how log lines
are displayed.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
As we now allow users to set a management password (for persistent
connections), the max size of password should match what openvpn.exe
can handle (128 or 4096 bytes depending on build options).
Increase the buffer size to 4096 though such large passwords
may not work in practice. 127 bytes + NUL, may be a safe upper limit.
For the random password used for connections spawned by the GUI,
the current size of 15 bytes + NUL is retained.
Fixes: #567
Signed-off-by: Selva Nair <selva.nair@gmail.com>
WEB_AUTH is treated same way as OPEN_URL with no support
for optional flags in WEB_AUTH message. In particular
proxy is not supported.
Closes#414
Signed-off-by: Selva Nair <selva.nair@gmail.com>
PR 418 introduced repeatedly trying to connect to the
management interface on timeout. User can exit out of this loop
by "disconnect", but for this to work an explict
call to OnStop() is required. Normally OnStop() is triggered
by management disconnect which won't happen if not connected
in the first place.
Also call OnStop() on all iservice errors instead of waiting for
management timeout which may never happen.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
- Add a function to check flow direction of currently selected
UI language
- Add MB_RIGHT|MB_RTLREADING to message boxes when language is RTL
Note: though we use MessageBoxEx() for popups, and pass langId to it,
buttons like OK/Cancel are not automatically localized. It seems these
get localized based on the current locale, not the langID passed in.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
In case of persistent connections, openvpn.exe is still running
after a disconnect, and another user can restart it without needing
credentials using cached passwords. Avoid this by sending
"forget-passwords" to the management interface before disconnect.
Only persistent connections are affected.
In openvpn.exe versions >= 2.5.8, this will also clear cached
auth_token, if present.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
If '--management' option cannot be parsed in the config file of a
persistent profile (due to missing option, unreadable password etc.),
connecting it from the GUI menu fails.
In such cases show an error message instead of silently failing.
The message is shown only during manual connect attempts,
not during auto-connect or resume.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
- Early state change from the main thread makes it synchronous and
thus easier to wait on the connection to complete when started
programmatically.
Made use of in Connect() in the PLAP implementation that follows.
Does not affect on the current mode of operation.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
OpenVPN3 doesn't yet support "state"
management command without parameters.
While this has to be fixed on OpenVPN3
side, it doesn't mean that gui could simply crash.
Signed-off-by: Lev Stipakov <lev@openvpn.net>
(i)
State is changed to detached before auto-starting
so that OnHold() will see state = resuming and keep the hold.
State is set to disconnected instead of detached on detach
so that manual starts will release the hold automatically.
End result: While connecting automatically, do not release if
management-hold is on. But while started manually, release
from hold so that connection can complete without further
user action.
In normal use of automatic service, one would not add management
hold into the config. However, if the user disconnects the connection
the GUI puts it on hold, and we do not want to auto-start it after a
lock-unlock or some other automatic action.
(ii)
Also, currently, for persistent connections, the status
window is not shown automatically which feels unnatural in
real use. Instead, popup the status window when connection
is manually initiated. Its not popped up when automatically
attached to or if silent_connection is on.
Only persistent connections are affected by the change.
fixup: config file list is not recreated from scratch when
enable_persistent == 2 (auto attach mode) to avoid losing info
such as auto_connect = false on detached connections.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Useful for releasing the management interface if the user wants to
connect to it by other means.
Detached connections are set to state = detached (no disconnected)
and auto_connect disabled, so that they could be handled properly
during a re-attach.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Three options are provided to control scanning of persistent
(pre-satrted) connections in config-auto folder, and how they
are attached to.
Auto: Scan and list persistent connections and attach to their
management i/f automatically at startup, and periodically
retry on failure to attach.
Manual: Scan and list as above, but do not attach automatically.
User can attach to such connections by manually clicking
connect.
Never: Do not scan config-auto folder.
Default is "Auto"
Change of this setting in the settings menu will take full effect
only if none of the connections are in connecting/connected/detached
state so that the connection list can be updated. Otherwise
restart the GUI.
TODO: Copying the settings dialog changes to all languages
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Distinguish between management socket connected and
ready for interaction with the server. The former can
happen even if the server is connected to another client
and thus non-responsive.
Use manage.connected = 1 in place of true when connected
and = 2 when handshake with server completed and ready for
input.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
- Parse the management interface address and password
from the config file
- Hide the status Window by default for persistent
connections --- their startup is automated and may
distract the user otherwise. The user can use the
menu to review status when required.
- Seed srand() using threadId instead of time. Although we
use rand() only for cosmetics, the latter is almost
never unique among threads when multiple connections can
get started in a succession with this patch set.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
- Parse the config-auto folder used by automatic service
and mark these profiles as persistent.
- These connections are marked as auto_connect to try
attaching to them at start up with periodic retry in
case the daemon or service are restarted.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Persistent connections have openvpn.exe daemon started
external to the GUI (e.g., by the automatic service).
This patch adds support for attaching to the management
i/f of such daemons from the GUI and control the connection.
The GUI never stops or starts the openvpn.exe process in this
case. Instead, connect and disconnect buttons signal the
management interface of a running openvpn.exe process to start
the tunnel by attaching to mgmt i/f and sending hold-release if
needed or stop it and wait in management-hold state
(see DisconnectDaemon()).
When the GUI process exits, persistent connections are left in their
current state using DetachOpenVPN().
No connections are marked as persistent as yet. That is done
in a following commit.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
- Split starting the status thread and launch of the
daemon process (openvpn.exe) into separate functions.
This is useful for implementing control of persistent connections
where the daemon is launched externally: e.g., by the
automatic service.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Add support for selecting pkcs11-id from the GUI.
Requires --management-pkcs11-id in the config file.
This option is not added by the GUI.
A list of all available pkcs11 certificates are presented to the
user with buttons OK, Cancel, Retry. OK submits the selected
entry, Cancel closes the connection, Retry reconstructs the
list of certificates by querying the daemon again. The latter
can be used to retry after inserting a token.
If no certificates are found, a message suggesting to insert
a token and press 'Retry' is displayed.
The list shows the "Issued-to", "Issued-by" names
(usually the subject & issuer common names) and valid-until
date in current locale for each certificate.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Bind a socket and then close to identify
a free port and use it when starting openvpn.exe.
Try port = offset + config-index is first, matching
the current usage, and fallback to a dynamic port if
the former fails.
Trac: #1051
Signed-off-by: Selva Nair <selva.nair@gmail.com>
In some cases the service may take a while to startup openvpn.exe,
causing connection to the management interface to timeout. This could
leave behind the OpenVPN process if/when it eventually starts up.
(Trac 905, 1050).
As errors in starting up the OpenVPN daemon are independently
handled, its better to keep retrying the management interface connection
until aborted due to errors or by the user.
- On timeout, log a message on the status window and retry the
management interface connection
- Eliminate the timed-out state that is no longer used
- Call StopOpenVPN() before abort so that OpenVPN daemon
is not left running in case it starts up later.
- In the unlikely event that OpenManagement() fails, show an error
- User can abort by pressing disconnect
A "retrying.." message is logged on to the status window every
15 seconds.
See Trac: #905, #1050
Signed-off-by: Selva Nair <selva.nair@gmail.com>
This adds optional support for using OpenVPN3 client
as an alternative to openvpn2.
Just replacing one client with another will not work:
- OpenVPN3 doesn't use interactive service, it uses
"agent" service with completely different protocol. OpenVPN GUI
needs to talk to agent using HTTP and JSON.
- OpenVPN3 management interface realtime notifications must be
explicitly turned on in order for GUI to work.
To enable using openvpn3:
- use any of *-ovpn3 presets (cmake build system)
- ./configure --enable-ovpn3 (mingw)
To switch betweet openvpn2 and openvpn3, see "OpenVPN Engine"
radiobutton group in Settings -> Advanced dialog.
OnReady() implementation was slighly changed - "log all on"
replaced with "log on all" - according to management interface
documentation this is the right way to do it, and also OpenVPN3
only supports "on all" order.
Management interface - enabled OpenVPN3 client (omiclient.exe) and
agent (ovpnagent.exe) are now part of openvpn3 repo.
Co-authored-by: Christopher Ng <facboy@gmail.com>
Signed-off-by: Christopher Ng <facboy@gmail.com>
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Escape the username string before passing to management
interface. For other dialogs this is already done.
Move string-escape to a function and process the username
through it.
Also escape space, single quote in addition to double quote
and backslash.
Reported by: Jakob Curdes <jc@info-systems.de>
Signed-off-by: Selva Nair <selva.nair@gmail.com>
* Provide more space for challenge dialog text
We do use a re-sizeable dialog box for dynamic challenge-response
to cater for potentially long lines of challenge text. But the
space specified for the widget is enough for only a single short line
(~60 characters) of text.
Increase the horizontal and vertical space to allow for up to
two lines of ~120 characters per line.
The default size of the Window is not changed. But it is
automatically resized if the space required for the text
is longer than the window width minus some margin. The max
horizontal size of the window is capped at 640 nominal pixels
as longer text will be wrapped in to two lines.
Github issue #468
Signed-off-by: Selva Nair <selva.nair@gmail.com>
%S --> %hs in wide format strings, %ls otherwise
%s --> %ls in wide format strings, unchanged otherwise
%c --> %lc in wide format strings
Resource files together have about 970 lines affected and
were edited by looping through all with
sed -i 's/%S/%hs/g' $file
sed -i 's/%s/%ls/g' $file
All other files were manually changed (about 85 lines).
Recent versions of mingw-w64 implicitly turns on __USE_MINGW_ANSI_STDIO
if _GNU_SOURCE, _XOPEN_SOURCE etc are defined (which we do usei).
This breaks non-standard spec such as %S. Anyway, we have been
gradually getting rid of those.
MSVC builds should not be affected.
v2: multiple occurrences in same line was missed in v1 (/g missing in
sed expression). Fixed.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
When 2FA is enabled, server (such as AS)
replies with HTTP 401 and issues a challenge.
Use existing facilities to parse CRV message
and prompt user for a response, then call REST
method again with encoded response as HTTP auth password.
See https://github.com/OpenVPN/openvpn3/blob/master/doc/webauth.md#challengeresponse-authentication
for more information.
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Commit 131c75e5 ("Notify dialog windows when OpenVPN state changes") added callback
function, but forgot to specify __stdcall calling convention with CALLBACK keyword.
This is not an issue for x64 builds, but x86 requires __stdcall calling convention for callbacks,
otherwise compiler throws an error:
Error: D:\a\openvpn-gui\openvpn-gui\openvpn.c(292): error C2440: 'function': cannot convert from 'BOOL (__cdecl *)(HWND,LPARAM)' to 'WNDENUMPROC'
Reported-by: Samuli Seppänen <samuli@openvpn.net>
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Currently we show a messagebox with OK/CANCEL when response is
not required but that cannot handle state change messages.
Instead, show the "GenericPass" dialog with input disabled.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Use a custom message to pass state change notification from OpenVPN
to all top level windows in the thread. Currently only the pending auth
dialog responds to this message by closing when the state changes.
The state change could be due to timeout, errors or success via
out-of-band authentication which makes the dialog no longer valid.
The case of CR_TEXT messages that do not require a response is handled
in the next commit.
See also issue #440https://github.com/OpenVPN/openvpn-gui/issues/440
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Selva Nair