Skip to content

Commit

Permalink
Increase max size of management password buffer
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
selvanair committed Dec 22, 2022
1 parent 78bdc6f commit 8b1976c
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 9 deletions.
22 changes: 14 additions & 8 deletions openvpn.c
Original file line number Diff line number Diff line change
Expand Up @@ -2390,6 +2390,12 @@ LaunchOpenVPN(connection_t *c)
HANDLE hNul = NULL;
DWORD written;
BOOL retval = FALSE;
DWORD passwd_len = 16; /* incuding NUL */

if (passwd_len > sizeof(c->manage.password))
{
passwd_len = sizeof(c->manage.password);
}

RunPreconnectScript(c);

Expand All @@ -2403,7 +2409,7 @@ LaunchOpenVPN(connection_t *c)
}

/* Create a management interface password */
GetRandomPassword(c->manage.password, sizeof(c->manage.password) - 1);
GetRandomPassword(c->manage.password, passwd_len - 1);

find_free_tcp_port(&c->manage.skaddr);

Expand Down Expand Up @@ -2434,7 +2440,7 @@ LaunchOpenVPN(connection_t *c)
}
else
{
DWORD size = _tcslen(c->config_dir) + _tcslen(options) + sizeof(c->manage.password) + 3;
DWORD size = _tcslen(c->config_dir) + _tcslen(options) + passwd_len + 3;
TCHAR startup_info[1024];

if (!AuthorizeConfig(c))
Expand All @@ -2445,15 +2451,15 @@ LaunchOpenVPN(connection_t *c)
}

c->hProcess = NULL;
c->manage.password[sizeof(c->manage.password) - 1] = '\n';
c->manage.password[passwd_len - 1] = '\n';

/* Ignore pushed route-method when service is in use */
const wchar_t* extra_options = L" --pull-filter ignore route-method";
size += wcslen(extra_options);

_sntprintf_0(startup_info, L"%ls%lc%ls%ls%lc%.*hs", c->config_dir, L'\0',
options, extra_options, L'\0', sizeof(c->manage.password), c->manage.password);
c->manage.password[sizeof(c->manage.password) - 1] = '\0';
options, extra_options, L'\0', passwd_len, c->manage.password);
c->manage.password[passwd_len - 1] = '\0';

res = WritePipe(c->iserv.pipe, startup_info, size * sizeof(TCHAR));
}
Expand Down Expand Up @@ -2553,9 +2559,9 @@ LaunchOpenVPN(connection_t *c)
CloseHandleEx(&hNul);

/* Pass management password to OpenVPN process */
c->manage.password[sizeof(c->manage.password) - 1] = '\n';
WriteFile(hStdInWrite, c->manage.password, sizeof(c->manage.password), &written, NULL);
c->manage.password[sizeof(c->manage.password) - 1] = '\0';
c->manage.password[passwd_len - 1] = '\n';
WriteFile(hStdInWrite, c->manage.password, passwd_len, &written, NULL);
c->manage.password[passwd_len - 1] = '\0';

c->hProcess = pi.hProcess; /* Will be closed in the event loop on exit */
CloseHandle(pi.hThread);
Expand Down
2 changes: 1 addition & 1 deletion options.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ struct connection {
SOCKET sk;
SOCKADDR_IN skaddr;
time_t timeout;
char password[16];
char password[4096]; /* match with largest possible passwd in openvpn.exe */
char *saved_data;
size_t saved_size;
mgmt_cmd_t *cmd_queue;
Expand Down

0 comments on commit 8b1976c

Please sign in to comment.