From bb956eaa94757ad058ff43631c3a7d6c94d38c2f Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Wed, 23 Jan 2019 00:30:41 +0000 Subject: [PATCH] upstream: pass most arguments to the KEX hash functions as sshbuf rather than pointer+length; ok markus@ OpenBSD-Commit-ID: ef0c89c52ccc89817a13a5205725148a28492bf7 --- kex.h | 4 ++-- kexgen.c | 67 +++++++++++++++++++++++++++++++------------------------ kexgex.c | 18 +++++++-------- kexgexc.c | 29 ++++++++++++++---------- kexgexs.c | 25 ++++++++++++--------- 5 files changed, 80 insertions(+), 63 deletions(-) diff --git a/kex.h b/kex.h index 44e6d1972..6d446d1cc 100644 --- a/kex.h +++ b/kex.h @@ -1,4 +1,4 @@ -/* $OpenBSD: kex.h,v 1.106 2019/01/21 10:40:11 djm Exp $ */ +/* $OpenBSD: kex.h,v 1.107 2019/01/23 00:30:41 djm Exp $ */ /* * Copyright (c) 2000, 2001 Markus Friedl. All rights reserved. @@ -229,7 +229,7 @@ int kex_dh_keygen(struct kex *); int kex_dh_compute_key(struct kex *, BIGNUM *, struct sshbuf *); int kexgex_hash(int, const struct sshbuf *, const struct sshbuf *, - const u_char *, size_t, const u_char *, size_t, const u_char *, size_t, + const struct sshbuf *, const struct sshbuf *, const struct sshbuf *, int, int, int, const BIGNUM *, const BIGNUM *, const BIGNUM *, const BIGNUM *, const u_char *, size_t, diff --git a/kexgen.c b/kexgen.c index 91c960fe6..bb0cb5c1b 100644 --- a/kexgen.c +++ b/kexgen.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kexgen.c,v 1.1 2019/01/21 11:22:00 djm Exp $ */ +/* $OpenBSD: kexgen.c,v 1.2 2019/01/23 00:30:41 djm Exp $ */ /* * Copyright (c) 2019 Markus Friedl. All rights reserved. * @@ -48,9 +48,9 @@ kex_gen_hash( int hash_alg, const struct sshbuf *client_version, const struct sshbuf *server_version, - const u_char *ckexinit, size_t ckexinitlen, - const u_char *skexinit, size_t skexinitlen, - const u_char *serverhostkeyblob, size_t sbloblen, + const struct sshbuf *client_kexinit, + const struct sshbuf *server_kexinit, + const struct sshbuf *server_host_key_blob, const struct sshbuf *client_pub, const struct sshbuf *server_pub, const struct sshbuf *shared_secret, @@ -66,13 +66,13 @@ kex_gen_hash( if ((r = sshbuf_put_stringb(b, client_version)) != 0 || (r = sshbuf_put_stringb(b, server_version)) != 0 || /* kexinit messages: fake header: len+SSH2_MSG_KEXINIT */ - (r = sshbuf_put_u32(b, ckexinitlen+1)) != 0 || + (r = sshbuf_put_u32(b, sshbuf_len(client_kexinit) + 1)) != 0 || (r = sshbuf_put_u8(b, SSH2_MSG_KEXINIT)) != 0 || - (r = sshbuf_put(b, ckexinit, ckexinitlen)) != 0 || - (r = sshbuf_put_u32(b, skexinitlen+1)) != 0 || + (r = sshbuf_putb(b, client_kexinit)) != 0 || + (r = sshbuf_put_u32(b, sshbuf_len(server_kexinit) + 1)) != 0 || (r = sshbuf_put_u8(b, SSH2_MSG_KEXINIT)) != 0 || - (r = sshbuf_put(b, skexinit, skexinitlen)) != 0 || - (r = sshbuf_put_string(b, serverhostkeyblob, sbloblen)) != 0 || + (r = sshbuf_putb(b, server_kexinit)) != 0 || + (r = sshbuf_put_stringb(b, server_host_key_blob)) != 0 || (r = sshbuf_put_stringb(b, client_pub)) != 0 || (r = sshbuf_put_stringb(b, server_pub)) != 0 || (r = sshbuf_putb(b, shared_secret)) != 0) { @@ -139,16 +139,21 @@ input_kex_gen_reply(int type, u_int32_t seq, struct ssh *ssh) struct sshkey *server_host_key = NULL; struct sshbuf *shared_secret = NULL; struct sshbuf *server_blob = NULL; - u_char *server_host_key_blob = NULL, *signature = NULL; + struct sshbuf *tmp = NULL, *server_host_key_blob = NULL; + u_char *signature = NULL; u_char hash[SSH_DIGEST_MAX_LENGTH]; - size_t slen, sbloblen, hashlen; + size_t slen, hashlen; int r; /* hostkey */ - if ((r = sshpkt_get_string(ssh, &server_host_key_blob, - &sbloblen)) != 0 || - (r = sshkey_from_blob(server_host_key_blob, sbloblen, - &server_host_key)) != 0) + if ((r = sshpkt_getb_froms(ssh, &server_host_key_blob)) != 0) + goto out; + /* sshkey_fromb() consumes its buffer, so make a copy */ + if ((tmp = sshbuf_fromb(server_host_key_blob)) == NULL) { + r = SSH_ERR_ALLOC_FAIL; + goto out; + } + if ((r = sshkey_fromb(tmp, &server_host_key)) != 0) goto out; if ((r = kex_verify_host_key(ssh, server_host_key)) != 0) goto out; @@ -192,9 +197,9 @@ input_kex_gen_reply(int type, u_int32_t seq, struct ssh *ssh) kex->hash_alg, kex->client_version, kex->server_version, - sshbuf_ptr(kex->my), sshbuf_len(kex->my), - sshbuf_ptr(kex->peer), sshbuf_len(kex->peer), - server_host_key_blob, sbloblen, + kex->my, + kex->peer, + server_host_key_blob, kex->client_pub, server_blob, shared_secret, @@ -212,8 +217,9 @@ out: explicit_bzero(kex->c25519_client_key, sizeof(kex->c25519_client_key)); explicit_bzero(kex->sntrup4591761_client_key, sizeof(kex->sntrup4591761_client_key)); - free(server_host_key_blob); + sshbuf_free(server_host_key_blob); free(signature); + sshbuf_free(tmp); sshkey_free(server_host_key); sshbuf_free(server_blob); sshbuf_free(shared_secret); @@ -238,9 +244,9 @@ input_kex_gen_init(int type, u_int32_t seq, struct ssh *ssh) struct sshbuf *shared_secret = NULL; struct sshbuf *server_pubkey = NULL; struct sshbuf *client_pubkey = NULL; - u_char *server_host_key_blob = NULL, *signature = NULL; - u_char hash[SSH_DIGEST_MAX_LENGTH]; - size_t slen, sbloblen, hashlen; + struct sshbuf *server_host_key_blob = NULL; + u_char *signature = NULL, hash[SSH_DIGEST_MAX_LENGTH]; + size_t slen, hashlen; int r; if ((r = kex_load_hostkey(ssh, &server_host_private, @@ -281,17 +287,20 @@ input_kex_gen_init(int type, u_int32_t seq, struct ssh *ssh) goto out; /* calc H */ - if ((r = sshkey_to_blob(server_host_public, &server_host_key_blob, - &sbloblen)) != 0) + if ((server_host_key_blob = sshbuf_new()) == NULL) { + r = SSH_ERR_ALLOC_FAIL; + goto out; + } + if ((r = sshkey_putb(server_host_public, server_host_key_blob)) != 0) goto out; hashlen = sizeof(hash); if ((r = kex_gen_hash( kex->hash_alg, kex->client_version, kex->server_version, - sshbuf_ptr(kex->peer), sshbuf_len(kex->peer), - sshbuf_ptr(kex->my), sshbuf_len(kex->my), - server_host_key_blob, sbloblen, + kex->peer, + kex->my, + server_host_key_blob, client_pubkey, server_pubkey, shared_secret, @@ -305,7 +314,7 @@ input_kex_gen_init(int type, u_int32_t seq, struct ssh *ssh) /* send server hostkey, ECDH pubkey 'Q_S' and signed H */ if ((r = sshpkt_start(ssh, SSH2_MSG_KEX_ECDH_REPLY)) != 0 || - (r = sshpkt_put_string(ssh, server_host_key_blob, sbloblen)) != 0 || + (r = sshpkt_put_stringb(ssh, server_host_key_blob)) != 0 || (r = sshpkt_put_stringb(ssh, server_pubkey)) != 0 || (r = sshpkt_put_string(ssh, signature, slen)) != 0 || (r = sshpkt_send(ssh)) != 0) @@ -315,7 +324,7 @@ input_kex_gen_init(int type, u_int32_t seq, struct ssh *ssh) r = kex_send_newkeys(ssh); out: explicit_bzero(hash, sizeof(hash)); - free(server_host_key_blob); + sshbuf_free(server_host_key_blob); free(signature); sshbuf_free(shared_secret); sshbuf_free(client_pubkey); diff --git a/kexgex.c b/kexgex.c index f828f2b20..8040a1320 100644 --- a/kexgex.c +++ b/kexgex.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kexgex.c,v 1.31 2019/01/21 10:03:37 djm Exp $ */ +/* $OpenBSD: kexgex.c,v 1.32 2019/01/23 00:30:41 djm Exp $ */ /* * Copyright (c) 2000 Niels Provos. All rights reserved. * Copyright (c) 2001 Markus Friedl. All rights reserved. @@ -48,9 +48,9 @@ kexgex_hash( int hash_alg, const struct sshbuf *client_version, const struct sshbuf *server_version, - const u_char *ckexinit, size_t ckexinitlen, - const u_char *skexinit, size_t skexinitlen, - const u_char *serverhostkeyblob, size_t sbloblen, + const struct sshbuf *client_kexinit, + const struct sshbuf *server_kexinit, + const struct sshbuf *server_host_key_blob, int min, int wantbits, int max, const BIGNUM *prime, const BIGNUM *gen, @@ -69,13 +69,13 @@ kexgex_hash( if ((r = sshbuf_put_stringb(b, client_version)) < 0 || (r = sshbuf_put_stringb(b, server_version)) < 0 || /* kexinit messages: fake header: len+SSH2_MSG_KEXINIT */ - (r = sshbuf_put_u32(b, ckexinitlen+1)) != 0 || + (r = sshbuf_put_u32(b, sshbuf_len(client_kexinit) + 1)) != 0 || (r = sshbuf_put_u8(b, SSH2_MSG_KEXINIT)) != 0 || - (r = sshbuf_put(b, ckexinit, ckexinitlen)) != 0 || - (r = sshbuf_put_u32(b, skexinitlen+1)) != 0 || + (r = sshbuf_putb(b, client_kexinit)) != 0 || + (r = sshbuf_put_u32(b, sshbuf_len(server_kexinit) + 1)) != 0 || (r = sshbuf_put_u8(b, SSH2_MSG_KEXINIT)) != 0 || - (r = sshbuf_put(b, skexinit, skexinitlen)) != 0 || - (r = sshbuf_put_string(b, serverhostkeyblob, sbloblen)) != 0 || + (r = sshbuf_putb(b, server_kexinit)) != 0 || + (r = sshbuf_put_stringb(b, server_host_key_blob)) != 0 || (min != -1 && (r = sshbuf_put_u32(b, min)) != 0) || (r = sshbuf_put_u32(b, wantbits)) != 0 || (max != -1 && (r = sshbuf_put_u32(b, max)) != 0) || diff --git a/kexgexc.c b/kexgexc.c index ac42127af..1c65b8a18 100644 --- a/kexgexc.c +++ b/kexgexc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kexgexc.c,v 1.33 2019/01/21 10:07:22 djm Exp $ */ +/* $OpenBSD: kexgexc.c,v 1.34 2019/01/23 00:30:41 djm Exp $ */ /* * Copyright (c) 2000 Niels Provos. All rights reserved. * Copyright (c) 2001 Markus Friedl. All rights reserved. @@ -146,20 +146,24 @@ input_kex_dh_gex_reply(int type, u_int32_t seq, struct ssh *ssh) BIGNUM *dh_server_pub = NULL; const BIGNUM *pub_key, *dh_p, *dh_g; struct sshbuf *shared_secret = NULL; + struct sshbuf *tmp = NULL, *server_host_key_blob = NULL; struct sshkey *server_host_key = NULL; - u_char *signature = NULL, *server_host_key_blob = NULL; + u_char *signature = NULL; u_char hash[SSH_DIGEST_MAX_LENGTH]; - size_t slen, sbloblen, hashlen; + size_t slen, hashlen; int r; debug("got SSH2_MSG_KEX_DH_GEX_REPLY"); /* key, cert */ - if ((r = sshpkt_get_string(ssh, &server_host_key_blob, - &sbloblen)) != 0 || - (r = sshkey_from_blob(server_host_key_blob, sbloblen, - &server_host_key)) != 0) + if ((r = sshpkt_getb_froms(ssh, &server_host_key_blob)) != 0) goto out; - if ((r = kex_verify_host_key(ssh, server_host_key)) != 0) + /* sshkey_fromb() consumes its buffer, so make a copy */ + if ((tmp = sshbuf_fromb(server_host_key_blob)) == NULL) { + r = SSH_ERR_ALLOC_FAIL; + goto out; + } + if ((r = sshkey_fromb(tmp, &server_host_key)) != 0 || + (r = kex_verify_host_key(ssh, server_host_key)) != 0) goto out; /* DH parameter f, server public DH key, signed H */ if ((r = sshpkt_get_bignum2(ssh, &dh_server_pub)) != 0 || @@ -183,9 +187,9 @@ input_kex_dh_gex_reply(int type, u_int32_t seq, struct ssh *ssh) kex->hash_alg, kex->client_version, kex->server_version, - sshbuf_ptr(kex->my), sshbuf_len(kex->my), - sshbuf_ptr(kex->peer), sshbuf_len(kex->peer), - server_host_key_blob, sbloblen, + kex->my, + kex->peer, + server_host_key_blob, kex->min, kex->nbits, kex->max, dh_p, dh_g, pub_key, @@ -207,7 +211,8 @@ input_kex_dh_gex_reply(int type, u_int32_t seq, struct ssh *ssh) BN_clear_free(dh_server_pub); sshbuf_free(shared_secret); sshkey_free(server_host_key); - free(server_host_key_blob); + sshbuf_free(tmp); + sshbuf_free(server_host_key_blob); free(signature); return r; } diff --git a/kexgexs.c b/kexgexs.c index a617d4453..8ee3aaccb 100644 --- a/kexgexs.c +++ b/kexgexs.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kexgexs.c,v 1.41 2019/01/21 10:05:09 djm Exp $ */ +/* $OpenBSD: kexgexs.c,v 1.42 2019/01/23 00:30:41 djm Exp $ */ /* * Copyright (c) 2000 Niels Provos. All rights reserved. * Copyright (c) 2001 Markus Friedl. All rights reserved. @@ -129,11 +129,11 @@ input_kex_dh_gex_init(int type, u_int32_t seq, struct ssh *ssh) BIGNUM *dh_client_pub = NULL; const BIGNUM *pub_key, *dh_p, *dh_g; struct sshbuf *shared_secret = NULL; + struct sshbuf *server_host_key_blob = NULL; struct sshkey *server_host_public, *server_host_private; - u_char *signature = NULL, *server_host_key_blob = NULL; + u_char *signature = NULL; u_char hash[SSH_DIGEST_MAX_LENGTH]; - size_t sbloblen, slen; - size_t hashlen; + size_t slen, hashlen; int r; if ((r = kex_load_hostkey(ssh, &server_host_private, @@ -150,8 +150,11 @@ input_kex_dh_gex_init(int type, u_int32_t seq, struct ssh *ssh) } if ((r = kex_dh_compute_key(kex, dh_client_pub, shared_secret)) != 0) goto out; - if ((r = sshkey_to_blob(server_host_public, &server_host_key_blob, - &sbloblen)) != 0) + if ((server_host_key_blob = sshbuf_new()) == NULL) { + r = SSH_ERR_ALLOC_FAIL; + goto out; + } + if ((r = sshkey_putb(server_host_public, server_host_key_blob)) != 0) goto out; /* calc H */ @@ -162,9 +165,9 @@ input_kex_dh_gex_init(int type, u_int32_t seq, struct ssh *ssh) kex->hash_alg, kex->client_version, kex->server_version, - sshbuf_ptr(kex->peer), sshbuf_len(kex->peer), - sshbuf_ptr(kex->my), sshbuf_len(kex->my), - server_host_key_blob, sbloblen, + kex->peer, + kex->my, + server_host_key_blob, kex->min, kex->nbits, kex->max, dh_p, dh_g, dh_client_pub, @@ -180,7 +183,7 @@ input_kex_dh_gex_init(int type, u_int32_t seq, struct ssh *ssh) /* send server hostkey, DH pubkey 'f' and signed H */ if ((r = sshpkt_start(ssh, SSH2_MSG_KEX_DH_GEX_REPLY)) != 0 || - (r = sshpkt_put_string(ssh, server_host_key_blob, sbloblen)) != 0 || + (r = sshpkt_put_stringb(ssh, server_host_key_blob)) != 0 || (r = sshpkt_put_bignum2(ssh, pub_key)) != 0 || /* f */ (r = sshpkt_put_string(ssh, signature, slen)) != 0 || (r = sshpkt_send(ssh)) != 0) @@ -194,7 +197,7 @@ input_kex_dh_gex_init(int type, u_int32_t seq, struct ssh *ssh) kex->dh = NULL; BN_clear_free(dh_client_pub); sshbuf_free(shared_secret); - free(server_host_key_blob); + sshbuf_free(server_host_key_blob); free(signature); return r; }