diff --git a/pageant.c b/pageant.c index 553fcf44..f6fe74af 100644 --- a/pageant.c +++ b/pageant.c @@ -392,11 +392,8 @@ static bool request_passphrase(PageantClient *pc, PageantKey *pk) if (!pk->decryption_prompt_active) { assert(!gui_request_in_progress); - strbuf *sb = strbuf_new(); - strbuf_catf(sb, "Enter passphrase to decrypt key '%s'", pk->comment); bool created_dlg = pageant_client_ask_passphrase( - pc, &pk->dlgid, sb->s); - strbuf_free(sb); + pc, &pk->dlgid, pk->comment); if (!created_dlg) return false; @@ -1525,11 +1522,11 @@ static void pageant_conn_got_response( } static bool pageant_conn_ask_passphrase( - PageantClient *pc, PageantClientDialogId *dlgid, const char *msg) + PageantClient *pc, PageantClientDialogId *dlgid, const char *comment) { struct pageant_conn_state *pcs = container_of(pc, struct pageant_conn_state, pc); - return pageant_listener_client_ask_passphrase(pcs->plc, dlgid, msg); + return pageant_listener_client_ask_passphrase(pcs->plc, dlgid, comment); } static const PageantClientVtable pageant_connection_clientvt = { @@ -1719,7 +1716,7 @@ static void internal_client_got_response( } static bool internal_client_ask_passphrase( - PageantClient *pc, PageantClientDialogId *dlgid, const char *msg) + PageantClient *pc, PageantClientDialogId *dlgid, const char *comment) { /* No delaying operations are permitted in this mode */ return false; diff --git a/pageant.h b/pageant.h index e7eaa47c..86c42c1f 100644 --- a/pageant.h +++ b/pageant.h @@ -34,7 +34,7 @@ struct PageantClientVtable { void (*got_response)(PageantClient *pc, PageantClientRequestId *reqid, ptrlen response); bool (*ask_passphrase)(PageantClient *pc, PageantClientDialogId *dlgid, - const char *msg); + const char *key_comment); }; static inline void pageant_client_log_v( @@ -58,8 +58,8 @@ static inline void pageant_client_got_response( PageantClient *pc, PageantClientRequestId *reqid, ptrlen response) { pc->vt->got_response(pc, reqid, response); } static inline bool pageant_client_ask_passphrase( - PageantClient *pc, PageantClientDialogId *dlgid, const char *msg) -{ return pc->vt->ask_passphrase(pc, dlgid, msg); } + PageantClient *pc, PageantClientDialogId *dlgid, const char *comment) +{ return pc->vt->ask_passphrase(pc, dlgid, comment); } /* PageantClientRequestId is used to match up responses to the agent * requests they refer to. A client may allocate one of these for each @@ -159,7 +159,8 @@ struct PageantListenerClient { struct PageantListenerClientVtable { void (*log)(PageantListenerClient *, const char *fmt, va_list ap); bool (*ask_passphrase)(PageantListenerClient *pc, - PageantClientDialogId *dlgid, const char *msg); + PageantClientDialogId *dlgid, + const char *key_comment); }; static inline void pageant_listener_client_log_v( @@ -179,8 +180,9 @@ static inline PRINTF_LIKE(2, 3) void pageant_listener_client_log( } } static inline bool pageant_listener_client_ask_passphrase( - PageantListenerClient *plc, PageantClientDialogId *dlgid, const char *msg) -{ return plc->vt->ask_passphrase(plc, dlgid, msg); } + PageantListenerClient *plc, PageantClientDialogId *dlgid, + const char *comment) +{ return plc->vt->ask_passphrase(plc, dlgid, comment); } struct pageant_listen_state; struct pageant_listen_state *pageant_listener_new( diff --git a/unix/uxpgnt.c b/unix/uxpgnt.c index f859c555..24dc0e45 100644 --- a/unix/uxpgnt.c +++ b/unix/uxpgnt.c @@ -103,24 +103,34 @@ static int make_pipe_to_askpass(const char *msg) } static bool uxpgnt_ask_passphrase( - PageantListenerClient *plc, PageantClientDialogId *dlgid, const char *msg) + PageantListenerClient *plc, PageantClientDialogId *dlgid, + const char *comment) { struct uxpgnt_client *upc = container_of(plc, struct uxpgnt_client, plc); assert(!upc->dlgid); /* Pageant core should be serialising requests */ + char *msg = dupprintf( + "A client of Pageant wants to use the following encrypted key:\n" + "%s\n" + "If you intended this, enter the passphrase to decrypt the key.", + comment); + switch (upc->prompt_type) { case RTPROMPT_UNAVAILABLE: + sfree(msg); return false; case RTPROMPT_GUI: upc->passphrase_fd = make_pipe_to_askpass(msg); + sfree(msg); if (upc->passphrase_fd < 0) return false; /* something went wrong */ break; case RTPROMPT_DEBUG: fprintf(upc->logfp, "pageant passphrase request: %s\n", msg); + sfree(msg); break; } diff --git a/windows/pageant.rc b/windows/pageant.rc index 2469070c..2ef79347 100644 --- a/windows/pageant.rc +++ b/windows/pageant.rc @@ -14,16 +14,29 @@ 210 DIALOG DISCARDABLE 0, 0, 140, 60 STYLE DS_MODALFRAME | WS_POPUP | WS_CAPTION | WS_SYSMENU -CAPTION "Pageant: Enter Passphrase" +CAPTION "Pageant: Loading Encrypted Key" FONT 8, "MS Shell Dlg" BEGIN - CTEXT "Enter passphrase for key", 100, 10, 6, 120, 8 + CTEXT "Enter passphrase to load key", 100, 10, 6, 120, 8 CTEXT "", 101, 10, 16, 120, 8 EDITTEXT 102, 10, 26, 120, 12, ES_PASSWORD | ES_AUTOHSCROLL DEFPUSHBUTTON "O&K", IDOK, 20, 42, 40, 14 PUSHBUTTON "&Cancel", IDCANCEL, 80, 42, 40, 14 END +212 DIALOG DISCARDABLE 0, 0, 250, 70 +STYLE DS_MODALFRAME | WS_POPUP | WS_CAPTION | WS_SYSMENU +CAPTION "Pageant: Decrypting Stored Key" +FONT 8, "MS Shell Dlg" +BEGIN + CTEXT "A client of Pageant wants to use the following encrypted key:", 100, 10, 6, 230, 8 + CTEXT "", 101, 10, 16, 230, 8 + CTEXT "If you intended this, enter the passphrase to decrypt the key.", 101, 10, 26, 230, 8 + EDITTEXT 102, 10, 36, 230, 12, ES_PASSWORD | ES_AUTOHSCROLL + DEFPUSHBUTTON "O&K", IDOK, 75, 52, 40, 14 + PUSHBUTTON "&Cancel", IDCANCEL, 135, 52, 40, 14 +END + 211 DIALOG DISCARDABLE 0, 0, 450, 211 STYLE DS_MODALFRAME | WS_POPUP | WS_CAPTION | WS_SYSMENU CAPTION "Pageant Key List" diff --git a/windows/winpgnt.c b/windows/winpgnt.c index bd07aba7..84455e77 100644 --- a/windows/winpgnt.c +++ b/windows/winpgnt.c @@ -863,7 +863,7 @@ static void wm_copydata_got_response( } static bool ask_passphrase_common(PageantClientDialogId *dlgid, - const char *msg) + const char *comment) { /* Pageant core should be serialising requests, so we never expect * a passphrase prompt to exist already at this point */ @@ -873,18 +873,55 @@ static bool ask_passphrase_common(PageantClientDialogId *dlgid, pps->modal = false; pps->dlgid = dlgid; pps->passphrase = NULL; - pps->comment = msg; + pps->comment = comment; nonmodal_passphrase_hwnd = CreateDialogParam( - hinst, MAKEINTRESOURCE(210), NULL, PassphraseProc, (LPARAM)pps); + hinst, MAKEINTRESOURCE(212), 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 *msg) + PageantClient *pc, PageantClientDialogId *dlgid, const char *comment) { - return ask_passphrase_common(dlgid, msg); + return ask_passphrase_common(dlgid, comment); } static const PageantClientVtable wmcpc_vtable = { @@ -1270,9 +1307,10 @@ void cleanup_exit(int code) } static bool winpgnt_listener_ask_passphrase( - PageantListenerClient *plc, PageantClientDialogId *dlgid, const char *msg) + PageantListenerClient *plc, PageantClientDialogId *dlgid, + const char *comment) { - return ask_passphrase_common(dlgid, msg); + return ask_passphrase_common(dlgid, comment); } struct winpgnt_client {