From d7d116b6d9e6cb79cc235e9801caa683d3db3181 Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Mon, 14 Oct 2019 06:00:02 +0000 Subject: upstream: memleak in error path; spotted by oss-fuzz, ok markus@ OpenBSD-Commit-ID: d6ed260cbbc297ab157ad63931802fb1ef7a4266 --- sshkey-xmss.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) (limited to 'sshkey-xmss.c') diff --git a/sshkey-xmss.c b/sshkey-xmss.c index 9e5f5e475..e8e2e3816 100644 --- a/sshkey-xmss.c +++ b/sshkey-xmss.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sshkey-xmss.c,v 1.6 2019/10/09 00:02:57 djm Exp $ */ +/* $OpenBSD: sshkey-xmss.c,v 1.7 2019/10/14 06:00:02 djm Exp $ */ /* * Copyright (c) 2017 Markus Friedl. All rights reserved. * @@ -748,7 +748,7 @@ sshkey_xmss_deserialize_state(struct sshkey *k, struct sshbuf *b) u_int32_t i, lh, node; size_t ls, lsl, la, lk, ln, lr; char *magic; - int r; + int r = SSH_ERR_INTERNAL_ERROR; if (state == NULL) return SSH_ERR_INVALID_ARGUMENT; @@ -767,9 +767,11 @@ sshkey_xmss_deserialize_state(struct sshkey *k, struct sshbuf *b) (r = sshbuf_get_string(b, &state->th_nodes, &ln)) != 0 || (r = sshbuf_get_string(b, &state->retain, &lr)) != 0 || (r = sshbuf_get_u32(b, &lh)) != 0) - return r; - if (strcmp(magic, SSH_XMSS_K2_MAGIC) != 0) - return SSH_ERR_INVALID_ARGUMENT; + goto out; + if (strcmp(magic, SSH_XMSS_K2_MAGIC) != 0) { + r = SSH_ERR_INVALID_ARGUMENT; + goto out; + } /* XXX check stackoffset */ if (ls != num_stack(state) || lsl != num_stacklevels(state) || @@ -777,8 +779,10 @@ sshkey_xmss_deserialize_state(struct sshkey *k, struct sshbuf *b) lk != num_keep(state) || ln != num_th_nodes(state) || lr != num_retain(state) || - lh != num_treehash(state)) - return SSH_ERR_INVALID_ARGUMENT; + lh != num_treehash(state)) { + r = SSH_ERR_INVALID_ARGUMENT; + goto out; + } for (i = 0; i < num_treehash(state); i++) { th = &state->treehash[i]; if ((r = sshbuf_get_u32(b, &th->h)) != 0 || @@ -786,7 +790,7 @@ sshkey_xmss_deserialize_state(struct sshkey *k, struct sshbuf *b) (r = sshbuf_get_u32(b, &th->stackusage)) != 0 || (r = sshbuf_get_u8(b, &th->completed)) != 0 || (r = sshbuf_get_u32(b, &node)) != 0) - return r; + goto out; if (node < num_th_nodes(state)) th->node = &state->th_nodes[node]; } @@ -794,7 +798,11 @@ sshkey_xmss_deserialize_state(struct sshkey *k, struct sshbuf *b) xmss_set_bds_state(&state->bds, state->stack, state->stackoffset, state->stacklevels, state->auth, state->keep, state->treehash, state->retain, 0); - return 0; + /* success */ + r = 0; + out: + free(magic); + return r; } int -- cgit v1.2.3 From bf219920b70cafbf29ebc9890ef67d0efa54e738 Mon Sep 17 00:00:00 2001 From: "markus@openbsd.org" Date: Wed, 13 Nov 2019 07:53:10 +0000 Subject: upstream: fix shield/unshield for xmss keys: - in ssh-agent we need to delay the call to shield until we have received key specific options. - when serializing xmss keys for shield we need to deal with all optional components (e.g. state might not be loaded). ok djm@ OpenBSD-Commit-ID: cc2db82524b209468eb176d6b4d6b9486422f41f --- ssh-agent.c | 10 +++++----- sshkey-xmss.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++---- sshkey.c | 4 ++-- sshkey.h | 9 +++++---- 4 files changed, 64 insertions(+), 15 deletions(-) (limited to 'sshkey-xmss.c') diff --git a/ssh-agent.c b/ssh-agent.c index eb17b18b2..c62c263a6 100644 --- a/ssh-agent.c +++ b/ssh-agent.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-agent.c,v 1.241 2019/11/12 22:36:44 djm Exp $ */ +/* $OpenBSD: ssh-agent.c,v 1.242 2019/11/13 07:53:10 markus Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -560,10 +560,6 @@ process_add_identity(SocketEntry *e) error("%s: decode private key: %s", __func__, ssh_err(r)); goto err; } - if ((r = sshkey_shield_private(k)) != 0) { - error("%s: shield private key: %s", __func__, ssh_err(r)); - goto err; - } while (sshbuf_len(e->request)) { if ((r = sshbuf_get_u8(e->request, &ctype)) != 0) { error("%s: buffer error: %s", __func__, ssh_err(r)); @@ -645,6 +641,10 @@ process_add_identity(SocketEntry *e) goto send; } } + if ((r = sshkey_shield_private(k)) != 0) { + error("%s: shield private key: %s", __func__, ssh_err(r)); + goto err; + } success = 1; if (lifetime && !death) diff --git a/sshkey-xmss.c b/sshkey-xmss.c index e8e2e3816..88e9ddf4d 100644 --- a/sshkey-xmss.c +++ b/sshkey-xmss.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sshkey-xmss.c,v 1.7 2019/10/14 06:00:02 djm Exp $ */ +/* $OpenBSD: sshkey-xmss.c,v 1.8 2019/11/13 07:53:10 markus Exp $ */ /* * Copyright (c) 2017 Markus Friedl. All rights reserved. * @@ -69,7 +69,7 @@ struct ssh_xmss_state { u_int32_t maxidx; /* restricted # of signatures */ int have_state; /* .state file exists */ int lockfd; /* locked in sshkey_xmss_get_state() */ - int allow_update; /* allow sshkey_xmss_update_state() */ + u_char allow_update; /* allow sshkey_xmss_update_state() */ char *enc_ciphername;/* encrypt state with cipher */ u_char *enc_keyiv; /* encrypt state with key */ u_int32_t enc_keyiv_len; /* length of enc_keyiv */ @@ -716,6 +716,7 @@ sshkey_xmss_serialize_state_opt(const struct sshkey *k, struct sshbuf *b, { struct ssh_xmss_state *state = k->xmss_state; int r = SSH_ERR_INVALID_ARGUMENT; + u_char have_stack, have_filename, have_enc; if (state == NULL) return SSH_ERR_INVALID_ARGUMENT; @@ -727,9 +728,35 @@ sshkey_xmss_serialize_state_opt(const struct sshkey *k, struct sshbuf *b, break; case SSHKEY_SERIALIZE_FULL: if ((r = sshkey_xmss_serialize_enc_key(k, b)) != 0) - break; + return r; r = sshkey_xmss_serialize_state(k, b); break; + case SSHKEY_SERIALIZE_SHIELD: + /* all of stack/filename/enc are optional */ + have_stack = state->stack != NULL; + if ((r = sshbuf_put_u8(b, have_stack)) != 0) + return r; + if (have_stack) { + state->idx = PEEK_U32(k->xmss_sk); /* update */ + if ((r = sshkey_xmss_serialize_state(k, b)) != 0) + return r; + } + have_filename = k->xmss_filename != NULL; + if ((r = sshbuf_put_u8(b, have_filename)) != 0) + return r; + if (have_filename && + (r = sshbuf_put_cstring(b, k->xmss_filename)) != 0) + return r; + have_enc = state->enc_keyiv != NULL; + if ((r = sshbuf_put_u8(b, have_enc)) != 0) + return r; + if (have_enc && + (r = sshkey_xmss_serialize_enc_key(k, b)) != 0) + return r; + if ((r = sshbuf_put_u32(b, state->maxidx)) != 0 || + (r = sshbuf_put_u8(b, state->allow_update)) != 0) + return r; + break; case SSHKEY_SERIALIZE_DEFAULT: r = 0; break; @@ -808,8 +835,9 @@ sshkey_xmss_deserialize_state(struct sshkey *k, struct sshbuf *b) int sshkey_xmss_deserialize_state_opt(struct sshkey *k, struct sshbuf *b) { + struct ssh_xmss_state *state = k->xmss_state; enum sshkey_serialize_rep opts; - u_char have_state; + u_char have_state, have_stack, have_filename, have_enc; int r; if ((r = sshbuf_get_u8(b, &have_state)) != 0) @@ -820,6 +848,26 @@ sshkey_xmss_deserialize_state_opt(struct sshkey *k, struct sshbuf *b) case SSHKEY_SERIALIZE_DEFAULT: r = 0; break; + case SSHKEY_SERIALIZE_SHIELD: + if ((r = sshbuf_get_u8(b, &have_stack)) != 0) + return r; + if (have_stack && + (r = sshkey_xmss_deserialize_state(k, b)) != 0) + return r; + if ((r = sshbuf_get_u8(b, &have_filename)) != 0) + return r; + if (have_filename && + (r = sshbuf_get_cstring(b, &k->xmss_filename, NULL)) != 0) + return r; + if ((r = sshbuf_get_u8(b, &have_enc)) != 0) + return r; + if (have_enc && + (r = sshkey_xmss_deserialize_enc_key(k, b)) != 0) + return r; + if ((r = sshbuf_get_u32(b, &state->maxidx)) != 0 || + (r = sshbuf_get_u8(b, &state->allow_update)) != 0) + return r; + break; case SSHKEY_SERIALIZE_STATE: if ((r = sshkey_xmss_deserialize_state(k, b)) != 0) return r; diff --git a/sshkey.c b/sshkey.c index 80186206c..190426e28 100644 --- a/sshkey.c +++ b/sshkey.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sshkey.c,v 1.90 2019/11/12 19:33:08 markus Exp $ */ +/* $OpenBSD: sshkey.c,v 1.91 2019/11/13 07:53:10 markus Exp $ */ /* * Copyright (c) 2000, 2001 Markus Friedl. All rights reserved. * Copyright (c) 2008 Alexander von Gernler. All rights reserved. @@ -2045,7 +2045,7 @@ sshkey_shield_private(struct sshkey *k) if (sshkey_is_shielded(k) && (r = sshkey_unshield_private(k)) != 0) goto out; if ((r = sshkey_private_serialize_opt(k, prvbuf, - SSHKEY_SERIALIZE_FULL)) != 0) + SSHKEY_SERIALIZE_SHIELD)) != 0) goto out; /* pad to cipher blocksize */ i = 0; diff --git a/sshkey.h b/sshkey.h index 1fb8369f0..a34a4cb48 100644 --- a/sshkey.h +++ b/sshkey.h @@ -1,4 +1,4 @@ -/* $OpenBSD: sshkey.h,v 1.38 2019/11/12 19:33:08 markus Exp $ */ +/* $OpenBSD: sshkey.h,v 1.39 2019/11/13 07:53:10 markus Exp $ */ /* * Copyright (c) 2000, 2001 Markus Friedl. All rights reserved. @@ -87,9 +87,10 @@ enum sshkey_fp_rep { /* Private key serialisation formats, used on the wire */ enum sshkey_serialize_rep { SSHKEY_SERIALIZE_DEFAULT = 0, - SSHKEY_SERIALIZE_STATE = 1, - SSHKEY_SERIALIZE_FULL = 2, - SSHKEY_SERIALIZE_INFO = 254, + SSHKEY_SERIALIZE_STATE = 1, /* only state is serialized */ + SSHKEY_SERIALIZE_FULL = 2, /* include keys for saving to disk */ + SSHKEY_SERIALIZE_SHIELD = 3, /* everything, for encrypting in ram */ + SSHKEY_SERIALIZE_INFO = 254, /* minimal information */ }; /* Private key disk formats */ -- cgit v1.2.3