nhyatt/putty-source
1
0
Fork 0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-09 17:38:00 +00:00
Code Issues Projects Releases Wiki Activity

Move sanitisation of k-i prompts into the SSH code.

Browse Source 
Now, instead of each seat's prompt-handling function doing the
control-char sanitisation of prompt text, the SSH code does it. This
means we can do it differently depending on the prompt.

In particular, prompts _we_ generate (e.g. a genuine request for your
private key's passphrase) are not sanitised; but prompts coming from
the server (in keyboard-interactive mode, or its more restricted SSH-1
analogues, TIS and CryptoCard) are not only sanitised but also
line-length limited and surrounded by uncounterfeitable headers, like
I've just done to the authentication banners.

This should mean that if a malicious server tries to fake the local
passphrase prompt (perhaps because it's somehow already got a copy of
your _encrypted_ private key), you can tell the difference.
This commit is contained in:
Simon Tatham 2019-03-09 15:51:38 +00:00
parent 767a9c6e45
commit e21afff605
6 changed files with 182 additions and 142 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

90
ssh1login.c
View File

@ -57,6 +57,9 @@ struct ssh1_login_state {
RSAKey servkey, hostkey;
bool want_user_input;
StripCtrlChars *tis_scc;
bool tis_scc_initialised;
PacketProtocolLayer ppl;
};
@ -135,6 +138,8 @@ static PktIn *ssh1_login_pop(struct ssh1_login_state *s)
return pq_pop(s->ppl.in_pq);
}
static void ssh1_login_setup_tis_scc(struct ssh1_login_state *s);
static void ssh1_login_process_queue(PacketProtocolLayer *ppl)
{
struct ssh1_login_state *s =
@ -784,6 +789,7 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl)
if (conf_get_bool(s->conf, CONF_try_tis_auth) &&
(s->supported_auths_mask & (1 << SSH1_AUTH_TIS)) &&
!s->tis_auth_refused) {
ssh1_login_setup_tis_scc(s);
s->pwpkt_type = SSH1_CMSG_AUTH_TIS_RESPONSE;
ppl_logevent("Requested TIS authentication");
pkt = ssh_bpp_new_pktout(s->ppl.bpp, SSH1_CMSG_AUTH_TIS);
@ -796,10 +802,7 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl)
s->tis_auth_refused = true;
continue;
} else if (pktin->type == SSH1_SMSG_AUTH_TIS_CHALLENGE) {
ptrlen challenge;
char *instr_suf, *prompt;
challenge = get_string(pktin);
ptrlen challenge = get_string(pktin);
if (get_err(pktin)) {
ssh_proto_error(s->ppl.ssh, "TIS challenge packet was "
"badly formed");
@ -809,21 +812,28 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl)
s->cur_prompt->to_server = true;
s->cur_prompt->from_server = true;
s->cur_prompt->name = dupstr("SSH TIS authentication");
/* Prompt heuristic comes from OpenSSH */
if (!memchr(challenge.ptr, '\n', challenge.len)) {
instr_suf = dupstr("");
prompt = mkstr(challenge);
strbuf *sb = strbuf_new();
put_datapl(sb, PTRLEN_LITERAL("\
-- TIS authentication challenge from server: ---------------------------------\
\r\n"));
if (s->tis_scc) {
stripctrl_retarget(s->tis_scc, BinarySink_UPCAST(sb));
put_datapl(s->tis_scc, challenge);
stripctrl_retarget(s->tis_scc, NULL);
} else {
instr_suf = mkstr(challenge);
prompt = dupstr("Response: ");
put_datapl(sb, challenge);
}
s->cur_prompt->instruction =
dupprintf("Using TIS authentication.%s%s",
(*instr_suf) ? "\n" : "",
instr_suf);
if (!ptrlen_endswith(challenge, PTRLEN_LITERAL("\n"), NULL))
put_datapl(sb, PTRLEN_LITERAL("\r\n"));
put_datapl(sb, PTRLEN_LITERAL("\
-- End of TIS authentication challenge from server: --------------------------\
\r\n"));
s->cur_prompt->instruction = strbuf_to_str(sb);
s->cur_prompt->instr_reqd = true;
add_prompt(s->cur_prompt, prompt, false);
sfree(instr_suf);
add_prompt(s->cur_prompt, dupstr(
"TIS authentication response: "), false);
} else {
ssh_proto_error(s->ppl.ssh, "Received unexpected packet"
" in response to TIS authentication, "
@ -834,6 +844,7 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl)
} else if (conf_get_bool(s->conf, CONF_try_tis_auth) &&
(s->supported_auths_mask & (1 << SSH1_AUTH_CCARD)) &&
!s->ccard_auth_refused) {
ssh1_login_setup_tis_scc(s);
s->pwpkt_type = SSH1_CMSG_AUTH_CCARD_RESPONSE;
ppl_logevent("Requested CryptoCard authentication");
pkt = ssh_bpp_new_pktout(s->ppl.bpp, SSH1_CMSG_AUTH_CCARD);
@ -845,10 +856,7 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl)
s->ccard_auth_refused = true;
continue;
} else if (pktin->type == SSH1_SMSG_AUTH_CCARD_CHALLENGE) {
ptrlen challenge;
char *instr_suf, *prompt;
challenge = get_string(pktin);
ptrlen challenge = get_string(pktin);
if (get_err(pktin)) {
ssh_proto_error(s->ppl.ssh, "CryptoCard challenge packet "
"was badly formed");
@ -858,22 +866,28 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl)
s->cur_prompt->to_server = true;
s->cur_prompt->from_server = true;
s->cur_prompt->name = dupstr("SSH CryptoCard authentication");
s->cur_prompt->name_reqd = false;
/* Prompt heuristic comes from OpenSSH */
if (!memchr(challenge.ptr, '\n', challenge.len)) {
instr_suf = dupstr("");
prompt = mkstr(challenge);
strbuf *sb = strbuf_new();
put_datapl(sb, PTRLEN_LITERAL("\
-- CryptoCard authentication challenge from server: --------------------------\
\r\n"));
if (s->tis_scc) {
stripctrl_retarget(s->tis_scc, BinarySink_UPCAST(sb));
put_datapl(s->tis_scc, challenge);
stripctrl_retarget(s->tis_scc, NULL);
} else {
instr_suf = mkstr(challenge);
prompt = dupstr("Response: ");
put_datapl(sb, challenge);
}
s->cur_prompt->instruction =
dupprintf("Using CryptoCard authentication.%s%s",
(*instr_suf) ? "\n" : "",
instr_suf);
if (!ptrlen_endswith(challenge, PTRLEN_LITERAL("\n"), NULL))
put_datapl(sb, PTRLEN_LITERAL("\r\n"));
put_datapl(sb, PTRLEN_LITERAL("\
-- End of CryptoCard authentication challenge from server: -------------------\
\r\n"));
s->cur_prompt->instruction = strbuf_to_str(sb);
s->cur_prompt->instr_reqd = true;
add_prompt(s->cur_prompt, prompt, false);
sfree(instr_suf);
add_prompt(s->cur_prompt, dupstr(
"CryptoCard authentication response: "), false);
} else {
ssh_proto_error(s->ppl.ssh, "Received unexpected packet"
" in response to TIS authentication, "
@ -1085,6 +1099,16 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl)
crFinishV;
}
static void ssh1_login_setup_tis_scc(struct ssh1_login_state *s)
{
if (s->tis_scc_initialised)
return;
s->tis_scc = seat_stripctrl_new(s->ppl.seat, NULL, SIC_KI_PROMPTS);
if (s->tis_scc)
stripctrl_enable_line_limiting(s->tis_scc);
s->tis_scc_initialised = true;
}
static void ssh1_login_dialog_callback(void *loginv, int ret)
{
struct ssh1_login_state *s = (struct ssh1_login_state *)loginv;

110
ssh2userauth.c
View File

@ -81,6 +81,10 @@ struct ssh2_userauth_state {
StripCtrlChars *banner_scc;
bool banner_scc_initialised;
StripCtrlChars *ki_scc;
bool ki_scc_initialised;
bool ki_printed_header;
PacketProtocolLayer ppl;
};
@ -176,6 +180,8 @@ static void ssh2_userauth_free(PacketProtocolLayer *ppl)
strbuf_free(s->last_methods_string);
if (s->banner_scc)
stripctrl_free(s->banner_scc);
if (s->ki_scc)
stripctrl_free(s->ki_scc);
sfree(s);
}
@ -1174,6 +1180,14 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl)
ppl_logevent("Attempting keyboard-interactive authentication");
if (!s->ki_scc_initialised) {
s->ki_scc = seat_stripctrl_new(
s->ppl.seat, NULL, SIC_KI_PROMPTS);
if (s->ki_scc)
stripctrl_enable_line_limiting(s->ki_scc);
s->ki_scc_initialised = true;
}
crMaybeWaitUntilV((pktin = ssh2_userauth_pop(s)) != NULL);
if (pktin->type != SSH2_MSG_USERAUTH_INFO_REQUEST) {
/* Server is not willing to do keyboard-interactive
@ -1186,12 +1200,15 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl)
continue;
}
s->ki_printed_header = false;
/*
* Loop while the server continues to send INFO_REQUESTs.
*/
while (pktin->type == SSH2_MSG_USERAUTH_INFO_REQUEST) {
ptrlen name, inst;
strbuf *sb;
int i;
/*
@ -1210,52 +1227,78 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl)
*/
s->num_prompts = get_uint32(pktin);
for (i = 0; i < s->num_prompts; i++) {
ptrlen prompt;
bool echo;
static char noprompt[] =
"<server failed to send prompt>: ";
ptrlen prompt = get_string(pktin);
bool echo = get_bool(pktin);
prompt = get_string(pktin);
echo = get_bool(pktin);
sb = strbuf_new();
if (!prompt.len) {
prompt.ptr = noprompt;
prompt.len = lenof(noprompt)-1;
put_datapl(sb, PTRLEN_LITERAL(
"<server failed to send prompt>: "));
} else if (s->ki_scc) {
stripctrl_retarget(
s->ki_scc, BinarySink_UPCAST(sb));
put_datapl(s->ki_scc, prompt);
stripctrl_retarget(s->ki_scc, NULL);
} else {
put_datapl(sb, prompt);
}
add_prompt(s->cur_prompt, mkstr(prompt), echo);
add_prompt(s->cur_prompt, strbuf_to_str(sb), echo);
}
/*
* Make the header strings. This includes the
* 'name' (optional dialog-box title) and
* 'instruction' from the server.
*
* First, display our disambiguating header line
* if this is the first time round the loop -
* _unless_ the server has sent a completely empty
* k-i packet with no prompts _or_ text, which
* apparently some do. In that situation there's
* no need to alert the user that the following
* text is server- supplied, because, well, _what_
* text?
*
* We also only do this if we got a stripctrl,
* because if we didn't, that suggests this is all
* being done via dialog boxes anyway.
*/
if (!s->ki_printed_header && s->ki_scc &&
(s->num_prompts || name.len || inst.len)) {
ssh2_userauth_antispoof_msg(
s, "Keyboard-interactive authentication "
"prompts from server:");
s->ki_printed_header = true;
}
sb = strbuf_new();
if (name.len) {
/* FIXME: better prefix to distinguish from
* local prompts? */
s->cur_prompt->name =
dupprintf("SSH server: %.*s", PTRLEN_PRINTF(name));
stripctrl_retarget(s->ki_scc, BinarySink_UPCAST(sb));
put_datapl(s->ki_scc, name);
stripctrl_retarget(s->ki_scc, NULL);
s->cur_prompt->name_reqd = true;
} else {
s->cur_prompt->name =
dupstr("SSH server authentication");
put_datapl(sb, PTRLEN_LITERAL(
"SSH server authentication"));
s->cur_prompt->name_reqd = false;
}
/* We add a prefix to try to make it clear that a prompt
* has come from the server.
* FIXME: ugly to print "Using..." in prompt _every_
* time round. Can this be done more subtly? */
/* Special case: for reasons best known to themselves,
* some servers send k-i requests with no prompts and
* nothing to display. Keep quiet in this case. */
if (s->num_prompts || name.len || inst.len) {
s->cur_prompt->instruction =
dupprintf("Using keyboard-interactive "
"authentication.%s%.*s",
inst.len ? "\n" : "",
PTRLEN_PRINTF(inst));
s->cur_prompt->name = strbuf_to_str(sb);
sb = strbuf_new();
if (inst.len) {
stripctrl_retarget(s->ki_scc, BinarySink_UPCAST(sb));
put_datapl(s->ki_scc, inst);
stripctrl_retarget(s->ki_scc, NULL);
s->cur_prompt->instr_reqd = true;
} else {
s->cur_prompt->instr_reqd = false;
}
/*
* Display any instructions, and get the user's
* response(s).
* Our prompts_t is fully constructed now. Get the
* user's response(s).
*/
s->userpass_ret = seat_get_userpass_input(
s->ppl.seat, s->cur_prompt, NULL);
@ -1313,6 +1356,13 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl)
}
/*
* Print our trailer line, if we printed a header.
*/
if (s->ki_printed_header)
ssh2_userauth_antispoof_msg(
s, "End of keyboard-interactive prompts from server");
/*
* We should have SUCCESS or FAILURE now.
*/

44
terminal.c
View File

@ -1682,9 +1682,6 @@ Terminal *term_init(Conf *myconf, struct unicode_data *ucsdata, TermWin *win)
term->paste_buffer = NULL;
term->paste_len = 0;
bufchain_init(&term->inbuf);
bufchain_sink_init(&term->inbuf_bs, &term->inbuf);
term->inbuf_scc = stripctrl_new_term(
BinarySink_UPCAST(&term->inbuf_bs), false, 0, term);
bufchain_init(&term->printer_buf);
term->printing = term->only_printing = false;
term->print_job = NULL;
@ -1779,7 +1776,6 @@ void term_free(Terminal *term)
term->beephead = beep->next;
sfree(beep);
}
stripctrl_free(term->inbuf_scc);
bufchain_clear(&term->inbuf);
if(term->print_job)
printer_finish_job(term->print_job);
@ -6837,12 +6833,6 @@ size_t term_data(Terminal *term, bool is_stderr, const void *data, size_t len)
return 0;
}
static void term_data_untrusted(Terminal *term, const void *data, size_t len)
{
put_data(term->inbuf_scc, data, len);
term_added_data(term);
}
void term_provide_logctx(Terminal *term, LogContext *logctx)
{
term->logctx = logctx;
@ -6877,6 +6867,12 @@ struct term_userpass_state {
size_t pos; /* cursor position */
};
/* Tiny wrapper to make it easier to write lots of little strings */
static inline void term_write(Terminal *term, ptrlen data)
{
term_data(term, false, data.ptr, data.len);
}
/*
* Process some terminal data in the course of username/password
* input.
@ -6893,17 +6889,17 @@ int term_get_userpass_input(Terminal *term, prompts_t *p, bufchain *input)
s->done_prompt = false;
/* We only print the `name' caption if we have to... */
if (p->name_reqd && p->name) {
size_t l = strlen(p->name);
term_data_untrusted(term, p->name, l);
if (p->name[l-1] != '\n')
term_data_untrusted(term, "\n", 1);
ptrlen plname = ptrlen_from_asciz(p->name);
term_write(term, plname);
if (!ptrlen_endswith(plname, PTRLEN_LITERAL("\n"), NULL))
term_write(term, PTRLEN_LITERAL("\r\n"));
}
/* ...but we always print any `instruction'. */
if (p->instruction) {
size_t l = strlen(p->instruction);
term_data_untrusted(term, p->instruction, l);
if (p->instruction[l-1] != '\n')
term_data_untrusted(term, "\n", 1);
ptrlen plinst = ptrlen_from_asciz(p->instruction);
term_write(term, plinst);
if (!ptrlen_endswith(plinst, PTRLEN_LITERAL("\n"), NULL))
term_write(term, PTRLEN_LITERAL("\r\n"));
}
/*
* Zero all the results, in case we abort half-way through.
@ -6921,7 +6917,7 @@ int term_get_userpass_input(Terminal *term, prompts_t *p, bufchain *input)
bool finished_prompt = false;
if (!s->done_prompt) {
term_data_untrusted(term, pr->prompt, strlen(pr->prompt));
term_write(term, ptrlen_from_asciz(pr->prompt));
s->done_prompt = true;
s->pos = 0;
}
@ -6937,7 +6933,7 @@ int term_get_userpass_input(Terminal *term, prompts_t *p, bufchain *input)
switch (c) {
case 10:
case 13:
term_data(term, false, "\r\n", 2);
term_write(term, PTRLEN_LITERAL("\r\n"));
prompt_ensure_result_size(pr, s->pos + 1);
pr->result[s->pos] = '\0';
/* go to next prompt, if any */
@ -6949,7 +6945,7 @@ int term_get_userpass_input(Terminal *term, prompts_t *p, bufchain *input)
case 127:
if (s->pos > 0) {
if (pr->echo)
term_data(term, false, "\b \b", 3);
term_write(term, PTRLEN_LITERAL("\b \b"));
s->pos--;
}
break;
@ -6957,14 +6953,14 @@ int term_get_userpass_input(Terminal *term, prompts_t *p, bufchain *input)
case 27:
while (s->pos > 0) {
if (pr->echo)
term_data(term, false, "\b \b", 3);
term_write(term, PTRLEN_LITERAL("\b \b"));
s->pos--;
}
break;
case 3:
case 4:
/* Immediate abort. */
term_data(term, false, "\r\n", 2);
term_write(term, PTRLEN_LITERAL("\r\n"));
sfree(s);
p->data = NULL;
return 0; /* user abort */
@ -6979,7 +6975,7 @@ int term_get_userpass_input(Terminal *term, prompts_t *p, bufchain *input)
prompt_ensure_result_size(pr, s->pos + 1);
pr->result[s->pos++] = c;
if (pr->echo)
term_data(term, false, &c, 1);
term_write(term, make_ptrlen(&c, 1));
}
break;
}

