1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-09 17:38:00 +00:00
putty-source/windows/pageant.c

1975 lines
63 KiB
C
Raw Permalink Normal View History

/*
* Pageant: the PuTTY Authentication Agent.
*/
#include <stdio.h>
#include <stdlib.h>
winpgnt.c: handle arbitrarily large file mappings. I heard recently that at least one third-party client of Pageant exists, and that it's used to generate signatures to use with TLS client certificates. Apparently the signature scheme is compatible, but TLS tends to need signatures over more data than will fit in AGENT_MAX_MSGLEN. Before the BinarySink refactor in commit b6cbad89f, this was OK because the Windows Pageant IPC didn't check the size of the _input_ message against AGENT_MAX_MSGLEN, only the output one. But then we started checking both, so that third-party TLS client started failing. Now we use VirtualQuery to find out the actual size of the file mapping we've been passed, and our only requirement is that the input and output messages should both fit in _that_. So TLS should work again, and also, other clients should be able to retrieve longer lists of public keys if they pass a larger file mapping. One side effect of this change is that Pageant's reply message is now written directly into the shared-memory region. Previously, it was written into a separate buffer and then memcpy()ed over after pageant_handle_msg returned, but now the buffer is variable-size, it seems to me to make more sense to avoid that extra not-entirely controlled malloc. So I've done one very small reordering of statements in the cross-platform pageant_handle_msg(), which fixes the only case I could find where that function started writing its output before it had finished using the contents of the input buffer.
2018-07-08 15:46:32 +00:00
#include <stddef.h>
#include <ctype.h>
#include <assert.h>
#include <tchar.h>
#include "putty.h"
#include "ssh.h"
#include "misc.h"
#include "tree234.h"
#include "security-api.h"
#include "cryptoapi.h"
#include "pageant.h"
#include "licence.h"
#include "pageant-rc.h"
#include <shellapi.h>
#include <aclapi.h>
#ifdef DEBUG_IPC
#define _WIN32_WINNT 0x0500 /* for ConvertSidToStringSid */
#include <sddl.h>
#endif
#define WM_SYSTRAY (WM_APP + 6)
#define WM_SYSTRAY2 (WM_APP + 7)
#define APPNAME "Pageant"
winpgnt: handle WM_COPYDATA requests in a subthread. This is preparation to allow Pageant to be able to return to its GUI main loop in the middle of handling a request (e.g. present a dialog box to the user related to that particular request, and wait for the user's response). In order to do that, we need the main thread's Windows message loop to never be blocked by a WM_COPYDATA agent request. So I've split Pageant's previous hidden window into two hidden windows, each with a subset of the original roles, and created in different threads so that they get independent message loops. The one in the main thread receives messages relating to Pageant's system tray icon; the one in the subthread has the identity known to (old) Pageant clients, and receives WM_COPYDATA messages only. Each WM_COPYDATA is handled by passing the request back to the main thread via an event object integrated into the Pageant main loop, and then waiting for a second event object that the main thread will signal when the answer comes back, and not returning from the WndProc handler until the response arrives. Hence, if an agent request received via WM_COPYDATA requires GUI activity, then the main thread's GUI message loop will be able to do that in parallel with all Pageant's other activity, including other GUI activity (like the key list dialog box) and including responding to other requests via named pipe. I can't stop WM_COPYDATA requests from blocking _each other_, but this allows them not to block anything else. And named-pipe requests block nothing at all, so as clients switch over to the new IPC, even that blockage will become less and less common.
2020-01-25 17:24:17 +00:00
/* Titles and class names for invisible windows. IPCWINTITLE and
* IPCCLASSNAME are critical to backwards compatibility: WM_COPYDATA
* based Pageant clients will call FindWindow with those parameters
* and expect to find the Pageant IPC receiver. */
#define TRAYWINTITLE "Pageant"
#define TRAYCLASSNAME "PageantSysTray"
#define IPCWINTITLE "Pageant"
#define IPCCLASSNAME "Pageant"
static HWND traywindow;
static HWND keylist;
static HWND aboutbox;
static HMENU systray_menu, session_menu;
Convert a lot of 'int' variables to 'bool'. My normal habit these days, in new code, is to treat int and bool as _almost_ completely separate types. I'm still willing to use C's implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine, no need to spell it out as blob.len != 0), but generally, if a variable is going to be conceptually a boolean, I like to declare it bool and assign to it using 'true' or 'false' rather than 0 or 1. PuTTY is an exception, because it predates the C99 bool, and I've stuck to its existing coding style even when adding new code to it. But it's been annoying me more and more, so now that I've decided C99 bool is an acceptable thing to require from our toolchain in the first place, here's a quite thorough trawl through the source doing 'boolification'. Many variables and function parameters are now typed as bool rather than int; many assignments of 0 or 1 to those variables are now spelled 'true' or 'false'. I managed this thorough conversion with the help of a custom clang plugin that I wrote to trawl the AST and apply heuristics to point out where things might want changing. So I've even managed to do a decent job on parts of the code I haven't looked at in years! To make the plugin's work easier, I pushed platform front ends generally in the direction of using standard 'bool' in preference to platform-specific boolean types like Windows BOOL or GTK's gboolean; I've left the platform booleans in places they _have_ to be for the platform APIs to work right, but variables only used by my own code have been converted wherever I found them. In a few places there are int values that look very like booleans in _most_ of the places they're used, but have a rarely-used third value, or a distinction between different nonzero values that most users don't care about. In these cases, I've _removed_ uses of 'true' and 'false' for the return values, to emphasise that there's something more subtle going on than a simple boolean answer: - the 'multisel' field in dialog.h's list box structure, for which the GTK front end in particular recognises a difference between 1 and 2 but nearly everything else treats as boolean - the 'urgent' parameter to plug_receive, where 1 vs 2 tells you something about the specific location of the urgent pointer, but most clients only care about 0 vs 'something nonzero' - the return value of wc_match, where -1 indicates a syntax error in the wildcard. - the return values from SSH-1 RSA-key loading functions, which use -1 for 'wrong passphrase' and 0 for all other failures (so any caller which already knows it's not loading an _encrypted private_ key can treat them as boolean) - term->esc_query, and the 'query' parameter in toggle_mode in terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h, but can also hold -1 for some other intervening character that we don't support. In a few places there's an integer that I haven't turned into a bool even though it really _can_ only take values 0 or 1 (and, as above, tried to make the call sites consistent in not calling those values true and false), on the grounds that I thought it would make it more confusing to imply that the 0 value was in some sense 'negative' or bad and the 1 positive or good: - the return value of plug_accepting uses the POSIXish convention of 0=success and nonzero=error; I think if I made it bool then I'd also want to reverse its sense, and that's a job for a separate piece of work. - the 'screen' parameter to lineptr() in terminal.c, where 0 and 1 represent the default and alternate screens. There's no obvious reason why one of those should be considered 'true' or 'positive' or 'success' - they're just indices - so I've left it as int. ssh_scp_recv had particularly confusing semantics for its previous int return value: its call sites used '<= 0' to check for error, but it never actually returned a negative number, just 0 or 1. Now the function and its call sites agree that it's a bool. In a couple of places I've renamed variables called 'ret', because I don't like that name any more - it's unclear whether it means the return value (in preparation) for the _containing_ function or the return value received from a subroutine call, and occasionally I've accidentally used the same variable for both and introduced a bug. So where one of those got in my way, I've renamed it to 'toret' or 'retd' (the latter short for 'returned') in line with my usual modern practice, but I haven't done a thorough job of finding all of them. Finally, one amusing side effect of doing this is that I've had to separate quite a few chained assignments. It used to be perfectly fine to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a the 'true' defined by stdbool.h, that idiom provokes a warning from gcc: 'suggest parentheses around assignment used as truth value'!
2018-11-02 19:23:19 +00:00
static bool already_running;
static FingerprintType fptype = SSH_FPTYPE_DEFAULT;
static char *putty_path;
Convert a lot of 'int' variables to 'bool'. My normal habit these days, in new code, is to treat int and bool as _almost_ completely separate types. I'm still willing to use C's implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine, no need to spell it out as blob.len != 0), but generally, if a variable is going to be conceptually a boolean, I like to declare it bool and assign to it using 'true' or 'false' rather than 0 or 1. PuTTY is an exception, because it predates the C99 bool, and I've stuck to its existing coding style even when adding new code to it. But it's been annoying me more and more, so now that I've decided C99 bool is an acceptable thing to require from our toolchain in the first place, here's a quite thorough trawl through the source doing 'boolification'. Many variables and function parameters are now typed as bool rather than int; many assignments of 0 or 1 to those variables are now spelled 'true' or 'false'. I managed this thorough conversion with the help of a custom clang plugin that I wrote to trawl the AST and apply heuristics to point out where things might want changing. So I've even managed to do a decent job on parts of the code I haven't looked at in years! To make the plugin's work easier, I pushed platform front ends generally in the direction of using standard 'bool' in preference to platform-specific boolean types like Windows BOOL or GTK's gboolean; I've left the platform booleans in places they _have_ to be for the platform APIs to work right, but variables only used by my own code have been converted wherever I found them. In a few places there are int values that look very like booleans in _most_ of the places they're used, but have a rarely-used third value, or a distinction between different nonzero values that most users don't care about. In these cases, I've _removed_ uses of 'true' and 'false' for the return values, to emphasise that there's something more subtle going on than a simple boolean answer: - the 'multisel' field in dialog.h's list box structure, for which the GTK front end in particular recognises a difference between 1 and 2 but nearly everything else treats as boolean - the 'urgent' parameter to plug_receive, where 1 vs 2 tells you something about the specific location of the urgent pointer, but most clients only care about 0 vs 'something nonzero' - the return value of wc_match, where -1 indicates a syntax error in the wildcard. - the return values from SSH-1 RSA-key loading functions, which use -1 for 'wrong passphrase' and 0 for all other failures (so any caller which already knows it's not loading an _encrypted private_ key can treat them as boolean) - term->esc_query, and the 'query' parameter in toggle_mode in terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h, but can also hold -1 for some other intervening character that we don't support. In a few places there's an integer that I haven't turned into a bool even though it really _can_ only take values 0 or 1 (and, as above, tried to make the call sites consistent in not calling those values true and false), on the grounds that I thought it would make it more confusing to imply that the 0 value was in some sense 'negative' or bad and the 1 positive or good: - the return value of plug_accepting uses the POSIXish convention of 0=success and nonzero=error; I think if I made it bool then I'd also want to reverse its sense, and that's a job for a separate piece of work. - the 'screen' parameter to lineptr() in terminal.c, where 0 and 1 represent the default and alternate screens. There's no obvious reason why one of those should be considered 'true' or 'positive' or 'success' - they're just indices - so I've left it as int. ssh_scp_recv had particularly confusing semantics for its previous int return value: its call sites used '<= 0' to check for error, but it never actually returned a negative number, just 0 or 1. Now the function and its call sites agree that it's a bool. In a couple of places I've renamed variables called 'ret', because I don't like that name any more - it's unclear whether it means the return value (in preparation) for the _containing_ function or the return value received from a subroutine call, and occasionally I've accidentally used the same variable for both and introduced a bug. So where one of those got in my way, I've renamed it to 'toret' or 'retd' (the latter short for 'returned') in line with my usual modern practice, but I haven't done a thorough job of finding all of them. Finally, one amusing side effect of doing this is that I've had to separate quite a few chained assignments. It used to be perfectly fine to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a the 'true' defined by stdbool.h, that idiom provokes a warning from gcc: 'suggest parentheses around assignment used as truth value'!
2018-11-02 19:23:19 +00:00
static bool restrict_putty_acl = false;
/* CWD for "add key" file requester. */
static filereq *keypath = NULL;
/* From MSDN: In the WM_SYSCOMMAND message, the four low-order bits of
* wParam are used by Windows, and should be masked off, so we shouldn't
* attempt to store information in them. Hence all these identifiers have
* the low 4 bits clear. Also, identifiers should < 0xF000. */
#define IDM_CLOSE 0x0010
#define IDM_VIEWKEYS 0x0020
#define IDM_ADDKEY 0x0030
#define IDM_ADDKEY_ENCRYPTED 0x0040
#define IDM_REMOVE_ALL 0x0050
#define IDM_REENCRYPT_ALL 0x0060
#define IDM_HELP 0x0070
#define IDM_ABOUT 0x0080
#define IDM_PUTTY 0x0090
#define IDM_SESSIONS_BASE 0x1000
#define IDM_SESSIONS_MAX 0x2000
#define PUTTY_REGKEY "Software\\SimonTatham\\PuTTY\\Sessions"
#define PUTTY_DEFAULT "Default%20Settings"
static int initial_menuitems_count;
/*
* Print a modal (Really Bad) message box and perform a fatal exit.
*/
void modalfatalbox(const char *fmt, ...)
{
va_list ap;
char *buf;
va_start(ap, fmt);
buf = dupvprintf(fmt, ap);
va_end(ap);
MessageBox(traywindow, buf, "Pageant Fatal Error",
MB_SYSTEMMODAL | MB_ICONERROR | MB_OK);
sfree(buf);
exit(1);
}
struct PassphraseProcStruct {
bool modal;
const char *help_topic;
PageantClientDialogId *dlgid;
char *passphrase;
const char *comment;
};
/*
* Dialog-box function for the Licence box.
*/
static INT_PTR CALLBACK LicenceProc(HWND hwnd, UINT msg,
WPARAM wParam, LPARAM lParam)
{
switch (msg) {
case WM_INITDIALOG:
SetDlgItemText(hwnd, IDC_LICENCE_TEXTBOX, LICENCE_TEXT("\r\n\r\n"));
return 1;
case WM_COMMAND:
switch (LOWORD(wParam)) {
case IDOK:
case IDCANCEL:
EndDialog(hwnd, 1);
return 0;
}
return 0;
case WM_CLOSE:
EndDialog(hwnd, 1);
return 0;
}
return 0;
}
/*
* Dialog-box function for the About box.
*/
static INT_PTR CALLBACK AboutProc(HWND hwnd, UINT msg,
WPARAM wParam, LPARAM lParam)
{
switch (msg) {
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
case WM_INITDIALOG: {
char *buildinfo_text = buildinfo("\r\n");
char *text = dupprintf(
"Pageant\r\n\r\n%s\r\n\r\n%s\r\n\r\n%s",
ver, buildinfo_text,
"\251 " SHORT_COPYRIGHT_DETAILS ". All rights reserved.");
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
sfree(buildinfo_text);
SetDlgItemText(hwnd, IDC_ABOUT_TEXTBOX, text);
MakeDlgItemBorderless(hwnd, IDC_ABOUT_TEXTBOX);
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
sfree(text);
return 1;
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
}
case WM_COMMAND:
switch (LOWORD(wParam)) {
case IDOK:
case IDCANCEL:
aboutbox = NULL;
DestroyWindow(hwnd);
return 0;
case IDC_ABOUT_LICENCE:
EnableWindow(hwnd, 0);
DialogBox(hinst, MAKEINTRESOURCE(IDD_LICENCE), hwnd, LicenceProc);
EnableWindow(hwnd, 1);
SetActiveWindow(hwnd);
return 0;
case IDC_ABOUT_WEBSITE:
/* Load web browser */
ShellExecute(hwnd, "open",
"https://www.chiark.greenend.org.uk/~sgtatham/putty/",
0, 0, SW_SHOWDEFAULT);
return 0;
}
return 0;
case WM_CLOSE:
aboutbox = NULL;
DestroyWindow(hwnd);
return 0;
}
return 0;
}
static HWND modal_passphrase_hwnd = NULL;
static HWND nonmodal_passphrase_hwnd = NULL;
static void end_passphrase_dialog(HWND hwnd, INT_PTR result)
{
struct PassphraseProcStruct *p = (struct PassphraseProcStruct *)
GetWindowLongPtr(hwnd, GWLP_USERDATA);
if (p->modal) {
EndDialog(hwnd, result);
} else {
/*
* Destroy this passphrase dialog box before passing the
* results back to the main pageant.c, to avoid re-entrancy
* issues.
*
* If we successfully got a passphrase from the user, but it
* was _wrong_, then pageant_passphrase_request_success will
* respond by calling back - synchronously - to our
* ask_passphrase() implementation, which will expect the
* previous value of nonmodal_passphrase_hwnd to have already
* been cleaned up.
*/
SetWindowLongPtr(hwnd, GWLP_USERDATA, (LONG_PTR) NULL);
DestroyWindow(hwnd);
nonmodal_passphrase_hwnd = NULL;
if (result)
pageant_passphrase_request_success(
p->dlgid, ptrlen_from_asciz(p->passphrase));
else
pageant_passphrase_request_refused(p->dlgid);
burnstr(p->passphrase);
sfree(p);
}
}
/*
* Dialog-box function for the passphrase box.
*/
static INT_PTR CALLBACK PassphraseProc(HWND hwnd, UINT msg,
WPARAM wParam, LPARAM lParam)
{
struct PassphraseProcStruct *p;
if (msg == WM_INITDIALOG) {
p = (struct PassphraseProcStruct *) lParam;
SetWindowLongPtr(hwnd, GWLP_USERDATA, (LONG_PTR) p);
} else {
p = (struct PassphraseProcStruct *)
GetWindowLongPtr(hwnd, GWLP_USERDATA);
}
switch (msg) {
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
case WM_INITDIALOG: {
if (p->modal)
modal_passphrase_hwnd = hwnd;
/*
* Centre the window.
*/
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
RECT rs, rd;
HWND hw;
hw = GetDesktopWindow();
if (GetWindowRect(hw, &rs) && GetWindowRect(hwnd, &rd))
MoveWindow(hwnd,
(rs.right + rs.left + rd.left - rd.right) / 2,
(rs.bottom + rs.top + rd.top - rd.bottom) / 2,
rd.right - rd.left, rd.bottom - rd.top, true);
SetForegroundWindow(hwnd);
SetWindowPos(hwnd, HWND_TOP, 0, 0, 0, 0,
SWP_NOMOVE | SWP_NOSIZE | SWP_SHOWWINDOW);
if (!p->modal)
SetActiveWindow(hwnd); /* this won't have happened automatically */
if (p->comment)
SetDlgItemText(hwnd, IDC_PASSPHRASE_FINGERPRINT, p->comment);
burnstr(p->passphrase);
p->passphrase = dupstr("");
SetDlgItemText(hwnd, IDC_PASSPHRASE_EDITBOX, p->passphrase);
if (!p->help_topic || !has_help()) {
HWND item = GetDlgItem(hwnd, IDHELP);
if (item)
DestroyWindow(item);
}
return 0;
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
}
case WM_COMMAND:
switch (LOWORD(wParam)) {
case IDOK:
if (p->passphrase)
end_passphrase_dialog(hwnd, 1);
else
MessageBeep(0);
return 0;
case IDCANCEL:
end_passphrase_dialog(hwnd, 0);
return 0;
case IDHELP:
if (p->help_topic)
launch_help(hwnd, p->help_topic);
return 0;
case IDC_PASSPHRASE_EDITBOX:
if ((HIWORD(wParam) == EN_CHANGE) && p->passphrase) {
burnstr(p->passphrase);
p->passphrase = GetDlgItemText_alloc(
hwnd, IDC_PASSPHRASE_EDITBOX);
}
return 0;
}
return 0;
case WM_CLOSE:
end_passphrase_dialog(hwnd, 0);
return 0;
}
return 0;
}
/*
* Warn about the obsolescent key file format.
*/
void old_keyfile_warning(void)
{
static const char mbtitle[] = "PuTTY Key File Warning";
static const char message[] =
"You are loading an SSH-2 private key which has an\n"
"old version of the file format. This means your key\n"
"file is not fully tamperproof. Future versions of\n"
"PuTTY may stop supporting this private key format,\n"
"so we recommend you convert your key to the new\n"
"format.\n"
"\n"
"You can perform this conversion by loading the key\n"
"into PuTTYgen and then saving it again.";
MessageBox(NULL, message, mbtitle, MB_OK);
}
struct keylist_update_ctx {
HDC hdc;
int algbitswidth, algwidth, bitswidth, hashwidth;
bool enable_remove_controls;
bool enable_reencrypt_controls;
};
struct keylist_display_data {
strbuf *alg, *bits, *hash, *comment, *info;
};
static void keylist_update_callback(
void *vctx, char **fingerprints, const char *comment, uint32_t ext_flags,
struct pageant_pubkey *key)
{
struct keylist_update_ctx *ctx = (struct keylist_update_ctx *)vctx;
FingerprintType this_type = ssh2_pick_fingerprint(fingerprints, fptype);
ptrlen fingerprint = ptrlen_from_asciz(fingerprints[this_type]);
struct keylist_display_data *disp = snew(struct keylist_display_data);
disp->alg = strbuf_new();
disp->bits = strbuf_new();
disp->hash = strbuf_new();
disp->comment = strbuf_new();
disp->info = strbuf_new();
/* There is at least one key, so the controls for removing keys
* should be enabled */
ctx->enable_remove_controls = true;
switch (key->ssh_version) {
case 1: {
/*
* Expect the fingerprint to contain two words: bit count and
* hash.
*/
put_dataz(disp->alg, "SSH-1");
put_datapl(disp->bits, ptrlen_get_word(&fingerprint, " "));
put_datapl(disp->hash, ptrlen_get_word(&fingerprint, " "));
break;
}
case 2: {
/*
* Expect the fingerprint to contain three words: algorithm
* name, bit count, hash.
*/
const ssh_keyalg *alg = pubkey_blob_to_alg(
ptrlen_from_strbuf(key->blob));
ptrlen keytype_word = ptrlen_get_word(&fingerprint, " ");
if (alg) {
/* Use our own human-legible algorithm names if available,
* because they fit better in the space. (Certificate key
* algorithm names in particular are terribly long.) */
char *alg_desc = ssh_keyalg_desc(alg);
put_dataz(disp->alg, alg_desc);
sfree(alg_desc);
} else {
put_datapl(disp->alg, keytype_word);
}
ptrlen bits_word = ptrlen_get_word(&fingerprint, " ");
if (alg && ssh_keyalg_variable_size(alg))
put_datapl(disp->bits, bits_word);
put_datapl(disp->hash, ptrlen_get_word(&fingerprint, " "));
}
}
put_dataz(disp->comment, comment);
SIZE sz;
if (disp->bits->len) {
GetTextExtentPoint32(ctx->hdc, disp->alg->s, disp->alg->len, &sz);
if (ctx->algwidth < sz.cx) ctx->algwidth = sz.cx;
GetTextExtentPoint32(ctx->hdc, disp->bits->s, disp->bits->len, &sz);
if (ctx->bitswidth < sz.cx) ctx->bitswidth = sz.cx;
} else {
GetTextExtentPoint32(ctx->hdc, disp->alg->s, disp->alg->len, &sz);
if (ctx->algbitswidth < sz.cx) ctx->algbitswidth = sz.cx;
}
GetTextExtentPoint32(ctx->hdc, disp->hash->s, disp->hash->len, &sz);
if (ctx->hashwidth < sz.cx) ctx->hashwidth = sz.cx;
if (ext_flags & LIST_EXTENDED_FLAG_HAS_NO_CLEARTEXT_KEY) {
put_fmt(disp->info, "(encrypted)");
} else if (ext_flags & LIST_EXTENDED_FLAG_HAS_ENCRYPTED_KEY_FILE) {
put_fmt(disp->info, "(re-encryptable)");
/* At least one key can be re-encrypted */
ctx->enable_reencrypt_controls = true;
}
/* This list box is owner-drawn but doesn't have LBS_HASSTRINGS,
* so we can use LB_ADDSTRING to hand the list box our display
* info pointer */
SendDlgItemMessage(keylist, IDC_KEYLIST_LISTBOX,
LB_ADDSTRING, 0, (LPARAM)disp);
}
/* Column start positions for the list box, in pixels (not dialog units). */
static int colpos_bits, colpos_hash, colpos_comment;
/*
* Update the visible key list.
*/
void keylist_update(void)
{
if (keylist) {
/*
* Clear the previous list box content and free their display
* structures.
*/
{
int nitems = SendDlgItemMessage(keylist, IDC_KEYLIST_LISTBOX,
LB_GETCOUNT, 0, 0);
for (int i = 0; i < nitems; i++) {
struct keylist_display_data *disp =
(struct keylist_display_data *)SendDlgItemMessage(
keylist, IDC_KEYLIST_LISTBOX, LB_GETITEMDATA, i, 0);
strbuf_free(disp->alg);
strbuf_free(disp->bits);
strbuf_free(disp->hash);
strbuf_free(disp->comment);
strbuf_free(disp->info);
sfree(disp);
}
}
SendDlgItemMessage(keylist, IDC_KEYLIST_LISTBOX,
LB_RESETCONTENT, 0, 0);
char *errmsg;
struct keylist_update_ctx ctx[1];
ctx->enable_remove_controls = false;
ctx->enable_reencrypt_controls = false;
ctx->algbitswidth = ctx->algwidth = 0;
ctx->bitswidth = ctx->hashwidth = 0;
ctx->hdc = GetDC(keylist);
SelectObject(ctx->hdc, (HFONT)SendMessage(keylist, WM_GETFONT, 0, 0));
int status = pageant_enum_keys(keylist_update_callback, ctx, &errmsg);
SIZE sz;
GetTextExtentPoint32(ctx->hdc, "MM", 2, &sz);
int gutter = sz.cx;
DeleteDC(ctx->hdc);
colpos_hash = ctx->algwidth + ctx->bitswidth + 2*gutter;
if (colpos_hash < ctx->algbitswidth + gutter)
colpos_hash = ctx->algbitswidth + gutter;
colpos_bits = colpos_hash - ctx->bitswidth - gutter;
colpos_comment = colpos_hash + ctx->hashwidth + gutter;
assert(status == PAGEANT_ACTION_OK);
assert(!errmsg);
SendDlgItemMessage(keylist, IDC_KEYLIST_LISTBOX,
LB_SETCURSEL, (WPARAM) - 1, 0);
EnableWindow(GetDlgItem(keylist, IDC_KEYLIST_REMOVE),
ctx->enable_remove_controls);
EnableWindow(GetDlgItem(keylist, IDC_KEYLIST_REENCRYPT),
ctx->enable_reencrypt_controls);
}
}
static void win_add_keyfile(Filename *filename, bool encrypted)
{
char *err;
int ret;
/*
* Try loading the key without a passphrase. (Or rather, without a
* _new_ passphrase; pageant_add_keyfile will take care of trying
* all the passphrases we've already stored.)
*/
ret = pageant_add_keyfile(filename, NULL, &err, encrypted);
if (ret == PAGEANT_ACTION_OK) {
goto done;
} else if (ret == PAGEANT_ACTION_FAILURE) {
goto error;
}
/*
* OK, a passphrase is needed, and we've been given the key
* comment to use in the passphrase prompt.
*/
while (1) {
INT_PTR dlgret;
struct PassphraseProcStruct pps;
pps.modal = true;
pps.help_topic = NULL; /* this dialog has no help button */
pps.dlgid = NULL;
pps.passphrase = NULL;
pps.comment = err;
dlgret = DialogBoxParam(
hinst, MAKEINTRESOURCE(IDD_LOAD_PASSPHRASE),
NULL, PassphraseProc, (LPARAM) &pps);
modal_passphrase_hwnd = NULL;
if (!dlgret) {
burnstr(pps.passphrase);
goto done; /* operation cancelled */
}
sfree(err);
assert(pps.passphrase != NULL);
ret = pageant_add_keyfile(filename, pps.passphrase, &err, false);
burnstr(pps.passphrase);
if (ret == PAGEANT_ACTION_OK) {
goto done;
} else if (ret == PAGEANT_ACTION_FAILURE) {
goto error;
}
}
error:
message_box(traywindow, err, APPNAME, MB_OK | MB_ICONERROR, false,
HELPCTXID(errors_cantloadkey));
done:
sfree(err);
return;
}
/*
* Prompt for a key file to add, and add it.
*/
static void prompt_add_keyfile(bool encrypted)
{
OPENFILENAME of;
char *filelist = snewn(8192, char);
if (!keypath) keypath = filereq_new();
memset(&of, 0, sizeof(of));
of.hwndOwner = traywindow;
Some support for wide-character filenames in Windows. The Windows version of the Filename structure now contains three versions of the pathname, in UTF-16, UTF-8 and the system code page. Callers can use whichever is most convenient. All uses of filenames for actually opening files now use the UTF-16 version, which means they can tolerate 'exotic' filenames, by which I mean those including Unicode characters outside the host system's CP_ACP default code page. Other uses of Filename structures inside the 'windows' subdirectory do something appropriate, e.g. when printing a filename inside a message box or a console message, we use the UTF-8 version of the filename with the UTF-8 version of the appropriate API. There are three remaining pieces to full Unicode filename support: One is that the cross-platform code has many calls to filename_to_str(), embodying the assumption that a file name can be reliably converted into the unspecified current character set; those will all need changing in some way. Another is that write_setting_filename(), in windows/storage.c, still saves filenames to the Registry as an ordinary REG_SZ in the system code page. So even if an exotic filename were stored in a Conf, that Conf couldn't round-trip via the Registry and back without corrupting that filename by coercing it back to a string that fits in CP_ACP and therefore doesn't represent the same file. This can't be fixed without a compatibility break in the storage format, and I don't want to make a minimal change in that area: if we're going to break compatibility, then we should break it good and hard (the Nanny Ogg principle), and devise a completely fresh storage representation that fixes as many other legacy problems as possible at the same time. So that's my plan, not yet started. The final point, much more obviously, is that we're still short of methods to _construct_ any Filename structures using a Unicode input string! It should now work to enter one in the GUI configurer (either by manual text input or via the file selector), but it won't round-trip through a save and load (as discussed above), and there's still no way to specify one on the command line (the groundwork is laid by commit 10e1ac7752de928 but not yet linked up). But this is a start.
2023-05-28 10:30:59 +00:00
of.lpstrFilter = FILTER_KEY_FILES_C;
of.lpstrCustomFilter = NULL;
of.nFilterIndex = 1;
of.lpstrFile = filelist;
*filelist = '\0';
of.nMaxFile = 8192;
of.lpstrFileTitle = NULL;
of.lpstrTitle = "Select Private Key File";
of.Flags = OFN_ALLOWMULTISELECT | OFN_EXPLORER;
if (request_file(keypath, &of, true, false)) {
if (strlen(filelist) > of.nFileOffset) {
/* Only one filename returned? */
Filename *fn = filename_from_str(filelist);
win_add_keyfile(fn, encrypted);
filename_free(fn);
} else {
/* we are returned a bunch of strings, end to
* end. first string is the directory, the
* rest the filenames. terminated with an
* empty string.
*/
char *dir = filelist;
char *filewalker = filelist + strlen(dir) + 1;
while (*filewalker != '\0') {
char *filename = dupcat(dir, "\\", filewalker);
Filename *fn = filename_from_str(filename);
win_add_keyfile(fn, encrypted);
filename_free(fn);
sfree(filename);
filewalker += strlen(filewalker) + 1;
}
}
keylist_update();
pageant_forget_passphrases();
}
sfree(filelist);
}
/*
* Dialog-box function for the key list box.
*/
static INT_PTR CALLBACK KeyListProc(HWND hwnd, UINT msg,
WPARAM wParam, LPARAM lParam)
{
static const struct {
const char *name;
FingerprintType value;
} fptypes[] = {
{"SHA256", SSH_FPTYPE_SHA256},
{"MD5", SSH_FPTYPE_MD5},
{"SHA256 including certificate", SSH_FPTYPE_SHA256_CERT},
{"MD5 including certificate", SSH_FPTYPE_MD5_CERT},
};
switch (msg) {
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
case WM_INITDIALOG: {
/*
* Centre the window.
*/
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
RECT rs, rd;
HWND hw;
hw = GetDesktopWindow();
if (GetWindowRect(hw, &rs) && GetWindowRect(hwnd, &rd))
MoveWindow(hwnd,
(rs.right + rs.left + rd.left - rd.right) / 2,
(rs.bottom + rs.top + rd.top - rd.bottom) / 2,
rd.right - rd.left, rd.bottom - rd.top, true);
if (has_help())
SetWindowLongPtr(hwnd, GWL_EXSTYLE,
GetWindowLongPtr(hwnd, GWL_EXSTYLE) |
WS_EX_CONTEXTHELP);
else {
HWND item = GetDlgItem(hwnd, IDC_KEYLIST_HELP);
if (item)
DestroyWindow(item);
}
keylist = hwnd;
int selection = 0;
for (size_t i = 0; i < lenof(fptypes); i++) {
SendDlgItemMessage(hwnd, IDC_KEYLIST_FPTYPE, CB_ADDSTRING,
0, (LPARAM)fptypes[i].name);
if (fptype == fptypes[i].value)
selection = (int)i;
}
SendDlgItemMessage(hwnd, IDC_KEYLIST_FPTYPE,
CB_SETCURSEL, 0, selection);
keylist_update();
return 0;
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
}
case WM_MEASUREITEM: {
assert(wParam == IDC_KEYLIST_LISTBOX);
MEASUREITEMSTRUCT *mi = (MEASUREITEMSTRUCT *)lParam;
/*
* Our list box is owner-drawn, but we put normal text in it.
* So the line height is the same as it would normally be,
* which is 8 dialog units.
*/
RECT r;
r.left = r.right = r.top = 0;
r.bottom = 8;
MapDialogRect(hwnd, &r);
mi->itemHeight = r.bottom;
return 0;
}
case WM_DRAWITEM: {
assert(wParam == IDC_KEYLIST_LISTBOX);
DRAWITEMSTRUCT *di = (DRAWITEMSTRUCT *)lParam;
if (di->itemAction == ODA_FOCUS) {
/* Just toggle the focus rectangle either on or off. This
* is an XOR-type function, so it's the same call in
* either case. */
DrawFocusRect(di->hDC, &di->rcItem);
} else {
/* Draw the full text. */
bool selected = (di->itemState & ODS_SELECTED);
COLORREF newfg = GetSysColor(
selected ? COLOR_HIGHLIGHTTEXT : COLOR_WINDOWTEXT);
COLORREF newbg = GetSysColor(
selected ? COLOR_HIGHLIGHT : COLOR_WINDOW);
COLORREF oldfg = SetTextColor(di->hDC, newfg);
COLORREF oldbg = SetBkColor(di->hDC, newbg);
HFONT font = (HFONT)SendMessage(hwnd, WM_GETFONT, 0, 0);
HFONT oldfont = SelectObject(di->hDC, font);
/* ExtTextOut("") is an easy way to just draw the
* background rectangle */
ExtTextOut(di->hDC, di->rcItem.left, di->rcItem.top,
ETO_OPAQUE | ETO_CLIPPED, &di->rcItem, "", 0, NULL);
struct keylist_display_data *disp =
(struct keylist_display_data *)di->itemData;
RECT r;
/* Apparently real list boxes start drawing at x=1, not x=0 */
r.left = r.top = r.bottom = 0;
r.right = 1;
MapDialogRect(hwnd, &r);
ExtTextOut(di->hDC, di->rcItem.left + r.right, di->rcItem.top,
ETO_CLIPPED, &di->rcItem, disp->alg->s,
disp->alg->len, NULL);
if (disp->bits->len) {
ExtTextOut(di->hDC, di->rcItem.left + r.right + colpos_bits,
di->rcItem.top, ETO_CLIPPED, &di->rcItem,
disp->bits->s, disp->bits->len, NULL);
}
ExtTextOut(di->hDC, di->rcItem.left + r.right + colpos_hash,
di->rcItem.top, ETO_CLIPPED, &di->rcItem,
disp->hash->s, disp->hash->len, NULL);
strbuf *sb = strbuf_new();
put_datapl(sb, ptrlen_from_strbuf(disp->comment));
if (disp->info->len) {
put_byte(sb, '\t');
put_datapl(sb, ptrlen_from_strbuf(disp->info));
}
TabbedTextOut(di->hDC, di->rcItem.left + r.right + colpos_comment,
di->rcItem.top, sb->s, sb->len, 0, NULL, 0);
strbuf_free(sb);
SetTextColor(di->hDC, oldfg);
SetBkColor(di->hDC, oldbg);
SelectObject(di->hDC, oldfont);
if (di->itemState & ODS_FOCUS)
DrawFocusRect(di->hDC, &di->rcItem);
}
return 0;
}
case WM_COMMAND:
switch (LOWORD(wParam)) {
case IDOK:
case IDCANCEL:
keylist = NULL;
DestroyWindow(hwnd);
return 0;
case IDC_KEYLIST_ADDKEY:
case IDC_KEYLIST_ADDKEY_ENC:
if (HIWORD(wParam) == BN_CLICKED ||
HIWORD(wParam) == BN_DOUBLECLICKED) {
if (modal_passphrase_hwnd) {
MessageBeep(MB_ICONERROR);
SetForegroundWindow(modal_passphrase_hwnd);
break;
}
prompt_add_keyfile(LOWORD(wParam) == IDC_KEYLIST_ADDKEY_ENC);
}
return 0;
case IDC_KEYLIST_REMOVE:
case IDC_KEYLIST_REENCRYPT:
if (HIWORD(wParam) == BN_CLICKED ||
HIWORD(wParam) == BN_DOUBLECLICKED) {
int i;
int rCount, sCount;
int *selectedArray;
/* our counter within the array of selected items */
int itemNum;
/* get the number of items selected in the list */
int numSelected = SendDlgItemMessage(
hwnd, IDC_KEYLIST_LISTBOX, LB_GETSELCOUNT, 0, 0);
/* none selected? that was silly */
if (numSelected == 0) {
MessageBeep(0);
break;
}
/* get item indices in an array */
selectedArray = snewn(numSelected, int);
SendDlgItemMessage(hwnd, IDC_KEYLIST_LISTBOX, LB_GETSELITEMS,
numSelected, (WPARAM)selectedArray);
itemNum = numSelected - 1;
rCount = pageant_count_ssh1_keys();
sCount = pageant_count_ssh2_keys();
/* go through the non-rsakeys until we've covered them all,
* and/or we're out of selected items to check. note that
* we go *backwards*, to avoid complications from deleting
* things hence altering the offset of subsequent items
*/
for (i = sCount - 1; (itemNum >= 0) && (i >= 0); i--) {
if (selectedArray[itemNum] == rCount + i) {
switch (LOWORD(wParam)) {
case IDC_KEYLIST_REMOVE:
pageant_delete_nth_ssh2_key(i);
break;
case IDC_KEYLIST_REENCRYPT:
pageant_reencrypt_nth_ssh2_key(i);
break;
}
itemNum--;
}
}
/* do the same for the rsa keys */
for (i = rCount - 1; (itemNum >= 0) && (i >= 0); i--) {
if (selectedArray[itemNum] == i) {
switch (LOWORD(wParam)) {
case IDC_KEYLIST_REMOVE:
pageant_delete_nth_ssh1_key(i);
break;
case IDC_KEYLIST_REENCRYPT:
/* SSH-1 keys can't be re-encrypted */
break;
}
itemNum--;
}
}
sfree(selectedArray);
keylist_update();
}
return 0;
case IDC_KEYLIST_HELP:
if (HIWORD(wParam) == BN_CLICKED ||
HIWORD(wParam) == BN_DOUBLECLICKED) {
launch_help(hwnd, WINHELP_CTX_pageant_general);
}
return 0;
case IDC_KEYLIST_FPTYPE:
if (HIWORD(wParam) == CBN_SELCHANGE) {
int selection = SendDlgItemMessage(
hwnd, IDC_KEYLIST_FPTYPE, CB_GETCURSEL, 0, 0);
if (selection >= 0 && (size_t)selection < lenof(fptypes)) {
fptype = fptypes[selection].value;
keylist_update();
}
}
return 0;
}
return 0;
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
case WM_HELP: {
int id = ((LPHELPINFO)lParam)->iCtrlId;
const char *topic = NULL;
switch (id) {
case IDC_KEYLIST_LISTBOX:
case IDC_KEYLIST_FPTYPE:
case IDC_KEYLIST_FPTYPE_STATIC:
topic = WINHELP_CTX_pageant_keylist; break;
case IDC_KEYLIST_ADDKEY: topic = WINHELP_CTX_pageant_addkey; break;
case IDC_KEYLIST_REMOVE: topic = WINHELP_CTX_pageant_remkey; break;
case IDC_KEYLIST_ADDKEY_ENC:
case IDC_KEYLIST_REENCRYPT:
topic = WINHELP_CTX_pageant_deferred; break;
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
}
if (topic) {
launch_help(hwnd, topic);
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
} else {
MessageBeep(0);
}
break;
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
}
case WM_CLOSE:
keylist = NULL;
DestroyWindow(hwnd);
return 0;
}
return 0;
}
/* Set up a system tray icon */
static BOOL AddTrayIcon(HWND hwnd)
{
BOOL res;
NOTIFYICONDATA tnid;
HICON hicon;
#ifdef NIM_SETVERSION
tnid.uVersion = 0;
res = Shell_NotifyIcon(NIM_SETVERSION, &tnid);
#endif
tnid.cbSize = sizeof(NOTIFYICONDATA);
tnid.hWnd = hwnd;
tnid.uID = 1; /* unique within this systray use */
tnid.uFlags = NIF_MESSAGE | NIF_ICON | NIF_TIP;
tnid.uCallbackMessage = WM_SYSTRAY;
tnid.hIcon = hicon = LoadIcon(hinst, MAKEINTRESOURCE(201));
strcpy(tnid.szTip, "Pageant (PuTTY authentication agent)");
res = Shell_NotifyIcon(NIM_ADD, &tnid);
if (hicon) DestroyIcon(hicon);
return res;
}
/* Update the saved-sessions menu. */
static void update_sessions(void)
{
int num_entries;
HKEY hkey;
TCHAR buf[MAX_PATH + 1];
MENUITEMINFO mii;
strbuf *sb;
int index_key, index_menu;
if (!putty_path)
return;
if (ERROR_SUCCESS != RegOpenKey(HKEY_CURRENT_USER, PUTTY_REGKEY, &hkey))
return;
for (num_entries = GetMenuItemCount(session_menu);
num_entries > initial_menuitems_count;
num_entries--)
RemoveMenu(session_menu, 0, MF_BYPOSITION);
index_key = 0;
index_menu = 0;
sb = strbuf_new();
while (ERROR_SUCCESS == RegEnumKey(hkey, index_key, buf, MAX_PATH)) {
if (strcmp(buf, PUTTY_DEFAULT) != 0) {
strbuf_clear(sb);
unescape_registry_key(buf, sb);
memset(&mii, 0, sizeof(mii));
mii.cbSize = sizeof(mii);
mii.fMask = MIIM_TYPE | MIIM_STATE | MIIM_ID;
mii.fType = MFT_STRING;
mii.fState = MFS_ENABLED;
mii.wID = (index_menu * 16) + IDM_SESSIONS_BASE;
mii.dwTypeData = sb->s;
InsertMenuItem(session_menu, index_menu, true, &mii);
index_menu++;
}
index_key++;
}
strbuf_free(sb);
RegCloseKey(hkey);
if (index_menu == 0) {
mii.cbSize = sizeof(mii);
mii.fMask = MIIM_TYPE | MIIM_STATE;
mii.fType = MFT_STRING;
mii.fState = MFS_GRAYED;
mii.dwTypeData = _T("(No sessions)");
InsertMenuItem(session_menu, index_menu, true, &mii);
}
}
/*
* Versions of Pageant prior to 0.61 expected this SID on incoming
* communications. For backwards compatibility, and more particularly
* for compatibility with derived works of PuTTY still using the old
* Pageant client code, we accept it as an alternative to the one
* returned from get_user_sid().
*/
PSID get_default_sid(void)
{
HANDLE proc = NULL;
DWORD sidlen;
PSECURITY_DESCRIPTOR psd = NULL;
PSID sid = NULL, copy = NULL, ret = NULL;
if ((proc = OpenProcess(MAXIMUM_ALLOWED, false,
GetCurrentProcessId())) == NULL)
goto cleanup;
if (p_GetSecurityInfo(proc, SE_KERNEL_OBJECT, OWNER_SECURITY_INFORMATION,
&sid, NULL, NULL, NULL, &psd) != ERROR_SUCCESS)
goto cleanup;
sidlen = GetLengthSid(sid);
copy = (PSID)smalloc(sidlen);
if (!CopySid(sidlen, copy, sid))
goto cleanup;
/* Success. Move sid into the return value slot, and null it out
* to stop the cleanup code freeing it. */
ret = copy;
copy = NULL;
cleanup:
if (proc != NULL)
CloseHandle(proc);
if (psd != NULL)
LocalFree(psd);
if (copy != NULL)
sfree(copy);
return ret;
}
winpgnt: handle WM_COPYDATA requests in a subthread. This is preparation to allow Pageant to be able to return to its GUI main loop in the middle of handling a request (e.g. present a dialog box to the user related to that particular request, and wait for the user's response). In order to do that, we need the main thread's Windows message loop to never be blocked by a WM_COPYDATA agent request. So I've split Pageant's previous hidden window into two hidden windows, each with a subset of the original roles, and created in different threads so that they get independent message loops. The one in the main thread receives messages relating to Pageant's system tray icon; the one in the subthread has the identity known to (old) Pageant clients, and receives WM_COPYDATA messages only. Each WM_COPYDATA is handled by passing the request back to the main thread via an event object integrated into the Pageant main loop, and then waiting for a second event object that the main thread will signal when the answer comes back, and not returning from the WndProc handler until the response arrives. Hence, if an agent request received via WM_COPYDATA requires GUI activity, then the main thread's GUI message loop will be able to do that in parallel with all Pageant's other activity, including other GUI activity (like the key list dialog box) and including responding to other requests via named pipe. I can't stop WM_COPYDATA requests from blocking _each other_, but this allows them not to block anything else. And named-pipe requests block nothing at all, so as clients switch over to the new IPC, even that blockage will become less and less common.
2020-01-25 17:24:17 +00:00
struct WmCopydataTransaction {
char *length, *body;
size_t bodysize, bodylen;
HANDLE ev_msg_ready, ev_reply_ready;
} wmct;
Pageant: new asynchronous internal APIs. This is a pure refactoring: no functional change expected. This commit introduces two new small vtable-style APIs. One is PageantClient, which identifies a particular client of the Pageant 'core' (meaning the code that handles each individual request). This changes pageant_handle_msg into an asynchronous operation: you pass in an agent request message and an identifier, and at some later point, the got_response method in your PageantClient will be called with the answer (and the same identifier, to allow you to match requests to responses). The trait vtable also contains a logging system. The main importance of PageantClient, and the reason why it has to exist instead of just passing pageant_handle_msg a bare callback function pointer and context parameter, is that it provides robustness if a client stops existing while a request is still pending. You call pageant_unregister_client, and any unfinished requests associated with that client in the Pageant core will be cleaned up, so that you're guaranteed that after the unregister operation, no stray callbacks will happen with a stale pointer to that client. The WM_COPYDATA interface of Windows Pageant is a direct client of this API. The other client is PageantListener, the system that lives in pageant.c and handles stream-based agent connections for both Unix Pageant and the new Windows named-pipe IPC. More specifically, each individual connection to the listening socket is a separate PageantClient, which means that if a socket is closed abruptly or suffers an OS error, that client can be unregistered and any pending requests cancelled without disrupting other connections. Users of PageantListener have a second client vtable they can use, called PageantListenerClient. That contains _only_ logging facilities, and at the moment, only Unix Pageant bothers to use it (and even that only in debugging mode). Finally, internally to the Pageant core, there's a new trait called PageantAsyncOp which describes an agent request in the process of being handled. But at the moment, it has only one trivial implementation, which is handed the full response message already constructed, and on the next toplevel callback, passes it back to the PageantClient.
2020-01-25 17:24:28 +00:00
static struct PageantClient wmcpc;
winpgnt: handle WM_COPYDATA requests in a subthread. This is preparation to allow Pageant to be able to return to its GUI main loop in the middle of handling a request (e.g. present a dialog box to the user related to that particular request, and wait for the user's response). In order to do that, we need the main thread's Windows message loop to never be blocked by a WM_COPYDATA agent request. So I've split Pageant's previous hidden window into two hidden windows, each with a subset of the original roles, and created in different threads so that they get independent message loops. The one in the main thread receives messages relating to Pageant's system tray icon; the one in the subthread has the identity known to (old) Pageant clients, and receives WM_COPYDATA messages only. Each WM_COPYDATA is handled by passing the request back to the main thread via an event object integrated into the Pageant main loop, and then waiting for a second event object that the main thread will signal when the answer comes back, and not returning from the WndProc handler until the response arrives. Hence, if an agent request received via WM_COPYDATA requires GUI activity, then the main thread's GUI message loop will be able to do that in parallel with all Pageant's other activity, including other GUI activity (like the key list dialog box) and including responding to other requests via named pipe. I can't stop WM_COPYDATA requests from blocking _each other_, but this allows them not to block anything else. And named-pipe requests block nothing at all, so as clients switch over to the new IPC, even that blockage will become less and less common.
2020-01-25 17:24:17 +00:00
static void wm_copydata_got_msg(void *vctx)
{
Pageant: new asynchronous internal APIs. This is a pure refactoring: no functional change expected. This commit introduces two new small vtable-style APIs. One is PageantClient, which identifies a particular client of the Pageant 'core' (meaning the code that handles each individual request). This changes pageant_handle_msg into an asynchronous operation: you pass in an agent request message and an identifier, and at some later point, the got_response method in your PageantClient will be called with the answer (and the same identifier, to allow you to match requests to responses). The trait vtable also contains a logging system. The main importance of PageantClient, and the reason why it has to exist instead of just passing pageant_handle_msg a bare callback function pointer and context parameter, is that it provides robustness if a client stops existing while a request is still pending. You call pageant_unregister_client, and any unfinished requests associated with that client in the Pageant core will be cleaned up, so that you're guaranteed that after the unregister operation, no stray callbacks will happen with a stale pointer to that client. The WM_COPYDATA interface of Windows Pageant is a direct client of this API. The other client is PageantListener, the system that lives in pageant.c and handles stream-based agent connections for both Unix Pageant and the new Windows named-pipe IPC. More specifically, each individual connection to the listening socket is a separate PageantClient, which means that if a socket is closed abruptly or suffers an OS error, that client can be unregistered and any pending requests cancelled without disrupting other connections. Users of PageantListener have a second client vtable they can use, called PageantListenerClient. That contains _only_ logging facilities, and at the moment, only Unix Pageant bothers to use it (and even that only in debugging mode). Finally, internally to the Pageant core, there's a new trait called PageantAsyncOp which describes an agent request in the process of being handled. But at the moment, it has only one trivial implementation, which is handed the full response message already constructed, and on the next toplevel callback, passes it back to the PageantClient.
2020-01-25 17:24:28 +00:00
pageant_handle_msg(&wmcpc, NULL, make_ptrlen(wmct.body, wmct.bodylen));
}
winpgnt: handle WM_COPYDATA requests in a subthread. This is preparation to allow Pageant to be able to return to its GUI main loop in the middle of handling a request (e.g. present a dialog box to the user related to that particular request, and wait for the user's response). In order to do that, we need the main thread's Windows message loop to never be blocked by a WM_COPYDATA agent request. So I've split Pageant's previous hidden window into two hidden windows, each with a subset of the original roles, and created in different threads so that they get independent message loops. The one in the main thread receives messages relating to Pageant's system tray icon; the one in the subthread has the identity known to (old) Pageant clients, and receives WM_COPYDATA messages only. Each WM_COPYDATA is handled by passing the request back to the main thread via an event object integrated into the Pageant main loop, and then waiting for a second event object that the main thread will signal when the answer comes back, and not returning from the WndProc handler until the response arrives. Hence, if an agent request received via WM_COPYDATA requires GUI activity, then the main thread's GUI message loop will be able to do that in parallel with all Pageant's other activity, including other GUI activity (like the key list dialog box) and including responding to other requests via named pipe. I can't stop WM_COPYDATA requests from blocking _each other_, but this allows them not to block anything else. And named-pipe requests block nothing at all, so as clients switch over to the new IPC, even that blockage will become less and less common.
2020-01-25 17:24:17 +00:00
static void wm_copydata_got_response(
PageantClient *pc, PageantClientRequestId *reqid, ptrlen response)
Pageant: new asynchronous internal APIs. This is a pure refactoring: no functional change expected. This commit introduces two new small vtable-style APIs. One is PageantClient, which identifies a particular client of the Pageant 'core' (meaning the code that handles each individual request). This changes pageant_handle_msg into an asynchronous operation: you pass in an agent request message and an identifier, and at some later point, the got_response method in your PageantClient will be called with the answer (and the same identifier, to allow you to match requests to responses). The trait vtable also contains a logging system. The main importance of PageantClient, and the reason why it has to exist instead of just passing pageant_handle_msg a bare callback function pointer and context parameter, is that it provides robustness if a client stops existing while a request is still pending. You call pageant_unregister_client, and any unfinished requests associated with that client in the Pageant core will be cleaned up, so that you're guaranteed that after the unregister operation, no stray callbacks will happen with a stale pointer to that client. The WM_COPYDATA interface of Windows Pageant is a direct client of this API. The other client is PageantListener, the system that lives in pageant.c and handles stream-based agent connections for both Unix Pageant and the new Windows named-pipe IPC. More specifically, each individual connection to the listening socket is a separate PageantClient, which means that if a socket is closed abruptly or suffers an OS error, that client can be unregistered and any pending requests cancelled without disrupting other connections. Users of PageantListener have a second client vtable they can use, called PageantListenerClient. That contains _only_ logging facilities, and at the moment, only Unix Pageant bothers to use it (and even that only in debugging mode). Finally, internally to the Pageant core, there's a new trait called PageantAsyncOp which describes an agent request in the process of being handled. But at the moment, it has only one trivial implementation, which is handed the full response message already constructed, and on the next toplevel callback, passes it back to the PageantClient.
2020-01-25 17:24:28 +00:00
{
if (response.len > wmct.bodysize) {
winpgnt: handle WM_COPYDATA requests in a subthread. This is preparation to allow Pageant to be able to return to its GUI main loop in the middle of handling a request (e.g. present a dialog box to the user related to that particular request, and wait for the user's response). In order to do that, we need the main thread's Windows message loop to never be blocked by a WM_COPYDATA agent request. So I've split Pageant's previous hidden window into two hidden windows, each with a subset of the original roles, and created in different threads so that they get independent message loops. The one in the main thread receives messages relating to Pageant's system tray icon; the one in the subthread has the identity known to (old) Pageant clients, and receives WM_COPYDATA messages only. Each WM_COPYDATA is handled by passing the request back to the main thread via an event object integrated into the Pageant main loop, and then waiting for a second event object that the main thread will signal when the answer comes back, and not returning from the WndProc handler until the response arrives. Hence, if an agent request received via WM_COPYDATA requires GUI activity, then the main thread's GUI message loop will be able to do that in parallel with all Pageant's other activity, including other GUI activity (like the key list dialog box) and including responding to other requests via named pipe. I can't stop WM_COPYDATA requests from blocking _each other_, but this allows them not to block anything else. And named-pipe requests block nothing at all, so as clients switch over to the new IPC, even that blockage will become less and less common.
2020-01-25 17:24:17 +00:00
/* Output would overflow message buffer. Replace with a
* failure message. */
Pageant: new asynchronous internal APIs. This is a pure refactoring: no functional change expected. This commit introduces two new small vtable-style APIs. One is PageantClient, which identifies a particular client of the Pageant 'core' (meaning the code that handles each individual request). This changes pageant_handle_msg into an asynchronous operation: you pass in an agent request message and an identifier, and at some later point, the got_response method in your PageantClient will be called with the answer (and the same identifier, to allow you to match requests to responses). The trait vtable also contains a logging system. The main importance of PageantClient, and the reason why it has to exist instead of just passing pageant_handle_msg a bare callback function pointer and context parameter, is that it provides robustness if a client stops existing while a request is still pending. You call pageant_unregister_client, and any unfinished requests associated with that client in the Pageant core will be cleaned up, so that you're guaranteed that after the unregister operation, no stray callbacks will happen with a stale pointer to that client. The WM_COPYDATA interface of Windows Pageant is a direct client of this API. The other client is PageantListener, the system that lives in pageant.c and handles stream-based agent connections for both Unix Pageant and the new Windows named-pipe IPC. More specifically, each individual connection to the listening socket is a separate PageantClient, which means that if a socket is closed abruptly or suffers an OS error, that client can be unregistered and any pending requests cancelled without disrupting other connections. Users of PageantListener have a second client vtable they can use, called PageantListenerClient. That contains _only_ logging facilities, and at the moment, only Unix Pageant bothers to use it (and even that only in debugging mode). Finally, internally to the Pageant core, there's a new trait called PageantAsyncOp which describes an agent request in the process of being handled. But at the moment, it has only one trivial implementation, which is handed the full response message already constructed, and on the next toplevel callback, passes it back to the PageantClient.
2020-01-25 17:24:28 +00:00
static const unsigned char failure[] = { SSH_AGENT_FAILURE };
response = make_ptrlen(failure, lenof(failure));
assert(response.len <= wmct.bodysize);
}
winpgnt: handle WM_COPYDATA requests in a subthread. This is preparation to allow Pageant to be able to return to its GUI main loop in the middle of handling a request (e.g. present a dialog box to the user related to that particular request, and wait for the user's response). In order to do that, we need the main thread's Windows message loop to never be blocked by a WM_COPYDATA agent request. So I've split Pageant's previous hidden window into two hidden windows, each with a subset of the original roles, and created in different threads so that they get independent message loops. The one in the main thread receives messages relating to Pageant's system tray icon; the one in the subthread has the identity known to (old) Pageant clients, and receives WM_COPYDATA messages only. Each WM_COPYDATA is handled by passing the request back to the main thread via an event object integrated into the Pageant main loop, and then waiting for a second event object that the main thread will signal when the answer comes back, and not returning from the WndProc handler until the response arrives. Hence, if an agent request received via WM_COPYDATA requires GUI activity, then the main thread's GUI message loop will be able to do that in parallel with all Pageant's other activity, including other GUI activity (like the key list dialog box) and including responding to other requests via named pipe. I can't stop WM_COPYDATA requests from blocking _each other_, but this allows them not to block anything else. And named-pipe requests block nothing at all, so as clients switch over to the new IPC, even that blockage will become less and less common.
2020-01-25 17:24:17 +00:00
Pageant: new asynchronous internal APIs. This is a pure refactoring: no functional change expected. This commit introduces two new small vtable-style APIs. One is PageantClient, which identifies a particular client of the Pageant 'core' (meaning the code that handles each individual request). This changes pageant_handle_msg into an asynchronous operation: you pass in an agent request message and an identifier, and at some later point, the got_response method in your PageantClient will be called with the answer (and the same identifier, to allow you to match requests to responses). The trait vtable also contains a logging system. The main importance of PageantClient, and the reason why it has to exist instead of just passing pageant_handle_msg a bare callback function pointer and context parameter, is that it provides robustness if a client stops existing while a request is still pending. You call pageant_unregister_client, and any unfinished requests associated with that client in the Pageant core will be cleaned up, so that you're guaranteed that after the unregister operation, no stray callbacks will happen with a stale pointer to that client. The WM_COPYDATA interface of Windows Pageant is a direct client of this API. The other client is PageantListener, the system that lives in pageant.c and handles stream-based agent connections for both Unix Pageant and the new Windows named-pipe IPC. More specifically, each individual connection to the listening socket is a separate PageantClient, which means that if a socket is closed abruptly or suffers an OS error, that client can be unregistered and any pending requests cancelled without disrupting other connections. Users of PageantListener have a second client vtable they can use, called PageantListenerClient. That contains _only_ logging facilities, and at the moment, only Unix Pageant bothers to use it (and even that only in debugging mode). Finally, internally to the Pageant core, there's a new trait called PageantAsyncOp which describes an agent request in the process of being handled. But at the moment, it has only one trivial implementation, which is handed the full response message already constructed, and on the next toplevel callback, passes it back to the PageantClient.
2020-01-25 17:24:28 +00:00
PUT_32BIT_MSB_FIRST(wmct.length, response.len);
memcpy(wmct.body, response.ptr, response.len);
winpgnt: handle WM_COPYDATA requests in a subthread. This is preparation to allow Pageant to be able to return to its GUI main loop in the middle of handling a request (e.g. present a dialog box to the user related to that particular request, and wait for the user's response). In order to do that, we need the main thread's Windows message loop to never be blocked by a WM_COPYDATA agent request. So I've split Pageant's previous hidden window into two hidden windows, each with a subset of the original roles, and created in different threads so that they get independent message loops. The one in the main thread receives messages relating to Pageant's system tray icon; the one in the subthread has the identity known to (old) Pageant clients, and receives WM_COPYDATA messages only. Each WM_COPYDATA is handled by passing the request back to the main thread via an event object integrated into the Pageant main loop, and then waiting for a second event object that the main thread will signal when the answer comes back, and not returning from the WndProc handler until the response arrives. Hence, if an agent request received via WM_COPYDATA requires GUI activity, then the main thread's GUI message loop will be able to do that in parallel with all Pageant's other activity, including other GUI activity (like the key list dialog box) and including responding to other requests via named pipe. I can't stop WM_COPYDATA requests from blocking _each other_, but this allows them not to block anything else. And named-pipe requests block nothing at all, so as clients switch over to the new IPC, even that blockage will become less and less common.
2020-01-25 17:24:17 +00:00
SetEvent(wmct.ev_reply_ready);
}
static bool ask_passphrase_common(PageantClientDialogId *dlgid,
const char *comment)
{
/* Pageant core should be serialising requests, so we never expect
* a passphrase prompt to exist already at this point */
assert(!nonmodal_passphrase_hwnd);
struct PassphraseProcStruct *pps = snew(struct PassphraseProcStruct);
pps->modal = false;
pps->help_topic = WINHELP_CTX_pageant_deferred;
pps->dlgid = dlgid;
pps->passphrase = NULL;
pps->comment = comment;
nonmodal_passphrase_hwnd = CreateDialogParam(
hinst, MAKEINTRESOURCE(IDD_ONDEMAND_PASSPHRASE),
NULL, PassphraseProc, (LPARAM)pps);
/*
* Try to put this passphrase prompt into the foreground.
*
* This will probably not succeed in giving it the actual keyboard
* focus, because Windows is quite opposed to applications being
* able to suddenly steal the focus on their own initiative.
*
* That makes sense in a lot of situations, as a defensive
* measure. If you were about to type a password or other secret
* data into the window you already had focused, and some
* malicious app stole the focus, it might manage to trick you
* into typing your secrets into _it_ instead.
*
* In this case it's possible to regard the same defensive measure
* as counterproductive, because the effect if we _do_ steal focus
* is that you type something into our passphrase prompt that
* isn't the passphrase, and we fail to decrypt the key, and no
* harm is done. Whereas the effect of the user wrongly _assuming_
* the new passphrase prompt has the focus is much worse: now you
* type your highly secret passphrase into some other window you
* didn't mean to trust with that information - such as the
* agent-forwarded PuTTY in which you just ran an ssh command,
* which the _whole point_ was to avoid telling your passphrase to!
*
* On the other hand, I'm sure _every_ application author can come
* up with an argument for why they think _they_ should be allowed
* to steal the focus. Probably most of them include the claim
* that no harm is done if their application receives data
* intended for something else, and of course that's not always
* true!
*
* In any case, I don't know of anything I can do about it, or
* anything I _should_ do about it if I could. If anyone thinks
* they can improve on all this, patches are welcome.
*/
SetForegroundWindow(nonmodal_passphrase_hwnd);
return true;
}
static bool wm_copydata_ask_passphrase(
PageantClient *pc, PageantClientDialogId *dlgid, const char *comment)
{
return ask_passphrase_common(dlgid, comment);
}
Pageant: new asynchronous internal APIs. This is a pure refactoring: no functional change expected. This commit introduces two new small vtable-style APIs. One is PageantClient, which identifies a particular client of the Pageant 'core' (meaning the code that handles each individual request). This changes pageant_handle_msg into an asynchronous operation: you pass in an agent request message and an identifier, and at some later point, the got_response method in your PageantClient will be called with the answer (and the same identifier, to allow you to match requests to responses). The trait vtable also contains a logging system. The main importance of PageantClient, and the reason why it has to exist instead of just passing pageant_handle_msg a bare callback function pointer and context parameter, is that it provides robustness if a client stops existing while a request is still pending. You call pageant_unregister_client, and any unfinished requests associated with that client in the Pageant core will be cleaned up, so that you're guaranteed that after the unregister operation, no stray callbacks will happen with a stale pointer to that client. The WM_COPYDATA interface of Windows Pageant is a direct client of this API. The other client is PageantListener, the system that lives in pageant.c and handles stream-based agent connections for both Unix Pageant and the new Windows named-pipe IPC. More specifically, each individual connection to the listening socket is a separate PageantClient, which means that if a socket is closed abruptly or suffers an OS error, that client can be unregistered and any pending requests cancelled without disrupting other connections. Users of PageantListener have a second client vtable they can use, called PageantListenerClient. That contains _only_ logging facilities, and at the moment, only Unix Pageant bothers to use it (and even that only in debugging mode). Finally, internally to the Pageant core, there's a new trait called PageantAsyncOp which describes an agent request in the process of being handled. But at the moment, it has only one trivial implementation, which is handed the full response message already constructed, and on the next toplevel callback, passes it back to the PageantClient.
2020-01-25 17:24:28 +00:00
static const PageantClientVtable wmcpc_vtable = {
.log = NULL, /* no logging in this client */
.got_response = wm_copydata_got_response,
.ask_passphrase = wm_copydata_ask_passphrase,
Pageant: new asynchronous internal APIs. This is a pure refactoring: no functional change expected. This commit introduces two new small vtable-style APIs. One is PageantClient, which identifies a particular client of the Pageant 'core' (meaning the code that handles each individual request). This changes pageant_handle_msg into an asynchronous operation: you pass in an agent request message and an identifier, and at some later point, the got_response method in your PageantClient will be called with the answer (and the same identifier, to allow you to match requests to responses). The trait vtable also contains a logging system. The main importance of PageantClient, and the reason why it has to exist instead of just passing pageant_handle_msg a bare callback function pointer and context parameter, is that it provides robustness if a client stops existing while a request is still pending. You call pageant_unregister_client, and any unfinished requests associated with that client in the Pageant core will be cleaned up, so that you're guaranteed that after the unregister operation, no stray callbacks will happen with a stale pointer to that client. The WM_COPYDATA interface of Windows Pageant is a direct client of this API. The other client is PageantListener, the system that lives in pageant.c and handles stream-based agent connections for both Unix Pageant and the new Windows named-pipe IPC. More specifically, each individual connection to the listening socket is a separate PageantClient, which means that if a socket is closed abruptly or suffers an OS error, that client can be unregistered and any pending requests cancelled without disrupting other connections. Users of PageantListener have a second client vtable they can use, called PageantListenerClient. That contains _only_ logging facilities, and at the moment, only Unix Pageant bothers to use it (and even that only in debugging mode). Finally, internally to the Pageant core, there's a new trait called PageantAsyncOp which describes an agent request in the process of being handled. But at the moment, it has only one trivial implementation, which is handed the full response message already constructed, and on the next toplevel callback, passes it back to the PageantClient.
2020-01-25 17:24:28 +00:00
};
static char *answer_filemapping_message(const char *mapname)
{
HANDLE maphandle = INVALID_HANDLE_VALUE;
void *mapaddr = NULL;
char *err = NULL;
winpgnt.c: handle arbitrarily large file mappings. I heard recently that at least one third-party client of Pageant exists, and that it's used to generate signatures to use with TLS client certificates. Apparently the signature scheme is compatible, but TLS tends to need signatures over more data than will fit in AGENT_MAX_MSGLEN. Before the BinarySink refactor in commit b6cbad89f, this was OK because the Windows Pageant IPC didn't check the size of the _input_ message against AGENT_MAX_MSGLEN, only the output one. But then we started checking both, so that third-party TLS client started failing. Now we use VirtualQuery to find out the actual size of the file mapping we've been passed, and our only requirement is that the input and output messages should both fit in _that_. So TLS should work again, and also, other clients should be able to retrieve longer lists of public keys if they pass a larger file mapping. One side effect of this change is that Pageant's reply message is now written directly into the shared-memory region. Previously, it was written into a separate buffer and then memcpy()ed over after pageant_handle_msg returned, but now the buffer is variable-size, it seems to me to make more sense to avoid that extra not-entirely controlled malloc. So I've done one very small reordering of statements in the cross-platform pageant_handle_msg(), which fixes the only case I could find where that function started writing its output before it had finished using the contents of the input buffer.
2018-07-08 15:46:32 +00:00
size_t mapsize;
unsigned msglen;
PSID mapsid = NULL;
PSID expectedsid = NULL;
PSID expectedsid_bc = NULL;
PSECURITY_DESCRIPTOR psd = NULL;
winpgnt: handle WM_COPYDATA requests in a subthread. This is preparation to allow Pageant to be able to return to its GUI main loop in the middle of handling a request (e.g. present a dialog box to the user related to that particular request, and wait for the user's response). In order to do that, we need the main thread's Windows message loop to never be blocked by a WM_COPYDATA agent request. So I've split Pageant's previous hidden window into two hidden windows, each with a subset of the original roles, and created in different threads so that they get independent message loops. The one in the main thread receives messages relating to Pageant's system tray icon; the one in the subthread has the identity known to (old) Pageant clients, and receives WM_COPYDATA messages only. Each WM_COPYDATA is handled by passing the request back to the main thread via an event object integrated into the Pageant main loop, and then waiting for a second event object that the main thread will signal when the answer comes back, and not returning from the WndProc handler until the response arrives. Hence, if an agent request received via WM_COPYDATA requires GUI activity, then the main thread's GUI message loop will be able to do that in parallel with all Pageant's other activity, including other GUI activity (like the key list dialog box) and including responding to other requests via named pipe. I can't stop WM_COPYDATA requests from blocking _each other_, but this allows them not to block anything else. And named-pipe requests block nothing at all, so as clients switch over to the new IPC, even that blockage will become less and less common.
2020-01-25 17:24:17 +00:00
wmct.length = wmct.body = NULL;
winpgnt.c: handle arbitrarily large file mappings. I heard recently that at least one third-party client of Pageant exists, and that it's used to generate signatures to use with TLS client certificates. Apparently the signature scheme is compatible, but TLS tends to need signatures over more data than will fit in AGENT_MAX_MSGLEN. Before the BinarySink refactor in commit b6cbad89f, this was OK because the Windows Pageant IPC didn't check the size of the _input_ message against AGENT_MAX_MSGLEN, only the output one. But then we started checking both, so that third-party TLS client started failing. Now we use VirtualQuery to find out the actual size of the file mapping we've been passed, and our only requirement is that the input and output messages should both fit in _that_. So TLS should work again, and also, other clients should be able to retrieve longer lists of public keys if they pass a larger file mapping. One side effect of this change is that Pageant's reply message is now written directly into the shared-memory region. Previously, it was written into a separate buffer and then memcpy()ed over after pageant_handle_msg returned, but now the buffer is variable-size, it seems to me to make more sense to avoid that extra not-entirely controlled malloc. So I've done one very small reordering of statements in the cross-platform pageant_handle_msg(), which fixes the only case I could find where that function started writing its output before it had finished using the contents of the input buffer.
2018-07-08 15:46:32 +00:00
#ifdef DEBUG_IPC
debug("mapname = \"%s\"\n", mapname);
#endif
maphandle = OpenFileMapping(FILE_MAP_ALL_ACCESS, false, mapname);
if (maphandle == NULL || maphandle == INVALID_HANDLE_VALUE) {
err = dupprintf("OpenFileMapping(\"%s\"): %s",
mapname, win_strerror(GetLastError()));
goto cleanup;
}
#ifdef DEBUG_IPC
debug("maphandle = %p\n", maphandle);
#endif
if (should_have_security()) {
DWORD retd;
if ((expectedsid = get_user_sid()) == NULL) {
err = dupstr("unable to get user SID");
goto cleanup;
}
if ((expectedsid_bc = get_default_sid()) == NULL) {
err = dupstr("unable to get default SID");
goto cleanup;
}
if ((retd = p_GetSecurityInfo(
maphandle, SE_KERNEL_OBJECT, OWNER_SECURITY_INFORMATION,
&mapsid, NULL, NULL, NULL, &psd) != ERROR_SUCCESS)) {
err = dupprintf("unable to get owner of file mapping: "
"GetSecurityInfo returned: %s",
win_strerror(retd));
goto cleanup;
}
#ifdef DEBUG_IPC
{
LPTSTR ours, ours2, theirs;
ConvertSidToStringSid(mapsid, &theirs);
ConvertSidToStringSid(expectedsid, &ours);
ConvertSidToStringSid(expectedsid_bc, &ours2);
debug("got sids:\n oursnew=%s\n oursold=%s\n"
" theirs=%s\n", ours, ours2, theirs);
LocalFree(ours);
LocalFree(ours2);
LocalFree(theirs);
}
#endif
if (!EqualSid(mapsid, expectedsid) &&
!EqualSid(mapsid, expectedsid_bc)) {
err = dupstr("wrong owning SID of file mapping");
goto cleanup;
}
} else {
#ifdef DEBUG_IPC
debug("security APIs not present\n");
#endif
}
mapaddr = MapViewOfFile(maphandle, FILE_MAP_WRITE, 0, 0, 0);
if (!mapaddr) {
err = dupprintf("unable to obtain view of file mapping: %s",
win_strerror(GetLastError()));
goto cleanup;
}
#ifdef DEBUG_IPC
debug("mapped address = %p\n", mapaddr);
#endif
winpgnt.c: handle arbitrarily large file mappings. I heard recently that at least one third-party client of Pageant exists, and that it's used to generate signatures to use with TLS client certificates. Apparently the signature scheme is compatible, but TLS tends to need signatures over more data than will fit in AGENT_MAX_MSGLEN. Before the BinarySink refactor in commit b6cbad89f, this was OK because the Windows Pageant IPC didn't check the size of the _input_ message against AGENT_MAX_MSGLEN, only the output one. But then we started checking both, so that third-party TLS client started failing. Now we use VirtualQuery to find out the actual size of the file mapping we've been passed, and our only requirement is that the input and output messages should both fit in _that_. So TLS should work again, and also, other clients should be able to retrieve longer lists of public keys if they pass a larger file mapping. One side effect of this change is that Pageant's reply message is now written directly into the shared-memory region. Previously, it was written into a separate buffer and then memcpy()ed over after pageant_handle_msg returned, but now the buffer is variable-size, it seems to me to make more sense to avoid that extra not-entirely controlled malloc. So I've done one very small reordering of statements in the cross-platform pageant_handle_msg(), which fixes the only case I could find where that function started writing its output before it had finished using the contents of the input buffer.
2018-07-08 15:46:32 +00:00
{
MEMORY_BASIC_INFORMATION mbi;
size_t mbiSize = VirtualQuery(mapaddr, &mbi, sizeof(mbi));
if (mbiSize == 0) {
err = dupprintf("unable to query view of file mapping: %s",
win_strerror(GetLastError()));
goto cleanup;
}
if (mbiSize < (offsetof(MEMORY_BASIC_INFORMATION, RegionSize) +
sizeof(mbi.RegionSize))) {
err = dupstr("VirtualQuery returned too little data to get "
"region size");
goto cleanup;
}
mapsize = mbi.RegionSize;
}
#ifdef DEBUG_IPC
debug("region size = %"SIZEu"\n", mapsize);
winpgnt.c: handle arbitrarily large file mappings. I heard recently that at least one third-party client of Pageant exists, and that it's used to generate signatures to use with TLS client certificates. Apparently the signature scheme is compatible, but TLS tends to need signatures over more data than will fit in AGENT_MAX_MSGLEN. Before the BinarySink refactor in commit b6cbad89f, this was OK because the Windows Pageant IPC didn't check the size of the _input_ message against AGENT_MAX_MSGLEN, only the output one. But then we started checking both, so that third-party TLS client started failing. Now we use VirtualQuery to find out the actual size of the file mapping we've been passed, and our only requirement is that the input and output messages should both fit in _that_. So TLS should work again, and also, other clients should be able to retrieve longer lists of public keys if they pass a larger file mapping. One side effect of this change is that Pageant's reply message is now written directly into the shared-memory region. Previously, it was written into a separate buffer and then memcpy()ed over after pageant_handle_msg returned, but now the buffer is variable-size, it seems to me to make more sense to avoid that extra not-entirely controlled malloc. So I've done one very small reordering of statements in the cross-platform pageant_handle_msg(), which fixes the only case I could find where that function started writing its output before it had finished using the contents of the input buffer.
2018-07-08 15:46:32 +00:00
#endif
if (mapsize < 5) {
err = dupstr("mapping smaller than smallest possible request");
goto cleanup;
}
winpgnt: handle WM_COPYDATA requests in a subthread. This is preparation to allow Pageant to be able to return to its GUI main loop in the middle of handling a request (e.g. present a dialog box to the user related to that particular request, and wait for the user's response). In order to do that, we need the main thread's Windows message loop to never be blocked by a WM_COPYDATA agent request. So I've split Pageant's previous hidden window into two hidden windows, each with a subset of the original roles, and created in different threads so that they get independent message loops. The one in the main thread receives messages relating to Pageant's system tray icon; the one in the subthread has the identity known to (old) Pageant clients, and receives WM_COPYDATA messages only. Each WM_COPYDATA is handled by passing the request back to the main thread via an event object integrated into the Pageant main loop, and then waiting for a second event object that the main thread will signal when the answer comes back, and not returning from the WndProc handler until the response arrives. Hence, if an agent request received via WM_COPYDATA requires GUI activity, then the main thread's GUI message loop will be able to do that in parallel with all Pageant's other activity, including other GUI activity (like the key list dialog box) and including responding to other requests via named pipe. I can't stop WM_COPYDATA requests from blocking _each other_, but this allows them not to block anything else. And named-pipe requests block nothing at all, so as clients switch over to the new IPC, even that blockage will become less and less common.
2020-01-25 17:24:17 +00:00
wmct.length = (char *)mapaddr;
msglen = GET_32BIT_MSB_FIRST(wmct.length);
#ifdef DEBUG_IPC
debug("msg length=%08x, msg type=%02x\n",
msglen, (unsigned)((unsigned char *) mapaddr)[4]);
#endif
winpgnt: handle WM_COPYDATA requests in a subthread. This is preparation to allow Pageant to be able to return to its GUI main loop in the middle of handling a request (e.g. present a dialog box to the user related to that particular request, and wait for the user's response). In order to do that, we need the main thread's Windows message loop to never be blocked by a WM_COPYDATA agent request. So I've split Pageant's previous hidden window into two hidden windows, each with a subset of the original roles, and created in different threads so that they get independent message loops. The one in the main thread receives messages relating to Pageant's system tray icon; the one in the subthread has the identity known to (old) Pageant clients, and receives WM_COPYDATA messages only. Each WM_COPYDATA is handled by passing the request back to the main thread via an event object integrated into the Pageant main loop, and then waiting for a second event object that the main thread will signal when the answer comes back, and not returning from the WndProc handler until the response arrives. Hence, if an agent request received via WM_COPYDATA requires GUI activity, then the main thread's GUI message loop will be able to do that in parallel with all Pageant's other activity, including other GUI activity (like the key list dialog box) and including responding to other requests via named pipe. I can't stop WM_COPYDATA requests from blocking _each other_, but this allows them not to block anything else. And named-pipe requests block nothing at all, so as clients switch over to the new IPC, even that blockage will become less and less common.
2020-01-25 17:24:17 +00:00
wmct.body = wmct.length + 4;
wmct.bodysize = mapsize - 4;
if (msglen > wmct.bodysize) {
/* Incoming length field is too large. Emit a failure response
* without even trying to handle the request.
*
* (We know this must fit, because we checked mapsize >= 5
* above.) */
PUT_32BIT_MSB_FIRST(wmct.length, 1);
*wmct.body = SSH_AGENT_FAILURE;
} else {
winpgnt: handle WM_COPYDATA requests in a subthread. This is preparation to allow Pageant to be able to return to its GUI main loop in the middle of handling a request (e.g. present a dialog box to the user related to that particular request, and wait for the user's response). In order to do that, we need the main thread's Windows message loop to never be blocked by a WM_COPYDATA agent request. So I've split Pageant's previous hidden window into two hidden windows, each with a subset of the original roles, and created in different threads so that they get independent message loops. The one in the main thread receives messages relating to Pageant's system tray icon; the one in the subthread has the identity known to (old) Pageant clients, and receives WM_COPYDATA messages only. Each WM_COPYDATA is handled by passing the request back to the main thread via an event object integrated into the Pageant main loop, and then waiting for a second event object that the main thread will signal when the answer comes back, and not returning from the WndProc handler until the response arrives. Hence, if an agent request received via WM_COPYDATA requires GUI activity, then the main thread's GUI message loop will be able to do that in parallel with all Pageant's other activity, including other GUI activity (like the key list dialog box) and including responding to other requests via named pipe. I can't stop WM_COPYDATA requests from blocking _each other_, but this allows them not to block anything else. And named-pipe requests block nothing at all, so as clients switch over to the new IPC, even that blockage will become less and less common.
2020-01-25 17:24:17 +00:00
wmct.bodylen = msglen;
SetEvent(wmct.ev_msg_ready);
WaitForSingleObject(wmct.ev_reply_ready, INFINITE);
}
cleanup:
/* expectedsid has the lifetime of the program, so we don't free it */
sfree(expectedsid_bc);
if (psd)
LocalFree(psd);
if (mapaddr)
UnmapViewOfFile(mapaddr);
if (maphandle != NULL && maphandle != INVALID_HANDLE_VALUE)
CloseHandle(maphandle);
return err;
}
static void create_keylist_window(void)
{
if (keylist)
return;
keylist = CreateDialog(hinst, MAKEINTRESOURCE(IDD_KEYLIST),
NULL, KeyListProc);
ShowWindow(keylist, SW_SHOWNORMAL);
}
winpgnt: handle WM_COPYDATA requests in a subthread. This is preparation to allow Pageant to be able to return to its GUI main loop in the middle of handling a request (e.g. present a dialog box to the user related to that particular request, and wait for the user's response). In order to do that, we need the main thread's Windows message loop to never be blocked by a WM_COPYDATA agent request. So I've split Pageant's previous hidden window into two hidden windows, each with a subset of the original roles, and created in different threads so that they get independent message loops. The one in the main thread receives messages relating to Pageant's system tray icon; the one in the subthread has the identity known to (old) Pageant clients, and receives WM_COPYDATA messages only. Each WM_COPYDATA is handled by passing the request back to the main thread via an event object integrated into the Pageant main loop, and then waiting for a second event object that the main thread will signal when the answer comes back, and not returning from the WndProc handler until the response arrives. Hence, if an agent request received via WM_COPYDATA requires GUI activity, then the main thread's GUI message loop will be able to do that in parallel with all Pageant's other activity, including other GUI activity (like the key list dialog box) and including responding to other requests via named pipe. I can't stop WM_COPYDATA requests from blocking _each other_, but this allows them not to block anything else. And named-pipe requests block nothing at all, so as clients switch over to the new IPC, even that blockage will become less and less common.
2020-01-25 17:24:17 +00:00
static LRESULT CALLBACK TrayWndProc(HWND hwnd, UINT message,
WPARAM wParam, LPARAM lParam)
{
Convert a lot of 'int' variables to 'bool'. My normal habit these days, in new code, is to treat int and bool as _almost_ completely separate types. I'm still willing to use C's implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine, no need to spell it out as blob.len != 0), but generally, if a variable is going to be conceptually a boolean, I like to declare it bool and assign to it using 'true' or 'false' rather than 0 or 1. PuTTY is an exception, because it predates the C99 bool, and I've stuck to its existing coding style even when adding new code to it. But it's been annoying me more and more, so now that I've decided C99 bool is an acceptable thing to require from our toolchain in the first place, here's a quite thorough trawl through the source doing 'boolification'. Many variables and function parameters are now typed as bool rather than int; many assignments of 0 or 1 to those variables are now spelled 'true' or 'false'. I managed this thorough conversion with the help of a custom clang plugin that I wrote to trawl the AST and apply heuristics to point out where things might want changing. So I've even managed to do a decent job on parts of the code I haven't looked at in years! To make the plugin's work easier, I pushed platform front ends generally in the direction of using standard 'bool' in preference to platform-specific boolean types like Windows BOOL or GTK's gboolean; I've left the platform booleans in places they _have_ to be for the platform APIs to work right, but variables only used by my own code have been converted wherever I found them. In a few places there are int values that look very like booleans in _most_ of the places they're used, but have a rarely-used third value, or a distinction between different nonzero values that most users don't care about. In these cases, I've _removed_ uses of 'true' and 'false' for the return values, to emphasise that there's something more subtle going on than a simple boolean answer: - the 'multisel' field in dialog.h's list box structure, for which the GTK front end in particular recognises a difference between 1 and 2 but nearly everything else treats as boolean - the 'urgent' parameter to plug_receive, where 1 vs 2 tells you something about the specific location of the urgent pointer, but most clients only care about 0 vs 'something nonzero' - the return value of wc_match, where -1 indicates a syntax error in the wildcard. - the return values from SSH-1 RSA-key loading functions, which use -1 for 'wrong passphrase' and 0 for all other failures (so any caller which already knows it's not loading an _encrypted private_ key can treat them as boolean) - term->esc_query, and the 'query' parameter in toggle_mode in terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h, but can also hold -1 for some other intervening character that we don't support. In a few places there's an integer that I haven't turned into a bool even though it really _can_ only take values 0 or 1 (and, as above, tried to make the call sites consistent in not calling those values true and false), on the grounds that I thought it would make it more confusing to imply that the 0 value was in some sense 'negative' or bad and the 1 positive or good: - the return value of plug_accepting uses the POSIXish convention of 0=success and nonzero=error; I think if I made it bool then I'd also want to reverse its sense, and that's a job for a separate piece of work. - the 'screen' parameter to lineptr() in terminal.c, where 0 and 1 represent the default and alternate screens. There's no obvious reason why one of those should be considered 'true' or 'positive' or 'success' - they're just indices - so I've left it as int. ssh_scp_recv had particularly confusing semantics for its previous int return value: its call sites used '<= 0' to check for error, but it never actually returned a negative number, just 0 or 1. Now the function and its call sites agree that it's a bool. In a couple of places I've renamed variables called 'ret', because I don't like that name any more - it's unclear whether it means the return value (in preparation) for the _containing_ function or the return value received from a subroutine call, and occasionally I've accidentally used the same variable for both and introduced a bug. So where one of those got in my way, I've renamed it to 'toret' or 'retd' (the latter short for 'returned') in line with my usual modern practice, but I haven't done a thorough job of finding all of them. Finally, one amusing side effect of doing this is that I've had to separate quite a few chained assignments. It used to be perfectly fine to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a the 'true' defined by stdbool.h, that idiom provokes a warning from gcc: 'suggest parentheses around assignment used as truth value'!
2018-11-02 19:23:19 +00:00
static bool menuinprogress;
static UINT msgTaskbarCreated = 0;
switch (message) {
case WM_CREATE:
msgTaskbarCreated = RegisterWindowMessage(_T("TaskbarCreated"));
break;
default:
if (message==msgTaskbarCreated) {
/*
* Explorer has been restarted, so the tray icon will
* have been lost.
*/
AddTrayIcon(hwnd);
}
break;
case WM_SYSTRAY:
if (lParam == WM_RBUTTONUP) {
POINT cursorpos;
GetCursorPos(&cursorpos);
PostMessage(hwnd, WM_SYSTRAY2, cursorpos.x, cursorpos.y);
} else if (lParam == WM_LBUTTONDBLCLK) {
/* Run the default menu item. */
UINT menuitem = GetMenuDefaultItem(systray_menu, false, 0);
if (menuitem != -1)
PostMessage(hwnd, WM_COMMAND, menuitem, 0);
}
break;
case WM_SYSTRAY2:
if (!menuinprogress) {
menuinprogress = true;
update_sessions();
SetForegroundWindow(hwnd);
TrackPopupMenu(systray_menu,
TPM_RIGHTALIGN | TPM_BOTTOMALIGN |
TPM_RIGHTBUTTON,
wParam, lParam, 0, hwnd, NULL);
menuinprogress = false;
}
break;
case WM_COMMAND:
case WM_SYSCOMMAND: {
unsigned command = wParam & ~0xF; /* low 4 bits reserved to Windows */
switch (command) {
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
case IDM_PUTTY: {
TCHAR cmdline[10];
cmdline[0] = '\0';
if (restrict_putty_acl)
strcat(cmdline, "&R");
if ((INT_PTR)ShellExecute(hwnd, NULL, putty_path, cmdline,
_T(""), SW_SHOW) <= 32) {
MessageBox(NULL, "Unable to execute PuTTY!",
"Error", MB_OK | MB_ICONERROR);
}
break;
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
}
case IDM_CLOSE:
if (modal_passphrase_hwnd)
SendMessage(modal_passphrase_hwnd, WM_CLOSE, 0, 0);
SendMessage(hwnd, WM_CLOSE, 0, 0);
break;
case IDM_VIEWKEYS:
create_keylist_window();
/*
* Sometimes the window comes up minimised / hidden for
* no obvious reason. Prevent this. This also brings it
* to the front if it's already present (the user
* selected View Keys because they wanted to _see_ the
* thing).
*/
SetForegroundWindow(keylist);
SetWindowPos(keylist, HWND_TOP, 0, 0, 0, 0,
SWP_NOMOVE | SWP_NOSIZE | SWP_SHOWWINDOW);
break;
case IDM_ADDKEY:
case IDM_ADDKEY_ENCRYPTED:
if (modal_passphrase_hwnd) {
MessageBeep(MB_ICONERROR);
SetForegroundWindow(modal_passphrase_hwnd);
break;
}
prompt_add_keyfile(command == IDM_ADDKEY_ENCRYPTED);
break;
case IDM_REMOVE_ALL:
pageant_delete_all();
keylist_update();
break;
case IDM_REENCRYPT_ALL:
pageant_reencrypt_all();
keylist_update();
break;
case IDM_ABOUT:
if (!aboutbox) {
aboutbox = CreateDialog(hinst, MAKEINTRESOURCE(IDD_ABOUT),
NULL, AboutProc);
ShowWindow(aboutbox, SW_SHOWNORMAL);
/*
* Sometimes the window comes up minimised / hidden
* for no obvious reason. Prevent this.
*/
SetForegroundWindow(aboutbox);
SetWindowPos(aboutbox, HWND_TOP, 0, 0, 0, 0,
SWP_NOMOVE | SWP_NOSIZE | SWP_SHOWWINDOW);
}
break;
case IDM_HELP:
launch_help(hwnd, WINHELP_CTX_pageant_general);
break;
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
default: {
if (wParam >= IDM_SESSIONS_BASE && wParam <= IDM_SESSIONS_MAX) {
MENUITEMINFO mii;
TCHAR buf[MAX_PATH + 1];
TCHAR param[MAX_PATH + 1];
memset(&mii, 0, sizeof(mii));
mii.cbSize = sizeof(mii);
mii.fMask = MIIM_TYPE;
mii.cch = MAX_PATH;
mii.dwTypeData = buf;
GetMenuItemInfo(session_menu, wParam, false, &mii);
param[0] = '\0';
if (restrict_putty_acl)
strcat(param, "&R");
strcat(param, "@");
strcat(param, mii.dwTypeData);
if ((INT_PTR)ShellExecute(hwnd, NULL, putty_path, param,
_T(""), SW_SHOW) <= 32) {
MessageBox(NULL, "Unable to execute PuTTY!", "Error",
MB_OK | MB_ICONERROR);
}
}
break;
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
}
}
break;
}
case WM_NETEVENT:
winselgui_response(wParam, lParam);
return 0;
case WM_DESTROY:
quit_help(hwnd);
PostQuitMessage(0);
return 0;
winpgnt: handle WM_COPYDATA requests in a subthread. This is preparation to allow Pageant to be able to return to its GUI main loop in the middle of handling a request (e.g. present a dialog box to the user related to that particular request, and wait for the user's response). In order to do that, we need the main thread's Windows message loop to never be blocked by a WM_COPYDATA agent request. So I've split Pageant's previous hidden window into two hidden windows, each with a subset of the original roles, and created in different threads so that they get independent message loops. The one in the main thread receives messages relating to Pageant's system tray icon; the one in the subthread has the identity known to (old) Pageant clients, and receives WM_COPYDATA messages only. Each WM_COPYDATA is handled by passing the request back to the main thread via an event object integrated into the Pageant main loop, and then waiting for a second event object that the main thread will signal when the answer comes back, and not returning from the WndProc handler until the response arrives. Hence, if an agent request received via WM_COPYDATA requires GUI activity, then the main thread's GUI message loop will be able to do that in parallel with all Pageant's other activity, including other GUI activity (like the key list dialog box) and including responding to other requests via named pipe. I can't stop WM_COPYDATA requests from blocking _each other_, but this allows them not to block anything else. And named-pipe requests block nothing at all, so as clients switch over to the new IPC, even that blockage will become less and less common.
2020-01-25 17:24:17 +00:00
}
return DefWindowProc(hwnd, message, wParam, lParam);
}
static LRESULT CALLBACK wm_copydata_WndProc(HWND hwnd, UINT message,
WPARAM wParam, LPARAM lParam)
{
switch (message) {
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
case WM_COPYDATA: {
COPYDATASTRUCT *cds;
char *mapname, *err;
cds = (COPYDATASTRUCT *) lParam;
if (cds->dwData != AGENT_COPYDATA_ID)
return 0; /* not our message, mate */
mapname = (char *) cds->lpData;
if (mapname[cds->cbData - 1] != '\0')
return 0; /* failure to be ASCIZ! */
err = answer_filemapping_message(mapname);
if (err) {
#ifdef DEBUG_IPC
debug("IPC failed: %s\n", err);
#endif
sfree(err);
return 0;
}
Formatting change to braces around one case of a switch. Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
2020-02-16 07:49:52 +00:00
return 1;
}
}
return DefWindowProc(hwnd, message, wParam, lParam);
}
winpgnt: handle WM_COPYDATA requests in a subthread. This is preparation to allow Pageant to be able to return to its GUI main loop in the middle of handling a request (e.g. present a dialog box to the user related to that particular request, and wait for the user's response). In order to do that, we need the main thread's Windows message loop to never be blocked by a WM_COPYDATA agent request. So I've split Pageant's previous hidden window into two hidden windows, each with a subset of the original roles, and created in different threads so that they get independent message loops. The one in the main thread receives messages relating to Pageant's system tray icon; the one in the subthread has the identity known to (old) Pageant clients, and receives WM_COPYDATA messages only. Each WM_COPYDATA is handled by passing the request back to the main thread via an event object integrated into the Pageant main loop, and then waiting for a second event object that the main thread will signal when the answer comes back, and not returning from the WndProc handler until the response arrives. Hence, if an agent request received via WM_COPYDATA requires GUI activity, then the main thread's GUI message loop will be able to do that in parallel with all Pageant's other activity, including other GUI activity (like the key list dialog box) and including responding to other requests via named pipe. I can't stop WM_COPYDATA requests from blocking _each other_, but this allows them not to block anything else. And named-pipe requests block nothing at all, so as clients switch over to the new IPC, even that blockage will become less and less common.
2020-01-25 17:24:17 +00:00
static DWORD WINAPI wm_copydata_threadfunc(void *param)
{
HINSTANCE inst = *(HINSTANCE *)param;
HWND ipchwnd = CreateWindow(IPCCLASSNAME, IPCWINTITLE,
WS_OVERLAPPEDWINDOW | WS_VSCROLL,
CW_USEDEFAULT, CW_USEDEFAULT,
100, 100, NULL, NULL, inst, NULL);
ShowWindow(ipchwnd, SW_HIDE);
MSG msg;
while (GetMessage(&msg, NULL, 0, 0) == 1) {
TranslateMessage(&msg);
DispatchMessage(&msg);
}
return 0;
}
/*
* Fork and Exec the command in cmdline. [DBW]
*/
void spawn_cmd(const char *cmdline, const char *args, int show)
{
if (ShellExecute(NULL, _T("open"), cmdline,
args, NULL, show) <= (HINSTANCE) 32) {
char *msg;
msg = dupprintf("Failed to run \"%s\": %s", cmdline,
win_strerror(GetLastError()));
MessageBox(NULL, msg, APPNAME, MB_OK | MB_ICONEXCLAMATION);
sfree(msg);
}
}
void noise_ultralight(NoiseSourceId id, unsigned long data)
{
/* Pageant doesn't use random numbers, so we ignore this */
}
void cleanup_exit(int code)
{
shutdown_help();
exit(code);
}
static bool winpgnt_listener_ask_passphrase(
PageantListenerClient *plc, PageantClientDialogId *dlgid,
const char *comment)
{
return ask_passphrase_common(dlgid, comment);
}
Pageant: new asynchronous internal APIs. This is a pure refactoring: no functional change expected. This commit introduces two new small vtable-style APIs. One is PageantClient, which identifies a particular client of the Pageant 'core' (meaning the code that handles each individual request). This changes pageant_handle_msg into an asynchronous operation: you pass in an agent request message and an identifier, and at some later point, the got_response method in your PageantClient will be called with the answer (and the same identifier, to allow you to match requests to responses). The trait vtable also contains a logging system. The main importance of PageantClient, and the reason why it has to exist instead of just passing pageant_handle_msg a bare callback function pointer and context parameter, is that it provides robustness if a client stops existing while a request is still pending. You call pageant_unregister_client, and any unfinished requests associated with that client in the Pageant core will be cleaned up, so that you're guaranteed that after the unregister operation, no stray callbacks will happen with a stale pointer to that client. The WM_COPYDATA interface of Windows Pageant is a direct client of this API. The other client is PageantListener, the system that lives in pageant.c and handles stream-based agent connections for both Unix Pageant and the new Windows named-pipe IPC. More specifically, each individual connection to the listening socket is a separate PageantClient, which means that if a socket is closed abruptly or suffers an OS error, that client can be unregistered and any pending requests cancelled without disrupting other connections. Users of PageantListener have a second client vtable they can use, called PageantListenerClient. That contains _only_ logging facilities, and at the moment, only Unix Pageant bothers to use it (and even that only in debugging mode). Finally, internally to the Pageant core, there's a new trait called PageantAsyncOp which describes an agent request in the process of being handled. But at the moment, it has only one trivial implementation, which is handed the full response message already constructed, and on the next toplevel callback, passes it back to the PageantClient.
2020-01-25 17:24:28 +00:00
struct winpgnt_client {
PageantListenerClient plc;
};
static const PageantListenerClientVtable winpgnt_vtable = {
.log = NULL, /* no logging */
.ask_passphrase = winpgnt_listener_ask_passphrase,
Pageant: new asynchronous internal APIs. This is a pure refactoring: no functional change expected. This commit introduces two new small vtable-style APIs. One is PageantClient, which identifies a particular client of the Pageant 'core' (meaning the code that handles each individual request). This changes pageant_handle_msg into an asynchronous operation: you pass in an agent request message and an identifier, and at some later point, the got_response method in your PageantClient will be called with the answer (and the same identifier, to allow you to match requests to responses). The trait vtable also contains a logging system. The main importance of PageantClient, and the reason why it has to exist instead of just passing pageant_handle_msg a bare callback function pointer and context parameter, is that it provides robustness if a client stops existing while a request is still pending. You call pageant_unregister_client, and any unfinished requests associated with that client in the Pageant core will be cleaned up, so that you're guaranteed that after the unregister operation, no stray callbacks will happen with a stale pointer to that client. The WM_COPYDATA interface of Windows Pageant is a direct client of this API. The other client is PageantListener, the system that lives in pageant.c and handles stream-based agent connections for both Unix Pageant and the new Windows named-pipe IPC. More specifically, each individual connection to the listening socket is a separate PageantClient, which means that if a socket is closed abruptly or suffers an OS error, that client can be unregistered and any pending requests cancelled without disrupting other connections. Users of PageantListener have a second client vtable they can use, called PageantListenerClient. That contains _only_ logging facilities, and at the moment, only Unix Pageant bothers to use it (and even that only in debugging mode). Finally, internally to the Pageant core, there's a new trait called PageantAsyncOp which describes an agent request in the process of being handled. But at the moment, it has only one trivial implementation, which is handed the full response message already constructed, and on the next toplevel callback, passes it back to the PageantClient.
2020-01-25 17:24:28 +00:00
};
static struct winpgnt_client wpc[1];
HINSTANCE hinst;
static NORETURN void opt_error(const char *fmt, ...)
{
va_list ap;
va_start(ap, fmt);
char *msg = dupvprintf(fmt, ap);
va_end(ap);
MessageBox(NULL, msg, "Pageant command line error", MB_ICONERROR | MB_OK);
exit(1);
}
#ifdef LEGACY_WINDOWS
BOOL sw_PeekMessage(LPMSG msg, HWND hwnd, UINT min, UINT max, UINT remove)
{
static bool unicode_unavailable = false;
if (!unicode_unavailable) {
BOOL ret = PeekMessageW(msg, hwnd, min, max, remove);
if (!ret && GetLastError() == ERROR_CALL_NOT_IMPLEMENTED)
unicode_unavailable = true; /* don't try again */
else
return ret;
}
return PeekMessageA(msg, hwnd, min, max, remove);
}
#else
#define sw_PeekMessage PeekMessageW
#endif
int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show)
{
MSG msg;
const char *command = NULL;
const char *unixsocket = NULL;
bool show_keylist_on_startup = false;
Filename *openssh_config_file = NULL;
Windows Pageant: make atomic client/server decision. In the previous state of the code, we first tested agent_exists() to decide whether to be the long-running Pageant server or a short-lived client; then, during the command-line parsing loop, we prompted for passphrases to add keys presented on the command line (to ourself or the server, respectively); *then* we set up the named pipe and WM_COPYDATA receiver window to actually start functioning as a server, if we decided that was our role. A consequence is that if a user started up two Pageants each with an encrypted key on the command line, there would be a race condition: each one would decide that it was _going_ to be the server, then prompt for a passphrase, and then try to set itself up as the server once the passphrase is entered. So whichever one's passphrase prompt was answered second would add its key to its own internal data structures, then fail to set up the server's named pipe, terminate with an error, and end up not having added its key to the _surviving_ server. This change reorders the setup steps so that the command-line parsing loop does not add the keys immediately; instead it merely caches the key filenames provided. Then we make the decision about whether we're the server, and set up both the named pipe and WM_COPYDATA window if we are; and finally, we go back to our list of key filenames and actually add them, either to ourself (if we're the server) or to some other Pageant (if we're a client). Moreover, the decision about whether to be the server is now wrapped in an interprocess mutex similar to the one used in connection sharing, which means that even if two or more Pageants are started up as close to simultaneously as possible, they should avoid a race condition and successfully manage to agree on exactly one of themselves to be the server. For example, a user reported that this could occur if you put shortcuts to multiple private key files in your Windows Startup folder, so that they were all launched simultaneously at startup. One slightly odd behaviour that remains: if the server Pageant has to prompt for private key passphrases at startup, then it won't actually start _servicing_ requests from other Pageants until it's finished dealing with its own prompts. As a result, if you do start up two Pageants at once each with an encrypted key file on its command line, the second one won't even manage to present its passphrase prompt until the first one's prompt is dismissed, because it will block waiting for the initial check of the key list. But it will get there in the end, so that's only a cosmetic oddity. It would be nice to arrange that Pageant GUI operations don't block unrelated agent requests (e.g. by having the GUI and the agent code run in separate threads). But that's a bigger problem, not specific to startup time - the same thing happens if you interactively load a key via Pageant's file dialog. And it would require a major reorganisation to fix that fully, because currently the GUI code depends on being able to call _internal_ Pageant query functions like pageant_count_ssh2_keys() that don't work by constructing an agent request at all.
2022-01-03 12:17:15 +00:00
typedef struct CommandLineKey {
Filename *fn;
bool add_encrypted;
} CommandLineKey;
CommandLineKey *clkeys = NULL;
size_t nclkeys = 0, clkeysize = 0;
dll_hijacking_protection();
hinst = inst;
if (should_have_security()) {
/*
* Attempt to get the security API we need.
*/
if (!got_advapi()) {
MessageBox(NULL,
"Unable to access security APIs. Pageant will\n"
"not run, in case it causes a security breach.",
"Pageant Fatal Error", MB_ICONERROR | MB_OK);
return 1;
}
}
/*
* See if we can find our Help file.
*/
init_help();
/*
* Look for the PuTTY binary (we will enable the saved session
* submenu if we find it).
*/
{
char b[2048], *p, *q, *r;
FILE *fp;
GetModuleFileName(NULL, b, sizeof(b) - 16);
r = b;
p = strrchr(b, '\\');
if (p && p >= r) r = p+1;
q = strrchr(b, ':');
if (q && q >= r) r = q+1;
strcpy(r, "putty.exe");
if ( (fp = fopen(b, "r")) != NULL) {
putty_path = dupstr(b);
fclose(fp);
} else
putty_path = NULL;
}
/*
Windows Pageant: make atomic client/server decision. In the previous state of the code, we first tested agent_exists() to decide whether to be the long-running Pageant server or a short-lived client; then, during the command-line parsing loop, we prompted for passphrases to add keys presented on the command line (to ourself or the server, respectively); *then* we set up the named pipe and WM_COPYDATA receiver window to actually start functioning as a server, if we decided that was our role. A consequence is that if a user started up two Pageants each with an encrypted key on the command line, there would be a race condition: each one would decide that it was _going_ to be the server, then prompt for a passphrase, and then try to set itself up as the server once the passphrase is entered. So whichever one's passphrase prompt was answered second would add its key to its own internal data structures, then fail to set up the server's named pipe, terminate with an error, and end up not having added its key to the _surviving_ server. This change reorders the setup steps so that the command-line parsing loop does not add the keys immediately; instead it merely caches the key filenames provided. Then we make the decision about whether we're the server, and set up both the named pipe and WM_COPYDATA window if we are; and finally, we go back to our list of key filenames and actually add them, either to ourself (if we're the server) or to some other Pageant (if we're a client). Moreover, the decision about whether to be the server is now wrapped in an interprocess mutex similar to the one used in connection sharing, which means that even if two or more Pageants are started up as close to simultaneously as possible, they should avoid a race condition and successfully manage to agree on exactly one of themselves to be the server. For example, a user reported that this could occur if you put shortcuts to multiple private key files in your Windows Startup folder, so that they were all launched simultaneously at startup. One slightly odd behaviour that remains: if the server Pageant has to prompt for private key passphrases at startup, then it won't actually start _servicing_ requests from other Pageants until it's finished dealing with its own prompts. As a result, if you do start up two Pageants at once each with an encrypted key file on its command line, the second one won't even manage to present its passphrase prompt until the first one's prompt is dismissed, because it will block waiting for the initial check of the key list. But it will get there in the end, so that's only a cosmetic oddity. It would be nice to arrange that Pageant GUI operations don't block unrelated agent requests (e.g. by having the GUI and the agent code run in separate threads). But that's a bigger problem, not specific to startup time - the same thing happens if you interactively load a key via Pageant's file dialog. And it would require a major reorganisation to fix that fully, because currently the GUI code depends on being able to call _internal_ Pageant query functions like pageant_count_ssh2_keys() that don't work by constructing an agent request at all.
2022-01-03 12:17:15 +00:00
* Process the command line, handling anything that can be done
* immediately, but deferring adding keys until after we've
* started up the main agent. Details of keys to be added are
* stored in the 'clkeys' array.
*/
bool add_keys_encrypted = false;
New abstraction for command-line arguments. This begins the process of enabling our Windows applications to handle Unicode characters on their command lines which don't fit in the system code page. Instead of passing plain strings to cmdline_process_param, we now pass a partially opaque and platform-specific thing called a CmdlineArg. This has a method that extracts the argument word as a default-encoded string, and another one that tries to extract it as UTF-8 (though it may fail if the UTF-8 isn't available). On Windows, the command line is now constructed by calling split_into_argv_w on the Unicode command line returned by GetCommandLineW(), and the UTF-8 method returns text converted directly from that wide-character form, not going via the system code page. So it _can_ include UTF-8 characters that wouldn't have round-tripped via CP_ACP. This commit introduces the abstraction and switches over the cross-platform and Windows argv-handling code to use it, with minimal functional change. Nothing yet tries to call cmdline_arg_get_utf8(). I say 'cross-platform and Windows' because on the Unix side there's still a lot of use of plain old argv which I haven't converted. That would be a much larger project, and isn't currently needed: the _current_ aim of this abstraction is to get the right things to happen relating to Unicode on Windows, so for code that doesn't run on Windows anyway, it's not adding value. (Also there's a tension with GTK, which wants to talk to standard argv and extract arguments _it_ knows about, so at the very least we'd have to let it munge argv before importing it into this new system.)
2024-09-25 09:18:38 +00:00
AuxMatchOpt amo = aux_match_opt_init(opt_error);
while (!aux_match_done(&amo)) {
New abstraction for command-line arguments. This begins the process of enabling our Windows applications to handle Unicode characters on their command lines which don't fit in the system code page. Instead of passing plain strings to cmdline_process_param, we now pass a partially opaque and platform-specific thing called a CmdlineArg. This has a method that extracts the argument word as a default-encoded string, and another one that tries to extract it as UTF-8 (though it may fail if the UTF-8 isn't available). On Windows, the command line is now constructed by calling split_into_argv_w on the Unicode command line returned by GetCommandLineW(), and the UTF-8 method returns text converted directly from that wide-character form, not going via the system code page. So it _can_ include UTF-8 characters that wouldn't have round-tripped via CP_ACP. This commit introduces the abstraction and switches over the cross-platform and Windows argv-handling code to use it, with minimal functional change. Nothing yet tries to call cmdline_arg_get_utf8(). I say 'cross-platform and Windows' because on the Unix side there's still a lot of use of plain old argv which I haven't converted. That would be a much larger project, and isn't currently needed: the _current_ aim of this abstraction is to get the right things to happen relating to Unicode on Windows, so for code that doesn't run on Windows anyway, it's not adding value. (Also there's a tension with GTK, which wants to talk to standard argv and extract arguments _it_ knows about, so at the very least we'd have to let it munge argv before importing it into this new system.)
2024-09-25 09:18:38 +00:00
CmdlineArg *valarg;
#define match_opt(...) aux_match_opt( \
&amo, NULL, __VA_ARGS__, (const char *)NULL)
#define match_optval(...) aux_match_opt( \
New abstraction for command-line arguments. This begins the process of enabling our Windows applications to handle Unicode characters on their command lines which don't fit in the system code page. Instead of passing plain strings to cmdline_process_param, we now pass a partially opaque and platform-specific thing called a CmdlineArg. This has a method that extracts the argument word as a default-encoded string, and another one that tries to extract it as UTF-8 (though it may fail if the UTF-8 isn't available). On Windows, the command line is now constructed by calling split_into_argv_w on the Unicode command line returned by GetCommandLineW(), and the UTF-8 method returns text converted directly from that wide-character form, not going via the system code page. So it _can_ include UTF-8 characters that wouldn't have round-tripped via CP_ACP. This commit introduces the abstraction and switches over the cross-platform and Windows argv-handling code to use it, with minimal functional change. Nothing yet tries to call cmdline_arg_get_utf8(). I say 'cross-platform and Windows' because on the Unix side there's still a lot of use of plain old argv which I haven't converted. That would be a much larger project, and isn't currently needed: the _current_ aim of this abstraction is to get the right things to happen relating to Unicode on Windows, so for code that doesn't run on Windows anyway, it's not adding value. (Also there's a tension with GTK, which wants to talk to standard argv and extract arguments _it_ knows about, so at the very least we'd have to let it munge argv before importing it into this new system.)
2024-09-25 09:18:38 +00:00
&amo, &valarg, __VA_ARGS__, (const char *)NULL)
New abstraction for command-line arguments. This begins the process of enabling our Windows applications to handle Unicode characters on their command lines which don't fit in the system code page. Instead of passing plain strings to cmdline_process_param, we now pass a partially opaque and platform-specific thing called a CmdlineArg. This has a method that extracts the argument word as a default-encoded string, and another one that tries to extract it as UTF-8 (though it may fail if the UTF-8 isn't available). On Windows, the command line is now constructed by calling split_into_argv_w on the Unicode command line returned by GetCommandLineW(), and the UTF-8 method returns text converted directly from that wide-character form, not going via the system code page. So it _can_ include UTF-8 characters that wouldn't have round-tripped via CP_ACP. This commit introduces the abstraction and switches over the cross-platform and Windows argv-handling code to use it, with minimal functional change. Nothing yet tries to call cmdline_arg_get_utf8(). I say 'cross-platform and Windows' because on the Unix side there's still a lot of use of plain old argv which I haven't converted. That would be a much larger project, and isn't currently needed: the _current_ aim of this abstraction is to get the right things to happen relating to Unicode on Windows, so for code that doesn't run on Windows anyway, it's not adding value. (Also there's a tension with GTK, which wants to talk to standard argv and extract arguments _it_ knows about, so at the very least we'd have to let it munge argv before importing it into this new system.)
2024-09-25 09:18:38 +00:00
if (aux_match_arg(&amo, &valarg)) {
/*
* Non-option arguments are expected to be key files, and
* added to clkeys.
*/
Windows Pageant: make atomic client/server decision. In the previous state of the code, we first tested agent_exists() to decide whether to be the long-running Pageant server or a short-lived client; then, during the command-line parsing loop, we prompted for passphrases to add keys presented on the command line (to ourself or the server, respectively); *then* we set up the named pipe and WM_COPYDATA receiver window to actually start functioning as a server, if we decided that was our role. A consequence is that if a user started up two Pageants each with an encrypted key on the command line, there would be a race condition: each one would decide that it was _going_ to be the server, then prompt for a passphrase, and then try to set itself up as the server once the passphrase is entered. So whichever one's passphrase prompt was answered second would add its key to its own internal data structures, then fail to set up the server's named pipe, terminate with an error, and end up not having added its key to the _surviving_ server. This change reorders the setup steps so that the command-line parsing loop does not add the keys immediately; instead it merely caches the key filenames provided. Then we make the decision about whether we're the server, and set up both the named pipe and WM_COPYDATA window if we are; and finally, we go back to our list of key filenames and actually add them, either to ourself (if we're the server) or to some other Pageant (if we're a client). Moreover, the decision about whether to be the server is now wrapped in an interprocess mutex similar to the one used in connection sharing, which means that even if two or more Pageants are started up as close to simultaneously as possible, they should avoid a race condition and successfully manage to agree on exactly one of themselves to be the server. For example, a user reported that this could occur if you put shortcuts to multiple private key files in your Windows Startup folder, so that they were all launched simultaneously at startup. One slightly odd behaviour that remains: if the server Pageant has to prompt for private key passphrases at startup, then it won't actually start _servicing_ requests from other Pageants until it's finished dealing with its own prompts. As a result, if you do start up two Pageants at once each with an encrypted key file on its command line, the second one won't even manage to present its passphrase prompt until the first one's prompt is dismissed, because it will block waiting for the initial check of the key list. But it will get there in the end, so that's only a cosmetic oddity. It would be nice to arrange that Pageant GUI operations don't block unrelated agent requests (e.g. by having the GUI and the agent code run in separate threads). But that's a bigger problem, not specific to startup time - the same thing happens if you interactively load a key via Pageant's file dialog. And it would require a major reorganisation to fix that fully, because currently the GUI code depends on being able to call _internal_ Pageant query functions like pageant_count_ssh2_keys() that don't work by constructing an agent request at all.
2022-01-03 12:17:15 +00:00
sgrowarray(clkeys, clkeysize, nclkeys);
CommandLineKey *clkey = &clkeys[nclkeys++];
clkey->fn = cmdline_arg_to_filename(valarg);
Windows Pageant: make atomic client/server decision. In the previous state of the code, we first tested agent_exists() to decide whether to be the long-running Pageant server or a short-lived client; then, during the command-line parsing loop, we prompted for passphrases to add keys presented on the command line (to ourself or the server, respectively); *then* we set up the named pipe and WM_COPYDATA receiver window to actually start functioning as a server, if we decided that was our role. A consequence is that if a user started up two Pageants each with an encrypted key on the command line, there would be a race condition: each one would decide that it was _going_ to be the server, then prompt for a passphrase, and then try to set itself up as the server once the passphrase is entered. So whichever one's passphrase prompt was answered second would add its key to its own internal data structures, then fail to set up the server's named pipe, terminate with an error, and end up not having added its key to the _surviving_ server. This change reorders the setup steps so that the command-line parsing loop does not add the keys immediately; instead it merely caches the key filenames provided. Then we make the decision about whether we're the server, and set up both the named pipe and WM_COPYDATA window if we are; and finally, we go back to our list of key filenames and actually add them, either to ourself (if we're the server) or to some other Pageant (if we're a client). Moreover, the decision about whether to be the server is now wrapped in an interprocess mutex similar to the one used in connection sharing, which means that even if two or more Pageants are started up as close to simultaneously as possible, they should avoid a race condition and successfully manage to agree on exactly one of themselves to be the server. For example, a user reported that this could occur if you put shortcuts to multiple private key files in your Windows Startup folder, so that they were all launched simultaneously at startup. One slightly odd behaviour that remains: if the server Pageant has to prompt for private key passphrases at startup, then it won't actually start _servicing_ requests from other Pageants until it's finished dealing with its own prompts. As a result, if you do start up two Pageants at once each with an encrypted key file on its command line, the second one won't even manage to present its passphrase prompt until the first one's prompt is dismissed, because it will block waiting for the initial check of the key list. But it will get there in the end, so that's only a cosmetic oddity. It would be nice to arrange that Pageant GUI operations don't block unrelated agent requests (e.g. by having the GUI and the agent code run in separate threads). But that's a bigger problem, not specific to startup time - the same thing happens if you interactively load a key via Pageant's file dialog. And it would require a major reorganisation to fix that fully, because currently the GUI code depends on being able to call _internal_ Pageant query functions like pageant_count_ssh2_keys() that don't work by constructing an agent request at all.
2022-01-03 12:17:15 +00:00
clkey->add_encrypted = add_keys_encrypted;
} else if (match_opt("-pgpfp")) {
pgp_fingerprints_msgbox(NULL);
return 0;
} else if (match_opt("-restrict-acl", "-restrict_acl",
"-restrictacl")) {
restrict_process_acl();
} else if (match_opt("-restrict-putty-acl", "-restrict_putty_acl")) {
restrict_putty_acl = true;
} else if (match_opt("-no-decrypt", "-no_decrypt",
"-nodecrypt", "-encrypted")) {
add_keys_encrypted = true;
} else if (match_opt("-keylist")) {
show_keylist_on_startup = true;
Windows Pageant: integrate with Windows OpenSSH. After a discussion with a user recently, I investigated the Windows native ssh.exe, and found it uses a Windows named pipe to talk to its ssh-agent, in exactly the same way Pageant does. So if you tell ssh.exe where to find Pageant's pipe, it can talk directly to Pageant, and then you can have just one SSH agent. The slight problem is that Pageant's pipe name is not stable. It's generated using the same system as connection-sharing pipe names, and contains a hex hash value whose preimage was fed through CryptProtectData. And the problem with _that_ is that CryptProtectData apparently reinitialises its seed between login sessions (though it's stable within a login session), which I hadn't fully realised when I reused the same pipe-name construction code. One possibility, of course, would be to change Pageant so that it uses a fixed pipe name. But after a bit of thought, I think I actually like this feature, because the Windows named pipe namespace isn't segregated into areas writable by only particular users, so anyone using that namespace on a multiuser Windows box is potentially vulnerable to someone else squatting on the name you wanted to use. Using this system makes that harder, because the squatter won't be able to predict what the name is going to be! (Unless you shut down Pageant and start it up again within one login session - but there's only so much we can do. And squatting is at most a DoS, because PuTTY's named-pipe client code checks ownership of the other end of the pipe in all cases.) So instead I've gone for a different approach. Windows Pageant now supports an extra command-line option to write out a snippet of OpenSSH config file format on startup, containing an 'IdentityAgent' directive which points at the location of its named pipe. So you can use the 'Include' directive in your main .ssh/config to include this extra snippet, and then ssh.exe invocations will be able to find wherever the current Pageant has put its pipe.
2022-01-15 18:36:22 +00:00
} else if (match_optval("-openssh-config", "-openssh_config")) {
openssh_config_file = cmdline_arg_to_filename(valarg);
} else if (match_optval("-unix")) {
/* UNICODE: should this be a Unicode filename? Is there a
* Unicode version of connect() that lets you give a
* Unicode pathname when making an AF_UNIX socket? */
New abstraction for command-line arguments. This begins the process of enabling our Windows applications to handle Unicode characters on their command lines which don't fit in the system code page. Instead of passing plain strings to cmdline_process_param, we now pass a partially opaque and platform-specific thing called a CmdlineArg. This has a method that extracts the argument word as a default-encoded string, and another one that tries to extract it as UTF-8 (though it may fail if the UTF-8 isn't available). On Windows, the command line is now constructed by calling split_into_argv_w on the Unicode command line returned by GetCommandLineW(), and the UTF-8 method returns text converted directly from that wide-character form, not going via the system code page. So it _can_ include UTF-8 characters that wouldn't have round-tripped via CP_ACP. This commit introduces the abstraction and switches over the cross-platform and Windows argv-handling code to use it, with minimal functional change. Nothing yet tries to call cmdline_arg_get_utf8(). I say 'cross-platform and Windows' because on the Unix side there's still a lot of use of plain old argv which I haven't converted. That would be a much larger project, and isn't currently needed: the _current_ aim of this abstraction is to get the right things to happen relating to Unicode on Windows, so for code that doesn't run on Windows anyway, it's not adding value. (Also there's a tension with GTK, which wants to talk to standard argv and extract arguments _it_ knows about, so at the very least we'd have to let it munge argv before importing it into this new system.)
2024-09-25 09:18:38 +00:00
unixsocket = cmdline_arg_to_str(valarg);
} else if (match_opt("-c")) {
/*
* If we see `-c', then the rest of the command line
* should be treated as a command to be spawned.
*/
New abstraction for command-line arguments. This begins the process of enabling our Windows applications to handle Unicode characters on their command lines which don't fit in the system code page. Instead of passing plain strings to cmdline_process_param, we now pass a partially opaque and platform-specific thing called a CmdlineArg. This has a method that extracts the argument word as a default-encoded string, and another one that tries to extract it as UTF-8 (though it may fail if the UTF-8 isn't available). On Windows, the command line is now constructed by calling split_into_argv_w on the Unicode command line returned by GetCommandLineW(), and the UTF-8 method returns text converted directly from that wide-character form, not going via the system code page. So it _can_ include UTF-8 characters that wouldn't have round-tripped via CP_ACP. This commit introduces the abstraction and switches over the cross-platform and Windows argv-handling code to use it, with minimal functional change. Nothing yet tries to call cmdline_arg_get_utf8(). I say 'cross-platform and Windows' because on the Unix side there's still a lot of use of plain old argv which I haven't converted. That would be a much larger project, and isn't currently needed: the _current_ aim of this abstraction is to get the right things to happen relating to Unicode on Windows, so for code that doesn't run on Windows anyway, it's not adding value. (Also there's a tension with GTK, which wants to talk to standard argv and extract arguments _it_ knows about, so at the very least we'd have to let it munge argv before importing it into this new system.)
2024-09-25 09:18:38 +00:00
if (amo.arglist->args[amo.index]) {
/* UNICODE: should use the UTF-8 or wide version, and
* CreateProcessW, to pass through arbitrary command lines */
command = cmdline_arg_remainder_acp(
amo.arglist->args[amo.index]);
} else {
command = "";
New abstraction for command-line arguments. This begins the process of enabling our Windows applications to handle Unicode characters on their command lines which don't fit in the system code page. Instead of passing plain strings to cmdline_process_param, we now pass a partially opaque and platform-specific thing called a CmdlineArg. This has a method that extracts the argument word as a default-encoded string, and another one that tries to extract it as UTF-8 (though it may fail if the UTF-8 isn't available). On Windows, the command line is now constructed by calling split_into_argv_w on the Unicode command line returned by GetCommandLineW(), and the UTF-8 method returns text converted directly from that wide-character form, not going via the system code page. So it _can_ include UTF-8 characters that wouldn't have round-tripped via CP_ACP. This commit introduces the abstraction and switches over the cross-platform and Windows argv-handling code to use it, with minimal functional change. Nothing yet tries to call cmdline_arg_get_utf8(). I say 'cross-platform and Windows' because on the Unix side there's still a lot of use of plain old argv which I haven't converted. That would be a much larger project, and isn't currently needed: the _current_ aim of this abstraction is to get the right things to happen relating to Unicode on Windows, so for code that doesn't run on Windows anyway, it's not adding value. (Also there's a tension with GTK, which wants to talk to standard argv and extract arguments _it_ knows about, so at the very least we'd have to let it munge argv before importing it into this new system.)
2024-09-25 09:18:38 +00:00
}
break;
} else {
New abstraction for command-line arguments. This begins the process of enabling our Windows applications to handle Unicode characters on their command lines which don't fit in the system code page. Instead of passing plain strings to cmdline_process_param, we now pass a partially opaque and platform-specific thing called a CmdlineArg. This has a method that extracts the argument word as a default-encoded string, and another one that tries to extract it as UTF-8 (though it may fail if the UTF-8 isn't available). On Windows, the command line is now constructed by calling split_into_argv_w on the Unicode command line returned by GetCommandLineW(), and the UTF-8 method returns text converted directly from that wide-character form, not going via the system code page. So it _can_ include UTF-8 characters that wouldn't have round-tripped via CP_ACP. This commit introduces the abstraction and switches over the cross-platform and Windows argv-handling code to use it, with minimal functional change. Nothing yet tries to call cmdline_arg_get_utf8(). I say 'cross-platform and Windows' because on the Unix side there's still a lot of use of plain old argv which I haven't converted. That would be a much larger project, and isn't currently needed: the _current_ aim of this abstraction is to get the right things to happen relating to Unicode on Windows, so for code that doesn't run on Windows anyway, it's not adding value. (Also there's a tension with GTK, which wants to talk to standard argv and extract arguments _it_ knows about, so at the very least we'd have to let it munge argv before importing it into this new system.)
2024-09-25 09:18:38 +00:00
opt_error("unrecognised option '%s'\n",
cmdline_arg_to_str(amo.arglist->args[amo.index]));
Windows Pageant: make atomic client/server decision. In the previous state of the code, we first tested agent_exists() to decide whether to be the long-running Pageant server or a short-lived client; then, during the command-line parsing loop, we prompted for passphrases to add keys presented on the command line (to ourself or the server, respectively); *then* we set up the named pipe and WM_COPYDATA receiver window to actually start functioning as a server, if we decided that was our role. A consequence is that if a user started up two Pageants each with an encrypted key on the command line, there would be a race condition: each one would decide that it was _going_ to be the server, then prompt for a passphrase, and then try to set itself up as the server once the passphrase is entered. So whichever one's passphrase prompt was answered second would add its key to its own internal data structures, then fail to set up the server's named pipe, terminate with an error, and end up not having added its key to the _surviving_ server. This change reorders the setup steps so that the command-line parsing loop does not add the keys immediately; instead it merely caches the key filenames provided. Then we make the decision about whether we're the server, and set up both the named pipe and WM_COPYDATA window if we are; and finally, we go back to our list of key filenames and actually add them, either to ourself (if we're the server) or to some other Pageant (if we're a client). Moreover, the decision about whether to be the server is now wrapped in an interprocess mutex similar to the one used in connection sharing, which means that even if two or more Pageants are started up as close to simultaneously as possible, they should avoid a race condition and successfully manage to agree on exactly one of themselves to be the server. For example, a user reported that this could occur if you put shortcuts to multiple private key files in your Windows Startup folder, so that they were all launched simultaneously at startup. One slightly odd behaviour that remains: if the server Pageant has to prompt for private key passphrases at startup, then it won't actually start _servicing_ requests from other Pageants until it's finished dealing with its own prompts. As a result, if you do start up two Pageants at once each with an encrypted key file on its command line, the second one won't even manage to present its passphrase prompt until the first one's prompt is dismissed, because it will block waiting for the initial check of the key list. But it will get there in the end, so that's only a cosmetic oddity. It would be nice to arrange that Pageant GUI operations don't block unrelated agent requests (e.g. by having the GUI and the agent code run in separate threads). But that's a bigger problem, not specific to startup time - the same thing happens if you interactively load a key via Pageant's file dialog. And it would require a major reorganisation to fix that fully, because currently the GUI code depends on being able to call _internal_ Pageant query functions like pageant_count_ssh2_keys() that don't work by constructing an agent request at all.
2022-01-03 12:17:15 +00:00
}
}
/*
* Create and lock an interprocess mutex while we figure out
* whether we're going to be the Pageant server or a client. That
* way, two Pageant processes started up simultaneously will be
* able to agree on which one becomes the server without a race
* condition.
*/
HANDLE mutex;
{
char *err;
char *mutexname = agent_mutex_name();
mutex = lock_interprocess_mutex(mutexname, &err);
sfree(mutexname);
if (!mutex) {
MessageBox(NULL, err, "Pageant Error", MB_ICONERROR | MB_OK);
return 1;
}
}
/*
* Find out if Pageant is already running.
*/
already_running = agent_exists();
/*
* If it isn't, we're going to be the primary Pageant that stays
* running, so set up all the machinery to answer requests.
*/
if (!already_running) {
/*
* Set up the window class for the hidden window that receives
* all the messages to do with our presence in the system tray.
*/
if (!prev) {
WNDCLASS wndclass;
memset(&wndclass, 0, sizeof(wndclass));
wndclass.lpfnWndProc = TrayWndProc;
wndclass.hInstance = inst;
wndclass.hIcon = LoadIcon(inst, MAKEINTRESOURCE(IDI_MAINICON));
wndclass.lpszClassName = TRAYCLASSNAME;
RegisterClass(&wndclass);
}
keylist = NULL;
traywindow = CreateWindow(TRAYCLASSNAME, TRAYWINTITLE,
WS_OVERLAPPEDWINDOW | WS_VSCROLL,
CW_USEDEFAULT, CW_USEDEFAULT,
100, 100, NULL, NULL, inst, NULL);
winselgui_set_hwnd(traywindow);
Windows Pageant: make atomic client/server decision. In the previous state of the code, we first tested agent_exists() to decide whether to be the long-running Pageant server or a short-lived client; then, during the command-line parsing loop, we prompted for passphrases to add keys presented on the command line (to ourself or the server, respectively); *then* we set up the named pipe and WM_COPYDATA receiver window to actually start functioning as a server, if we decided that was our role. A consequence is that if a user started up two Pageants each with an encrypted key on the command line, there would be a race condition: each one would decide that it was _going_ to be the server, then prompt for a passphrase, and then try to set itself up as the server once the passphrase is entered. So whichever one's passphrase prompt was answered second would add its key to its own internal data structures, then fail to set up the server's named pipe, terminate with an error, and end up not having added its key to the _surviving_ server. This change reorders the setup steps so that the command-line parsing loop does not add the keys immediately; instead it merely caches the key filenames provided. Then we make the decision about whether we're the server, and set up both the named pipe and WM_COPYDATA window if we are; and finally, we go back to our list of key filenames and actually add them, either to ourself (if we're the server) or to some other Pageant (if we're a client). Moreover, the decision about whether to be the server is now wrapped in an interprocess mutex similar to the one used in connection sharing, which means that even if two or more Pageants are started up as close to simultaneously as possible, they should avoid a race condition and successfully manage to agree on exactly one of themselves to be the server. For example, a user reported that this could occur if you put shortcuts to multiple private key files in your Windows Startup folder, so that they were all launched simultaneously at startup. One slightly odd behaviour that remains: if the server Pageant has to prompt for private key passphrases at startup, then it won't actually start _servicing_ requests from other Pageants until it's finished dealing with its own prompts. As a result, if you do start up two Pageants at once each with an encrypted key file on its command line, the second one won't even manage to present its passphrase prompt until the first one's prompt is dismissed, because it will block waiting for the initial check of the key list. But it will get there in the end, so that's only a cosmetic oddity. It would be nice to arrange that Pageant GUI operations don't block unrelated agent requests (e.g. by having the GUI and the agent code run in separate threads). But that's a bigger problem, not specific to startup time - the same thing happens if you interactively load a key via Pageant's file dialog. And it would require a major reorganisation to fix that fully, because currently the GUI code depends on being able to call _internal_ Pageant query functions like pageant_count_ssh2_keys() that don't work by constructing an agent request at all.
2022-01-03 12:17:15 +00:00
/*
* Initialise the cross-platform Pageant code.
*/
pageant_init();
/*
* Set up a named-pipe listener.
*/
wpc->plc.vt = &winpgnt_vtable;
wpc->plc.suppress_logging = true;
if (should_have_security()) {
Plug *pl_plug;
struct pageant_listen_state *pl =
pageant_listener_new(&pl_plug, &wpc->plc);
char *pipename = agent_named_pipe_name();
Socket *sock = new_named_pipe_listener(pipename, pl_plug);
if (sk_socket_error(sock)) {
char *err = dupprintf("Unable to open named pipe at %s "
"for SSH agent:\n%s", pipename,
sk_socket_error(sock));
MessageBox(NULL, err, "Pageant Error", MB_ICONERROR | MB_OK);
return 1;
}
pageant_listener_got_socket(pl, sock);
/*
* If we've been asked to write out an OpenSSH config file
* pointing at the named pipe, do so.
*/
if (openssh_config_file) {
FILE *fp = f_open(openssh_config_file, "w", true);
if (!fp) {
char *err = dupprintf(
"Unable to write OpenSSH config file to %s",
filename_to_str(openssh_config_file));
MessageBox(NULL, err, "Pageant Error",
MB_ICONERROR | MB_OK);
return 1;
}
fputs("IdentityAgent \"", fp);
/* Some versions of Windows OpenSSH prefer / to \ as the path
* separator; others don't mind, but as far as we know, no
* version _objects_ to /, so we use it unconditionally. */
for (const char *p = pipename; *p; p++) {
char c = *p;
if (c == '\\')
c = '/';
fputc(c, fp);
}
fputs("\"\n", fp);
fclose(fp);
}
sfree(pipename);
Windows Pageant: make atomic client/server decision. In the previous state of the code, we first tested agent_exists() to decide whether to be the long-running Pageant server or a short-lived client; then, during the command-line parsing loop, we prompted for passphrases to add keys presented on the command line (to ourself or the server, respectively); *then* we set up the named pipe and WM_COPYDATA receiver window to actually start functioning as a server, if we decided that was our role. A consequence is that if a user started up two Pageants each with an encrypted key on the command line, there would be a race condition: each one would decide that it was _going_ to be the server, then prompt for a passphrase, and then try to set itself up as the server once the passphrase is entered. So whichever one's passphrase prompt was answered second would add its key to its own internal data structures, then fail to set up the server's named pipe, terminate with an error, and end up not having added its key to the _surviving_ server. This change reorders the setup steps so that the command-line parsing loop does not add the keys immediately; instead it merely caches the key filenames provided. Then we make the decision about whether we're the server, and set up both the named pipe and WM_COPYDATA window if we are; and finally, we go back to our list of key filenames and actually add them, either to ourself (if we're the server) or to some other Pageant (if we're a client). Moreover, the decision about whether to be the server is now wrapped in an interprocess mutex similar to the one used in connection sharing, which means that even if two or more Pageants are started up as close to simultaneously as possible, they should avoid a race condition and successfully manage to agree on exactly one of themselves to be the server. For example, a user reported that this could occur if you put shortcuts to multiple private key files in your Windows Startup folder, so that they were all launched simultaneously at startup. One slightly odd behaviour that remains: if the server Pageant has to prompt for private key passphrases at startup, then it won't actually start _servicing_ requests from other Pageants until it's finished dealing with its own prompts. As a result, if you do start up two Pageants at once each with an encrypted key file on its command line, the second one won't even manage to present its passphrase prompt until the first one's prompt is dismissed, because it will block waiting for the initial check of the key list. But it will get there in the end, so that's only a cosmetic oddity. It would be nice to arrange that Pageant GUI operations don't block unrelated agent requests (e.g. by having the GUI and the agent code run in separate threads). But that's a bigger problem, not specific to startup time - the same thing happens if you interactively load a key via Pageant's file dialog. And it would require a major reorganisation to fix that fully, because currently the GUI code depends on being able to call _internal_ Pageant query functions like pageant_count_ssh2_keys() that don't work by constructing an agent request at all.
2022-01-03 12:17:15 +00:00
}
Windows Pageant: integrate with Windows OpenSSH. After a discussion with a user recently, I investigated the Windows native ssh.exe, and found it uses a Windows named pipe to talk to its ssh-agent, in exactly the same way Pageant does. So if you tell ssh.exe where to find Pageant's pipe, it can talk directly to Pageant, and then you can have just one SSH agent. The slight problem is that Pageant's pipe name is not stable. It's generated using the same system as connection-sharing pipe names, and contains a hex hash value whose preimage was fed through CryptProtectData. And the problem with _that_ is that CryptProtectData apparently reinitialises its seed between login sessions (though it's stable within a login session), which I hadn't fully realised when I reused the same pipe-name construction code. One possibility, of course, would be to change Pageant so that it uses a fixed pipe name. But after a bit of thought, I think I actually like this feature, because the Windows named pipe namespace isn't segregated into areas writable by only particular users, so anyone using that namespace on a multiuser Windows box is potentially vulnerable to someone else squatting on the name you wanted to use. Using this system makes that harder, because the squatter won't be able to predict what the name is going to be! (Unless you shut down Pageant and start it up again within one login session - but there's only so much we can do. And squatting is at most a DoS, because PuTTY's named-pipe client code checks ownership of the other end of the pipe in all cases.) So instead I've gone for a different approach. Windows Pageant now supports an extra command-line option to write out a snippet of OpenSSH config file format on startup, containing an 'IdentityAgent' directive which points at the location of its named pipe. So you can use the 'Include' directive in your main .ssh/config to include this extra snippet, and then ssh.exe invocations will be able to find wherever the current Pageant has put its pipe.
2022-01-15 18:36:22 +00:00
/*
* Set up an AF_UNIX listener too, if we were asked to.
Windows Pageant: integrate with Windows OpenSSH. After a discussion with a user recently, I investigated the Windows native ssh.exe, and found it uses a Windows named pipe to talk to its ssh-agent, in exactly the same way Pageant does. So if you tell ssh.exe where to find Pageant's pipe, it can talk directly to Pageant, and then you can have just one SSH agent. The slight problem is that Pageant's pipe name is not stable. It's generated using the same system as connection-sharing pipe names, and contains a hex hash value whose preimage was fed through CryptProtectData. And the problem with _that_ is that CryptProtectData apparently reinitialises its seed between login sessions (though it's stable within a login session), which I hadn't fully realised when I reused the same pipe-name construction code. One possibility, of course, would be to change Pageant so that it uses a fixed pipe name. But after a bit of thought, I think I actually like this feature, because the Windows named pipe namespace isn't segregated into areas writable by only particular users, so anyone using that namespace on a multiuser Windows box is potentially vulnerable to someone else squatting on the name you wanted to use. Using this system makes that harder, because the squatter won't be able to predict what the name is going to be! (Unless you shut down Pageant and start it up again within one login session - but there's only so much we can do. And squatting is at most a DoS, because PuTTY's named-pipe client code checks ownership of the other end of the pipe in all cases.) So instead I've gone for a different approach. Windows Pageant now supports an extra command-line option to write out a snippet of OpenSSH config file format on startup, containing an 'IdentityAgent' directive which points at the location of its named pipe. So you can use the 'Include' directive in your main .ssh/config to include this extra snippet, and then ssh.exe invocations will be able to find wherever the current Pageant has put its pipe.
2022-01-15 18:36:22 +00:00
*/
if (unixsocket) {
sk_init();
/* FIXME: diagnose any error except file-not-found. Also,
* check the file type if possible? */
remove(unixsocket);
Plug *pl_plug;
struct pageant_listen_state *pl =
pageant_listener_new(&pl_plug, &wpc->plc);
Socket *sock = sk_newlistener_unix(unixsocket, pl_plug);
if (sk_socket_error(sock)) {
char *err = dupprintf("Unable to open AF_UNIX socket at %s "
"for SSH agent:\n%s", unixsocket,
sk_socket_error(sock));
Windows Pageant: integrate with Windows OpenSSH. After a discussion with a user recently, I investigated the Windows native ssh.exe, and found it uses a Windows named pipe to talk to its ssh-agent, in exactly the same way Pageant does. So if you tell ssh.exe where to find Pageant's pipe, it can talk directly to Pageant, and then you can have just one SSH agent. The slight problem is that Pageant's pipe name is not stable. It's generated using the same system as connection-sharing pipe names, and contains a hex hash value whose preimage was fed through CryptProtectData. And the problem with _that_ is that CryptProtectData apparently reinitialises its seed between login sessions (though it's stable within a login session), which I hadn't fully realised when I reused the same pipe-name construction code. One possibility, of course, would be to change Pageant so that it uses a fixed pipe name. But after a bit of thought, I think I actually like this feature, because the Windows named pipe namespace isn't segregated into areas writable by only particular users, so anyone using that namespace on a multiuser Windows box is potentially vulnerable to someone else squatting on the name you wanted to use. Using this system makes that harder, because the squatter won't be able to predict what the name is going to be! (Unless you shut down Pageant and start it up again within one login session - but there's only so much we can do. And squatting is at most a DoS, because PuTTY's named-pipe client code checks ownership of the other end of the pipe in all cases.) So instead I've gone for a different approach. Windows Pageant now supports an extra command-line option to write out a snippet of OpenSSH config file format on startup, containing an 'IdentityAgent' directive which points at the location of its named pipe. So you can use the 'Include' directive in your main .ssh/config to include this extra snippet, and then ssh.exe invocations will be able to find wherever the current Pageant has put its pipe.
2022-01-15 18:36:22 +00:00
MessageBox(NULL, err, "Pageant Error", MB_ICONERROR | MB_OK);
return 1;
}
pageant_listener_got_socket(pl, sock);
Windows Pageant: integrate with Windows OpenSSH. After a discussion with a user recently, I investigated the Windows native ssh.exe, and found it uses a Windows named pipe to talk to its ssh-agent, in exactly the same way Pageant does. So if you tell ssh.exe where to find Pageant's pipe, it can talk directly to Pageant, and then you can have just one SSH agent. The slight problem is that Pageant's pipe name is not stable. It's generated using the same system as connection-sharing pipe names, and contains a hex hash value whose preimage was fed through CryptProtectData. And the problem with _that_ is that CryptProtectData apparently reinitialises its seed between login sessions (though it's stable within a login session), which I hadn't fully realised when I reused the same pipe-name construction code. One possibility, of course, would be to change Pageant so that it uses a fixed pipe name. But after a bit of thought, I think I actually like this feature, because the Windows named pipe namespace isn't segregated into areas writable by only particular users, so anyone using that namespace on a multiuser Windows box is potentially vulnerable to someone else squatting on the name you wanted to use. Using this system makes that harder, because the squatter won't be able to predict what the name is going to be! (Unless you shut down Pageant and start it up again within one login session - but there's only so much we can do. And squatting is at most a DoS, because PuTTY's named-pipe client code checks ownership of the other end of the pipe in all cases.) So instead I've gone for a different approach. Windows Pageant now supports an extra command-line option to write out a snippet of OpenSSH config file format on startup, containing an 'IdentityAgent' directive which points at the location of its named pipe. So you can use the 'Include' directive in your main .ssh/config to include this extra snippet, and then ssh.exe invocations will be able to find wherever the current Pageant has put its pipe.
2022-01-15 18:36:22 +00:00
}
Windows Pageant: make atomic client/server decision. In the previous state of the code, we first tested agent_exists() to decide whether to be the long-running Pageant server or a short-lived client; then, during the command-line parsing loop, we prompted for passphrases to add keys presented on the command line (to ourself or the server, respectively); *then* we set up the named pipe and WM_COPYDATA receiver window to actually start functioning as a server, if we decided that was our role. A consequence is that if a user started up two Pageants each with an encrypted key on the command line, there would be a race condition: each one would decide that it was _going_ to be the server, then prompt for a passphrase, and then try to set itself up as the server once the passphrase is entered. So whichever one's passphrase prompt was answered second would add its key to its own internal data structures, then fail to set up the server's named pipe, terminate with an error, and end up not having added its key to the _surviving_ server. This change reorders the setup steps so that the command-line parsing loop does not add the keys immediately; instead it merely caches the key filenames provided. Then we make the decision about whether we're the server, and set up both the named pipe and WM_COPYDATA window if we are; and finally, we go back to our list of key filenames and actually add them, either to ourself (if we're the server) or to some other Pageant (if we're a client). Moreover, the decision about whether to be the server is now wrapped in an interprocess mutex similar to the one used in connection sharing, which means that even if two or more Pageants are started up as close to simultaneously as possible, they should avoid a race condition and successfully manage to agree on exactly one of themselves to be the server. For example, a user reported that this could occur if you put shortcuts to multiple private key files in your Windows Startup folder, so that they were all launched simultaneously at startup. One slightly odd behaviour that remains: if the server Pageant has to prompt for private key passphrases at startup, then it won't actually start _servicing_ requests from other Pageants until it's finished dealing with its own prompts. As a result, if you do start up two Pageants at once each with an encrypted key file on its command line, the second one won't even manage to present its passphrase prompt until the first one's prompt is dismissed, because it will block waiting for the initial check of the key list. But it will get there in the end, so that's only a cosmetic oddity. It would be nice to arrange that Pageant GUI operations don't block unrelated agent requests (e.g. by having the GUI and the agent code run in separate threads). But that's a bigger problem, not specific to startup time - the same thing happens if you interactively load a key via Pageant's file dialog. And it would require a major reorganisation to fix that fully, because currently the GUI code depends on being able to call _internal_ Pageant query functions like pageant_count_ssh2_keys() that don't work by constructing an agent request at all.
2022-01-03 12:17:15 +00:00
/*
* Set up the window class for the hidden window that receives
* the WM_COPYDATA message used by the old-style Pageant IPC
* system.
*/
if (!prev) {
WNDCLASS wndclass;
memset(&wndclass, 0, sizeof(wndclass));
wndclass.lpfnWndProc = wm_copydata_WndProc;
wndclass.hInstance = inst;
wndclass.lpszClassName = IPCCLASSNAME;
RegisterClass(&wndclass);
}
Windows Pageant: make atomic client/server decision. In the previous state of the code, we first tested agent_exists() to decide whether to be the long-running Pageant server or a short-lived client; then, during the command-line parsing loop, we prompted for passphrases to add keys presented on the command line (to ourself or the server, respectively); *then* we set up the named pipe and WM_COPYDATA receiver window to actually start functioning as a server, if we decided that was our role. A consequence is that if a user started up two Pageants each with an encrypted key on the command line, there would be a race condition: each one would decide that it was _going_ to be the server, then prompt for a passphrase, and then try to set itself up as the server once the passphrase is entered. So whichever one's passphrase prompt was answered second would add its key to its own internal data structures, then fail to set up the server's named pipe, terminate with an error, and end up not having added its key to the _surviving_ server. This change reorders the setup steps so that the command-line parsing loop does not add the keys immediately; instead it merely caches the key filenames provided. Then we make the decision about whether we're the server, and set up both the named pipe and WM_COPYDATA window if we are; and finally, we go back to our list of key filenames and actually add them, either to ourself (if we're the server) or to some other Pageant (if we're a client). Moreover, the decision about whether to be the server is now wrapped in an interprocess mutex similar to the one used in connection sharing, which means that even if two or more Pageants are started up as close to simultaneously as possible, they should avoid a race condition and successfully manage to agree on exactly one of themselves to be the server. For example, a user reported that this could occur if you put shortcuts to multiple private key files in your Windows Startup folder, so that they were all launched simultaneously at startup. One slightly odd behaviour that remains: if the server Pageant has to prompt for private key passphrases at startup, then it won't actually start _servicing_ requests from other Pageants until it's finished dealing with its own prompts. As a result, if you do start up two Pageants at once each with an encrypted key file on its command line, the second one won't even manage to present its passphrase prompt until the first one's prompt is dismissed, because it will block waiting for the initial check of the key list. But it will get there in the end, so that's only a cosmetic oddity. It would be nice to arrange that Pageant GUI operations don't block unrelated agent requests (e.g. by having the GUI and the agent code run in separate threads). But that's a bigger problem, not specific to startup time - the same thing happens if you interactively load a key via Pageant's file dialog. And it would require a major reorganisation to fix that fully, because currently the GUI code depends on being able to call _internal_ Pageant query functions like pageant_count_ssh2_keys() that don't work by constructing an agent request at all.
2022-01-03 12:17:15 +00:00
/*
* And launch the subthread which will open that hidden window and
* handle WM_COPYDATA messages on it.
*/
wmcpc.vt = &wmcpc_vtable;
wmcpc.suppress_logging = true;
pageant_register_client(&wmcpc);
DWORD wm_copydata_threadid;
wmct.ev_msg_ready = CreateEvent(NULL, false, false, NULL);
wmct.ev_reply_ready = CreateEvent(NULL, false, false, NULL);
HANDLE hThread = CreateThread(NULL, 0, wm_copydata_threadfunc,
&inst, 0, &wm_copydata_threadid);
if (hThread)
CloseHandle(hThread); /* we don't need the thread handle */
add_handle_wait(wmct.ev_msg_ready, wm_copydata_got_msg, NULL);
}
/*
Windows Pageant: make atomic client/server decision. In the previous state of the code, we first tested agent_exists() to decide whether to be the long-running Pageant server or a short-lived client; then, during the command-line parsing loop, we prompted for passphrases to add keys presented on the command line (to ourself or the server, respectively); *then* we set up the named pipe and WM_COPYDATA receiver window to actually start functioning as a server, if we decided that was our role. A consequence is that if a user started up two Pageants each with an encrypted key on the command line, there would be a race condition: each one would decide that it was _going_ to be the server, then prompt for a passphrase, and then try to set itself up as the server once the passphrase is entered. So whichever one's passphrase prompt was answered second would add its key to its own internal data structures, then fail to set up the server's named pipe, terminate with an error, and end up not having added its key to the _surviving_ server. This change reorders the setup steps so that the command-line parsing loop does not add the keys immediately; instead it merely caches the key filenames provided. Then we make the decision about whether we're the server, and set up both the named pipe and WM_COPYDATA window if we are; and finally, we go back to our list of key filenames and actually add them, either to ourself (if we're the server) or to some other Pageant (if we're a client). Moreover, the decision about whether to be the server is now wrapped in an interprocess mutex similar to the one used in connection sharing, which means that even if two or more Pageants are started up as close to simultaneously as possible, they should avoid a race condition and successfully manage to agree on exactly one of themselves to be the server. For example, a user reported that this could occur if you put shortcuts to multiple private key files in your Windows Startup folder, so that they were all launched simultaneously at startup. One slightly odd behaviour that remains: if the server Pageant has to prompt for private key passphrases at startup, then it won't actually start _servicing_ requests from other Pageants until it's finished dealing with its own prompts. As a result, if you do start up two Pageants at once each with an encrypted key file on its command line, the second one won't even manage to present its passphrase prompt until the first one's prompt is dismissed, because it will block waiting for the initial check of the key list. But it will get there in the end, so that's only a cosmetic oddity. It would be nice to arrange that Pageant GUI operations don't block unrelated agent requests (e.g. by having the GUI and the agent code run in separate threads). But that's a bigger problem, not specific to startup time - the same thing happens if you interactively load a key via Pageant's file dialog. And it would require a major reorganisation to fix that fully, because currently the GUI code depends on being able to call _internal_ Pageant query functions like pageant_count_ssh2_keys() that don't work by constructing an agent request at all.
2022-01-03 12:17:15 +00:00
* Now we're either a fully set up Pageant server, or we know one
* is running somewhere else. Either way, now it's safe to unlock
* the mutex.
*/
unlock_interprocess_mutex(mutex);
/*
* Add any keys provided on the command line.
*/
Windows Pageant: make atomic client/server decision. In the previous state of the code, we first tested agent_exists() to decide whether to be the long-running Pageant server or a short-lived client; then, during the command-line parsing loop, we prompted for passphrases to add keys presented on the command line (to ourself or the server, respectively); *then* we set up the named pipe and WM_COPYDATA receiver window to actually start functioning as a server, if we decided that was our role. A consequence is that if a user started up two Pageants each with an encrypted key on the command line, there would be a race condition: each one would decide that it was _going_ to be the server, then prompt for a passphrase, and then try to set itself up as the server once the passphrase is entered. So whichever one's passphrase prompt was answered second would add its key to its own internal data structures, then fail to set up the server's named pipe, terminate with an error, and end up not having added its key to the _surviving_ server. This change reorders the setup steps so that the command-line parsing loop does not add the keys immediately; instead it merely caches the key filenames provided. Then we make the decision about whether we're the server, and set up both the named pipe and WM_COPYDATA window if we are; and finally, we go back to our list of key filenames and actually add them, either to ourself (if we're the server) or to some other Pageant (if we're a client). Moreover, the decision about whether to be the server is now wrapped in an interprocess mutex similar to the one used in connection sharing, which means that even if two or more Pageants are started up as close to simultaneously as possible, they should avoid a race condition and successfully manage to agree on exactly one of themselves to be the server. For example, a user reported that this could occur if you put shortcuts to multiple private key files in your Windows Startup folder, so that they were all launched simultaneously at startup. One slightly odd behaviour that remains: if the server Pageant has to prompt for private key passphrases at startup, then it won't actually start _servicing_ requests from other Pageants until it's finished dealing with its own prompts. As a result, if you do start up two Pageants at once each with an encrypted key file on its command line, the second one won't even manage to present its passphrase prompt until the first one's prompt is dismissed, because it will block waiting for the initial check of the key list. But it will get there in the end, so that's only a cosmetic oddity. It would be nice to arrange that Pageant GUI operations don't block unrelated agent requests (e.g. by having the GUI and the agent code run in separate threads). But that's a bigger problem, not specific to startup time - the same thing happens if you interactively load a key via Pageant's file dialog. And it would require a major reorganisation to fix that fully, because currently the GUI code depends on being able to call _internal_ Pageant query functions like pageant_count_ssh2_keys() that don't work by constructing an agent request at all.
2022-01-03 12:17:15 +00:00
for (size_t i = 0; i < nclkeys; i++) {
CommandLineKey *clkey = &clkeys[i];
win_add_keyfile(clkey->fn, clkey->add_encrypted);
filename_free(clkey->fn);
}
sfree(clkeys);
/* And forget any passphrases we stashed during that loop. */
pageant_forget_passphrases();
Windows Pageant: make atomic client/server decision. In the previous state of the code, we first tested agent_exists() to decide whether to be the long-running Pageant server or a short-lived client; then, during the command-line parsing loop, we prompted for passphrases to add keys presented on the command line (to ourself or the server, respectively); *then* we set up the named pipe and WM_COPYDATA receiver window to actually start functioning as a server, if we decided that was our role. A consequence is that if a user started up two Pageants each with an encrypted key on the command line, there would be a race condition: each one would decide that it was _going_ to be the server, then prompt for a passphrase, and then try to set itself up as the server once the passphrase is entered. So whichever one's passphrase prompt was answered second would add its key to its own internal data structures, then fail to set up the server's named pipe, terminate with an error, and end up not having added its key to the _surviving_ server. This change reorders the setup steps so that the command-line parsing loop does not add the keys immediately; instead it merely caches the key filenames provided. Then we make the decision about whether we're the server, and set up both the named pipe and WM_COPYDATA window if we are; and finally, we go back to our list of key filenames and actually add them, either to ourself (if we're the server) or to some other Pageant (if we're a client). Moreover, the decision about whether to be the server is now wrapped in an interprocess mutex similar to the one used in connection sharing, which means that even if two or more Pageants are started up as close to simultaneously as possible, they should avoid a race condition and successfully manage to agree on exactly one of themselves to be the server. For example, a user reported that this could occur if you put shortcuts to multiple private key files in your Windows Startup folder, so that they were all launched simultaneously at startup. One slightly odd behaviour that remains: if the server Pageant has to prompt for private key passphrases at startup, then it won't actually start _servicing_ requests from other Pageants until it's finished dealing with its own prompts. As a result, if you do start up two Pageants at once each with an encrypted key file on its command line, the second one won't even manage to present its passphrase prompt until the first one's prompt is dismissed, because it will block waiting for the initial check of the key list. But it will get there in the end, so that's only a cosmetic oddity. It would be nice to arrange that Pageant GUI operations don't block unrelated agent requests (e.g. by having the GUI and the agent code run in separate threads). But that's a bigger problem, not specific to startup time - the same thing happens if you interactively load a key via Pageant's file dialog. And it would require a major reorganisation to fix that fully, because currently the GUI code depends on being able to call _internal_ Pageant query functions like pageant_count_ssh2_keys() that don't work by constructing an agent request at all.
2022-01-03 12:17:15 +00:00
/*
* Now our keys are present, spawn a command, if we were asked to.
*/
if (command) {
char *args;
if (command[0] == '"')
args = strchr(++command, '"');
else
args = strchr(command, ' ');
if (args) {
*args++ = 0;
while (*args && isspace((unsigned char)*args)) args++;
}
spawn_cmd(command, args, show);
}
/*
* If Pageant was already running, we leave now. If we haven't
* even taken any auxiliary action (spawned a command or added
* keys), complain.
*/
if (already_running) {
Windows Pageant: make atomic client/server decision. In the previous state of the code, we first tested agent_exists() to decide whether to be the long-running Pageant server or a short-lived client; then, during the command-line parsing loop, we prompted for passphrases to add keys presented on the command line (to ourself or the server, respectively); *then* we set up the named pipe and WM_COPYDATA receiver window to actually start functioning as a server, if we decided that was our role. A consequence is that if a user started up two Pageants each with an encrypted key on the command line, there would be a race condition: each one would decide that it was _going_ to be the server, then prompt for a passphrase, and then try to set itself up as the server once the passphrase is entered. So whichever one's passphrase prompt was answered second would add its key to its own internal data structures, then fail to set up the server's named pipe, terminate with an error, and end up not having added its key to the _surviving_ server. This change reorders the setup steps so that the command-line parsing loop does not add the keys immediately; instead it merely caches the key filenames provided. Then we make the decision about whether we're the server, and set up both the named pipe and WM_COPYDATA window if we are; and finally, we go back to our list of key filenames and actually add them, either to ourself (if we're the server) or to some other Pageant (if we're a client). Moreover, the decision about whether to be the server is now wrapped in an interprocess mutex similar to the one used in connection sharing, which means that even if two or more Pageants are started up as close to simultaneously as possible, they should avoid a race condition and successfully manage to agree on exactly one of themselves to be the server. For example, a user reported that this could occur if you put shortcuts to multiple private key files in your Windows Startup folder, so that they were all launched simultaneously at startup. One slightly odd behaviour that remains: if the server Pageant has to prompt for private key passphrases at startup, then it won't actually start _servicing_ requests from other Pageants until it's finished dealing with its own prompts. As a result, if you do start up two Pageants at once each with an encrypted key file on its command line, the second one won't even manage to present its passphrase prompt until the first one's prompt is dismissed, because it will block waiting for the initial check of the key list. But it will get there in the end, so that's only a cosmetic oddity. It would be nice to arrange that Pageant GUI operations don't block unrelated agent requests (e.g. by having the GUI and the agent code run in separate threads). But that's a bigger problem, not specific to startup time - the same thing happens if you interactively load a key via Pageant's file dialog. And it would require a major reorganisation to fix that fully, because currently the GUI code depends on being able to call _internal_ Pageant query functions like pageant_count_ssh2_keys() that don't work by constructing an agent request at all.
2022-01-03 12:17:15 +00:00
if (!command && !nclkeys) {
MessageBox(NULL, "Pageant is already running", "Pageant Error",
MB_ICONERROR | MB_OK);
}
return 0;
}
/* Set up a system tray icon */
AddTrayIcon(traywindow);
/* Accelerators used: nsvkxa */
systray_menu = CreatePopupMenu();
if (putty_path) {
session_menu = CreateMenu();
AppendMenu(systray_menu, MF_ENABLED, IDM_PUTTY, "&New Session");
AppendMenu(systray_menu, MF_POPUP | MF_ENABLED,
(UINT_PTR) session_menu, "&Saved Sessions");
AppendMenu(systray_menu, MF_SEPARATOR, 0, 0);
}
AppendMenu(systray_menu, MF_ENABLED, IDM_VIEWKEYS,
"&View Keys");
AppendMenu(systray_menu, MF_ENABLED, IDM_ADDKEY, "Add &Key");
AppendMenu(systray_menu, MF_ENABLED, IDM_ADDKEY_ENCRYPTED,
"Add key (encrypted)");
AppendMenu(systray_menu, MF_SEPARATOR, 0, 0);
AppendMenu(systray_menu, MF_ENABLED, IDM_REMOVE_ALL,
"Remove All Keys");
AppendMenu(systray_menu, MF_ENABLED, IDM_REENCRYPT_ALL,
"Re-encrypt All Keys");
AppendMenu(systray_menu, MF_SEPARATOR, 0, 0);
if (has_help())
AppendMenu(systray_menu, MF_ENABLED, IDM_HELP, "&Help");
AppendMenu(systray_menu, MF_ENABLED, IDM_ABOUT, "&About");
AppendMenu(systray_menu, MF_SEPARATOR, 0, 0);
AppendMenu(systray_menu, MF_ENABLED, IDM_CLOSE, "E&xit");
initial_menuitems_count = GetMenuItemCount(session_menu);
/* Set the default menu item. */
SetMenuDefaultItem(systray_menu, IDM_VIEWKEYS, false);
ShowWindow(traywindow, SW_HIDE);
Windows Pageant: make atomic client/server decision. In the previous state of the code, we first tested agent_exists() to decide whether to be the long-running Pageant server or a short-lived client; then, during the command-line parsing loop, we prompted for passphrases to add keys presented on the command line (to ourself or the server, respectively); *then* we set up the named pipe and WM_COPYDATA receiver window to actually start functioning as a server, if we decided that was our role. A consequence is that if a user started up two Pageants each with an encrypted key on the command line, there would be a race condition: each one would decide that it was _going_ to be the server, then prompt for a passphrase, and then try to set itself up as the server once the passphrase is entered. So whichever one's passphrase prompt was answered second would add its key to its own internal data structures, then fail to set up the server's named pipe, terminate with an error, and end up not having added its key to the _surviving_ server. This change reorders the setup steps so that the command-line parsing loop does not add the keys immediately; instead it merely caches the key filenames provided. Then we make the decision about whether we're the server, and set up both the named pipe and WM_COPYDATA window if we are; and finally, we go back to our list of key filenames and actually add them, either to ourself (if we're the server) or to some other Pageant (if we're a client). Moreover, the decision about whether to be the server is now wrapped in an interprocess mutex similar to the one used in connection sharing, which means that even if two or more Pageants are started up as close to simultaneously as possible, they should avoid a race condition and successfully manage to agree on exactly one of themselves to be the server. For example, a user reported that this could occur if you put shortcuts to multiple private key files in your Windows Startup folder, so that they were all launched simultaneously at startup. One slightly odd behaviour that remains: if the server Pageant has to prompt for private key passphrases at startup, then it won't actually start _servicing_ requests from other Pageants until it's finished dealing with its own prompts. As a result, if you do start up two Pageants at once each with an encrypted key file on its command line, the second one won't even manage to present its passphrase prompt until the first one's prompt is dismissed, because it will block waiting for the initial check of the key list. But it will get there in the end, so that's only a cosmetic oddity. It would be nice to arrange that Pageant GUI operations don't block unrelated agent requests (e.g. by having the GUI and the agent code run in separate threads). But that's a bigger problem, not specific to startup time - the same thing happens if you interactively load a key via Pageant's file dialog. And it would require a major reorganisation to fix that fully, because currently the GUI code depends on being able to call _internal_ Pageant query functions like pageant_count_ssh2_keys() that don't work by constructing an agent request at all.
2022-01-03 12:17:15 +00:00
/* Open the visible key list window, if we've been asked to. */
if (show_keylist_on_startup)
create_keylist_window();
/*
* Main message loop.
*/
while (true) {
Reorganise Windows HANDLE management. Before commit 6e69223dc262755, Pageant would stop working after a certain number of PuTTYs were active at the same time. (At most about 60, but maybe fewer - see below.) This was because of two separate bugs. The easy one, fixed in 6e69223dc262755 itself, was that PuTTY left each named-pipe connection to Pageant open for the rest of its lifetime. So the real problem was that Pageant had too many active connections at once. (And since a given PuTTY might make multiple connections during userauth - one to list keys, and maybe another to actually make a signature - that was why the number of _PuTTYs_ might vary.) It was clearly a bug that PuTTY was leaving connections to Pageant needlessly open. But it was _also_ a bug that Pageant couldn't handle more than about 60 at once. In this commit, I fix that secondary bug. The cause of the bug is that the WaitForMultipleObjects function family in the Windows API have a limit on the number of HANDLE objects they can select between. The limit is MAXIMUM_WAIT_OBJECTS, defined to be 64. And handle-io.c was using a separate event object for each I/O subthread to communicate back to the main thread, so as soon as all those event objects (plus a handful of other HANDLEs) added up to more than 64, we'd start passing an overlarge handle array to WaitForMultipleObjects, and it would start not doing what we wanted. To fix this, I've reorganised handle-io.c so that all its subthreads share just _one_ event object to signal readiness back to the main thread. There's now a linked list of 'struct handle' objects that are ready to be processed, protected by a CRITICAL_SECTION. Each subthread signals readiness by adding itself to the linked list, and setting the event object to indicate that the list is now non-empty. When the main thread receives the event, it iterates over the whole list processing all the ready handles. (Each 'struct handle' still has a separate event object for the main thread to use to communicate _to_ the subthread. That's OK, because no thread is ever waiting on all those events at once: each subthread only waits on its own.) The previous HT_FOREIGN system didn't really fit into this framework. So I've moved it out into its own system. There's now a handle-wait.c which deals with the relatively simple job of managing a list of handles that need to be waited for, each with a callback function; that's what communicates a list of HANDLEs to event loops, and receives the notification when the event loop notices that one of them has done something. And handle-io.c is now just one client of handle-wait.c, providing a single HANDLE to the event loop, and dealing internally with everything that needs to be done when that handle fires. The new top-level handle-wait.c system *still* can't deal with more than MAXIMUM_WAIT_OBJECTS. At the moment, I'm reasonably convinced it doesn't need to: the only kind of HANDLE that any of our tools could previously have needed to wait on more than one of was the one in handle-io.c that I've just removed. But I've left some assertions and a TODO comment in there just in case we need to change that in future.
2021-05-24 12:06:10 +00:00
int n;
Reorganise Windows HANDLE management. Before commit 6e69223dc262755, Pageant would stop working after a certain number of PuTTYs were active at the same time. (At most about 60, but maybe fewer - see below.) This was because of two separate bugs. The easy one, fixed in 6e69223dc262755 itself, was that PuTTY left each named-pipe connection to Pageant open for the rest of its lifetime. So the real problem was that Pageant had too many active connections at once. (And since a given PuTTY might make multiple connections during userauth - one to list keys, and maybe another to actually make a signature - that was why the number of _PuTTYs_ might vary.) It was clearly a bug that PuTTY was leaving connections to Pageant needlessly open. But it was _also_ a bug that Pageant couldn't handle more than about 60 at once. In this commit, I fix that secondary bug. The cause of the bug is that the WaitForMultipleObjects function family in the Windows API have a limit on the number of HANDLE objects they can select between. The limit is MAXIMUM_WAIT_OBJECTS, defined to be 64. And handle-io.c was using a separate event object for each I/O subthread to communicate back to the main thread, so as soon as all those event objects (plus a handful of other HANDLEs) added up to more than 64, we'd start passing an overlarge handle array to WaitForMultipleObjects, and it would start not doing what we wanted. To fix this, I've reorganised handle-io.c so that all its subthreads share just _one_ event object to signal readiness back to the main thread. There's now a linked list of 'struct handle' objects that are ready to be processed, protected by a CRITICAL_SECTION. Each subthread signals readiness by adding itself to the linked list, and setting the event object to indicate that the list is now non-empty. When the main thread receives the event, it iterates over the whole list processing all the ready handles. (Each 'struct handle' still has a separate event object for the main thread to use to communicate _to_ the subthread. That's OK, because no thread is ever waiting on all those events at once: each subthread only waits on its own.) The previous HT_FOREIGN system didn't really fit into this framework. So I've moved it out into its own system. There's now a handle-wait.c which deals with the relatively simple job of managing a list of handles that need to be waited for, each with a callback function; that's what communicates a list of HANDLEs to event loops, and receives the notification when the event loop notices that one of them has done something. And handle-io.c is now just one client of handle-wait.c, providing a single HANDLE to the event loop, and dealing internally with everything that needs to be done when that handle fires. The new top-level handle-wait.c system *still* can't deal with more than MAXIMUM_WAIT_OBJECTS. At the moment, I'm reasonably convinced it doesn't need to: the only kind of HANDLE that any of our tools could previously have needed to wait on more than one of was the one in handle-io.c that I've just removed. But I've left some assertions and a TODO comment in there just in case we need to change that in future.
2021-05-24 12:06:10 +00:00
HandleWaitList *hwl = get_handle_wait_list();
DWORD timeout = toplevel_callback_pending() ? 0 : INFINITE;
Reorganise Windows HANDLE management. Before commit 6e69223dc262755, Pageant would stop working after a certain number of PuTTYs were active at the same time. (At most about 60, but maybe fewer - see below.) This was because of two separate bugs. The easy one, fixed in 6e69223dc262755 itself, was that PuTTY left each named-pipe connection to Pageant open for the rest of its lifetime. So the real problem was that Pageant had too many active connections at once. (And since a given PuTTY might make multiple connections during userauth - one to list keys, and maybe another to actually make a signature - that was why the number of _PuTTYs_ might vary.) It was clearly a bug that PuTTY was leaving connections to Pageant needlessly open. But it was _also_ a bug that Pageant couldn't handle more than about 60 at once. In this commit, I fix that secondary bug. The cause of the bug is that the WaitForMultipleObjects function family in the Windows API have a limit on the number of HANDLE objects they can select between. The limit is MAXIMUM_WAIT_OBJECTS, defined to be 64. And handle-io.c was using a separate event object for each I/O subthread to communicate back to the main thread, so as soon as all those event objects (plus a handful of other HANDLEs) added up to more than 64, we'd start passing an overlarge handle array to WaitForMultipleObjects, and it would start not doing what we wanted. To fix this, I've reorganised handle-io.c so that all its subthreads share just _one_ event object to signal readiness back to the main thread. There's now a linked list of 'struct handle' objects that are ready to be processed, protected by a CRITICAL_SECTION. Each subthread signals readiness by adding itself to the linked list, and setting the event object to indicate that the list is now non-empty. When the main thread receives the event, it iterates over the whole list processing all the ready handles. (Each 'struct handle' still has a separate event object for the main thread to use to communicate _to_ the subthread. That's OK, because no thread is ever waiting on all those events at once: each subthread only waits on its own.) The previous HT_FOREIGN system didn't really fit into this framework. So I've moved it out into its own system. There's now a handle-wait.c which deals with the relatively simple job of managing a list of handles that need to be waited for, each with a callback function; that's what communicates a list of HANDLEs to event loops, and receives the notification when the event loop notices that one of them has done something. And handle-io.c is now just one client of handle-wait.c, providing a single HANDLE to the event loop, and dealing internally with everything that needs to be done when that handle fires. The new top-level handle-wait.c system *still* can't deal with more than MAXIMUM_WAIT_OBJECTS. At the moment, I'm reasonably convinced it doesn't need to: the only kind of HANDLE that any of our tools could previously have needed to wait on more than one of was the one in handle-io.c that I've just removed. But I've left some assertions and a TODO comment in there just in case we need to change that in future.
2021-05-24 12:06:10 +00:00
n = MsgWaitForMultipleObjects(hwl->nhandles, hwl->handles, false,
timeout, QS_ALLINPUT);
Reorganise Windows HANDLE management. Before commit 6e69223dc262755, Pageant would stop working after a certain number of PuTTYs were active at the same time. (At most about 60, but maybe fewer - see below.) This was because of two separate bugs. The easy one, fixed in 6e69223dc262755 itself, was that PuTTY left each named-pipe connection to Pageant open for the rest of its lifetime. So the real problem was that Pageant had too many active connections at once. (And since a given PuTTY might make multiple connections during userauth - one to list keys, and maybe another to actually make a signature - that was why the number of _PuTTYs_ might vary.) It was clearly a bug that PuTTY was leaving connections to Pageant needlessly open. But it was _also_ a bug that Pageant couldn't handle more than about 60 at once. In this commit, I fix that secondary bug. The cause of the bug is that the WaitForMultipleObjects function family in the Windows API have a limit on the number of HANDLE objects they can select between. The limit is MAXIMUM_WAIT_OBJECTS, defined to be 64. And handle-io.c was using a separate event object for each I/O subthread to communicate back to the main thread, so as soon as all those event objects (plus a handful of other HANDLEs) added up to more than 64, we'd start passing an overlarge handle array to WaitForMultipleObjects, and it would start not doing what we wanted. To fix this, I've reorganised handle-io.c so that all its subthreads share just _one_ event object to signal readiness back to the main thread. There's now a linked list of 'struct handle' objects that are ready to be processed, protected by a CRITICAL_SECTION. Each subthread signals readiness by adding itself to the linked list, and setting the event object to indicate that the list is now non-empty. When the main thread receives the event, it iterates over the whole list processing all the ready handles. (Each 'struct handle' still has a separate event object for the main thread to use to communicate _to_ the subthread. That's OK, because no thread is ever waiting on all those events at once: each subthread only waits on its own.) The previous HT_FOREIGN system didn't really fit into this framework. So I've moved it out into its own system. There's now a handle-wait.c which deals with the relatively simple job of managing a list of handles that need to be waited for, each with a callback function; that's what communicates a list of HANDLEs to event loops, and receives the notification when the event loop notices that one of them has done something. And handle-io.c is now just one client of handle-wait.c, providing a single HANDLE to the event loop, and dealing internally with everything that needs to be done when that handle fires. The new top-level handle-wait.c system *still* can't deal with more than MAXIMUM_WAIT_OBJECTS. At the moment, I'm reasonably convinced it doesn't need to: the only kind of HANDLE that any of our tools could previously have needed to wait on more than one of was the one in handle-io.c that I've just removed. But I've left some assertions and a TODO comment in there just in case we need to change that in future.
2021-05-24 12:06:10 +00:00
if ((unsigned)(n - WAIT_OBJECT_0) < (unsigned)hwl->nhandles)
handle_wait_activate(hwl, n - WAIT_OBJECT_0);
handle_wait_list_free(hwl);
while (sw_PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) {
if (msg.message == WM_QUIT)
goto finished; /* two-level break */
if (IsWindow(keylist) && IsDialogMessage(keylist, &msg))
continue;
if (IsWindow(aboutbox) && IsDialogMessage(aboutbox, &msg))
continue;
if (IsWindow(nonmodal_passphrase_hwnd) &&
IsDialogMessage(nonmodal_passphrase_hwnd, &msg))
continue;
TranslateMessage(&msg);
DispatchMessage(&msg);
}
winpgnt: handle WM_COPYDATA requests in a subthread. This is preparation to allow Pageant to be able to return to its GUI main loop in the middle of handling a request (e.g. present a dialog box to the user related to that particular request, and wait for the user's response). In order to do that, we need the main thread's Windows message loop to never be blocked by a WM_COPYDATA agent request. So I've split Pageant's previous hidden window into two hidden windows, each with a subset of the original roles, and created in different threads so that they get independent message loops. The one in the main thread receives messages relating to Pageant's system tray icon; the one in the subthread has the identity known to (old) Pageant clients, and receives WM_COPYDATA messages only. Each WM_COPYDATA is handled by passing the request back to the main thread via an event object integrated into the Pageant main loop, and then waiting for a second event object that the main thread will signal when the answer comes back, and not returning from the WndProc handler until the response arrives. Hence, if an agent request received via WM_COPYDATA requires GUI activity, then the main thread's GUI message loop will be able to do that in parallel with all Pageant's other activity, including other GUI activity (like the key list dialog box) and including responding to other requests via named pipe. I can't stop WM_COPYDATA requests from blocking _each other_, but this allows them not to block anything else. And named-pipe requests block nothing at all, so as clients switch over to the new IPC, even that blockage will become less and less common.
2020-01-25 17:24:17 +00:00
run_toplevel_callbacks();
}
finished:
/* Clean up the system tray icon */
{
NOTIFYICONDATA tnid;
tnid.cbSize = sizeof(NOTIFYICONDATA);
tnid.hWnd = traywindow;
tnid.uID = 1;
Shell_NotifyIcon(NIM_DELETE, &tnid);
DestroyMenu(systray_menu);
}
if (keypath) filereq_free(keypath);
Windows Pageant: integrate with Windows OpenSSH. After a discussion with a user recently, I investigated the Windows native ssh.exe, and found it uses a Windows named pipe to talk to its ssh-agent, in exactly the same way Pageant does. So if you tell ssh.exe where to find Pageant's pipe, it can talk directly to Pageant, and then you can have just one SSH agent. The slight problem is that Pageant's pipe name is not stable. It's generated using the same system as connection-sharing pipe names, and contains a hex hash value whose preimage was fed through CryptProtectData. And the problem with _that_ is that CryptProtectData apparently reinitialises its seed between login sessions (though it's stable within a login session), which I hadn't fully realised when I reused the same pipe-name construction code. One possibility, of course, would be to change Pageant so that it uses a fixed pipe name. But after a bit of thought, I think I actually like this feature, because the Windows named pipe namespace isn't segregated into areas writable by only particular users, so anyone using that namespace on a multiuser Windows box is potentially vulnerable to someone else squatting on the name you wanted to use. Using this system makes that harder, because the squatter won't be able to predict what the name is going to be! (Unless you shut down Pageant and start it up again within one login session - but there's only so much we can do. And squatting is at most a DoS, because PuTTY's named-pipe client code checks ownership of the other end of the pipe in all cases.) So instead I've gone for a different approach. Windows Pageant now supports an extra command-line option to write out a snippet of OpenSSH config file format on startup, containing an 'IdentityAgent' directive which points at the location of its named pipe. So you can use the 'Include' directive in your main .ssh/config to include this extra snippet, and then ssh.exe invocations will be able to find wherever the current Pageant has put its pipe.
2022-01-15 18:36:22 +00:00
if (openssh_config_file) {
/*
* Leave this file around, but empty it, so that it doesn't
* refer to a pipe we aren't listening on any more.
*/
FILE *fp = f_open(openssh_config_file, "w", true);
Windows Pageant: integrate with Windows OpenSSH. After a discussion with a user recently, I investigated the Windows native ssh.exe, and found it uses a Windows named pipe to talk to its ssh-agent, in exactly the same way Pageant does. So if you tell ssh.exe where to find Pageant's pipe, it can talk directly to Pageant, and then you can have just one SSH agent. The slight problem is that Pageant's pipe name is not stable. It's generated using the same system as connection-sharing pipe names, and contains a hex hash value whose preimage was fed through CryptProtectData. And the problem with _that_ is that CryptProtectData apparently reinitialises its seed between login sessions (though it's stable within a login session), which I hadn't fully realised when I reused the same pipe-name construction code. One possibility, of course, would be to change Pageant so that it uses a fixed pipe name. But after a bit of thought, I think I actually like this feature, because the Windows named pipe namespace isn't segregated into areas writable by only particular users, so anyone using that namespace on a multiuser Windows box is potentially vulnerable to someone else squatting on the name you wanted to use. Using this system makes that harder, because the squatter won't be able to predict what the name is going to be! (Unless you shut down Pageant and start it up again within one login session - but there's only so much we can do. And squatting is at most a DoS, because PuTTY's named-pipe client code checks ownership of the other end of the pipe in all cases.) So instead I've gone for a different approach. Windows Pageant now supports an extra command-line option to write out a snippet of OpenSSH config file format on startup, containing an 'IdentityAgent' directive which points at the location of its named pipe. So you can use the 'Include' directive in your main .ssh/config to include this extra snippet, and then ssh.exe invocations will be able to find wherever the current Pageant has put its pipe.
2022-01-15 18:36:22 +00:00
if (fp)
fclose(fp);
}
cleanup_exit(msg.wParam);
return msg.wParam; /* just in case optimiser complains */
}