From fd6dcef2030d23c43f986d26979f84619c10589d Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Wed, 30 Nov 2016 02:57:40 +0000 Subject: upstream commit When a forced-command appears in both a certificate and an authorized keys/principals command= restriction, refuse to accept the certificate unless they are identical. The previous (documented) behaviour of having the certificate forced- command override the other could be a bit confused and more error-prone. Pointed out by Jann Horn of Project Zero; ok dtucker@ Upstream-ID: 79d811b6eb6bbe1221bf146dde6928f92d2cd05f --- auth-options.c | 27 +++++++++++++++++++++------ auth-options.h | 4 ++-- auth2-pubkey.c | 18 ++++++++---------- sshd.8 | 18 +++++++++++++----- 4 files changed, 44 insertions(+), 23 deletions(-) diff --git a/auth-options.c b/auth-options.c index b399b91e..57b49f7f 100644 --- a/auth-options.c +++ b/auth-options.c @@ -1,4 +1,4 @@ -/* $OpenBSD: auth-options.c,v 1.71 2016/03/07 19:02:43 djm Exp $ */ +/* $OpenBSD: auth-options.c,v 1.72 2016/11/30 02:57:40 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -601,7 +601,7 @@ parse_option_list(struct sshbuf *oblob, struct passwd *pw, * options so this must be called after auth_parse_options(). */ int -auth_cert_options(struct sshkey *k, struct passwd *pw) +auth_cert_options(struct sshkey *k, struct passwd *pw, const char **reason) { int cert_no_port_forwarding_flag = 1; int cert_no_agent_forwarding_flag = 1; @@ -611,6 +611,8 @@ auth_cert_options(struct sshkey *k, struct passwd *pw) char *cert_forced_command = NULL; int cert_source_address_done = 0; + *reason = "invalid certificate options"; + /* Separate options and extensions for v01 certs */ if (parse_option_list(k->cert->critical, pw, OPTIONS_CRITICAL, 1, NULL, NULL, NULL, NULL, NULL, @@ -632,11 +634,24 @@ auth_cert_options(struct sshkey *k, struct passwd *pw) no_x11_forwarding_flag |= cert_no_x11_forwarding_flag; no_pty_flag |= cert_no_pty_flag; no_user_rc |= cert_no_user_rc; - /* CA-specified forced command supersedes key option */ - if (cert_forced_command != NULL) { - free(forced_command); + /* + * Only permit both CA and key option forced-command if they match. + * Otherwise refuse the certificate. + */ + if (cert_forced_command != NULL && forced_command != NULL) { + if (strcmp(forced_command, cert_forced_command) == 0) { + free(forced_command); + forced_command = cert_forced_command; + } else { + *reason = "certificate and key options forced command " + "do not match"; + free(cert_forced_command); + return -1; + } + } else if (cert_forced_command != NULL) forced_command = cert_forced_command; - } + /* success */ + *reason = NULL; return 0; } diff --git a/auth-options.h b/auth-options.h index 34852e5c..52cbb42a 100644 --- a/auth-options.h +++ b/auth-options.h @@ -1,4 +1,4 @@ -/* $OpenBSD: auth-options.h,v 1.21 2015/01/14 10:30:34 markus Exp $ */ +/* $OpenBSD: auth-options.h,v 1.22 2016/11/30 02:57:40 djm Exp $ */ /* * Author: Tatu Ylonen @@ -35,6 +35,6 @@ extern char *authorized_principals; int auth_parse_options(struct passwd *, char *, char *, u_long); void auth_clear_options(void); -int auth_cert_options(struct sshkey *, struct passwd *); +int auth_cert_options(struct sshkey *, struct passwd *, const char **); #endif diff --git a/auth2-pubkey.c b/auth2-pubkey.c index 375d91cb..20f3309e 100644 --- a/auth2-pubkey.c +++ b/auth2-pubkey.c @@ -1,4 +1,4 @@ -/* $OpenBSD: auth2-pubkey.c,v 1.59 2016/09/21 17:44:20 djm Exp $ */ +/* $OpenBSD: auth2-pubkey.c,v 1.60 2016/11/30 02:57:40 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * @@ -757,17 +757,17 @@ static int check_authkeys_file(FILE *f, char *file, Key* key, struct passwd *pw) { char line[SSH_MAX_PUBKEY_BYTES]; - const char *reason; int found_key = 0; u_long linenum = 0; Key *found; - char *fp; found_key = 0; found = NULL; while (read_keyfile_line(f, file, line, sizeof(line), &linenum) != -1) { - char *cp, *key_options = NULL; + char *cp, *key_options = NULL, *fp = NULL; + const char *reason = NULL; + if (found != NULL) key_free(found); found = key_new(key_is_cert(key) ? KEY_UNSPEC : key->type); @@ -832,10 +832,8 @@ check_authkeys_file(FILE *f, char *file, Key* key, struct passwd *pw) authorized_principals == NULL ? pw->pw_name : NULL, &reason) != 0) goto fail_reason; - if (auth_cert_options(key, pw) != 0) { - free(fp); - continue; - } + if (auth_cert_options(key, pw, &reason) != 0) + goto fail_reason; verbose("Accepted certificate ID \"%s\" (serial %llu) " "signed by %s CA %s via %s", key->cert->key_id, (unsigned long long)key->cert->serial, @@ -913,8 +911,8 @@ user_cert_trusted_ca(struct passwd *pw, Key *key) if (key_cert_check_authority(key, 0, 1, use_authorized_principals ? NULL : pw->pw_name, &reason) != 0) goto fail_reason; - if (auth_cert_options(key, pw) != 0) - goto out; + if (auth_cert_options(key, pw, &reason) != 0) + goto fail_reason; verbose("Accepted certificate ID \"%s\" (serial %llu) signed by " "%s CA %s via %s", key->cert->key_id, diff --git a/sshd.8 b/sshd.8 index 4cf8fee4..41fc5051 100644 --- a/sshd.8 +++ b/sshd.8 @@ -33,8 +33,8 @@ .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. .\" -.\" $OpenBSD: sshd.8,v 1.286 2016/08/19 03:18:06 djm Exp $ -.Dd $Mdocdate: August 19 2016 $ +.\" $OpenBSD: sshd.8,v 1.287 2016/11/30 02:57:40 djm Exp $ +.Dd $Mdocdate: November 30 2016 $ .Dt SSHD 8 .Os .Sh NAME @@ -481,19 +481,27 @@ If an 8-bit clean channel is required, one must not request a pty or should specify .Cm no-pty . A quote may be included in the command by quoting it with a backslash. +.Pp This option might be useful to restrict certain public keys to perform just a specific operation. An example might be a key that permits remote backups but nothing else. Note that the client may specify TCP and/or X11 -forwarding unless they are explicitly prohibited. +forwarding unless they are explicitly prohibited, e.g. using the +.Cm restrict +key option. +.Pp The command originally supplied by the client is available in the .Ev SSH_ORIGINAL_COMMAND environment variable. Note that this option applies to shell, command or subsystem execution. -Also note that this command may be superseded by either a +Also note that this command may be superseded by a .Xr sshd_config 5 .Cm ForceCommand -directive or a command embedded in a certificate. +directive. +.Pp +If a command is specified and a forced-command is embedded in a certificate +used for authentication, then the certificate will be accepted only if the +two commands are identical. .It Cm environment="NAME=value" Specifies that the string is to be added to the environment when logging in using this key. -- cgit v1.2.3