2
terminal.h
View File

@ -100,8 +100,6 @@ struct terminal_tag {
termchar basic_erase_char, erase_char;
bufchain inbuf; /* terminal input buffer */
bufchain_sink inbuf_bs;
StripCtrlChars *inbuf_scc;
pos curs; /* cursor */
pos savecurs; /* saved cursor position */

37
unix/uxcons.c
View File

@ -494,22 +494,9 @@ static void console_close(FILE *outfp, int infd)
fclose(outfp); /* will automatically close infd too */
}
static void console_prompt_text(FILE *outfp, const char *data, size_t len)
static void console_write(FILE *outfp, ptrlen data)
{
bufchain sanitised;
bufchain_sink bs;
bufchain_init(&sanitised);
bufchain_sink_init(&bs, &sanitised);
StripCtrlChars *scc = stripctrl_new(BinarySink_UPCAST(&bs), false, 0);
put_data(scc, data, len);
stripctrl_free(scc);
while (bufchain_size(&sanitised) > 0) {
ptrlen sdata = bufchain_prefix(&sanitised);
fwrite(sdata.ptr, 1, sdata.len, outfp);
bufchain_consume(&sanitised, sdata.len);
}
fwrite(data.ptr, 1, data.len, outfp);
fflush(outfp);
}
@ -538,17 +525,17 @@ int console_get_userpass_input(prompts_t *p)
*/
/* We only print the `name' caption if we have to... */
if (p->name_reqd && p->name) {
size_t l = strlen(p->name);
console_prompt_text(outfp, p->name, l);
if (p->name[l-1] != '\n')
console_prompt_text(outfp, "\n", 1);
ptrlen plname = ptrlen_from_asciz(p->name);
console_write(outfp, plname);
if (!ptrlen_endswith(plname, PTRLEN_LITERAL("\n"), NULL))
console_write(outfp, PTRLEN_LITERAL("\n"));
}
/* ...but we always print any `instruction'. */
if (p->instruction) {
size_t l = strlen(p->instruction);
console_prompt_text(outfp, p->instruction, l);
if (p->instruction[l-1] != '\n')
console_prompt_text(outfp, "\n", 1);
ptrlen plinst = ptrlen_from_asciz(p->instruction);
console_write(outfp, plinst);
if (!ptrlen_endswith(plinst, PTRLEN_LITERAL("\n"), NULL))
console_write(outfp, PTRLEN_LITERAL("\n"));
}
for (curr_prompt = 0; curr_prompt < p->n_prompts; curr_prompt++) {
@ -566,7 +553,7 @@ int console_get_userpass_input(prompts_t *p)
newmode.c_lflag |= ECHO;
tcsetattr(infd, TCSANOW, &newmode);
console_prompt_text(outfp, pr->prompt, strlen(pr->prompt));
console_write(outfp, ptrlen_from_asciz(pr->prompt));
len = 0;
while (1) {
@ -588,7 +575,7 @@ int console_get_userpass_input(prompts_t *p)
tcsetattr(infd, TCSANOW, &oldmode);
if (!pr->echo)
console_prompt_text(outfp, "\n", 1);
console_write(outfp, PTRLEN_LITERAL("\n"));
if (len < 0) {
console_close(outfp, infd);

41
windows/wincons.c
View File

@ -394,23 +394,10 @@ StripCtrlChars *console_stripctrl_new(
return stripctrl_new(bs_out, false, 0);
}
static void console_data_untrusted(HANDLE hout, const char *data, size_t len)
static void console_write(HANDLE hout, ptrlen data)
{
DWORD dummy;
bufchain sanitised;
bufchain_sink bs;
bufchain_init(&sanitised);
bufchain_sink_init(&bs, &sanitised);
StripCtrlChars *scc = stripctrl_new(BinarySink_UPCAST(&bs), false, 0);
put_data(scc, data, len);
stripctrl_free(scc);
while (bufchain_size(&sanitised) > 0) {
ptrlen sdata = bufchain_prefix(&sanitised);
WriteFile(hout, sdata.ptr, sdata.len, &dummy, NULL);
bufchain_consume(&sanitised, sdata.len);
}
WriteFile(hout, data.ptr, data.len, &dummy, NULL);
}
int console_get_userpass_input(prompts_t *p)
@ -459,17 +446,17 @@ int console_get_userpass_input(prompts_t *p)
*/
/* We only print the `name' caption if we have to... */
if (p->name_reqd && p->name) {
size_t l = strlen(p->name);
console_data_untrusted(hout, p->name, l);
if (p->name[l-1] != '\n')
console_data_untrusted(hout, "\n", 1);
ptrlen plname = ptrlen_from_asciz(p->name);
console_write(hout, plname);
if (!ptrlen_endswith(plname, PTRLEN_LITERAL("\n"), NULL))
console_write(hout, PTRLEN_LITERAL("\n"));
}
/* ...but we always print any `instruction'. */
if (p->instruction) {
size_t l = strlen(p->instruction);
console_data_untrusted(hout, p->instruction, l);
if (p->instruction[l-1] != '\n')
console_data_untrusted(hout, "\n", 1);
ptrlen plinst = ptrlen_from_asciz(p->instruction);
console_write(hout, plinst);
if (!ptrlen_endswith(plinst, PTRLEN_LITERAL("\n"), NULL))
console_write(hout, PTRLEN_LITERAL("\n"));
}
for (curr_prompt = 0; curr_prompt < p->n_prompts; curr_prompt++) {
@ -486,7 +473,7 @@ int console_get_userpass_input(prompts_t *p)
newmode |= ENABLE_ECHO_INPUT;
SetConsoleMode(hin, newmode);
console_data_untrusted(hout, pr->prompt, strlen(pr->prompt));
console_write(hout, ptrlen_from_asciz(pr->prompt));
len = 0;
while (1) {
@ -510,10 +497,8 @@ int console_get_userpass_input(prompts_t *p)
SetConsoleMode(hin, savemode);
if (!pr->echo) {
DWORD dummy;
WriteFile(hout, "\r\n", 2, &dummy, NULL);
}
if (!pr->echo)
console_write(hout, PTRLEN_LITERAL("\r\n"));
if (len == (size_t)-1) {
return 0; /* failure due to read error */