From 3e19295c3a253c8dc8660cf45baad7f45fccb969 Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Sun, 29 Dec 2013 17:47:50 +1100 Subject: [PATCH] - djm@cvs.openbsd.org 2013/12/27 22:30:17 [ssh-dss.c ssh-ecdsa.c ssh-rsa.c] make the original RSA and DSA signing/verification code look more like the ECDSA/Ed25519 ones: use key_type_plain() when checking the key type rather than tediously listing all variants, use __func__ for debug/ error messages --- ChangeLog | 6 ++++++ ssh-dss.c | 29 +++++++++++++++-------------- ssh-ecdsa.c | 12 +++++++----- ssh-rsa.c | 39 +++++++++++++++++++++------------------ 4 files changed, 49 insertions(+), 37 deletions(-) diff --git a/ChangeLog b/ChangeLog index f3a72291b..442dec3e9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -34,6 +34,12 @@ - tedu@cvs.openbsd.org 2013/12/21 07:10:47 [ssh-keygen.1] small typo + - djm@cvs.openbsd.org 2013/12/27 22:30:17 + [ssh-dss.c ssh-ecdsa.c ssh-rsa.c] + make the original RSA and DSA signing/verification code look more like + the ECDSA/Ed25519 ones: use key_type_plain() when checking the key type + rather than tediously listing all variants, use __func__ for debug/ + error messages 20131221 - (dtucker) [regress/keytype.sh] Actually test ecdsa key types. diff --git a/ssh-dss.c b/ssh-dss.c index 322ec9fd8..a6292aa84 100644 --- a/ssh-dss.c +++ b/ssh-dss.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-dss.c,v 1.28 2013/05/17 00:13:14 djm Exp $ */ +/* $OpenBSD: ssh-dss.c,v 1.29 2013/12/27 22:30:17 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * @@ -53,11 +53,12 @@ ssh_dss_sign(const Key *key, u_char **sigp, u_int *lenp, u_int rlen, slen, len, dlen; Buffer b; - if (key == NULL || key->dsa == NULL || (key->type != KEY_DSA && - key->type != KEY_DSA_CERT && key->type != KEY_DSA_CERT_V00)) { - error("ssh_dss_sign: no DSA key"); + if (key == NULL || key_type_plain(key->type) != KEY_DSA || + key->dsa == NULL) { + error("%s: no DSA key", __func__); return -1; } + EVP_DigestInit(&md, evp_md); EVP_DigestUpdate(&md, data, datalen); EVP_DigestFinal(&md, digest, &dlen); @@ -117,9 +118,9 @@ ssh_dss_verify(const Key *key, const u_char *signature, u_int signaturelen, int rlen, ret; Buffer b; - if (key == NULL || key->dsa == NULL || (key->type != KEY_DSA && - key->type != KEY_DSA_CERT && key->type != KEY_DSA_CERT_V00)) { - error("ssh_dss_verify: no DSA key"); + if (key == NULL || key_type_plain(key->type) != KEY_DSA || + key->dsa == NULL) { + error("%s: no DSA key", __func__); return -1; } @@ -135,7 +136,7 @@ ssh_dss_verify(const Key *key, const u_char *signature, u_int signaturelen, buffer_append(&b, signature, signaturelen); ktype = buffer_get_cstring(&b, NULL); if (strcmp("ssh-dss", ktype) != 0) { - error("ssh_dss_verify: cannot handle type %s", ktype); + error("%s: cannot handle type %s", __func__, ktype); buffer_free(&b); free(ktype); return -1; @@ -145,8 +146,8 @@ ssh_dss_verify(const Key *key, const u_char *signature, u_int signaturelen, rlen = buffer_len(&b); buffer_free(&b); if (rlen != 0) { - error("ssh_dss_verify: " - "remaining bytes in signature %d", rlen); + error("%s: remaining bytes in signature %d", + __func__, rlen); free(sigblob); return -1; } @@ -158,14 +159,14 @@ ssh_dss_verify(const Key *key, const u_char *signature, u_int signaturelen, /* parse signature */ if ((sig = DSA_SIG_new()) == NULL) - fatal("ssh_dss_verify: DSA_SIG_new failed"); + fatal("%s: DSA_SIG_new failed", __func__); if ((sig->r = BN_new()) == NULL) - fatal("ssh_dss_verify: BN_new failed"); + fatal("%s: BN_new failed", __func__); if ((sig->s = BN_new()) == NULL) fatal("ssh_dss_verify: BN_new failed"); if ((BN_bin2bn(sigblob, INTBLOB_LEN, sig->r) == NULL) || (BN_bin2bn(sigblob+ INTBLOB_LEN, INTBLOB_LEN, sig->s) == NULL)) - fatal("ssh_dss_verify: BN_bin2bn failed"); + fatal("%s: BN_bin2bn failed", __func__); /* clean up */ memset(sigblob, 0, len); @@ -181,7 +182,7 @@ ssh_dss_verify(const Key *key, const u_char *signature, u_int signaturelen, DSA_SIG_free(sig); - debug("ssh_dss_verify: signature %s", + debug("%s: signature %s", __func__, ret == 1 ? "correct" : ret == 0 ? "incorrect" : "error"); return ret; } diff --git a/ssh-ecdsa.c b/ssh-ecdsa.c index 766338941..52f9e74c0 100644 --- a/ssh-ecdsa.c +++ b/ssh-ecdsa.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-ecdsa.c,v 1.6 2013/05/17 00:13:14 djm Exp $ */ +/* $OpenBSD: ssh-ecdsa.c,v 1.7 2013/12/27 22:30:17 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * Copyright (c) 2010 Damien Miller. All rights reserved. @@ -54,11 +54,12 @@ ssh_ecdsa_sign(const Key *key, u_char **sigp, u_int *lenp, u_int len, dlen; Buffer b, bb; - if (key == NULL || key->ecdsa == NULL || - (key->type != KEY_ECDSA && key->type != KEY_ECDSA_CERT)) { + if (key == NULL || key_type_plain(key->type) != KEY_ECDSA || + key->ecdsa == NULL) { error("%s: no ECDSA key", __func__); return -1; } + evp_md = key_ec_nid_to_evpmd(key->ecdsa_nid); EVP_DigestInit(&md, evp_md); EVP_DigestUpdate(&md, data, datalen); @@ -105,11 +106,12 @@ ssh_ecdsa_verify(const Key *key, const u_char *signature, u_int signaturelen, Buffer b, bb; char *ktype; - if (key == NULL || key->ecdsa == NULL || - (key->type != KEY_ECDSA && key->type != KEY_ECDSA_CERT)) { + if (key == NULL || key_type_plain(key->type) != KEY_ECDSA || + key->ecdsa == NULL) { error("%s: no ECDSA key", __func__); return -1; } + evp_md = key_ec_nid_to_evpmd(key->ecdsa_nid); /* fetch signature */ diff --git a/ssh-rsa.c b/ssh-rsa.c index 30f96abc2..782c85573 100644 --- a/ssh-rsa.c +++ b/ssh-rsa.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-rsa.c,v 1.46 2013/05/17 00:13:14 djm Exp $ */ +/* $OpenBSD: ssh-rsa.c,v 1.47 2013/12/27 22:30:17 djm Exp $ */ /* * Copyright (c) 2000, 2003 Markus Friedl * @@ -47,14 +47,15 @@ ssh_rsa_sign(const Key *key, u_char **sigp, u_int *lenp, int ok, nid; Buffer b; - if (key == NULL || key->rsa == NULL || (key->type != KEY_RSA && - key->type != KEY_RSA_CERT && key->type != KEY_RSA_CERT_V00)) { - error("ssh_rsa_sign: no RSA key"); + if (key == NULL || key_type_plain(key->type) != KEY_RSA || + key->rsa == NULL) { + error("%s: no RSA key", __func__); return -1; } + nid = (datafellows & SSH_BUG_RSASIGMD5) ? NID_md5 : NID_sha1; if ((evp_md = EVP_get_digestbynid(nid)) == NULL) { - error("ssh_rsa_sign: EVP_get_digestbynid %d failed", nid); + error("%s: EVP_get_digestbynid %d failed", __func__, nid); return -1; } EVP_DigestInit(&md, evp_md); @@ -70,7 +71,7 @@ ssh_rsa_sign(const Key *key, u_char **sigp, u_int *lenp, if (ok != 1) { int ecode = ERR_get_error(); - error("ssh_rsa_sign: RSA_sign failed: %s", + error("%s: RSA_sign failed: %s", __func__, ERR_error_string(ecode, NULL)); free(sig); return -1; @@ -81,7 +82,7 @@ ssh_rsa_sign(const Key *key, u_char **sigp, u_int *lenp, memmove(sig + diff, sig, len); memset(sig, 0, diff); } else if (len > slen) { - error("ssh_rsa_sign: slen %u slen2 %u", slen, len); + error("%s: slen %u slen2 %u", __func__, slen, len); free(sig); return -1; } @@ -115,21 +116,23 @@ ssh_rsa_verify(const Key *key, const u_char *signature, u_int signaturelen, u_int len, dlen, modlen; int rlen, ret, nid; - if (key == NULL || key->rsa == NULL || (key->type != KEY_RSA && - key->type != KEY_RSA_CERT && key->type != KEY_RSA_CERT_V00)) { - error("ssh_rsa_verify: no RSA key"); + if (key == NULL || key_type_plain(key->type) != KEY_RSA || + key->rsa == NULL) { + error("%s: no RSA key", __func__); return -1; } + if (BN_num_bits(key->rsa->n) < SSH_RSA_MINIMUM_MODULUS_SIZE) { - error("ssh_rsa_verify: RSA modulus too small: %d < minimum %d bits", - BN_num_bits(key->rsa->n), SSH_RSA_MINIMUM_MODULUS_SIZE); + error("%s: RSA modulus too small: %d < minimum %d bits", + __func__, BN_num_bits(key->rsa->n), + SSH_RSA_MINIMUM_MODULUS_SIZE); return -1; } buffer_init(&b); buffer_append(&b, signature, signaturelen); ktype = buffer_get_cstring(&b, NULL); if (strcmp("ssh-rsa", ktype) != 0) { - error("ssh_rsa_verify: cannot handle type %s", ktype); + error("%s: cannot handle type %s", __func__, ktype); buffer_free(&b); free(ktype); return -1; @@ -139,19 +142,19 @@ ssh_rsa_verify(const Key *key, const u_char *signature, u_int signaturelen, rlen = buffer_len(&b); buffer_free(&b); if (rlen != 0) { - error("ssh_rsa_verify: remaining bytes in signature %d", rlen); + error("%s: remaining bytes in signature %d", __func__, rlen); free(sigblob); return -1; } /* RSA_verify expects a signature of RSA_size */ modlen = RSA_size(key->rsa); if (len > modlen) { - error("ssh_rsa_verify: len %u > modlen %u", len, modlen); + error("%s: len %u > modlen %u", __func__, len, modlen); free(sigblob); return -1; } else if (len < modlen) { u_int diff = modlen - len; - debug("ssh_rsa_verify: add padding: modlen %u > len %u", + debug("%s: add padding: modlen %u > len %u", __func__, modlen, len); sigblob = xrealloc(sigblob, 1, modlen); memmove(sigblob + diff, sigblob, len); @@ -160,7 +163,7 @@ ssh_rsa_verify(const Key *key, const u_char *signature, u_int signaturelen, } nid = (datafellows & SSH_BUG_RSASIGMD5) ? NID_md5 : NID_sha1; if ((evp_md = EVP_get_digestbynid(nid)) == NULL) { - error("ssh_rsa_verify: EVP_get_digestbynid %d failed", nid); + error("%s: EVP_get_digestbynid %d failed", __func__, nid); free(sigblob); return -1; } @@ -172,7 +175,7 @@ ssh_rsa_verify(const Key *key, const u_char *signature, u_int signaturelen, memset(digest, 'd', sizeof(digest)); memset(sigblob, 's', len); free(sigblob); - debug("ssh_rsa_verify: signature %scorrect", (ret==0) ? "in" : ""); + debug("%s: signature %scorrect", __func__, (ret == 0) ? "in" : ""); return ret; }