1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-09 17:38:00 +00:00

Rework parsing of agent key list.

Now, in both SSH-1 and SSH-2, we go through the whole response from
the SSH agent, parse out the public blob and comment of every key, and
stash them in a data structure to iterate through later.

Previously, we were iterating through the agent response _in situ_,
while it was still stored in the s->agent_response memory buffer in
the form the agent sent it, and had the ongoing s->asrc BinarySource
pointing at it. This led to a remotely triggerable stale-pointer bug:
as soon as we send a _second_ agent request trying to authenticate
with one of the keys, it causes s->agent_response to be freed. In
normal usage this doesn't happen, because if a server sends PK_OK (or
an RSA1 challenge) then it's going to accept our response, so we never
go back to iterating over the rest of the agent's key list. But if a
server sends PK_OK or an RSA1 challenge and _then_ rejects
authentication after we go to the effort of responding, we'll go back
to iterating over the agent's key list and cause a crash.

So now, we extract everything we need from the key-list agent
response, and by the time we're making further agent requests, we
don't need it any more.
This commit is contained in:
Simon Tatham 2020-02-09 08:24:28 +00:00
parent 4510a622ea
commit 45287b627d
2 changed files with 260 additions and 183 deletions

View File

@ -12,6 +12,12 @@
#include "sshppl.h"
#include "sshcr.h"
typedef struct agent_key {
RSAKey key;
strbuf *comment;
ptrlen blob; /* only used during initial parsing of agent response */
} agent_key;
struct ssh1_login_state {
int crState;
@ -47,11 +53,11 @@ struct ssh1_login_state {
void *agent_response_to_free;
ptrlen agent_response;
BinarySource asrc[1]; /* response from SSH agent */
int keyi, nkeys;
size_t agent_keys_len;
agent_key *agent_keys;
size_t agent_key_index, agent_key_limit;
bool authed;
RSAKey key;
mp_int *challenge;
strbuf *agent_comment;
int dlgret;
Filename *keyfile;
RSAKey servkey, hostkey;
@ -99,7 +105,6 @@ PacketProtocolLayer *ssh1_login_new(
s->savedhost = dupstr(host);
s->savedport = port;
s->successor_layer = successor_layer;
s->agent_comment = strbuf_new();
return &s->ppl;
}
@ -118,9 +123,15 @@ static void ssh1_login_free(PacketProtocolLayer *ppl)
if (s->publickey_blob)
strbuf_free(s->publickey_blob);
sfree(s->publickey_comment);
strbuf_free(s->agent_comment);
if (s->cur_prompt)
free_prompts(s->cur_prompt);
if (s->agent_keys) {
for (size_t i = 0; i < s->agent_keys_len; i++) {
freersakey(&s->agent_keys[i].key);
strbuf_free(s->agent_keys[i].comment);
}
sfree(s->agent_keys);
}
sfree(s->agent_response_to_free);
if (s->auth_agent_query)
agent_cancel_query(s->auth_agent_query);
@ -504,122 +515,165 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl)
get_uint32(s->asrc); /* skip length field */
if (get_byte(s->asrc) == SSH1_AGENT_RSA_IDENTITIES_ANSWER) {
s->nkeys = toint(get_uint32(s->asrc));
if (s->nkeys < 0) {
ppl_logevent("Pageant reported negative key count %d",
s->nkeys);
s->nkeys = 0;
}
ppl_logevent("Pageant has %d SSH-1 keys", s->nkeys);
for (s->keyi = 0; s->keyi < s->nkeys; s->keyi++) {
size_t start, end;
start = s->asrc->pos;
get_rsa_ssh1_pub(s->asrc, &s->key,
RSA_SSH1_EXPONENT_FIRST);
end = s->asrc->pos;
strbuf_clear(s->agent_comment);
put_datapl(s->agent_comment, get_string(s->asrc));
if (get_err(s->asrc)) {
ppl_logevent("Pageant key list packet was truncated");
break;
}
if (s->publickey_blob) {
ptrlen keystr = make_ptrlen(
(const char *)s->asrc->data + start, end - start);
size_t nkeys = get_uint32(s->asrc);
size_t origpos = s->asrc->pos;
if (keystr.len == s->publickey_blob->len &&
!memcmp(keystr.ptr, s->publickey_blob->s,
s->publickey_blob->len)) {
ppl_logevent("Pageant key #%d matches "
"configured key file", s->keyi);
s->tried_publickey = true;
} else
/* Skip non-configured key */
continue;
/*
* Check that the agent response is well formed.
*/
for (size_t i = 0; i < nkeys; i++) {
get_rsa_ssh1_pub(s->asrc, NULL, RSA_SSH1_EXPONENT_FIRST);
get_string(s->asrc); /* comment */
if (get_err(s->asrc)) {
ppl_logevent("Pageant's response was truncated");
goto parsed_agent_query;
}
ppl_logevent("Trying Pageant key #%d", s->keyi);
pkt = ssh_bpp_new_pktout(s->ppl.bpp, SSH1_CMSG_AUTH_RSA);
put_mp_ssh1(pkt, s->key.modulus);
pq_push(s->ppl.out_pq, pkt);
crMaybeWaitUntilV((pktin = ssh1_login_pop(s))
!= NULL);
if (pktin->type != SSH1_SMSG_AUTH_RSA_CHALLENGE) {
ppl_logevent("Key refused");
continue;
}
/*
* Copy the list of public-key blobs out of the Pageant
* response.
*/
BinarySource_REWIND_TO(s->asrc, origpos);
s->agent_keys_len = nkeys;
s->agent_keys = snewn(s->agent_keys_len, agent_key);
for (size_t i = 0; i < nkeys; i++) {
memset(&s->agent_keys[i].key, 0,
sizeof(s->agent_keys[i].key));
const char *blobstart = get_ptr(s->asrc);
get_rsa_ssh1_pub(s->asrc, &s->agent_keys[i].key,
RSA_SSH1_EXPONENT_FIRST);
const char *blobend = get_ptr(s->asrc);
s->agent_keys[i].comment = strbuf_new();
put_datapl(s->agent_keys[i].comment, get_string(s->asrc));
s->agent_keys[i].blob = make_ptrlen(
blobstart, blobend - blobstart);
}
ppl_logevent("Pageant has %"SIZEu" SSH-1 keys", nkeys);
if (s->publickey_blob) {
/*
* If we've been given a specific public key blob,
* filter the list of keys to try from the agent
* down to only that one, or none if it's not
* there.
*/
ptrlen our_blob = ptrlen_from_strbuf(s->publickey_blob);
size_t i;
for (i = 0; i < nkeys; i++) {
if (ptrlen_eq_ptrlen(our_blob, s->agent_keys[i].blob))
break;
}
ppl_logevent("Received RSA challenge");
s->challenge = get_mp_ssh1(pktin);
if (i < nkeys) {
ppl_logevent("Pageant key #%"SIZEu" matches "
"configured key file", i);
s->agent_key_index = i;
s->agent_key_limit = i+1;
} else {
ppl_logevent("Configured key file not in Pageant");
s->agent_key_index = 0;
s->agent_key_limit = 0;
}
} else {
/*
* Otherwise, try them all.
*/
s->agent_key_index = 0;
s->agent_key_limit = nkeys;
}
} else {
ppl_logevent("Failed to get reply from Pageant");
}
parsed_agent_query:;
for (; s->agent_key_index < s->agent_key_limit;
s->agent_key_index++) {
ppl_logevent("Trying Pageant key #%"SIZEu, s->agent_key_index);
pkt = ssh_bpp_new_pktout(s->ppl.bpp, SSH1_CMSG_AUTH_RSA);
put_mp_ssh1(pkt,
s->agent_keys[s->agent_key_index].key.modulus);
pq_push(s->ppl.out_pq, pkt);
crMaybeWaitUntilV((pktin = ssh1_login_pop(s))
!= NULL);
if (pktin->type != SSH1_SMSG_AUTH_RSA_CHALLENGE) {
ppl_logevent("Key refused");
continue;
}
ppl_logevent("Received RSA challenge");
{
mp_int *challenge = get_mp_ssh1(pktin);
if (get_err(pktin)) {
mp_free(s->challenge);
mp_free(challenge);
ssh_proto_error(s->ppl.ssh, "Server's RSA challenge "
"was badly formatted");
return;
}
{
strbuf *agentreq;
const char *ret;
strbuf *agentreq = strbuf_new_for_agent_query();
put_byte(agentreq, SSH1_AGENTC_RSA_CHALLENGE);
agentreq = strbuf_new_for_agent_query();
put_byte(agentreq, SSH1_AGENTC_RSA_CHALLENGE);
put_uint32(agentreq, mp_get_nbits(s->key.modulus));
put_mp_ssh1(agentreq, s->key.exponent);
put_mp_ssh1(agentreq, s->key.modulus);
put_mp_ssh1(agentreq, s->challenge);
put_data(agentreq, s->session_id, 16);
put_uint32(agentreq, 1); /* response format */
ssh1_login_agent_query(s, agentreq);
strbuf_free(agentreq);
crMaybeWaitUntilV(!s->auth_agent_query);
rsa_ssh1_public_blob(
BinarySink_UPCAST(agentreq),
&s->agent_keys[s->agent_key_index].key,
RSA_SSH1_EXPONENT_FIRST);
ret = s->agent_response.ptr;
if (ret) {
if (s->agent_response.len >= 5+16 &&
ret[4] == SSH1_AGENT_RSA_RESPONSE) {
ppl_logevent("Sending Pageant's response");
pkt = ssh_bpp_new_pktout(
s->ppl.bpp, SSH1_CMSG_AUTH_RSA_RESPONSE);
put_data(pkt, ret + 5, 16);
pq_push(s->ppl.out_pq, pkt);
crMaybeWaitUntilV(
(pktin = ssh1_login_pop(s))
!= NULL);
if (pktin->type == SSH1_SMSG_SUCCESS) {
ppl_logevent("Pageant's response "
"accepted");
if (flags & FLAG_VERBOSE) {
ptrlen comment = ptrlen_from_strbuf(
s->agent_comment);
ppl_printf("Authenticated using RSA "
"key \"%.*s\" from "
"agent\r\n",
PTRLEN_PRINTF(comment));
}
s->authed = true;
} else
ppl_logevent("Pageant's response not "
"accepted");
} else {
ppl_logevent("Pageant failed to answer "
"challenge");
sfree((char *)ret);
}
} else {
ppl_logevent("No reply received from Pageant");
}
}
mp_free(s->key.exponent);
mp_free(s->key.modulus);
mp_free(s->challenge);
if (s->authed)
break;
put_mp_ssh1(agentreq, challenge);
mp_free(challenge);
put_data(agentreq, s->session_id, 16);
put_uint32(agentreq, 1); /* response format */
ssh1_login_agent_query(s, agentreq);
strbuf_free(agentreq);
crMaybeWaitUntilV(!s->auth_agent_query);
}
sfree(s->agent_response_to_free);
s->agent_response_to_free = NULL;
if (s->publickey_blob && !s->tried_publickey)
ppl_logevent("Configured key file not in Pageant");
} else {
ppl_logevent("Failed to get reply from Pageant");
{
const unsigned char *ret = s->agent_response.ptr;
if (ret) {
if (s->agent_response.len >= 5+16 &&
ret[4] == SSH1_AGENT_RSA_RESPONSE) {
ppl_logevent("Sending Pageant's response");
pkt = ssh_bpp_new_pktout(
s->ppl.bpp, SSH1_CMSG_AUTH_RSA_RESPONSE);
put_data(pkt, ret + 5, 16);
pq_push(s->ppl.out_pq, pkt);
crMaybeWaitUntilV(
(pktin = ssh1_login_pop(s))
!= NULL);
if (pktin->type == SSH1_SMSG_SUCCESS) {
ppl_logevent("Pageant's response "
"accepted");
if (flags & FLAG_VERBOSE) {
ptrlen comment = ptrlen_from_strbuf(
s->agent_keys[s->agent_key_index].
comment);
ppl_printf("Authenticated using RSA "
"key \"%.*s\" from "
"agent\r\n",
PTRLEN_PRINTF(comment));
}
s->authed = true;
} else
ppl_logevent("Pageant's response not "
"accepted");
} else {
ppl_logevent("Pageant failed to answer "
"challenge");
sfree((char *)ret);
}
} else {
ppl_logevent("No reply received from Pageant");
}
}
if (s->authed)
break;
}
if (s->authed)
break;

View File

@ -18,6 +18,11 @@
#define BANNER_LIMIT 131072
typedef struct agent_key {
strbuf *blob, *comment;
ptrlen algorithm;
} agent_key;
struct ssh2_userauth_state {
int crState;
@ -69,9 +74,9 @@ struct ssh2_userauth_state {
void *agent_response_to_free;
ptrlen agent_response;
BinarySource asrc[1]; /* for reading SSH agent response */
size_t pkblob_pos_in_agent;
int keyi, nkeys;
ptrlen pk, alg, comment;
size_t agent_keys_len;
agent_key *agent_keys;
size_t agent_key_index, agent_key_limit;
int len;
PktOut *pktout;
bool want_user_input;
@ -173,6 +178,13 @@ static void ssh2_userauth_free(PacketProtocolLayer *ppl)
if (s->successor_layer)
ssh_ppl_free(s->successor_layer);
if (s->agent_keys) {
for (size_t i = 0; i < s->agent_keys_len; i++) {
strbuf_free(s->agent_keys[i].blob);
strbuf_free(s->agent_keys[i].comment);
}
sfree(s->agent_keys);
}
sfree(s->agent_response_to_free);
if (s->auth_agent_query)
agent_cancel_query(s->auth_agent_query);
@ -300,8 +312,6 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl)
* Find out about any keys Pageant has (but if there's a public
* key configured, filter out all others).
*/
s->nkeys = 0;
s->pkblob_pos_in_agent = 0;
if (s->tryagent && agent_exists()) {
ppl_logevent("Pageant is running. Requesting keys.");
@ -317,48 +327,75 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl)
get_uint32(s->asrc); /* skip length field */
if (get_byte(s->asrc) == SSH2_AGENT_IDENTITIES_ANSWER) {
int keyi;
s->nkeys = toint(get_uint32(s->asrc));
size_t nkeys = get_uint32(s->asrc);
size_t origpos = s->asrc->pos;
/*
* Vet the Pageant response to ensure that the key count
* and blob lengths make sense.
* Check that the agent response is well formed.
*/
if (s->nkeys < 0) {
ppl_logevent("Pageant response contained a negative"
" key count %d", s->nkeys);
s->nkeys = 0;
goto done_agent_query;
} else {
ppl_logevent("Pageant has %d SSH-2 keys", s->nkeys);
for (size_t i = 0; i < nkeys; i++) {
get_string(s->asrc); /* blob */
get_string(s->asrc); /* comment */
if (get_err(s->asrc)) {
ppl_logevent("Pageant's response was truncated");
goto done_agent_query;
}
}
/* See if configured key is in agent. */
for (keyi = 0; keyi < s->nkeys; keyi++) {
size_t pos = s->asrc->pos;
ptrlen blob = get_string(s->asrc);
get_string(s->asrc); /* skip comment */
if (get_err(s->asrc)) {
ppl_logevent("Pageant response was truncated");
s->nkeys = 0;
goto done_agent_query;
}
/*
* Copy the list of public-key blobs out of the Pageant
* response.
*/
BinarySource_REWIND_TO(s->asrc, origpos);
s->agent_keys_len = nkeys;
s->agent_keys = snewn(s->agent_keys_len, agent_key);
for (size_t i = 0; i < nkeys; i++) {
s->agent_keys[i].blob = strbuf_new();
put_datapl(s->agent_keys[i].blob, get_string(s->asrc));
s->agent_keys[i].comment = strbuf_new();
put_datapl(s->agent_keys[i].comment, get_string(s->asrc));
if (s->publickey_blob &&
blob.len == s->publickey_blob->len &&
!memcmp(blob.ptr, s->publickey_blob->s,
s->publickey_blob->len)) {
ppl_logevent("Pageant key #%d matches "
"configured key file", keyi);
s->keyi = keyi;
s->pkblob_pos_in_agent = pos;
/* Also, extract the algorithm string from the start
* of the public-key blob. */
BinarySource src[1];
BinarySource_BARE_INIT_PL(src, ptrlen_from_strbuf(
s->agent_keys[i].blob));
s->agent_keys[i].algorithm = get_string(src);
}
ppl_logevent("Pageant has %"SIZEu" SSH-2 keys", nkeys);
if (s->publickey_blob) {
/*
* If we've been given a specific public key blob,
* filter the list of keys to try from the agent down
* to only that one, or none if it's not there.
*/
ptrlen our_blob = ptrlen_from_strbuf(s->publickey_blob);
size_t i;
for (i = 0; i < nkeys; i++) {
if (ptrlen_eq_ptrlen(our_blob, ptrlen_from_strbuf(
s->agent_keys[i].blob)))
break;
}
}
if (s->publickey_blob && !s->pkblob_pos_in_agent) {
if (i < nkeys) {
ppl_logevent("Pageant key #%"SIZEu" matches "
"configured key file", i);
s->agent_key_index = i;
s->agent_key_limit = i+1;
} else {
ppl_logevent("Configured key file not in Pageant");
s->nkeys = 0;
s->agent_key_index = 0;
s->agent_key_limit = 0;
}
} else {
/*
* Otherwise, try them all.
*/
s->agent_key_index = 0;
s->agent_key_limit = nkeys;
}
} else {
ppl_logevent("Failed to get reply from Pageant");
@ -457,17 +494,7 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl)
s->tried_pubkey_config = false;
s->kbd_inter_refused = false;
/* Reset agent request state. */
s->done_agent = false;
if (s->agent_response.ptr) {
if (s->pkblob_pos_in_agent) {
s->asrc->pos = s->pkblob_pos_in_agent;
} else {
s->asrc->pos = 9; /* skip length + type + key count */
s->keyi = 0;
}
}
while (1) {
/*
@ -688,7 +715,8 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl)
} else
#endif /* NO_GSSAPI */
if (s->can_pubkey && !s->done_agent && s->nkeys) {
if (s->can_pubkey && !s->done_agent &&
s->agent_key_index < s->agent_key_limit) {
/*
* Attempt public-key authentication using a key from Pageant.
@ -696,16 +724,7 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl)
s->ppl.bpp->pls->actx = SSH2_PKTCTX_PUBLICKEY;
ppl_logevent("Trying Pageant key #%d", s->keyi);
/* Unpack key from agent response */
s->pk = get_string(s->asrc);
s->comment = get_string(s->asrc);
{
BinarySource src[1];
BinarySource_BARE_INIT_PL(src, s->pk);
s->alg = get_string(src);
}
ppl_logevent("Trying Pageant key #%"SIZEu, s->agent_key_index);
/* See if server will accept it */
s->pktout = ssh_bpp_new_pktout(
@ -715,8 +734,10 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl)
put_stringz(s->pktout, "publickey");
/* method */
put_bool(s->pktout, false); /* no signature included */
put_stringpl(s->pktout, s->alg);
put_stringpl(s->pktout, s->pk);
put_stringpl(s->pktout,
s->agent_keys[s->agent_key_index].algorithm);
put_stringpl(s->pktout, ptrlen_from_strbuf(
s->agent_keys[s->agent_key_index].blob));
pq_push(s->ppl.out_pq, s->pktout);
s->type = AUTH_TYPE_PUBLICKEY_OFFER_QUIET;
@ -729,11 +750,13 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl)
} else {
strbuf *agentreq, *sigdata;
ptrlen comment = ptrlen_from_strbuf(
s->agent_keys[s->agent_key_index].comment);
if (flags & FLAG_VERBOSE)
ppl_printf("Authenticating with public key "
"\"%.*s\" from agent\r\n",
PTRLEN_PRINTF(s->comment));
PTRLEN_PRINTF(comment));
/*
* Server is willing to accept the key.
@ -746,13 +769,16 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl)
put_stringz(s->pktout, "publickey");
/* method */
put_bool(s->pktout, true); /* signature included */
put_stringpl(s->pktout, s->alg);
put_stringpl(s->pktout, s->pk);
put_stringpl(s->pktout,
s->agent_keys[s->agent_key_index].algorithm);
put_stringpl(s->pktout, ptrlen_from_strbuf(
s->agent_keys[s->agent_key_index].blob));
/* Ask agent for signature. */
agentreq = strbuf_new_for_agent_query();
put_byte(agentreq, SSH2_AGENTC_SIGN_REQUEST);
put_stringpl(agentreq, s->pk);
put_stringpl(agentreq, ptrlen_from_strbuf(
s->agent_keys[s->agent_key_index].blob));
/* Now the data to be signed... */
sigdata = strbuf_new();
ssh2_userauth_add_session_id(s, sigdata);
@ -774,8 +800,11 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl)
if (get_byte(src) == SSH2_AGENT_SIGN_RESPONSE &&
(sigblob = get_string(src), !get_err(src))) {
ppl_logevent("Sending Pageant's response");
ssh2_userauth_add_sigblob(s, s->pktout,
s->pk, sigblob);
ssh2_userauth_add_sigblob(
s, s->pktout,
ptrlen_from_strbuf(
s->agent_keys[s->agent_key_index].blob),
sigblob);
pq_push(s->ppl.out_pq, s->pktout);
s->type = AUTH_TYPE_PUBLICKEY;
} else {
@ -796,14 +825,8 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl)
}
/* Do we have any keys left to try? */
if (s->pkblob_pos_in_agent) {
if (++s->agent_key_index >= s->agent_key_limit)
s->done_agent = true;
s->tried_pubkey_config = true;
} else {
s->keyi++;
if (s->keyi >= s->nkeys)
s->done_agent = true;
}
} else if (s->can_pubkey && s->publickey_blob &&
s->privatekey_available && !s->tried_pubkey_config) {