Attachment 3100 Details for Bug 2799 – Check signature algorithm while verifying RSA signatures

[patch] Check signature algorithm while verifying RSA signatures

openssh-verify.patch (text/plain), 12.16 KB, created by Jakub Jelen on 2017-11-28 03:37:02 AEDT

(hide)

Description:

Filename:

MIME Type:

Creator: Jakub Jelen

Created: 2017-11-28 03:37:02 AEDT

Size: 12.16 KB

patch

obsolete

>From 3769c51183224351e1841c48d74e51d446dc7924 Mon Sep 17 00:00:00 2001
>From: Jakub Jelen <jjelen@redhat.com>
>Date: Mon, 27 Nov 2017 17:07:28 +0100
>Subject: [PATCH] Verify the pkalg is the same for both layers
>
>---
> auth2-hostbased.c |  2 +-
> auth2-pubkey.c    |  2 +-
> clientloop.c      |  2 +-
> kexc25519c.c      |  2 +-
> kexdhc.c          |  2 +-
> kexecdhc.c        |  2 +-
> kexgexc.c         |  2 +-
> key.c             |  2 +-
> krl.c             |  7 ++++++-
> monitor.c         |  6 ++++--
> monitor_wrap.c    |  3 ++-
> monitor_wrap.h    |  2 +-
> ssh-keygen.c      |  2 +-
> ssh-rsa.c         |  7 ++++++-
> sshkey.c          | 41 ++++++++++++++++++++++++++++++++++++++---
> sshkey.h          |  5 +++--
> 16 files changed, 69 insertions(+), 20 deletions(-)
>
>diff --git a/auth2-hostbased.c b/auth2-hostbased.c
>index 92758b38..2c0c4313 100644
>--- a/auth2-hostbased.c
>+++ b/auth2-hostbased.c
>@@ -144,7 +144,7 @@ userauth_hostbased(struct ssh *ssh)
> 	authenticated = 0;
> 	if (PRIVSEP(hostbased_key_allowed(authctxt->pw, cuser, chost, key)) &&
> 	    PRIVSEP(sshkey_verify(key, sig, slen,
>-	    sshbuf_ptr(b), sshbuf_len(b), ssh->compat)) == 0)
>+	    sshbuf_ptr(b), sshbuf_len(b), pkalg, ssh->compat)) == 0)
> 		authenticated = 1;
> 
> 	auth2_record_key(authctxt, authenticated, key);
>diff --git a/auth2-pubkey.c b/auth2-pubkey.c
>index 169839b0..288e030e 100644
>--- a/auth2-pubkey.c
>+++ b/auth2-pubkey.c
>@@ -198,7 +198,7 @@ userauth_pubkey(struct ssh *ssh)
> 		authenticated = 0;
> 		if (PRIVSEP(user_key_allowed(authctxt->pw, key, 1)) &&
> 		    PRIVSEP(sshkey_verify(key, sig, slen, sshbuf_ptr(b),
>-		    sshbuf_len(b), ssh->compat)) == 0) {
>+		    sshbuf_len(b), pkalg, ssh->compat)) == 0) {
> 			authenticated = 1;
> 		}
> 		sshbuf_free(b);
>diff --git a/clientloop.c b/clientloop.c
>index eb1cdfcd..69862aa3 100644
>--- a/clientloop.c
>+++ b/clientloop.c
>@@ -1960,7 +1960,7 @@ client_global_hostkeys_private_confirm(struct ssh *ssh, int type,
> 			goto out;
> 		}
> 		if ((r = sshkey_verify(ctx->keys[i], sig, siglen,
>-		    sshbuf_ptr(signdata), sshbuf_len(signdata), 0)) != 0) {
>+		    sshbuf_ptr(signdata), sshbuf_len(signdata), NULL, 0)) != 0) {
> 			error("%s: server gave bad signature for %s key %zu",
> 			    __func__, sshkey_type(ctx->keys[i]), i);
> 			goto out;
>diff --git a/kexc25519c.c b/kexc25519c.c
>index e488013e..f89dcbbf 100644
>--- a/kexc25519c.c
>+++ b/kexc25519c.c
>@@ -141,7 +141,7 @@ input_kex_c25519_reply(int type, u_int32_t seq, struct ssh *ssh)
> 		goto out;
> 
> 	if ((r = sshkey_verify(server_host_key, signature, slen, hash, hashlen,
>-	    ssh->compat)) != 0)
>+	    NULL, ssh->compat)) != 0)
> 		goto out;
> 
> 	/* save session id */
>diff --git a/kexdhc.c b/kexdhc.c
>index ef0dbc20..1aa284b6 100644
>--- a/kexdhc.c
>+++ b/kexdhc.c
>@@ -192,7 +192,7 @@ input_kex_dh(int type, u_int32_t seq, struct ssh *ssh)
> 		goto out;
> 
> 	if ((r = sshkey_verify(server_host_key, signature, slen, hash, hashlen,
>-	    ssh->compat)) != 0)
>+	    NULL, ssh->compat)) != 0)
> 		goto out;
> 
> 	/* save session id */
>diff --git a/kexecdhc.c b/kexecdhc.c
>index d8a8b660..68d62cb9 100644
>--- a/kexecdhc.c
>+++ b/kexecdhc.c
>@@ -188,7 +188,7 @@ input_kex_ecdh_reply(int type, u_int32_t seq, struct ssh *ssh)
> 		goto out;
> 
> 	if ((r = sshkey_verify(server_host_key, signature, slen, hash,
>-	    hashlen, ssh->compat)) != 0)
>+	    hashlen, NULL, ssh->compat)) != 0)
> 		goto out;
> 
> 	/* save session id */
>diff --git a/kexgexc.c b/kexgexc.c
>index 543ff749..cdf2601e 100644
>--- a/kexgexc.c
>+++ b/kexgexc.c
>@@ -238,7 +238,7 @@ input_kex_dh_gex_reply(int type, u_int32_t seq, struct ssh *ssh)
> 		goto out;
> 
> 	if ((r = sshkey_verify(server_host_key, signature, slen, hash,
>-	    hashlen, ssh->compat)) != 0)
>+	    hashlen, NULL, ssh->compat)) != 0)
> 		goto out;
> 
> 	/* save session id */
>diff --git a/key.c b/key.c
>index 6e338c49..da3b636a 100644
>--- a/key.c
>+++ b/key.c
>@@ -102,7 +102,7 @@ key_verify(const Key *key, const u_char *signature, u_int signaturelen,
> 	int r;
> 
> 	if ((r = sshkey_verify(key, signature, signaturelen,
>-	    data, datalen, datafellows)) != 0) {
>+	    data, datalen, NULL, datafellows)) != 0) {
> 		fatal_on_fatal_errors(r, __func__, 0);
> 		error("%s: %s", __func__, ssh_err(r));
> 		return r == SSH_ERR_SIGNATURE_INVALID ? 0 : -1;
>diff --git a/krl.c b/krl.c
>index 086fc20e..10fc8a26 100644
>--- a/krl.c
>+++ b/krl.c
>@@ -929,6 +929,7 @@ ssh_krl_from_blob(struct sshbuf *buf, struct ssh_krl **krlp,
> 	const u_char *blob;
> 	size_t i, j, sig_off, sects_off, rlen, blen, nca_used;
> 	u_int format_version;
>+	char *sigtype = NULL;
> 
> 	nca_used = 0;
> 	*krlp = NULL;
>@@ -1012,9 +1013,12 @@ ssh_krl_from_blob(struct sshbuf *buf, struct ssh_krl **krlp,
> 			r = SSH_ERR_INVALID_FORMAT;
> 			goto out;
> 		}
>+		if ((r = sshkey_sigtype(blob, blen, &sigtype)) != 0) {
>+			goto out;
>+		}
> 		/* Check signature over entire KRL up to this point */
> 		if ((r = sshkey_verify(key, blob, blen,
>-		    sshbuf_ptr(buf), sig_off, 0)) != 0)
>+		    sshbuf_ptr(buf), sig_off, sigtype, 0)) != 0)
> 			goto out;
> 		/* Check if this key has already signed this KRL */
> 		for (i = 0; i < nca_used; i++) {
>@@ -1154,6 +1158,7 @@ ssh_krl_from_blob(struct sshbuf *buf, struct ssh_krl **krlp,
> 	sshkey_free(key);
> 	sshbuf_free(copy);
> 	sshbuf_free(sect);
>+	sshbuf_free(sigtype);
> 	return r;
> }
> 
>diff --git a/monitor.c b/monitor.c
>index ae137241..a0bdb2c3 100644
>--- a/monitor.c
>+++ b/monitor.c
>@@ -1359,12 +1359,14 @@ mm_answer_keyverify(int sock, struct sshbuf *m)
> {
> 	struct sshkey *key;
> 	u_char *signature, *data, *blob;
>+	char *alg;
> 	size_t signaturelen, datalen, bloblen;
> 	int r, ret, valid_data = 0, encoded_ret;
> 
> 	if ((r = sshbuf_get_string(m, &blob, &bloblen)) != 0 ||
> 	    (r = sshbuf_get_string(m, &signature, &signaturelen)) != 0 ||
>-	    (r = sshbuf_get_string(m, &data, &datalen)) != 0)
>+	    (r = sshbuf_get_string(m, &data, &datalen)) != 0 ||
>+	    (r = sshbuf_get_cstring(m, &alg, NULL)) != 0)
> 		fatal("%s: buffer error: %s", __func__, ssh_err(r));
> 
> 	if (hostbased_cuser == NULL || hostbased_chost == NULL ||
>@@ -1393,7 +1395,7 @@ mm_answer_keyverify(int sock, struct sshbuf *m)
> 		fatal("%s: bad signature data blob", __func__);
> 
> 	ret = sshkey_verify(key, signature, signaturelen, data, datalen,
>-	    active_state->compat);
>+	    alg, active_state->compat);
> 	debug3("%s: %s %p signature %s", __func__, auth_method, key,
> 	    (ret == 0) ? "verified" : "unverified");
> 	auth2_record_key(authctxt, ret == 0, key);
>diff --git a/monitor_wrap.c b/monitor_wrap.c
>index 0e8e22f7..6e8a27d4 100644
>--- a/monitor_wrap.c
>+++ b/monitor_wrap.c
>@@ -441,7 +441,7 @@ mm_key_allowed(enum mm_keytype type, const char *user, const char *host,
> 
> int
> mm_sshkey_verify(const struct sshkey *key, const u_char *sig, size_t siglen,
>-    const u_char *data, size_t datalen, u_int compat)
>+    const u_char *data, size_t datalen, const char *alg, u_int compat)
> {
> 	Buffer m;
> 	u_char *blob;
>@@ -458,6 +458,7 @@ mm_sshkey_verify(const struct sshkey *key, const u_char *sig, size_t siglen,
> 	buffer_put_string(&m, blob, len);
> 	buffer_put_string(&m, sig, siglen);
> 	buffer_put_string(&m, data, datalen);
>+	buffer_put_cstring(&m, alg);
> 	free(blob);
> 
> 	mm_request_send(pmonitor->m_recvfd, MONITOR_REQ_KEYVERIFY, &m);
>diff --git a/monitor_wrap.h b/monitor_wrap.h
>index d991887d..a58742cd 100644
>--- a/monitor_wrap.h
>+++ b/monitor_wrap.h
>@@ -51,7 +51,7 @@ int mm_user_key_allowed(struct passwd *, struct sshkey *, int);
> int mm_hostbased_key_allowed(struct passwd *, const char *,
>     const char *, struct sshkey *);
> int mm_sshkey_verify(const struct sshkey *, const u_char *, size_t,
>-    const u_char *, size_t, u_int);
>+    const u_char *, size_t, const char *, u_int);
> 
> #ifdef GSSAPI
> OM_uint32 mm_ssh_gssapi_server_ctx(Gssctxt **, gss_OID);
>diff --git a/ssh-keygen.c b/ssh-keygen.c
>index f50b26a1..44087404 100644
>--- a/ssh-keygen.c
>+++ b/ssh-keygen.c
>@@ -566,7 +566,7 @@ do_convert_private_ssh2_from_blob(u_char *blob, u_int blen)
> 
> 	/* try the key */
> 	if (sshkey_sign(key, &sig, &slen, data, sizeof(data), NULL, 0) != 0 ||
>-	    sshkey_verify(key, sig, slen, data, sizeof(data), 0) != 0) {
>+	    sshkey_verify(key, sig, slen, data, sizeof(data), NULL, 0) != 0) {
> 		sshkey_free(key);
> 		free(sig);
> 		return NULL;
>diff --git a/ssh-rsa.c b/ssh-rsa.c
>index 6e2bba0d..e6c9813f 100644
>--- a/ssh-rsa.c
>+++ b/ssh-rsa.c
>@@ -207,7 +207,8 @@ ssh_rsa_sign(const struct sshkey *key, u_char **sigp, size_t *lenp,
> 
> int
> ssh_rsa_verify(const struct sshkey *key,
>-    const u_char *sig, size_t siglen, const u_char *data, size_t datalen)
>+    const u_char *sig, size_t siglen, const u_char *data, size_t datalen,
>+    const char *alg)
> {
> 	char *ktype = NULL;
> 	int hash_alg, ret = SSH_ERR_INTERNAL_ERROR;
>@@ -228,6 +229,10 @@ ssh_rsa_verify(const struct sshkey *key,
> 		ret = SSH_ERR_INVALID_FORMAT;
> 		goto out;
> 	}
>+	if (alg != NULL && strcmp(ktype, alg) != 0) {
>+		ret = SSH_ERR_KEY_TYPE_MISMATCH;
>+		goto out;
>+	}
> 	if ((hash_alg = rsa_hash_alg_from_ident(ktype)) == -1) {
> 		ret = SSH_ERR_KEY_TYPE_MISMATCH;
> 		goto out;
>diff --git a/sshkey.c b/sshkey.c
>index d5606ea7..c154763d 100644
>--- a/sshkey.c
>+++ b/sshkey.c
>@@ -1735,11 +1735,41 @@ sshkey_from_private(const struct sshkey *k, struct sshkey **pkp)
> }
> 
> static int
>+sshkey_ktype_from_blob(struct sshbuf *b, char **ktypep)
>+{
>+	int ret = SSH_ERR_INTERNAL_ERROR;
>+	struct sshbuf *copy;
>+	char *ktype = NULL;
>+
>+	if (ktypep != NULL)
>+		*ktypep = NULL;
>+	if ((copy = sshbuf_fromb(b)) == NULL) {
>+		ret = SSH_ERR_ALLOC_FAIL;
>+		goto out;
>+	}
>+	if (sshbuf_get_cstring(b, &ktype, NULL) != 0) {
>+		ret = SSH_ERR_INVALID_FORMAT;
>+		goto out;
>+	}
>+	if (ktypep != NULL) {
>+		*ktypep = ktype;
>+		ktype = NULL;
>+	}
>+	/* Success */
>+	ret = 0;
>+out:
>+	sshbuf_free(copy);
>+	free(ktype);
>+	return ret;
>+}
>+
>+static int
> cert_parse(struct sshbuf *b, struct sshkey *key, struct sshbuf *certbuf)
> {
> 	struct sshbuf *principals = NULL, *crit = NULL;
> 	struct sshbuf *exts = NULL, *ca = NULL;
> 	u_char *sig = NULL;
>+	char *alg = NULL;
> 	size_t signed_len = 0, slen = 0, kidlen = 0;
> 	int ret = SSH_ERR_INTERNAL_ERROR;
> 
>@@ -1842,8 +1872,12 @@ cert_parse(struct sshbuf *b, struct sshkey *key, struct sshbuf *certbuf)
> 		ret = SSH_ERR_KEY_CERT_INVALID_SIGN_KEY;
> 		goto out;
> 	}
>+	if (!sshkey_ktype_from_blob(ca, &alg)) {
>+		ret = SSH_ERR_KEY_CERT_INVALID_SIGN_KEY;
>+		goto out;
>+	}
> 	if ((ret = sshkey_verify(key->cert->signature_key, sig, slen,
>-	    sshbuf_ptr(key->cert->certblob), signed_len, 0)) != 0)
>+	    sshbuf_ptr(key->cert->certblob), signed_len, alg, 0)) != 0)
> 		goto out;
> 
> 	/* Success */
>@@ -1854,6 +1888,7 @@ cert_parse(struct sshbuf *b, struct sshkey *key, struct sshbuf *certbuf)
> 	sshbuf_free(exts);
> 	sshbuf_free(principals);
> 	free(sig);
>+	free(alg);
> 	return ret;
> }
> 
>@@ -2174,7 +2209,7 @@ sshkey_sign(const struct sshkey *key,
> int
> sshkey_verify(const struct sshkey *key,
>     const u_char *sig, size_t siglen,
>-    const u_char *data, size_t dlen, u_int compat)
>+    const u_char *data, size_t dlen, const char *alg, u_int compat)
> {
> 	if (siglen == 0 || dlen > SSH_KEY_MAX_SIGN_DATA_SIZE)
> 		return SSH_ERR_INVALID_ARGUMENT;
>@@ -2190,7 +2225,7 @@ sshkey_verify(const struct sshkey *key,
> # endif /* OPENSSL_HAS_ECC */
> 	case KEY_RSA_CERT:
> 	case KEY_RSA:
>-		return ssh_rsa_verify(key, sig, siglen, data, dlen);
>+		return ssh_rsa_verify(key, sig, siglen, data, dlen, alg);
> #endif /* WITH_OPENSSL */
> 	case KEY_ED25519:
> 	case KEY_ED25519_CERT:
>diff --git a/sshkey.h b/sshkey.h
>index 412ef0e4..fad51f04 100644
>--- a/sshkey.h
>+++ b/sshkey.h
>@@ -179,7 +179,7 @@ int	 sshkey_sigtype(const u_char *, size_t, char **);
> int	 sshkey_sign(const struct sshkey *, u_char **, size_t *,
>     const u_char *, size_t, const char *, u_int);
> int	 sshkey_verify(const struct sshkey *, const u_char *, size_t,
>-    const u_char *, size_t, u_int);
>+    const u_char *, size_t, const char *, u_int);
> 
> /* for debug */
> void	sshkey_dump_ec_point(const EC_GROUP *, const EC_POINT *);
>@@ -206,7 +206,8 @@ int ssh_rsa_sign(const struct sshkey *key,
>     u_char **sigp, size_t *lenp, const u_char *data, size_t datalen,
>     const char *ident);
> int ssh_rsa_verify(const struct sshkey *key,
>-    const u_char *sig, size_t siglen, const u_char *data, size_t datalen);
>+    const u_char *sig, size_t siglen, const u_char *data, size_t datalen,
>+    const char *alg);
> int ssh_dss_sign(const struct sshkey *key, u_char **sigp, size_t *lenp,
>     const u_char *data, size_t datalen, u_int compat);
> int ssh_dss_verify(const struct sshkey *key,
>-- 
>2.13.6
>

Actions: View | Diff

Attachments on bug 2799: 3090 | 3092 | 3100 | 3104 | 3135