1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-10 01:48:00 +00:00

Windows Pageant: turn 'has_security' into a global function.

Now it can be called from places other than Pageant's WinMain(). In
particular, the attempt to make a security descriptor in
lock_interprocess_mutex() is gated on it.

In return, however, I've tightened up the semantics. In normal PuTTY
builds that aren't trying to support pre-NT systems, the function
*unconditionally* returns true, on the grounds that we don't expect to
target any system that doesn't support the security APIs, and if
someone manages to contrive one anyway - or, more likely, if we some
day introduce a bug in our loading of the security API functions -
then this safety catch should make Pageant less likely to accidentally
fall back to 'never mind, just run in insecure mode'.
This commit is contained in:
Simon Tatham 2022-03-12 15:02:12 +00:00
parent f500d24a95
commit a2b376af96
4 changed files with 49 additions and 40 deletions

View File

@ -91,8 +91,6 @@ void modalfatalbox(const char *fmt, ...)
exit(1); exit(1);
} }
static bool has_security;
struct PassphraseProcStruct { struct PassphraseProcStruct {
bool modal; bool modal;
const char *help_topic; const char *help_topic;
@ -999,7 +997,7 @@ static char *answer_filemapping_message(const char *mapname)
debug("maphandle = %p\n", maphandle); debug("maphandle = %p\n", maphandle);
#endif #endif
if (has_security) { if (should_have_security()) {
DWORD retd; DWORD retd;
if ((expectedsid = get_user_sid()) == NULL) { if ((expectedsid = get_user_sid()) == NULL) {
@ -1405,14 +1403,7 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show)
hinst = inst; hinst = inst;
/* if (should_have_security()) {
* Determine whether we're an NT system (should have security
* APIs) or a non-NT system (don't do security).
*/
init_winver();
has_security = (osPlatformId == VER_PLATFORM_WIN32_NT);
if (has_security) {
/* /*
* Attempt to get the security API we need. * Attempt to get the security API we need.
*/ */
@ -1543,39 +1534,42 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show)
/* /*
* Set up a named-pipe listener. * Set up a named-pipe listener.
*/ */
Plug *pl_plug;
wpc->plc.vt = &winpgnt_vtable; wpc->plc.vt = &winpgnt_vtable;
wpc->plc.suppress_logging = true; wpc->plc.suppress_logging = true;
struct pageant_listen_state *pl = if (should_have_security()) {
pageant_listener_new(&pl_plug, &wpc->plc); Plug *pl_plug;
char *pipename = agent_named_pipe_name(); struct pageant_listen_state *pl =
Socket *sock = new_named_pipe_listener(pipename, pl_plug); pageant_listener_new(&pl_plug, &wpc->plc);
if (sk_socket_error(sock)) { char *pipename = agent_named_pipe_name();
char *err = dupprintf("Unable to open named pipe at %s " Socket *sock = new_named_pipe_listener(pipename, pl_plug);
"for SSH agent:\n%s", pipename, if (sk_socket_error(sock)) {
sk_socket_error(sock)); char *err = dupprintf("Unable to open named pipe at %s "
MessageBox(NULL, err, "Pageant Error", MB_ICONERROR | MB_OK); "for SSH agent:\n%s", pipename,
return 1; sk_socket_error(sock));
}
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 = fopen(openssh_config_file, "w");
if (!fp) {
char *err = dupprintf("Unable to write OpenSSH config file "
"to %s", openssh_config_file);
MessageBox(NULL, err, "Pageant Error", MB_ICONERROR | MB_OK); MessageBox(NULL, err, "Pageant Error", MB_ICONERROR | MB_OK);
return 1; return 1;
} }
fprintf(fp, "IdentityAgent %s\n", pipename); pageant_listener_got_socket(pl, sock);
fclose(fp);
}
sfree(pipename); /*
* 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 = fopen(openssh_config_file, "w");
if (!fp) {
char *err = dupprintf("Unable to write OpenSSH config "
"file to %s", openssh_config_file);
MessageBox(NULL, err, "Pageant Error",
MB_ICONERROR | MB_OK);
return 1;
}
fprintf(fp, "IdentityAgent %s\n", pipename);
fclose(fp);
}
sfree(pipename);
}
/* /*
* Set up the window class for the hidden window that receives * Set up the window class for the hidden window that receives

View File

@ -574,6 +574,7 @@ void dll_hijacking_protection(void);
const char *get_system_dir(void); const char *get_system_dir(void);
HMODULE load_system32_dll(const char *libname); HMODULE load_system32_dll(const char *libname);
const char *win_strerror(int error); const char *win_strerror(int error);
bool should_have_security(void);
void restrict_process_acl(void); void restrict_process_acl(void);
bool restricted_acl(void); bool restricted_acl(void);
void escape_registry_key(const char *in, strbuf *out); void escape_registry_key(const char *in, strbuf *out);

View File

@ -13,8 +13,8 @@ HANDLE lock_interprocess_mutex(const char *mutexname, char **error)
PACL acl = NULL; PACL acl = NULL;
HANDLE mutex = NULL; HANDLE mutex = NULL;
if (!make_private_security_descriptor(MUTEX_ALL_ACCESS, if (should_have_security() && !make_private_security_descriptor(
&psd, &acl, error)) MUTEX_ALL_ACCESS, &psd, &acl, error))
goto out; goto out;
SECURITY_ATTRIBUTES sa; SECURITY_ATTRIBUTES sa;

View File

@ -20,6 +20,20 @@ DEF_WINDOWS_FUNCTION(GetSecurityInfo);
DEF_WINDOWS_FUNCTION(SetSecurityInfo); DEF_WINDOWS_FUNCTION(SetSecurityInfo);
DEF_WINDOWS_FUNCTION(SetEntriesInAclA); DEF_WINDOWS_FUNCTION(SetEntriesInAclA);
bool should_have_security(void)
{
#ifdef LEGACY_WINDOWS
/* Legacy pre-NT platforms are not expected to have any of these APIs */
init_winver();
return (osPlatformId == VER_PLATFORM_WIN32_NT);
#else
/* In the up-to-date PuTTY builds which do not support those
* platforms, unconditionally return true, to minimise the risk of
* compiling out security checks. */
return true;
#endif
}
bool got_advapi(void) bool got_advapi(void)
{ {
static bool attempted = false; static bool attempted = false;