5 files changed, 45 insertions, 13 deletions

diff --git a/ChangeLog b/ChangeLog
index ff81d0522..2cb2dce0c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -23,6 +23,13 @@
    - dtucker@cvs.openbsd.org 2011/05/20 02:00:19
      [servconf.c]
      Add comment documenting what should be after the preauth check.  ok djm
+   - djm@cvs.openbsd.org 2011/05/20 03:25:45
+     [monitor.c monitor_wrap.c servconf.c servconf.h]
+     use a macro to define which string options to copy between configs
+     for Match. This avoids problems caused by forgetting to keep three
+     code locations in perfect sync and ordering
+     
+     "this is at once beautiful and horrible" + ok dtucker@
 
 20110515
  - (djm) OpenBSD CVS Sync
diff --git a/monitor.c b/monitor.c
index 732cb365d..c3a418319 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor.c,v 1.111 2011/05/15 08:09:01 djm Exp $ */
+/* $OpenBSD: monitor.c,v 1.112 2011/05/20 03:25:45 djm Exp $ */
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * Copyright 2002 Markus Friedl <markus@openbsd.org>
@@ -671,8 +671,15 @@ mm_answer_pwnamallow(int sock, Buffer *m)
 
  out:
         buffer_put_string(m, &options, sizeof(options));
-        if (options.banner != NULL)
-                buffer_put_cstring(m, options.banner);
+
+#define M_CP_STROPT(x) do { \
+                if (options.x != NULL) \
+                        buffer_put_cstring(m, options.x); \
+        } while (0)
+        /* See comment in servconf.h */
+        COPY_MATCH_STRING_OPTS();
+#undef M_CP_STROPT
+        
         debug3("%s: sending MONITOR_ANS_PWNAM: %d", __func__, allowed);
         mm_request_send(sock, MONITOR_ANS_PWNAM, m);
 
diff --git a/monitor_wrap.c b/monitor_wrap.c
index 1a5dda561..d3f274021 100644
--- a/monitor_wrap.c
+++ b/monitor_wrap.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor_wrap.c,v 1.70 2010/08/31 11:54:45 djm Exp $ */
+/* $OpenBSD: monitor_wrap.c,v 1.71 2011/05/20 03:25:45 djm Exp $ */
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * Copyright 2002 Markus Friedl <markus@openbsd.org>
@@ -245,8 +245,15 @@ out:
         newopts = buffer_get_string(&m, &len);
         if (len != sizeof(*newopts))
                 fatal("%s: option block size mismatch", __func__);
-        if (newopts->banner != NULL)
-                newopts->banner = buffer_get_string(&m, NULL);
+
+#define M_CP_STROPT(x) do { \
+                if (newopts->x != NULL) \
+                        newopts->x = buffer_get_string(&m, NULL); \
+        } while (0)
+        /* See comment in servconf.h */
+        COPY_MATCH_STRING_OPTS();
+#undef M_CP_STROPT
+
         copy_set_server_options(&options, newopts, 1);
         xfree(newopts);
 
diff --git a/servconf.c b/servconf.c
index 04a32973e..daed26a66 100644
--- a/servconf.c
+++ b/servconf.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: servconf.c,v 1.217 2011/05/20 02:00:19 dtucker Exp $ */
+/* $OpenBSD: servconf.c,v 1.218 2011/05/20 03:25:45 djm Exp $ */
 /*
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
  *                    All rights reserved
@@ -1499,11 +1499,8 @@ copy_set_server_options(ServerOptions *dst, ServerOptions *src, int preauth)
         M_CP_INTOPT(ip_qos_interactive);
         M_CP_INTOPT(ip_qos_bulk);
 
-        M_CP_STROPT(banner);
-        M_CP_STROPT(trusted_user_ca_keys);
-        M_CP_STROPT(revoked_keys_file);
-        M_CP_STROPT(authorized_keys_file);
-        M_CP_STROPT(authorized_principals_file);
+        /* See comment in servconf.h */
+        COPY_MATCH_STRING_OPTS();
 
         /*
          * The only things that should be below this point are string options
diff --git a/servconf.h b/servconf.h
index 3f04b8501..953ef8650 100644
--- a/servconf.h
+++ b/servconf.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: servconf.h,v 1.96 2011/05/11 04:47:06 djm Exp $ */
+/* $OpenBSD: servconf.h,v 1.97 2011/05/20 03:25:45 djm Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -161,6 +161,20 @@ typedef struct {
         char   *authorized_principals_file;
 }       ServerOptions;
 
+/*
+ * These are string config options that must be copied between the
+ * Match sub-config and the main config, and must be sent from the
+ * privsep slave to the privsep master. We use a macro to ensure all
+ * the options are copied and the copies are done in the correct order.
+ */
+#define COPY_MATCH_STRING_OPTS() do { \
+                M_CP_STROPT(banner); \
+                M_CP_STROPT(trusted_user_ca_keys); \
+                M_CP_STROPT(revoked_keys_file); \
+                M_CP_STROPT(authorized_keys_file); \
+                M_CP_STROPT(authorized_principals_file); \
+        } while (0)
+
 void     initialize_server_options(ServerOptions *);
 void     fill_default_server_options(ServerOptions *);
 int      process_server_config_line(ServerOptions *, char *, const char *, int,