diff --git a/aqsync.c b/aqsync.c index e16eadd1..dee048e7 100644 --- a/aqsync.c +++ b/aqsync.c @@ -11,11 +11,11 @@ #include "putty.h" -void agent_query_synchronous(void *in, int inlen, void **out, int *outlen) +void agent_query_synchronous(strbuf *query, void **out, int *outlen) { agent_pending_query *pending; - pending = agent_query(in, inlen, out, outlen, NULL, 0); + pending = agent_query(query, out, outlen, NULL, 0); assert(!pending); } diff --git a/misc.c b/misc.c index d87a01ca..b14f45d2 100644 --- a/misc.c +++ b/misc.c @@ -537,6 +537,19 @@ void strbuf_catf(strbuf *buf_o, const char *fmt, ...) va_end(ap); } +strbuf *strbuf_new_for_agent_query(void) +{ + strbuf *buf = strbuf_new(); + put_uint32(buf, 0); /* reserve space for length field */ + return buf; +} +void strbuf_finalise_agent_query(strbuf *buf_o) +{ + struct strbuf_impl *buf = FROMFIELD(buf_o, struct strbuf_impl, visible); + assert(buf->visible.len >= 5); + PUT_32BIT_MSB_FIRST(buf->visible.u, buf->visible.len - 4); +} + /* * Read an entire line of text from a file. Return a buffer * malloced to be as big as necessary (caller must free). diff --git a/misc.h b/misc.h index 299daa52..271d34f1 100644 --- a/misc.h +++ b/misc.h @@ -47,10 +47,14 @@ struct strbuf { }; strbuf *strbuf_new(void); void strbuf_free(strbuf *buf); +char *strbuf_append(strbuf *buf, size_t len); char *strbuf_to_str(strbuf *buf); /* does free buf, but you must free result */ void strbuf_catf(strbuf *buf, const char *fmt, ...); void strbuf_catfv(strbuf *buf, const char *fmt, va_list ap); +strbuf *strbuf_new_for_agent_query(void); +void strbuf_finalise_agent_query(strbuf *buf); + /* String-to-Unicode converters that auto-allocate the destination and * work around the rather deficient interface of mb_to_wc. * diff --git a/pageant.c b/pageant.c index 671cabab..c88230f8 100644 --- a/pageant.c +++ b/pageant.c @@ -1118,14 +1118,16 @@ void *pageant_get_keylist1(int *length) void *ret; if (!pageant_local) { - unsigned char request[5], *response; + strbuf *request; + unsigned char *response; void *vresponse; int resplen; - request[4] = SSH1_AGENTC_REQUEST_RSA_IDENTITIES; - PUT_32BIT(request, 1); + request = strbuf_new_for_agent_query(); + put_byte(request, SSH1_AGENTC_REQUEST_RSA_IDENTITIES); + agent_query_synchronous(request, &vresponse, &resplen); + strbuf_free(request); - agent_query_synchronous(request, 5, &vresponse, &resplen); response = vresponse; if (resplen < 5 || response[4] != SSH1_AGENT_RSA_IDENTITIES_ANSWER) { sfree(response); @@ -1149,14 +1151,16 @@ void *pageant_get_keylist2(int *length) void *ret; if (!pageant_local) { - unsigned char request[5], *response; + strbuf *request; + unsigned char *response; void *vresponse; int resplen; - request[4] = SSH2_AGENTC_REQUEST_IDENTITIES; - PUT_32BIT(request, 1); + request = strbuf_new_for_agent_query(); + put_byte(request, SSH2_AGENTC_REQUEST_IDENTITIES); + agent_query_synchronous(request, &vresponse, &resplen); + strbuf_free(request); - agent_query_synchronous(request, 5, &vresponse, &resplen); response = vresponse; if (resplen < 5 || response[4] != SSH2_AGENT_IDENTITIES_ANSWER) { sfree(response); @@ -1412,55 +1416,35 @@ int pageant_add_keyfile(Filename *filename, const char *passphrase, if (type == SSH_KEYTYPE_SSH1) { if (!pageant_local) { - unsigned char *request, *response; + strbuf *request; + unsigned char *response; void *vresponse; - int reqlen, clen, resplen; + int resplen; - clen = strlen(rkey->comment); + request = strbuf_new_for_agent_query(); + put_byte(request, SSH1_AGENTC_ADD_RSA_IDENTITY); + put_uint32(request, bignum_bitcount(rkey->modulus)); + put_mp_ssh1(request, rkey->modulus); + put_mp_ssh1(request, rkey->exponent); + put_mp_ssh1(request, rkey->private_exponent); + put_mp_ssh1(request, rkey->iqmp); + put_mp_ssh1(request, rkey->q); + put_mp_ssh1(request, rkey->p); + put_stringz(request, rkey->comment); + agent_query_synchronous(request, &vresponse, &resplen); + strbuf_free(request); - reqlen = 4 + 1 + /* length, message type */ - 4 + /* bit count */ - ssh1_bignum_length(rkey->modulus) + - ssh1_bignum_length(rkey->exponent) + - ssh1_bignum_length(rkey->private_exponent) + - ssh1_bignum_length(rkey->iqmp) + - ssh1_bignum_length(rkey->p) + - ssh1_bignum_length(rkey->q) + 4 + clen /* comment */ - ; - - request = snewn(reqlen, unsigned char); - - request[4] = SSH1_AGENTC_ADD_RSA_IDENTITY; - reqlen = 5; - PUT_32BIT(request + reqlen, bignum_bitcount(rkey->modulus)); - reqlen += 4; - reqlen += ssh1_write_bignum(request + reqlen, rkey->modulus); - reqlen += ssh1_write_bignum(request + reqlen, rkey->exponent); - reqlen += - ssh1_write_bignum(request + reqlen, - rkey->private_exponent); - reqlen += ssh1_write_bignum(request + reqlen, rkey->iqmp); - reqlen += ssh1_write_bignum(request + reqlen, rkey->q); - reqlen += ssh1_write_bignum(request + reqlen, rkey->p); - PUT_32BIT(request + reqlen, clen); - memcpy(request + reqlen + 4, rkey->comment, clen); - reqlen += 4 + clen; - PUT_32BIT(request, reqlen - 4); - - agent_query_synchronous(request, reqlen, &vresponse, &resplen); response = vresponse; if (resplen < 5 || response[4] != SSH_AGENT_SUCCESS) { *retstr = dupstr("The already running Pageant " "refused to add the key."); freersakey(rkey); sfree(rkey); - sfree(request); sfree(response); return PAGEANT_ACTION_FAILURE; } freersakey(rkey); sfree(rkey); - sfree(request); sfree(response); } else { if (!pageant_add_ssh1_key(rkey)) { @@ -1470,48 +1454,27 @@ int pageant_add_keyfile(Filename *filename, const char *passphrase, } } else { if (!pageant_local) { - unsigned char *request, *response; + strbuf *request; + unsigned char *response; void *vresponse; - strbuf *keyblob; - int reqlen, alglen, clen, resplen; - alglen = strlen(skey->alg->name); - clen = strlen(skey->comment); + int resplen; - keyblob = strbuf_new(); - skey->alg->openssh_fmtkey(skey->data, BinarySink_UPCAST(keyblob)); + request = strbuf_new_for_agent_query(); + put_byte(request, SSH2_AGENTC_ADD_IDENTITY); + put_stringz(request, skey->alg->name); + skey->alg->openssh_fmtkey(skey->data, BinarySink_UPCAST(request)); + put_stringz(request, skey->comment); + agent_query_synchronous(request, &vresponse, &resplen); + strbuf_free(request); - reqlen = 4 + 1 + /* length, message type */ - 4 + alglen + /* algorithm name */ - keyblob->len + /* key data */ - 4 + clen /* comment */ - ; - - request = snewn(reqlen, unsigned char); - - request[4] = SSH2_AGENTC_ADD_IDENTITY; - reqlen = 5; - PUT_32BIT(request + reqlen, alglen); - reqlen += 4; - memcpy(request + reqlen, skey->alg->name, alglen); - reqlen += alglen; - memcpy(request + reqlen, keyblob->s, keyblob->len); - reqlen += keyblob->len; - PUT_32BIT(request + reqlen, clen); - memcpy(request + reqlen + 4, skey->comment, clen); - reqlen += clen + 4; - PUT_32BIT(request, reqlen - 4); - - agent_query_synchronous(request, reqlen, &vresponse, &resplen); response = vresponse; if (resplen < 5 || response[4] != SSH_AGENT_SUCCESS) { *retstr = dupstr("The already running Pageant " "refused to add the key."); - sfree(request); sfree(response); return PAGEANT_ACTION_FAILURE; } - sfree(request); sfree(response); } else { if (!pageant_add_ssh2_key(skey)) { @@ -1674,26 +1637,24 @@ int pageant_enum_keys(pageant_key_enum_fn_t callback, void *callback_ctx, int pageant_delete_key(struct pageant_pubkey *key, char **retstr) { - unsigned char *request, *response; - int reqlen, resplen, ret; + strbuf *request; + unsigned char *response; + int resplen, ret; void *vresponse; + request = strbuf_new_for_agent_query(); + if (key->ssh_version == 1) { - reqlen = 5 + key->blob->len; - request = snewn(reqlen, unsigned char); - PUT_32BIT(request, reqlen - 4); - request[4] = SSH1_AGENTC_REMOVE_RSA_IDENTITY; - memcpy(request + 5, key->blob->s, key->blob->len); + put_byte(request, SSH1_AGENTC_REMOVE_RSA_IDENTITY); + put_data(request, key->blob->s, key->blob->len); } else { - reqlen = 9 + key->blob->len; - request = snewn(reqlen, unsigned char); - PUT_32BIT(request, reqlen - 4); - request[4] = SSH2_AGENTC_REMOVE_IDENTITY; - PUT_32BIT(request + 5, key->blob->len); - memcpy(request + 9, key->blob->s, key->blob->len); + put_byte(request, SSH2_AGENTC_REMOVE_IDENTITY); + put_string(request, key->blob->s, key->blob->len); } - agent_query_synchronous(request, reqlen, &vresponse, &resplen); + agent_query_synchronous(request, &vresponse, &resplen); + strbuf_free(request); + response = vresponse; if (resplen < 5 || response[4] != SSH_AGENT_SUCCESS) { *retstr = dupstr("Agent failed to delete key"); @@ -1702,21 +1663,21 @@ int pageant_delete_key(struct pageant_pubkey *key, char **retstr) *retstr = NULL; ret = PAGEANT_ACTION_OK; } - sfree(request); sfree(response); return ret; } int pageant_delete_all_keys(char **retstr) { - unsigned char request[5], *response; - int reqlen, resplen, success; + strbuf *request; + unsigned char *response; + int resplen, success; void *vresponse; - PUT_32BIT(request, 1); - request[4] = SSH2_AGENTC_REMOVE_ALL_IDENTITIES; - reqlen = 5; - agent_query_synchronous(request, reqlen, &vresponse, &resplen); + request = strbuf_new_for_agent_query(); + put_byte(request, SSH2_AGENTC_REMOVE_ALL_IDENTITIES); + agent_query_synchronous(request, &vresponse, &resplen); + strbuf_free(request); response = vresponse; success = (resplen >= 4 && response[4] == SSH_AGENT_SUCCESS); sfree(response); @@ -1725,10 +1686,10 @@ int pageant_delete_all_keys(char **retstr) return PAGEANT_ACTION_FAILURE; } - PUT_32BIT(request, 1); - request[4] = SSH1_AGENTC_REMOVE_ALL_RSA_IDENTITIES; - reqlen = 5; - agent_query_synchronous(request, reqlen, &vresponse, &resplen); + request = strbuf_new_for_agent_query(); + put_byte(request, SSH1_AGENTC_REMOVE_ALL_RSA_IDENTITIES); + agent_query_synchronous(request, &vresponse, &resplen); + strbuf_free(request); response = vresponse; success = (resplen >= 4 && response[4] == SSH_AGENT_SUCCESS); sfree(response); diff --git a/putty.h b/putty.h index 72f0c164..00334f44 100644 --- a/putty.h +++ b/putty.h @@ -1292,10 +1292,10 @@ void crypto_wrapup(); */ typedef struct agent_pending_query agent_pending_query; agent_pending_query *agent_query( - void *in, int inlen, void **out, int *outlen, + strbuf *in, void **out, int *outlen, void (*callback)(void *, void *, int), void *callback_ctx); void agent_cancel_query(agent_pending_query *); -void agent_query_synchronous(void *in, int inlen, void **out, int *outlen); +void agent_query_synchronous(strbuf *in, void **out, int *outlen); int agent_exists(void); /* diff --git a/ssh.c b/ssh.c index 464072e8..2ddbbbc9 100644 --- a/ssh.c +++ b/ssh.c @@ -3938,8 +3938,8 @@ static void ssh_agentf_callback(void *cv, void *reply, int replylen); static void ssh_agentf_try_forward(struct ssh_channel *c) { - unsigned datalen, lengthfield, messagelen; - unsigned char *message; + unsigned datalen, length; + strbuf *message; unsigned char msglen[4]; void *reply; int replylen; @@ -3982,9 +3982,9 @@ static void ssh_agentf_try_forward(struct ssh_channel *c) break; /* not even a length field available yet */ bufchain_fetch(&c->u.a.inbuffer, msglen, 4); - lengthfield = GET_32BIT(msglen); + length = GET_32BIT(msglen); - if (lengthfield > AGENT_MAX_MSGLEN) { + if (length > AGENT_MAX_MSGLEN-4) { /* * If the remote has sent a message that's just _too_ * long, we should reject it in advance of seeing the rest @@ -3998,16 +3998,17 @@ static void ssh_agentf_try_forward(struct ssh_channel *c) return; } - if (lengthfield > datalen - 4) + if (length > datalen - 4) break; /* a whole message is not yet available */ - messagelen = lengthfield + 4; + bufchain_consume(&c->u.a.inbuffer, 4); - message = snewn(messagelen, unsigned char); - bufchain_fetch_consume(&c->u.a.inbuffer, message, messagelen); + message = strbuf_new_for_agent_query(); + bufchain_fetch_consume( + &c->u.a.inbuffer, strbuf_append(message, length), length); c->u.a.pending = agent_query( - message, messagelen, &reply, &replylen, ssh_agentf_callback, c); - sfree(message); + message, &reply, &replylen, ssh_agentf_callback, c); + strbuf_free(message); if (c->u.a.pending) return; /* agent_query promised to reply in due course */ @@ -4172,7 +4173,7 @@ static void do_ssh1_login(void *vctx) int userpass_ret; char c; int pwpkt_type; - unsigned char request[5], *response, *p; + unsigned char *response, *p; int responselen; int keyi, nkeys; int authed; @@ -4545,16 +4546,18 @@ static void do_ssh1_login(void *vctx) * Attempt RSA authentication using Pageant. */ void *r; + strbuf *request; s->authed = FALSE; s->tried_agent = 1; logevent("Pageant is running. Requesting keys."); /* Request the keys held by the agent. */ - PUT_32BIT(s->request, 1); - s->request[4] = SSH1_AGENTC_REQUEST_RSA_IDENTITIES; + request = strbuf_new_for_agent_query(); + put_byte(request, SSH1_AGENTC_REQUEST_RSA_IDENTITIES); ssh->auth_agent_query = agent_query( - s->request, 5, &r, &s->responselen, ssh_agent_callback, ssh); + request, &r, &s->responselen, ssh_agent_callback, ssh); + strbuf_free(request); if (ssh->auth_agent_query) { ssh->agent_response = NULL; crWaitUntilV(ssh->agent_response); @@ -4634,38 +4637,31 @@ static void do_ssh1_login(void *vctx) } { - char *agentreq, *q, *ret; + strbuf *agentreq; + char *ret; void *vret; - int len, retlen; - len = 1 + 4; /* message type, bit count */ - len += ssh1_bignum_length(s->key.exponent); - len += ssh1_bignum_length(s->key.modulus); - len += ssh1_bignum_length(s->challenge); - len += 16; /* session id */ - len += 4; /* response format */ - agentreq = snewn(4 + len, char); - PUT_32BIT(agentreq, len); - q = agentreq + 4; - *q++ = SSH1_AGENTC_RSA_CHALLENGE; - PUT_32BIT(q, bignum_bitcount(s->key.modulus)); - q += 4; - q += ssh1_write_bignum(q, s->key.exponent); - q += ssh1_write_bignum(q, s->key.modulus); - q += ssh1_write_bignum(q, s->challenge); - memcpy(q, s->session_id, 16); - q += 16; - PUT_32BIT(q, 1); /* response format */ + int retlen; + + agentreq = strbuf_new_for_agent_query(); + put_byte(agentreq, SSH1_AGENTC_RSA_CHALLENGE); + put_uint32(agentreq, bignum_bitcount(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 */ ssh->auth_agent_query = agent_query( - agentreq, len + 4, &vret, &retlen, + agentreq, &vret, &retlen, ssh_agent_callback, ssh); + strbuf_free(agentreq); + if (ssh->auth_agent_query) { - sfree(agentreq); ssh->agent_response = NULL; crWaitUntilV(ssh->agent_response); vret = ssh->agent_response; retlen = ssh->agent_response_len; - } else - sfree(agentreq); + } + ret = vret; if (ret) { if (ret[4] == SSH1_AGENT_RSA_RESPONSE) { @@ -9921,14 +9917,14 @@ static void do_ssh2_userauth(void *vctx) int privatekey_available, privatekey_encrypted; char *publickey_algorithm; char *publickey_comment; - unsigned char agent_request[5], *agent_response, *agentp; + unsigned char *agent_response, *agentp; int agent_responselen; unsigned char *pkblob_in_agent; int keyi, nkeys; char *pkblob, *alg, *commentp; int pklen, alglen, commentlen; - int siglen, retlen, len; - char *q, *agentreq, *ret; + int retlen, len; + char *ret; struct Packet *pktout; Filename *keyfile; #ifndef NO_GSSAPI @@ -10061,15 +10057,18 @@ static void do_ssh2_userauth(void *vctx) if (conf_get_int(ssh->conf, CONF_tryagent) && agent_exists()) { void *r; + strbuf *agent_request; logevent("Pageant is running. Requesting keys."); /* Request the keys held by the agent. */ - PUT_32BIT(s->agent_request, 1); - s->agent_request[4] = SSH2_AGENTC_REQUEST_IDENTITIES; + agent_request = strbuf_new_for_agent_query(); + put_byte(agent_request, SSH2_AGENTC_REQUEST_IDENTITIES); ssh->auth_agent_query = agent_query( - s->agent_request, 5, &r, &s->agent_responselen, + agent_request, &r, &s->agent_responselen, ssh_agent_callback, ssh); + strbuf_free(agent_request); + if (ssh->auth_agent_query) { ssh->agent_response = NULL; crWaitUntilV(ssh->agent_response); @@ -10468,7 +10467,7 @@ static void do_ssh2_userauth(void *vctx) pq_push_front(&ssh->pq_ssh2_userauth, pktin); } else { - + strbuf *agentreq, *sigdata; void *vret; if (flags & FLAG_VERBOSE) { @@ -10493,40 +10492,28 @@ static void do_ssh2_userauth(void *vctx) put_string(s->pktout, s->pkblob, s->pklen); /* Ask agent for signature. */ - s->siglen = s->pktout->length - 5 + 4 + - ssh->v2_session_id_len; - if (ssh->remote_bugs & BUG_SSH2_PK_SESSIONID) - s->siglen -= 4; - s->len = 1; /* message type */ - s->len += 4 + s->pklen; /* key blob */ - s->len += 4 + s->siglen; /* data to sign */ - s->len += 4; /* flags */ - s->agentreq = snewn(4 + s->len, char); - PUT_32BIT(s->agentreq, s->len); - s->q = s->agentreq + 4; - *s->q++ = SSH2_AGENTC_SIGN_REQUEST; - PUT_32BIT(s->q, s->pklen); - s->q += 4; - memcpy(s->q, s->pkblob, s->pklen); - s->q += s->pklen; - PUT_32BIT(s->q, s->siglen); - s->q += 4; + agentreq = strbuf_new_for_agent_query(); + put_byte(agentreq, SSH2_AGENTC_SIGN_REQUEST); + put_string(agentreq, s->pkblob, s->pklen); /* Now the data to be signed... */ - if (!(ssh->remote_bugs & BUG_SSH2_PK_SESSIONID)) { - PUT_32BIT(s->q, ssh->v2_session_id_len); - s->q += 4; - } - memcpy(s->q, ssh->v2_session_id, - ssh->v2_session_id_len); - s->q += ssh->v2_session_id_len; - memcpy(s->q, s->pktout->data + 5, - s->pktout->length - 5); - s->q += s->pktout->length - 5; + sigdata = strbuf_new(); + if (ssh->remote_bugs & BUG_SSH2_PK_SESSIONID) { + put_data(sigdata, ssh->v2_session_id, + ssh->v2_session_id_len); + } else { + put_string(sigdata, ssh->v2_session_id, + ssh->v2_session_id_len); + } + put_data(sigdata, s->pktout->data + 5, + s->pktout->length - 5); + put_stringsb(agentreq, sigdata); /* And finally the (zero) flags word. */ - PUT_32BIT(s->q, 0); + put_uint32(agentreq, 0); ssh->auth_agent_query = agent_query( - s->agentreq, s->len + 4, &vret, &s->retlen, + agentreq, &vret, &s->retlen, ssh_agent_callback, ssh); + strbuf_free(agentreq); + if (ssh->auth_agent_query) { ssh->agent_response = NULL; crWaitUntilV(ssh->agent_response); @@ -10534,7 +10521,6 @@ static void do_ssh2_userauth(void *vctx) s->retlen = ssh->agent_response_len; } s->ret = vret; - sfree(s->agentreq); if (s->ret) { if (s->retlen >= 9 && s->ret[4] == SSH2_AGENT_SIGN_RESPONSE && diff --git a/unix/uxagentc.c b/unix/uxagentc.c index 2c748343..1e804a73 100644 --- a/unix/uxagentc.c +++ b/unix/uxagentc.c @@ -126,7 +126,7 @@ static void agent_select_result(int fd, int event) } agent_pending_query *agent_query( - void *in, int inlen, void **out, int *outlen, + strbuf *query, void **out, int *outlen, void (*callback)(void *, void *, int), void *callback_ctx) { char *name; @@ -154,8 +154,11 @@ agent_pending_query *agent_query( goto failure; } - for (done = 0; done < inlen ;) { - int ret = write(sock, (char *)in + done, inlen - done); + strbuf_finalise_agent_query(query); + + for (done = 0; done < query->len ;) { + int ret = write(sock, query->s + done, + query->len - done); if (ret <= 0) { close(sock); goto failure; diff --git a/windows/winpgntc.c b/windows/winpgntc.c index cd00f6a3..c6607715 100644 --- a/windows/winpgntc.c +++ b/windows/winpgntc.c @@ -31,7 +31,7 @@ void agent_cancel_query(agent_pending_query *q) } agent_pending_query *agent_query( - void *in, int inlen, void **out, int *outlen, + strbuf *query, void **out, int *outlen, void (*callback)(void *, void *, int), void *callback_ctx) { HWND hwnd; @@ -47,6 +47,9 @@ agent_pending_query *agent_query( *out = NULL; *outlen = 0; + if (query->len > AGENT_MAX_MSGLEN) + return NULL; /* query too large */ + hwnd = FindWindow("Pageant", "Pageant"); if (!hwnd) return NULL; /* *out == NULL, so failure */ @@ -93,7 +96,8 @@ agent_pending_query *agent_query( return NULL; /* *out == NULL, so failure */ } p = MapViewOfFile(filemap, FILE_MAP_WRITE, 0, 0, 0); - memcpy(p, in, inlen); + strbuf_finalise_agent_query(query); + memcpy(p, query->s, query->len); cds.dwData = AGENT_COPYDATA_ID; cds.cbData = 1 + strlen(mapname); cds.lpData = mapname;