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

1 files changed, 21 insertions, 6 deletions

diff --git a/auth-options.c b/auth-options.c
index b399b91e3..57b49f7fd 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 <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, 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;
 }