From b8669307550173b232fdd7c9239488de7ae64868 Mon Sep 17 00:00:00 2001 From: djm Date: Fri, 27 May 2022 05:01:25 +0000 Subject: [PATCH] refactor authorized_keys/principals handling remove "struct ssh *" from arguments - this was only used to pass the remote host/address. These can be passed in instead and the resulting code is less tightly coupled to ssh_api.[ch] ok dtucker@ --- usr.bin/ssh/auth.c | 10 ++--- usr.bin/ssh/auth.h | 10 ++--- usr.bin/ssh/auth2-pubkey.c | 92 +++++++++++++++++++++++--------------- usr.bin/ssh/monitor.c | 9 ++-- usr.bin/ssh/monitor_wrap.c | 7 +-- usr.bin/ssh/monitor_wrap.h | 6 +-- 6 files changed, 77 insertions(+), 57 deletions(-) diff --git a/usr.bin/ssh/auth.c b/usr.bin/ssh/auth.c index 3f2654ebeb0..9fff5c1156e 100644 --- a/usr.bin/ssh/auth.c +++ b/usr.bin/ssh/auth.c @@ -1,4 +1,4 @@ -/* $OpenBSD: auth.c,v 1.155 2022/04/26 07:41:44 dtucker Exp $ */ +/* $OpenBSD: auth.c,v 1.156 2022/05/27 05:01:25 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * @@ -835,12 +835,10 @@ auth_restrict_session(struct ssh *ssh) } int -auth_authorise_keyopts(struct ssh *ssh, struct passwd *pw, - struct sshauthopt *opts, int allow_cert_authority, const char *loc) +auth_authorise_keyopts(struct passwd *pw, struct sshauthopt *opts, + int allow_cert_authority, const char *remote_ip, const char *remote_host, + const char *loc) { - const char *remote_ip = ssh_remote_ipaddr(ssh); - const char *remote_host = auth_get_canonical_hostname(ssh, - options.use_dns); time_t now = time(NULL); char buf[64]; diff --git a/usr.bin/ssh/auth.h b/usr.bin/ssh/auth.h index d9f129518b7..ea2ca7f89aa 100644 --- a/usr.bin/ssh/auth.h +++ b/usr.bin/ssh/auth.h @@ -1,4 +1,4 @@ -/* $OpenBSD: auth.h,v 1.102 2021/12/19 22:12:07 djm Exp $ */ +/* $OpenBSD: auth.h,v 1.103 2022/05/27 05:01:25 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. @@ -124,8 +124,8 @@ int auth_password(struct ssh *, const char *); int hostbased_key_allowed(struct ssh *, struct passwd *, const char *, char *, struct sshkey *); -int user_key_allowed(struct ssh *, struct passwd *, struct sshkey *, int, - struct sshauthopt **); +int user_key_allowed(struct passwd *, struct sshkey *, int, + const char *, const char *, struct sshauthopt **); int auth2_key_already_used(Authctxt *, const struct sshkey *); /* @@ -195,8 +195,8 @@ int sshd_hostkey_sign(struct ssh *, struct sshkey *, struct sshkey *, const struct sshauthopt *auth_options(struct ssh *); int auth_activate_options(struct ssh *, struct sshauthopt *); void auth_restrict_session(struct ssh *); -int auth_authorise_keyopts(struct ssh *, struct passwd *pw, - struct sshauthopt *, int, const char *); +int auth_authorise_keyopts(struct passwd *pw, struct sshauthopt *, int, + const char *, const char *, const char *); void auth_log_authopts(const char *, const struct sshauthopt *, int); /* debug messages during authentication */ diff --git a/usr.bin/ssh/auth2-pubkey.c b/usr.bin/ssh/auth2-pubkey.c index 115cf9ed696..1a479fb32e5 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.113 2022/02/27 01:33:59 naddy Exp $ */ +/* $OpenBSD: auth2-pubkey.c,v 1.114 2022/05/27 05:01:25 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * @@ -97,6 +97,9 @@ userauth_pubkey(struct ssh *ssh, const char *method) int req_presence = 0, req_verify = 0, authenticated = 0; struct sshauthopt *authopts = NULL; struct sshkey_sig_details *sig_details = NULL; + const char *remote_ip = ssh_remote_ipaddr(ssh); + const char *remote_host = auth_get_canonical_hostname(ssh, + options.use_dns); hostbound = strcmp(method, "publickey-hostbound-v00@openssh.com") == 0; @@ -219,7 +222,8 @@ userauth_pubkey(struct ssh *ssh, const char *method) #endif /* test for correct signature */ authenticated = 0; - if (PRIVSEP(user_key_allowed(ssh, pw, key, 1, &authopts)) && + if (PRIVSEP(user_key_allowed(pw, key, 1, remote_ip, + remote_host, &authopts)) && PRIVSEP(sshkey_verify(key, sig, slen, sshbuf_ptr(b), sshbuf_len(b), (ssh->compat & SSH_BUG_SIGTYPE) == 0 ? pkalg : NULL, @@ -281,7 +285,8 @@ userauth_pubkey(struct ssh *ssh, const char *method) * if a user is not allowed to login. is this an * issue? -markus */ - if (PRIVSEP(user_key_allowed(ssh, pw, key, 0, NULL))) { + if (PRIVSEP(user_key_allowed(pw, key, 0, remote_ip, + remote_host, NULL))) { if ((r = sshpkt_start(ssh, SSH2_MSG_USERAUTH_PK_OK)) != 0 || (r = sshpkt_put_cstring(ssh, pkalg)) != 0 || @@ -339,7 +344,7 @@ match_principals_option(const char *principal_list, struct sshkey_cert *cert) * log preamble for file/line information. */ static int -check_principals_line(struct ssh *ssh, char *cp, const struct sshkey_cert *cert, +check_principals_line(char *cp, const struct sshkey_cert *cert, const char *loc, struct sshauthopt **authoptsp) { u_int i, found = 0; @@ -389,7 +394,7 @@ check_principals_line(struct ssh *ssh, char *cp, const struct sshkey_cert *cert, } static int -process_principals(struct ssh *ssh, FILE *f, const char *file, +process_principals(FILE *f, const char *file, const struct sshkey_cert *cert, struct sshauthopt **authoptsp) { char loc[256], *line = NULL, *cp, *ep; @@ -417,7 +422,7 @@ process_principals(struct ssh *ssh, FILE *f, const char *file, nonblank++; snprintf(loc, sizeof(loc), "%.200s:%lu", file, linenum); - if (check_principals_line(ssh, cp, cert, loc, authoptsp) == 0) + if (check_principals_line(cp, cert, loc, authoptsp) == 0) found_principal = 1; } debug2_f("%s: processed %lu/%lu lines", file, nonblank, linenum); @@ -428,7 +433,7 @@ process_principals(struct ssh *ssh, FILE *f, const char *file, /* XXX remove pw args here and elsewhere once ssh->authctxt is guaranteed */ static int -match_principals_file(struct ssh *ssh, struct passwd *pw, char *file, +match_principals_file(struct passwd *pw, char *file, struct sshkey_cert *cert, struct sshauthopt **authoptsp) { FILE *f; @@ -443,7 +448,7 @@ match_principals_file(struct ssh *ssh, struct passwd *pw, char *file, restore_uid(); return 0; } - success = process_principals(ssh, f, file, cert, authoptsp); + success = process_principals(f, file, cert, authoptsp); fclose(f); restore_uid(); return success; @@ -454,7 +459,7 @@ match_principals_file(struct ssh *ssh, struct passwd *pw, char *file, * returns 1 if the principal is allowed or 0 otherwise. */ static int -match_principals_command(struct ssh *ssh, struct passwd *user_pw, +match_principals_command(struct passwd *user_pw, const struct sshkey *key, struct sshauthopt **authoptsp) { struct passwd *runas_pw = NULL; @@ -559,7 +564,7 @@ match_principals_command(struct ssh *ssh, struct passwd *user_pw, uid_swapped = 1; temporarily_use_uid(runas_pw); - ok = process_principals(ssh, f, "(command)", cert, authoptsp); + ok = process_principals(f, "(command)", cert, authoptsp); fclose(f); f = NULL; @@ -593,8 +598,9 @@ match_principals_command(struct ssh *ssh, struct passwd *user_pw, * on success. "loc" is used as file/line location in log messages. */ static int -check_authkey_line(struct ssh *ssh, struct passwd *pw, struct sshkey *key, - char *cp, const char *loc, struct sshauthopt **authoptsp) +check_authkey_line(struct passwd *pw, struct sshkey *key, + char *cp, const char *remote_ip, const char *remote_host, const char *loc, + struct sshauthopt **authoptsp) { int want_keytype = sshkey_is_cert(key) ? KEY_UNSPEC : key->type; struct sshkey *found = NULL; @@ -654,8 +660,8 @@ check_authkey_line(struct ssh *ssh, struct passwd *pw, struct sshkey *key, debug("%s: matching %s found: %s %s", loc, sshkey_is_cert(key) ? "CA" : "key", sshkey_type(found), fp); - if (auth_authorise_keyopts(ssh, pw, keyopts, - sshkey_is_cert(key), loc) != 0) { + if (auth_authorise_keyopts(pw, keyopts, + sshkey_is_cert(key), remote_ip, remote_host, loc) != 0) { reason = "Refused by key options"; goto fail_reason; } @@ -677,7 +683,8 @@ check_authkey_line(struct ssh *ssh, struct passwd *pw, struct sshkey *key, reason = "Invalid certificate options"; goto fail_reason; } - if (auth_authorise_keyopts(ssh, pw, certopts, 0, loc) != 0) { + if (auth_authorise_keyopts(pw, certopts, 0, + remote_ip, remote_host, loc) != 0) { reason = "Refused by certificate options"; goto fail_reason; } @@ -733,8 +740,9 @@ check_authkey_line(struct ssh *ssh, struct passwd *pw, struct sshkey *key, * returns 1 if the key is allowed or 0 otherwise. */ static int -check_authkeys_file(struct ssh *ssh, struct passwd *pw, FILE *f, - char *file, struct sshkey *key, struct sshauthopt **authoptsp) +check_authkeys_file(struct passwd *pw, FILE *f, char *file, + struct sshkey *key, const char *remote_ip, + const char *remote_host, struct sshauthopt **authoptsp) { char *cp, *line = NULL, loc[256]; size_t linesize = 0; @@ -758,7 +766,8 @@ check_authkeys_file(struct ssh *ssh, struct passwd *pw, FILE *f, nonblank++; snprintf(loc, sizeof(loc), "%.200s:%lu", file, linenum); - if (check_authkey_line(ssh, pw, key, cp, loc, authoptsp) == 0) + if (check_authkey_line(pw, key, cp, + remote_ip, remote_host, loc, authoptsp) == 0) found_key = 1; } free(line); @@ -768,7 +777,8 @@ check_authkeys_file(struct ssh *ssh, struct passwd *pw, FILE *f, /* Authenticate a certificate key against TrustedUserCAKeys */ static int -user_cert_trusted_ca(struct ssh *ssh, struct passwd *pw, struct sshkey *key, +user_cert_trusted_ca(struct passwd *pw, struct sshkey *key, + const char *remote_ip, const char *remote_host, struct sshauthopt **authoptsp) { char *ca_fp, *principals_file = NULL; @@ -800,12 +810,12 @@ user_cert_trusted_ca(struct ssh *ssh, struct passwd *pw, struct sshkey *key, * against the username. */ if ((principals_file = authorized_principals_file(pw)) != NULL) { - if (match_principals_file(ssh, pw, principals_file, + if (match_principals_file(pw, principals_file, key->cert, &principals_opts)) found_principal = 1; } /* Try querying command if specified */ - if (!found_principal && match_principals_command(ssh, pw, key, + if (!found_principal && match_principals_command(pw, key, &principals_opts)) found_principal = 1; /* If principals file or command is specified, then require a match */ @@ -826,7 +836,8 @@ user_cert_trusted_ca(struct ssh *ssh, struct passwd *pw, struct sshkey *key, reason = "Invalid certificate options"; goto fail_reason; } - if (auth_authorise_keyopts(ssh, pw, cert_opts, 0, "cert") != 0) { + if (auth_authorise_keyopts(pw, cert_opts, 0, + remote_ip, remote_host, "cert") != 0) { reason = "Refused by certificate options"; goto fail_reason; } @@ -834,8 +845,8 @@ user_cert_trusted_ca(struct ssh *ssh, struct passwd *pw, struct sshkey *key, final_opts = cert_opts; cert_opts = NULL; } else { - if (auth_authorise_keyopts(ssh, pw, principals_opts, 0, - "principals") != 0) { + if (auth_authorise_keyopts(pw, principals_opts, 0, + remote_ip, remote_host, "principals") != 0) { reason = "Refused by certificate principals options"; goto fail_reason; } @@ -873,8 +884,9 @@ user_cert_trusted_ca(struct ssh *ssh, struct passwd *pw, struct sshkey *key, * returns 1 if the key is allowed or 0 otherwise. */ static int -user_key_allowed2(struct ssh *ssh, struct passwd *pw, struct sshkey *key, - char *file, struct sshauthopt **authoptsp) +user_key_allowed2(struct passwd *pw, struct sshkey *key, + char *file, const char *remote_ip, const char *remote_host, + struct sshauthopt **authoptsp) { FILE *f; int found_key = 0; @@ -887,8 +899,8 @@ user_key_allowed2(struct ssh *ssh, struct passwd *pw, struct sshkey *key, debug("trying public key file %s", file); if ((f = auth_openkeyfile(file, pw, options.strict_modes)) != NULL) { - found_key = check_authkeys_file(ssh, pw, f, file, - key, authoptsp); + found_key = check_authkeys_file(pw, f, file, + key, remote_ip, remote_host, authoptsp); fclose(f); } @@ -901,8 +913,9 @@ user_key_allowed2(struct ssh *ssh, struct passwd *pw, struct sshkey *key, * returns 1 if the key is allowed or 0 otherwise. */ static int -user_key_command_allowed2(struct ssh *ssh, struct passwd *user_pw, - struct sshkey *key, struct sshauthopt **authoptsp) +user_key_command_allowed2(struct passwd *user_pw, struct sshkey *key, + const char *remote_ip, const char *remote_host, + struct sshauthopt **authoptsp) { struct passwd *runas_pw = NULL; FILE *f = NULL; @@ -1002,8 +1015,9 @@ user_key_command_allowed2(struct ssh *ssh, struct passwd *user_pw, uid_swapped = 1; temporarily_use_uid(runas_pw); - ok = check_authkeys_file(ssh, user_pw, f, - options.authorized_keys_command, key, authoptsp); + ok = check_authkeys_file(user_pw, f, + options.authorized_keys_command, key, remote_ip, + remote_host, authoptsp); fclose(f); f = NULL; @@ -1033,8 +1047,9 @@ user_key_command_allowed2(struct ssh *ssh, struct passwd *user_pw, * Check whether key authenticates and authorises the user. */ int -user_key_allowed(struct ssh *ssh, struct passwd *pw, struct sshkey *key, - int auth_attempt, struct sshauthopt **authoptsp) +user_key_allowed(struct passwd *pw, struct sshkey *key, + int auth_attempt, const char *remote_ip, const char *remote_host, + struct sshauthopt **authoptsp) { u_int success = 0, i; char *file; @@ -1054,7 +1069,8 @@ user_key_allowed(struct ssh *ssh, struct passwd *pw, struct sshkey *key, continue; file = expand_authorized_keys( options.authorized_keys_files[i], pw); - success = user_key_allowed2(ssh, pw, key, file, &opts); + success = user_key_allowed2(pw, key, file, + remote_ip, remote_host, &opts); free(file); if (!success) { sshauthopt_free(opts); @@ -1064,12 +1080,14 @@ user_key_allowed(struct ssh *ssh, struct passwd *pw, struct sshkey *key, if (success) goto out; - if ((success = user_cert_trusted_ca(ssh, pw, key, &opts)) != 0) + if ((success = user_cert_trusted_ca(pw, key, remote_ip, remote_host, + &opts)) != 0) goto out; sshauthopt_free(opts); opts = NULL; - if ((success = user_key_command_allowed2(ssh, pw, key, &opts)) != 0) + if ((success = user_key_command_allowed2(pw, key, remote_ip, + remote_host, &opts)) != 0) goto out; sshauthopt_free(opts); opts = NULL; diff --git a/usr.bin/ssh/monitor.c b/usr.bin/ssh/monitor.c index baaa6219bbf..5f812f8f219 100644 --- a/usr.bin/ssh/monitor.c +++ b/usr.bin/ssh/monitor.c @@ -1,4 +1,4 @@ -/* $OpenBSD: monitor.c,v 1.232 2022/02/25 02:09:27 djm Exp $ */ +/* $OpenBSD: monitor.c,v 1.233 2022/05/27 05:01:25 djm Exp $ */ /* * Copyright 2002 Niels Provos * Copyright 2002 Markus Friedl @@ -902,6 +902,9 @@ mm_answer_keyallowed(struct ssh *ssh, int sock, struct sshbuf *m) u_int type = 0; int r, allowed = 0; struct sshauthopt *opts = NULL; + const char *remote_ip = ssh_remote_ipaddr(ssh); + const char *remote_host = auth_get_canonical_hostname(ssh, + options.use_dns); debug3_f("entering"); if ((r = sshbuf_get_u32(m, &type)) != 0 || @@ -927,8 +930,8 @@ mm_answer_keyallowed(struct ssh *ssh, int sock, struct sshbuf *m) if (!key_base_type_match(auth_method, key, options.pubkey_accepted_algos)) break; - allowed = user_key_allowed(ssh, authctxt->pw, key, - pubkey_auth_attempt, &opts); + allowed = user_key_allowed(authctxt->pw, key, + pubkey_auth_attempt, remote_ip, remote_host, &opts); break; case MM_HOSTKEY: auth_method = "hostbased"; diff --git a/usr.bin/ssh/monitor_wrap.c b/usr.bin/ssh/monitor_wrap.c index 3a7145aa6f4..b61ea1301ef 100644 --- a/usr.bin/ssh/monitor_wrap.c +++ b/usr.bin/ssh/monitor_wrap.c @@ -1,4 +1,4 @@ -/* $OpenBSD: monitor_wrap.c,v 1.123 2021/04/15 16:24:31 markus Exp $ */ +/* $OpenBSD: monitor_wrap.c,v 1.124 2022/05/27 05:01:25 djm Exp $ */ /* * Copyright 2002 Niels Provos * Copyright 2002 Markus Friedl @@ -411,8 +411,9 @@ mm_auth_password(struct ssh *ssh, char *password) } int -mm_user_key_allowed(struct ssh *ssh, struct passwd *pw, struct sshkey *key, - int pubkey_auth_attempt, struct sshauthopt **authoptp) +mm_user_key_allowed(struct passwd *pw, struct sshkey *key, + int pubkey_auth_attempt, const char *remote_ip, const char *remote_host, + struct sshauthopt **authoptp) { return (mm_key_allowed(MM_USERKEY, NULL, NULL, key, pubkey_auth_attempt, authoptp)); diff --git a/usr.bin/ssh/monitor_wrap.h b/usr.bin/ssh/monitor_wrap.h index e4bf0fa3116..912dfef0ad5 100644 --- a/usr.bin/ssh/monitor_wrap.h +++ b/usr.bin/ssh/monitor_wrap.h @@ -1,4 +1,4 @@ -/* $OpenBSD: monitor_wrap.h,v 1.47 2021/04/15 16:24:31 markus Exp $ */ +/* $OpenBSD: monitor_wrap.h,v 1.48 2022/05/27 05:01:25 djm Exp $ */ /* * Copyright 2002 Niels Provos @@ -54,8 +54,8 @@ char *mm_auth2_read_banner(void); int mm_auth_password(struct ssh *, char *); int mm_key_allowed(enum mm_keytype, const char *, const char *, struct sshkey *, int, struct sshauthopt **); -int mm_user_key_allowed(struct ssh *, struct passwd *, struct sshkey *, int, - struct sshauthopt **); +int mm_user_key_allowed(struct passwd *, struct sshkey *, int, + const char *, const char *, struct sshauthopt **); int mm_hostbased_key_allowed(struct ssh *, struct passwd *, const char *, const char *, struct sshkey *); int mm_sshkey_verify(const struct sshkey *, const u_char *, size_t, -- 2.20.1