diff --git a/Recipe b/Recipe index 48cad133..6dc306fa 100644 --- a/Recipe +++ b/Recipe @@ -54,8 +54,7 @@ # security grounds (although it will run fine on Win95-series # OSes where there is no access control anyway). # - SSH connection sharing is disabled. -# - There is no restriction of the process ACLs (on all versions -# of Windows, without warning), as if UNPROTECT below were set. +# - There is no support for restriction of the process ACLs. # # - COMPAT=/DNO_MULTIMON (Windows only) # Disables PuTTY's use of , which is not available @@ -108,10 +107,6 @@ # - XFLAGS=/DDEBUG # Causes PuTTY to enable internal debugging. # -# - XFLAGS=/DUNPROTECT -# Disable tightened ACL on PuTTY process so that e.g. debuggers -# can attach to it. -# # - XFLAGS=/DMALLOC_LOG # Causes PuTTY to emit a file called putty_mem.log, logging every # memory allocation and free, so you can track memory leaks. diff --git a/cmdline.c b/cmdline.c index e6b69073..73ede342 100644 --- a/cmdline.c +++ b/cmdline.c @@ -609,6 +609,17 @@ int cmdline_process_param(const char *p, char *value, conf_set_str(conf, CONF_proxy_telnet_command, value); } +#ifdef _WINDOWS + /* + * Cross-tool options only available on Windows. + */ + if (!strcmp(p, "-restrict-acl") || !strcmp(p, "-restrict_acl") || + !strcmp(p, "-restrictacl")) { + RETURN(1); + restrict_process_acl(); + } +#endif + return ret; /* unrecognised */ } diff --git a/doc/using.but b/doc/using.but index 8a4b95db..b21a9a8b 100644 --- a/doc/using.but +++ b/doc/using.but @@ -1010,3 +1010,24 @@ connection. It expects a shell command string as an argument. See \k{config-proxy-type} for more information on this, and on other proxy settings. + +\S2{using-cmdline-restrict-acl} \i\c{-restrict-acl}: restrict the +Windows process ACL + +This option (on Windows only) causes PuTTY to try to lock down the +operating system's access control on its own process. If this +succeeds, it should present an extra obstacle to malware that has +managed to run under the same user id as the PuTTY process, by +preventing it from attaching to PuTTY using the same interfaces +debuggers use and either reading sensitive information out of its +memory or hijacking its network session. + +This option is not enabled by default, because this form of +interaction between Windows programs has many legitimate uses, +including accessibility software such as screen readers. Also, it +cannot provide full security against this class of attack in any case, +because PuTTY can only lock down its own ACL \e{after} it has started +up, and malware could still get in if it attacks the process between +startup and lockdown. So it trades away noticeable convenience, and +delivers less real security than you might want. However, if you do +want to make that tradeoff anyway, the option is available. diff --git a/windows/window.c b/windows/window.c index 044d3ce3..5ef3a460 100644 --- a/windows/window.c +++ b/windows/window.c @@ -403,21 +403,6 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) return 1; } - /* - * Protect our process - */ - { -#if !defined UNPROTECT && !defined NO_SECURITY - char *error = NULL; - if (! setprocessacl(error)) { - char *message = dupprintf("Could not restrict process ACL: %s", - error); - logevent(NULL, message); - sfree(message); - sfree(error); - } -#endif - } /* * Process the command line. */ diff --git a/windows/winpgen.c b/windows/winpgen.c index 4dd8272a..98c2097e 100644 --- a/windows/winpgen.c +++ b/windows/winpgen.c @@ -1517,7 +1517,7 @@ void cleanup_exit(int code) int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) { - int argc; + int argc, i; char **argv; int ret; @@ -1534,36 +1534,24 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) split_into_argv(cmdline, &argc, &argv, NULL); - if (argc > 0) { - if (!strcmp(argv[0], "-pgpfp")) { + for (i = 0; i < argc; i++) { + if (!strcmp(argv[i], "-pgpfp")) { pgp_fingerprints(); - exit(1); + return 1; + } else if (!strcmp(argv[i], "-restrict-acl") || + !strcmp(argv[i], "-restrict_acl") || + !strcmp(argv[i], "-restrictacl")) { + restrict_process_acl(); } else { /* * Assume the first argument to be a private key file, and * attempt to load it. */ - cmdline_keyfile = argv[0]; + cmdline_keyfile = argv[i]; + break; } } -#if !defined UNPROTECT && !defined NO_SECURITY - /* - * Protect our process. - */ - { - char *error = NULL; - if (!setprocessacl(error)) { - char *message = dupprintf("Could not restrict process ACL: %s", - error); - MessageBox(NULL, message, "PuTTYgen Warning", - MB_ICONWARNING | MB_OK); - sfree(message); - sfree(error); - } - } -#endif - random_ref(); ret = DialogBox(hinst, MAKEINTRESOURCE(201), NULL, MainDlgProc) != IDOK; diff --git a/windows/winpgnt.c b/windows/winpgnt.c index 604d092f..fe0822e4 100644 --- a/windows/winpgnt.c +++ b/windows/winpgnt.c @@ -1159,6 +1159,10 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) if (!strcmp(argv[i], "-pgpfp")) { pgp_fingerprints(); return 1; + } else if (!strcmp(argv[i], "-restrict-acl") || + !strcmp(argv[i], "-restrict_acl") || + !strcmp(argv[i], "-restrictacl")) { + restrict_process_acl(); } else if (!strcmp(argv[i], "-c")) { /* * If we see `-c', then the rest of the @@ -1178,23 +1182,6 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) } } -#if !defined UNPROTECT && !defined NO_SECURITY - /* - * Protect our process. - */ - { - char *error = NULL; - if (!setprocessacl(error)) { - char *message = dupprintf("Could not restrict process ACL: %s", - error); - MessageBox(NULL, message, "Pageant Warning", - MB_ICONWARNING | MB_OK); - sfree(message); - sfree(error); - } - } -#endif - /* * Forget any passphrase that we retained while going over * command line keyfiles. diff --git a/windows/winplink.c b/windows/winplink.c index d916104e..0f86b41c 100644 --- a/windows/winplink.c +++ b/windows/winplink.c @@ -502,22 +502,6 @@ int main(int argc, char **argv) } } -#if !defined UNPROTECT && !defined NO_SECURITY - /* - * Protect our process. - */ - { - char *error = NULL; - if (!setprocessacl(error)) { - char *message = dupprintf("Could not restrict process ACL: %s", - error); - logevent(NULL, message); - sfree(message); - sfree(error); - } - } -#endif - if (errors) return 1; diff --git a/windows/winsecur.c b/windows/winsecur.c index 8b3c7f16..d07150d0 100644 --- a/windows/winsecur.c +++ b/windows/winsecur.c @@ -224,7 +224,7 @@ int make_private_security_descriptor(DWORD permissions, return ret; } -int setprocessacl(char *error) +static int really_restrict_process_acl(char *error) { EXPLICIT_ACCESS ea[2]; int acl_err; @@ -287,3 +287,35 @@ int setprocessacl(char *error) return ret; } #endif /* !defined NO_SECURITY */ + +/* + * Lock down our process's ACL, to present an obstacle to malware + * trying to write into its memory. This can't be a full defence, + * because well timed malware could attack us before this code runs - + * even if it was unconditionally run at the very start of main(), + * which we wouldn't want to do anyway because it turns out in practie + * that interfering with other processes in this way has significant + * non-infringing uses on Windows (e.g. screen reader software). + * + * If we've been requested to do this and are unsuccessful, bomb out + * via modalfatalbox rather than continue in a less protected mode. + * + * This function is intentionally outside the #ifndef NO_SECURITY that + * covers the rest of this file, because when PuTTY is compiled + * without the ability to restrict its ACL, we don't want it to + * silently pretend to honour the instruction to do so. + */ +void restrict_process_acl(void) +{ + char *error = NULL; + int ret; + +#if !defined NO_SECURITY + ret = really_restrict_process_acl(error); +#else + ret = FALSE; + error = dupstr("ACL restrictions not compiled into this binary"); +#endif + if (!ret) + modalfatalbox("Could not restrict process ACL: %s", error); +} diff --git a/windows/winsecur.h b/windows/winsecur.h index ed3151c5..a56f7fb8 100644 --- a/windows/winsecur.h +++ b/windows/winsecur.h @@ -56,6 +56,4 @@ int make_private_security_descriptor(DWORD permissions, PACL *acl, char **error); -int setprocessacl(char *error); - #endif diff --git a/windows/winsftp.c b/windows/winsftp.c index 7b08970f..c85c24aa 100644 --- a/windows/winsftp.c +++ b/windows/winsftp.c @@ -749,21 +749,6 @@ char *ssh_sftp_get_cmdline(const char *prompt, int no_fds_ok) void platform_psftp_post_option_setup(void) { -#if !defined UNPROTECT && !defined NO_SECURITY - /* - * Protect our process. - */ - { - char *error = NULL; - if (!setprocessacl(error)) { - char *message = dupprintf("Could not restrict process ACL: %s", - error); - logevent(NULL, message); - sfree(message); - sfree(error); - } - } -#endif } /* ---------------------------------------------------------------------- diff --git a/windows/winstuff.h b/windows/winstuff.h index 9829f087..a120a735 100644 --- a/windows/winstuff.h +++ b/windows/winstuff.h @@ -484,6 +484,7 @@ void dll_hijacking_protection(void); BOOL init_winver(void); HMODULE load_system32_dll(const char *libname); const char *win_strerror(int error); +void restrict_process_acl(void); /* * Exports from sizetip.c.