From fb19656940552b0db7188a4a498f59bf8824b925 Mon Sep 17 00:00:00 2001 From: djm Date: Tue, 26 Jan 2021 00:49:30 +0000 Subject: [PATCH] move check_host_cert() from sshconnect,c to sshkey.c and refactor it to make it more generally usable and testable. ok markus@ --- usr.bin/ssh/auth2-hostbased.c | 4 +-- usr.bin/ssh/auth2-pubkey.c | 6 ++-- usr.bin/ssh/hostfile.c | 3 +- usr.bin/ssh/sshconnect.c | 56 +++++++++++++---------------------- usr.bin/ssh/sshkey.c | 39 +++++++++++++++++++++--- usr.bin/ssh/sshkey.h | 6 ++-- usr.bin/ssh/sshsig.c | 6 ++-- 7 files changed, 69 insertions(+), 51 deletions(-) diff --git a/usr.bin/ssh/auth2-hostbased.c b/usr.bin/ssh/auth2-hostbased.c index 6bbdec25e0f..7db7f3310f2 100644 --- a/usr.bin/ssh/auth2-hostbased.c +++ b/usr.bin/ssh/auth2-hostbased.c @@ -1,4 +1,4 @@ -/* $OpenBSD: auth2-hostbased.c,v 1.43 2020/10/18 11:32:01 djm Exp $ */ +/* $OpenBSD: auth2-hostbased.c,v 1.44 2021/01/26 00:49:30 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * @@ -213,7 +213,7 @@ hostbased_key_allowed(struct ssh *ssh, struct passwd *pw, debug2_f("access allowed by auth_rhosts2"); if (sshkey_is_cert(key) && - sshkey_cert_check_authority(key, 1, 0, lookup, &reason)) { + sshkey_cert_check_authority(key, 1, 0, 0, lookup, &reason)) { error("%s", reason); auth_debug_add("%s", reason); return 0; diff --git a/usr.bin/ssh/auth2-pubkey.c b/usr.bin/ssh/auth2-pubkey.c index 9b5cd2441ca..8acb5531610 100644 --- a/usr.bin/ssh/auth2-pubkey.c +++ b/usr.bin/ssh/auth2-pubkey.c @@ -1,4 +1,4 @@ -/* $OpenBSD: auth2-pubkey.c,v 1.104 2021/01/22 02:44:58 dtucker Exp $ */ +/* $OpenBSD: auth2-pubkey.c,v 1.105 2021/01/26 00:49:30 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * @@ -671,7 +671,7 @@ check_authkey_line(struct ssh *ssh, struct passwd *pw, struct sshkey *key, reason = "Certificate does not contain an authorized principal"; goto fail_reason; } - if (sshkey_cert_check_authority(key, 0, 0, + if (sshkey_cert_check_authority(key, 0, 0, 0, keyopts->cert_principals == NULL ? pw->pw_name : NULL, &reason) != 0) goto fail_reason; @@ -790,7 +790,7 @@ user_cert_trusted_ca(struct ssh *ssh, struct passwd *pw, struct sshkey *key, } if (use_authorized_principals && principals_opts == NULL) fatal_f("internal error: missing principals_opts"); - if (sshkey_cert_check_authority(key, 0, 1, + if (sshkey_cert_check_authority(key, 0, 1, 0, use_authorized_principals ? NULL : pw->pw_name, &reason) != 0) goto fail_reason; diff --git a/usr.bin/ssh/hostfile.c b/usr.bin/ssh/hostfile.c index 6f21c368823..287ea1a35b7 100644 --- a/usr.bin/ssh/hostfile.c +++ b/usr.bin/ssh/hostfile.c @@ -1,4 +1,4 @@ -/* $OpenBSD: hostfile.c,v 1.87 2020/12/20 23:36:51 djm Exp $ */ +/* $OpenBSD: hostfile.c,v 1.88 2021/01/26 00:49:30 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -59,6 +59,7 @@ #include "ssherr.h" #include "digest.h" #include "hmac.h" +#include "sshbuf.h" struct hostkeys { struct hostkey_entry *entries; diff --git a/usr.bin/ssh/sshconnect.c b/usr.bin/ssh/sshconnect.c index 65f2773870b..47904020d6b 100644 --- a/usr.bin/ssh/sshconnect.c +++ b/usr.bin/ssh/sshconnect.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sshconnect.c,v 1.349 2020/12/22 00:15:23 djm Exp $ */ +/* $OpenBSD: sshconnect.c,v 1.350 2021/01/26 00:49:30 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -567,36 +567,6 @@ confirm(const char *prompt, const char *fingerprint) } } -static int -check_host_cert(const char *host, const struct sshkey *key) -{ - const char *reason; - int r; - - if (sshkey_cert_check_authority(key, 1, 0, host, &reason) != 0) { - error("%s", reason); - return 0; - } - if (sshbuf_len(key->cert->critical) != 0) { - error("Certificate for %s contains unsupported " - "critical options(s)", host); - return 0; - } - if ((r = sshkey_check_cert_sigtype(key, - options.ca_sign_algorithms)) != 0) { - logit_fr(r, "certificate signature algorithm %s", - (key->cert == NULL || key->cert->signature_type == NULL) ? - "(null)" : key->cert->signature_type); - return 0; - } - /* Do not attempt hostkey update if a certificate was successful */ - if (options.update_hostkeys != 0) { - options.update_hostkeys = 0; - debug3_f("certificate host key in use; disabling UpdateHostkeys"); - } - return 1; -} - static int sockaddr_is_local(struct sockaddr *hostaddr) { @@ -925,7 +895,7 @@ check_host_key(char *hostname, const struct ssh_conn_info *cinfo, char *ip = NULL, *host = NULL; char hostline[1000], *hostp, *fp, *ra; char msg[1024]; - const char *type; + const char *type, *fail_reason; const struct hostkey_entry *host_found = NULL, *ip_found = NULL; int len, cancelled_forwarding = 0, confirmed; int local = sockaddr_is_local(hostaddr); @@ -1031,10 +1001,24 @@ check_host_key(char *hostname, const struct ssh_conn_info *cinfo, host, type, want_cert ? "certificate" : "key"); debug("Found %s in %s:%lu", want_cert ? "CA key" : "key", host_found->file, host_found->line); - if (want_cert && - !check_host_cert(options.host_key_alias == NULL ? - hostname : options.host_key_alias, host_key)) - goto fail; + if (want_cert) { + if (sshkey_cert_check_host(host_key, + options.host_key_alias == NULL ? + hostname : options.host_key_alias, 0, + options.ca_sign_algorithms, &fail_reason) != 0) { + error("%s", fail_reason); + goto fail; + } + /* + * Do not attempt hostkey update if a certificate was + * successfully matched. + */ + if (options.update_hostkeys != 0) { + options.update_hostkeys = 0; + debug3_f("certificate host key in use; " + "disabling UpdateHostkeys"); + } + } /* Turn off UpdateHostkeys if key was in system known_hosts */ if (options.update_hostkeys != 0 && (path_in_hostfiles(host_found->file, diff --git a/usr.bin/ssh/sshkey.c b/usr.bin/ssh/sshkey.c index e866a791c61..941e9fda124 100644 --- a/usr.bin/ssh/sshkey.c +++ b/usr.bin/ssh/sshkey.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sshkey.c,v 1.113 2021/01/15 04:31:25 dtucker Exp $ */ +/* $OpenBSD: sshkey.c,v 1.114 2021/01/26 00:49:30 djm Exp $ */ /* * Copyright (c) 2000, 2001 Markus Friedl. All rights reserved. * Copyright (c) 2008 Alexander von Gernler. All rights reserved. @@ -3025,7 +3025,7 @@ sshkey_certify(struct sshkey *k, struct sshkey *ca, const char *alg, int sshkey_cert_check_authority(const struct sshkey *k, - int want_host, int require_principal, + int want_host, int require_principal, int wildcard_pattern, const char *name, const char **reason) { u_int i, principal_matches; @@ -3033,7 +3033,10 @@ sshkey_cert_check_authority(const struct sshkey *k, if (reason == NULL) return SSH_ERR_INVALID_ARGUMENT; - + if (!sshkey_is_cert(k)) { + *reason = "Key is not a certificate"; + return SSH_ERR_KEY_CERT_INVALID; + } if (want_host) { if (k->cert->type != SSH2_CERT_TYPE_HOST) { *reason = "Certificate invalid: not a host certificate"; @@ -3066,7 +3069,13 @@ sshkey_cert_check_authority(const struct sshkey *k, } else if (name != NULL) { principal_matches = 0; for (i = 0; i < k->cert->nprincipals; i++) { - if (strcmp(name, k->cert->principals[i]) == 0) { + if (wildcard_pattern) { + if (match_pattern(k->cert->principals[i], + name)) { + principal_matches = 1; + break; + } + } else if (strcmp(name, k->cert->principals[i]) == 0) { principal_matches = 1; break; } @@ -3080,6 +3089,28 @@ sshkey_cert_check_authority(const struct sshkey *k, return 0; } +int +sshkey_cert_check_host(const struct sshkey *key, const char *host, + int wildcard_principals, const char *ca_sign_algorithms, + const char **reason) +{ + int r; + + if ((r = sshkey_cert_check_authority(key, 1, 0, wildcard_principals, + host, reason)) != 0) + return r; + if (sshbuf_len(key->cert->critical) != 0) { + *reason = "Certificate contains unsupported critical options"; + return SSH_ERR_KEY_CERT_INVALID; + } + if (ca_sign_algorithms != NULL && + (r = sshkey_check_cert_sigtype(key, ca_sign_algorithms)) != 0) { + *reason = "Certificate signed with disallowed algorithm"; + return SSH_ERR_KEY_CERT_INVALID; + } + return 0; +} + size_t sshkey_format_cert_validity(const struct sshkey_cert *cert, char *s, size_t l) { diff --git a/usr.bin/ssh/sshkey.h b/usr.bin/ssh/sshkey.h index c56267b5395..18b11b5db9a 100644 --- a/usr.bin/ssh/sshkey.h +++ b/usr.bin/ssh/sshkey.h @@ -1,4 +1,4 @@ -/* $OpenBSD: sshkey.h,v 1.48 2020/11/08 11:46:12 dtucker Exp $ */ +/* $OpenBSD: sshkey.h,v 1.49 2021/01/26 00:49:30 djm Exp $ */ /* * Copyright (c) 2000, 2001 Markus Friedl. All rights reserved. @@ -189,8 +189,10 @@ int sshkey_type_plain(int); int sshkey_to_certified(struct sshkey *); int sshkey_drop_cert(struct sshkey *); int sshkey_cert_copy(const struct sshkey *, struct sshkey *); -int sshkey_cert_check_authority(const struct sshkey *, int, int, +int sshkey_cert_check_authority(const struct sshkey *, int, int, int, const char *, const char **); +int sshkey_cert_check_host(const struct sshkey *, const char *, + int , const char *, const char **); size_t sshkey_format_cert_validity(const struct sshkey_cert *, char *, size_t) __attribute__((__bounded__(__string__, 2, 3))); int sshkey_check_cert_sigtype(const struct sshkey *, const char *); diff --git a/usr.bin/ssh/sshsig.c b/usr.bin/ssh/sshsig.c index 8371c60a712..2638253958e 100644 --- a/usr.bin/ssh/sshsig.c +++ b/usr.bin/ssh/sshsig.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sshsig.c,v 1.18 2020/10/18 11:32:02 djm Exp $ */ +/* $OpenBSD: sshsig.c,v 1.19 2021/01/26 00:49:30 djm Exp $ */ /* * Copyright (c) 2019 Google LLC * @@ -807,7 +807,7 @@ check_allowed_keys_line(const char *path, u_long linenum, char *line, } else if (sigopts->ca && sshkey_is_cert(sign_key) && sshkey_equal_public(sign_key->cert->signature_key, found_key)) { /* Match of certificate's CA key */ - if ((r = sshkey_cert_check_authority(sign_key, 0, 1, + if ((r = sshkey_cert_check_authority(sign_key, 0, 1, 0, principal, &reason)) != 0) { error("%s:%lu: certificate not authorized: %s", path, linenum, reason); @@ -890,7 +890,7 @@ cert_filter_principals(const char *path, u_long linenum, continue; } /* Check against principals list in certificate */ - if ((r = sshkey_cert_check_authority(cert, 0, 1, + if ((r = sshkey_cert_check_authority(cert, 0, 1, 0, cp, &reason)) != 0) { debug("%s:%lu: principal \"%s\" not authorized: %s", path, linenum, cp, reason); -- 2.20.1