* 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>
As with CRV1, submit an empty string as the response.
Our base64-encode functiton can handle empty input to generate
an empty string as output.
Also make ensure the message box is shown in foreground,
and not dependent on the status window which may be hidden.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
This adds support for crtext method of pending authentication,
used by Access Server 2.7 and newer.
When enabled on the server side and on the client side (IV_SSO=crtext),
server returns AUTH_PENDING with Info command like:
CR_TEXT:R,E:Enter Authenticator Code
Client prompts user for the response and sends base64-encoded response
to the server via management interface command:
cr-response SGFsbG8gV2VsdCE=
See https://github.com/OpenVPN/openvpn/blob/master/doc/management-notes.txt (crtext part)
for more information.
Signed-off-by: Lev Stipakov <lev@openvpn.net>
This adds support for web-based extra authentication, which may be
used by OpenVPN Cloud. When enabled and client sends IV_SSO=openurl,
server pushes Info command OPEN_URL:<url>. The client opens that URL and
user authenticates.
Signed-off-by: Lev Stipakov <lev@openvpn.net>
In this case, as per management-notes.txt, a CRV1 response with
an empty password should be submitted. Currently we ignore
such "challenges" causing the regular user-auth dialog to be shown
instead.
Fix by displaying the message received from the server. Depending
on user action (OK or CANCEL), a properly formatted reponse with an
empty password is returned or the connection is aborted.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
In private key passphrase and dynamic-challenge/pkcs11 PIN
dialogs:
- Disable the OK button by default
- Require non-empty user input before the OK button is enabled
Signed-off-by: Selva Nair <selva.nair@gmail.com>
We had earlier supported blank passwords or OTPs to be submitted. Change
this by enabling the OK button only if some minimal inputs are present.
- In static challenge dialog require username and either password or
challenge-reponse (OTP) fields to be non-empty
- In normal user-auth dialog require username and password to be non-empty
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Process four new echo commands to construct messages to be
displayed to the user:
echo msg message-text
echo msg-n message-text
echo msg-window message-title
echo msg-notify message-title
Note: All rules of push and echo processing apply and determine
what is received as echo commands by the GUI. In addition,
'url-encoded' characters (% followed by two hex digits) are
decoded and displayed.
The message is constructed in the GUI by concatenating the text
specified in one or more "echo msg text" or "echo msg-n text"
commands. In case of "echo msg text" text is appended with a new
line. An empty text in this case will
just add a new line.
The message ends and gets displayed when one of the following
are receieved:
echo msg-window title
echo msg-notify title
where "title" becomes the title of the message window. In case of
msg-window, a modeless window shows the message, in the latter case
a notification balloon is shown.
Example: when pushed from the server:
push "echo msg I say let the world go to hell%2C"
push "echo msg I must have my cup of tea."
push "echo msg-window Notes from the underground"
will display a modeless window with title
"Notes from the underground" and a two line body
--
I say let the world go to hell,
I must have my cup of tea.
--
Note that the message itself is not quoted in the above examples
and so it relies on the server's option-parser combining
individual words into a space separated string. Number of words
on a line is limited by the maximum number of parameters allowed
in openvpn commands (16). This limitation may be avoided by quoting
the text that follows so that the option parser sees it as one
parameter.
The comma character is not allowed in pushed strings, so
it has to be sent encoded as %2C as shown above.
Such encoding of arbitrary bytes is suppored. For example,
newlines may be embedded as %0A, though discouraged. Instead
use multiple "echo msg" commands to separate lines by new line.
An example with embedded spaces and multiple lines concatenated
without a new line in between (note use of single quotes):
push "echo msg-n I swear to you gentlemen%2C that to be"
push "echo msg-n ' overly conscious is a sickness%2C ' "
push "echo msg-n a real%2C thorough sickness."
push "echo msg-notify Quote of the Day"
will show up as a notification that displays for an
OS-dependent interval as:
--
Quote of the Day
I swear to you gentlemen, that to be overly conscious
is a sickness, a real, thorough sickness.
--
where the location of the line break is automatically determined
by the notification API and is OS version-dependent.
Commands like "echo msg ..." in the config file are also
processed the same way. It gets displayed when the GUI connects
to the management interface and receives all pending echo.
Pushed message(s) get displayed when the client daemon
processes push-reply and passes on echo directives to the
GUI.
TODO: The actual window that displays the messages is
implemented in the next commit.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Currently we pop up the status window on double click only
if one connection is active though there is no strong reason to
limit this behaviour. In fact, when multiple connections are
stuck in the connecting state, its very useful to have a quick
way to examine their progress instead of having to drill down
the menu. Especially so when nested menu is in use.
A random variation of up to 100 pixel is added to the initial
position of the status window to avoid all windows falling on
top of each other.
To prevent an explosion of new windows in the very unlikely event
of numerous active connections, restrict the maximum windows
shown to 10.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
We didn't use interactive service when gui was running
under admin because of some privilege escalation vulnerability in Vista.
Apparently this issue doesn't exist on Win7 and newer versions so
it is safe to use iservice on those systems.
Introduce "Always use interactive service" option,
which is "on" by default. This should enable users,
who by various reasons run gui as admin, use Wintun.
When gui is running as admin and interactive service
cannot be started or not installed, warn that wintun will not work.
Signed-off-by: Lev Stipakov <lev@openvpn.net>
The current code re-issues the next read request in the I/O
completion routine before the previous message is fully handled.
This could potentially lead to lost messages as the message buffer
is reused.
Fix by re-queuing the next read from OnService() after duplicating
the previous message.
The length check of the read message is omitted as it is implicitly
checked when scanning the message. Makes the logic simpler.
Reported by Lev Stipakov <lstipakov@gmail.com>
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Parent keeping the handle to write end of child's stdout will
cause ERROR_BROKEN_PIPE not signalled if/when the child exits.
Also add a wrapper for CloseHandle()
Fixes the GUI process hanging in read from child
if the latter unexpectedly dies due to some error.
Trac #1203
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Selva Nair