From c14f079863ba6fb48137c7a96be7de5f7e5a8007 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Wed, 14 Sep 2022 16:04:14 +0100 Subject: [PATCH] windows/utils/registry.c: allow opening reg keys RO. These handy wrappers on the verbose underlying Win32 registry API have to lose some expressiveness, and one thing they lost was the ability to open a registry key without asking for both read and write access. This meant they couldn't be used for accessing keys not owned by the calling user. So far, I've only used them for accessing PuTTY's own saved data, which means that hasn't been a problem. But I want to use them elsewhere in an upcoming commit, so I need to fix that. The obvious thing would be to change the meaning of the existing 'create' boolean flag so that if it's false, we also don't request write access. The rationale would be that you're either reading or writing, and if you're writing you want both RW access and to create keys that don't already exist. But in fact that's not true: you do want to set create==false and have write access in the case where you're _deleting_ things from the key (or the whole key). So we really do need three ways to call the wrapper function. Rather than add another boolean field to every call site or mess about with an 'access type' enum, I've taken an in-between route: the underlying open_regkey_fn *function* takes a 'create' and a 'write' flag, but at call sites, it's wrapped with a macro anyway (to append NULL to the variadic argument list), so I've just made three macros whose names request different access. That makes call sites marginally _less_ verbose, while still (cherry picked from commit 7339e00f4ac8a0e5f0e101d2a0effa18ef794034) --- windows/platform.h | 10 +++++++--- windows/storage.c | 39 +++++++++++++++++++-------------------- windows/utils/registry.c | 10 +++++----- 3 files changed, 31 insertions(+), 28 deletions(-) diff --git a/windows/platform.h b/windows/platform.h index cd9ef989..aa221a88 100644 --- a/windows/platform.h +++ b/windows/platform.h @@ -734,9 +734,13 @@ char *get_jumplist_registry_entries(void); #define CLIPUI_DEFAULT_INS CLIPUI_EXPLICIT /* In utils */ -HKEY open_regkey_fn(bool create, HKEY base, const char *path, ...); -#define open_regkey(create, base, ...) \ - open_regkey_fn(create, base, __VA_ARGS__, (const char *)NULL) +HKEY open_regkey_fn(bool create, bool write, HKEY base, const char *path, ...); +#define open_regkey_ro(base, ...) \ + open_regkey_fn(false, false, base, __VA_ARGS__, (const char *)NULL) +#define open_regkey_rw(base, ...) \ + open_regkey_fn(false, true, base, __VA_ARGS__, (const char *)NULL) +#define create_regkey(base, ...) \ + open_regkey_fn(true, true, base, __VA_ARGS__, (const char *)NULL) void close_regkey(HKEY key); void del_regkey(HKEY key, const char *name); char *enum_regkey(HKEY key, int index); diff --git a/windows/storage.c b/windows/storage.c index f2c33feb..29095939 100644 --- a/windows/storage.c +++ b/windows/storage.c @@ -42,7 +42,7 @@ settings_w *open_settings_w(const char *sessionname, char **errmsg) strbuf *sb = strbuf_new(); escape_registry_key(sessionname, sb); - HKEY sesskey = open_regkey(true, HKEY_CURRENT_USER, puttystr, sb->s); + HKEY sesskey = create_regkey(HKEY_CURRENT_USER, puttystr, sb->s); if (!sesskey) { *errmsg = dupprintf("Unable to create registry key\n" "HKEY_CURRENT_USER\\%s\\%s", puttystr, sb->s); @@ -85,7 +85,7 @@ settings_r *open_settings_r(const char *sessionname) strbuf *sb = strbuf_new(); escape_registry_key(sessionname, sb); - HKEY sesskey = open_regkey(false, HKEY_CURRENT_USER, puttystr, sb->s); + HKEY sesskey = open_regkey_ro(HKEY_CURRENT_USER, puttystr, sb->s); strbuf_free(sb); if (!sesskey) @@ -196,7 +196,7 @@ void close_settings_r(settings_r *handle) void del_settings(const char *sessionname) { - HKEY rkey = open_regkey(false, HKEY_CURRENT_USER, puttystr); + HKEY rkey = open_regkey_rw(HKEY_CURRENT_USER, puttystr); if (!rkey) return; @@ -217,7 +217,7 @@ struct settings_e { settings_e *enum_settings_start(void) { - HKEY key = open_regkey(false, HKEY_CURRENT_USER, puttystr); + HKEY key = open_regkey_ro(HKEY_CURRENT_USER, puttystr); if (!key) return NULL; @@ -264,8 +264,8 @@ int check_stored_host_key(const char *hostname, int port, strbuf *regname = strbuf_new(); hostkey_regname(regname, hostname, port, keytype); - HKEY rkey = open_regkey(false, HKEY_CURRENT_USER, - PUTTY_REG_POS "\\SshHostKeys"); + HKEY rkey = open_regkey_ro(HKEY_CURRENT_USER, + PUTTY_REG_POS "\\SshHostKeys"); if (!rkey) { strbuf_free(regname); return 1; /* key does not exist in registry */ @@ -363,8 +363,8 @@ void store_host_key(const char *hostname, int port, strbuf *regname = strbuf_new(); hostkey_regname(regname, hostname, port, keytype); - HKEY rkey = open_regkey(true, HKEY_CURRENT_USER, - PUTTY_REG_POS "\\SshHostKeys"); + HKEY rkey = create_regkey(HKEY_CURRENT_USER, + PUTTY_REG_POS "\\SshHostKeys"); if (rkey) { put_reg_sz(rkey, regname->s, key); close_regkey(rkey); @@ -383,7 +383,7 @@ host_ca_enum *enum_host_ca_start(void) host_ca_enum *e; HKEY key; - if (!(key = open_regkey(false, HKEY_CURRENT_USER, host_ca_key))) + if (!(key = open_regkey_ro(HKEY_CURRENT_USER, host_ca_key))) return NULL; e = snew(host_ca_enum); @@ -418,7 +418,7 @@ host_ca *host_ca_load(const char *name) sb = strbuf_new(); escape_registry_key(name, sb); - HKEY rkey = open_regkey(false, HKEY_CURRENT_USER, host_ca_key, sb->s); + HKEY rkey = open_regkey_ro(HKEY_CURRENT_USER, host_ca_key, sb->s); strbuf_free(sb); if (!rkey) @@ -466,7 +466,7 @@ char *host_ca_save(host_ca *hca) strbuf *sb = strbuf_new(); escape_registry_key(hca->name, sb); - HKEY rkey = open_regkey(true, HKEY_CURRENT_USER, host_ca_key, sb->s); + HKEY rkey = create_regkey(HKEY_CURRENT_USER, host_ca_key, sb->s); if (!rkey) { char *err = dupprintf("Unable to create registry key\n" "HKEY_CURRENT_USER\\%s\\%s", host_ca_key, sb->s); @@ -495,7 +495,7 @@ char *host_ca_save(host_ca *hca) char *host_ca_delete(const char *name) { - HKEY rkey = open_regkey(false, HKEY_CURRENT_USER, host_ca_key); + HKEY rkey = open_regkey_rw(HKEY_CURRENT_USER, host_ca_key); if (!rkey) return NULL; @@ -561,7 +561,7 @@ static HANDLE access_random_seed(int action) * Registry, if any. */ { - HKEY rkey = open_regkey(false, HKEY_CURRENT_USER, PUTTY_REG_POS); + HKEY rkey = open_regkey_ro(HKEY_CURRENT_USER, PUTTY_REG_POS); if (rkey) { char *regpath = get_reg_sz(rkey, "RandSeedFile"); close_regkey(rkey); @@ -688,7 +688,7 @@ void write_random_seed(void *data, int len) static int transform_jumplist_registry( const char *add, const char *rem, char **out) { - HKEY rkey = open_regkey(true, HKEY_CURRENT_USER, reg_jumplist_key); + HKEY rkey = create_regkey(HKEY_CURRENT_USER, reg_jumplist_key); if (!rkey) return JUMPLISTREG_ERROR_KEYOPENCREATE_FAILURE; @@ -785,7 +785,7 @@ static void registry_recursive_remove(HKEY key) DWORD i = 0; while ((name = enum_regkey(key, i)) != NULL) { - HKEY subkey = open_regkey(false, key, name); + HKEY subkey = open_regkey_rw(key, name); if (subkey) { registry_recursive_remove(subkey); close_regkey(subkey); @@ -816,7 +816,7 @@ void cleanup_all(void) /* * Open the main PuTTY registry key and remove everything in it. */ - HKEY key = open_regkey(false, HKEY_CURRENT_USER, PUTTY_REG_POS); + HKEY key = open_regkey_rw(HKEY_CURRENT_USER, PUTTY_REG_POS); if (key) { registry_recursive_remove(key); close_regkey(key); @@ -826,8 +826,7 @@ void cleanup_all(void) * we've done that, see if the parent key has any other * children. */ - if ((key = open_regkey(false, HKEY_CURRENT_USER, - PUTTY_REG_PARENT)) != NULL) { + if ((key = open_regkey_rw(HKEY_CURRENT_USER, PUTTY_REG_PARENT)) != NULL) { del_regkey(key, PUTTY_REG_PARENT_CHILD); char *name = enum_regkey(key, 0); close_regkey(key); @@ -840,8 +839,8 @@ void cleanup_all(void) if (name) { sfree(name); } else { - if ((key = open_regkey(false, HKEY_CURRENT_USER, - PUTTY_REG_GPARENT)) != NULL) { + if ((key = open_regkey_rw(HKEY_CURRENT_USER, + PUTTY_REG_GPARENT)) != NULL) { del_regkey(key, PUTTY_REG_GPARENT_CHILD); close_regkey(key); } diff --git a/windows/utils/registry.c b/windows/utils/registry.c index 1f50e67a..9d65df1c 100644 --- a/windows/utils/registry.c +++ b/windows/utils/registry.c @@ -5,7 +5,7 @@ #include "putty.h" -HKEY open_regkey_fn(bool create, HKEY hk, const char *path, ...) +HKEY open_regkey_fn(bool create, bool write, HKEY hk, const char *path, ...) { HKEY toret = NULL; bool hk_needs_close = false; @@ -15,14 +15,14 @@ HKEY open_regkey_fn(bool create, HKEY hk, const char *path, ...) for (; path; path = va_arg(ap, const char *)) { HKEY hk_sub = NULL; + DWORD access = KEY_READ | (write ? KEY_WRITE : 0); LONG status; if (create) status = RegCreateKeyEx( hk, path, 0, NULL, REG_OPTION_NON_VOLATILE, - KEY_READ | KEY_WRITE, NULL, &hk_sub, NULL); + access, NULL, &hk_sub, NULL); else - status = RegOpenKeyEx( - hk, path, 0, KEY_READ | KEY_WRITE, &hk_sub); + status = RegOpenKeyEx(hk, path, 0, access, &hk_sub); if (status != ERROR_SUCCESS) goto out; @@ -175,7 +175,7 @@ bool put_reg_multi_sz(HKEY key, const char *name, strbuf *str) char *get_reg_sz_simple(HKEY key, const char *name, const char *leaf) { - HKEY subkey = open_regkey(false, key, name); + HKEY subkey = open_regkey_ro(key, name); if (!subkey) return NULL; char *toret = get_reg_sz(subkey, leaf);