cmdmon: make open commands configurable

Replace the hardcoded list of open commands (accessible over UDP),
with a list that can be configured with a new "opencommands" directive.
The default matches the original list. All read-only commands except
accheck and cmdaccheck can be enabled. The naming follows the chronyc
naming. Enable the N_SOURCES request only when needed.

This makes it possible to have a full monitoring access without access
to the Unix domain socket. It also allows restricting the monitoring
access to a smaller number of commands if some commands from the default
list are not needed.

Mention in the man page that the protocol of the non-default commands is
not consider stable and the information they provide may have security
implications.

diff --git a/cmdmon.c b/cmdmon.c
index ea562fc11a817a84ffae2dd4b5bd59b3cdb2a89f..57867c7debd96bb901f5258f8f8d092350ea2751 100644 (file)
--- a/cmdmon.c
+++ b/cmdmon.c
@@ -1507,25 +1507,16 @@ handle_readwrite_commands(int command, CMD_Request *request, CMD_Reply *reply)
 static int
 handle_readonly_commands(int command, int full_access, CMD_Request *request, CMD_Reply *reply)
 {
+  ARR_Instance open_commands;
   int i, allowed = 0;
 
-  const unsigned char open_commands[] = {
-    REQ_N_SOURCES,
-    REQ_SOURCE_DATA,
-    REQ_TRACKING,
-    REQ_SOURCESTATS,
-    REQ_RTCREPORT,
-    REQ_MANUAL_LIST,
-    REQ_ACTIVITY,
-    REQ_SMOOTHING,
-    REQ_NTP_SOURCE_NAME,
-  };
-
   if (full_access) {
     allowed = 1;
   } else {
-    for (i = 0; i < sizeof (open_commands); i++) {
-      if (open_commands[i] == command) {
+    open_commands = CNF_GetOpenCommands();
+
+    for (i = 0; i < ARR_GetSize(open_commands); i++) {
+      if (*(int *)ARR_GetElement(open_commands, i) == command) {
         allowed = 1;
         break;
       }

diff --git a/conf.c b/conf.c
index 8d2074928c8be52917020ede3510787f8087b816..5f60048042e3b5631c3d9c6ced341b83802d078e 100644 (file)
--- a/conf.c
+++ b/conf.c
@@ -80,6 +80,7 @@ static void parse_makestep(char *);
 static void parse_maxchange(char *);
 static void parse_ntsserver(char *, ARR_Instance files);
 static void parse_ntstrustedcerts(char *);
+static void parse_open_commands(char *line);
 static void parse_pidfile(char *line);
 static void parse_ratelimit(char *line, int *enabled, int *interval,
                             int *burst, int *leak, int *kod);
@@ -326,6 +327,11 @@ typedef struct _AllowDeny {
 static ARR_Instance ntp_restrictions;
 static ARR_Instance cmd_restrictions;
 
+#define DEFAULT_OPEN_COMMANDS "activity manual rtcdata smoothing sourcename sources sourcestats tracking"
+
+/* Array of int specifying commands allowed from network */
+static ARR_Instance open_commands;
+
 typedef struct {
   NTP_Remote_Address addr;
   int interval;
@@ -403,7 +409,7 @@ check_number_of_args(char *line, int num)
 void
 CNF_Initialise(int r, int client_only)
 {
-  char buf[10];
+  char buf[128];
 
   restarted = r;
 
@@ -418,6 +424,10 @@ CNF_Initialise(int r, int client_only)
   ntp_restrictions = ARR_CreateInstance(sizeof (AllowDeny));
   cmd_restrictions = ARR_CreateInstance(sizeof (AllowDeny));
 
+  open_commands = ARR_CreateInstance(sizeof (int));
+  snprintf(buf, sizeof (buf), DEFAULT_OPEN_COMMANDS);
+  parse_open_commands(buf);
+
   nts_aeads = ARR_CreateInstance(sizeof (int));
   snprintf(buf, sizeof (buf), DEFAULT_NTS_AEADS);
   parse_ints(buf, nts_aeads);
@@ -477,6 +487,8 @@ CNF_Finalise(void)
   ARR_DestroyInstance(refclock_sources);
   ARR_DestroyInstance(broadcasts);
 
+  ARR_DestroyInstance(open_commands);
+
   ARR_DestroyInstance(ntp_restrictions);
   ARR_DestroyInstance(cmd_restrictions);
 
@@ -715,6 +727,8 @@ CNF_ParseLine(const char *filename, int number, char *line)
     parse_ntsserver(p, nts_server_key_files);
   } else if (!strcasecmp(command, "ntstrustedcerts")) {
     parse_ntstrustedcerts(p);
+  } else if (!strcasecmp(command, "opencommands")) {
+    parse_open_commands(p);
   } else if (!strcasecmp(command, "peer")) {
     parse_source(p, command, 1);
   } else if (!strcasecmp(command, "pidfile")) {
@@ -1271,6 +1285,72 @@ parse_ntstrustedcerts(char *line)
 
 /* ================================================== */
 
+static void
+add_open_command(int command)
+{
+  int i;
+
+  /* Avoid duplicates */
+  for (i = 0; i < ARR_GetSize(open_commands); i++) {
+    if (*(int *)ARR_GetElement(open_commands, i) == command)
+      return;
+  }
+
+  ARR_AppendElement(open_commands, &command);
+}
+
+/* ================================================== */
+
+static void
+parse_open_commands(char *line)
+{
+  char *s;
+
+  ARR_SetSize(open_commands, 0);
+
+  while (*line) {
+    s = line;
+    line = CPS_SplitWord(line);
+
+    if (strcasecmp(s, "activity") == 0) {
+      add_open_command(REQ_ACTIVITY);
+    } else if (strcasecmp(s, "authdata") == 0) {
+      add_open_command(REQ_N_SOURCES);
+      add_open_command(REQ_AUTH_DATA);
+    } else if (strcasecmp(s, "clients") == 0) {
+      add_open_command(REQ_CLIENT_ACCESSES_BY_INDEX3);
+    } else if (strcasecmp(s, "manual") == 0) {
+      add_open_command(REQ_MANUAL_LIST);
+    } else if (strcasecmp(s, "ntpdata") == 0) {
+      add_open_command(REQ_N_SOURCES);
+      add_open_command(REQ_NTP_DATA);
+    } else if (strcasecmp(s, "rtcdata") == 0) {
+      add_open_command(REQ_RTCREPORT);
+    } else if (strcasecmp(s, "selectdata") == 0) {
+      add_open_command(REQ_N_SOURCES);
+      add_open_command(REQ_SELECT_DATA);
+    } else if (strcasecmp(s, "serverstats") == 0) {
+      add_open_command(REQ_SERVER_STATS);
+    } else if (strcasecmp(s, "smoothing") == 0) {
+      add_open_command(REQ_SMOOTHING);
+    } else if (strcasecmp(s, "sourcename") == 0) {
+      add_open_command(REQ_NTP_SOURCE_NAME);
+    } else if (strcasecmp(s, "sources") == 0) {
+      add_open_command(REQ_N_SOURCES);
+      add_open_command(REQ_SOURCE_DATA);
+    } else if (strcasecmp(s, "sourcestats") == 0) {
+      add_open_command(REQ_N_SOURCES);
+      add_open_command(REQ_SOURCESTATS);
+    } else if (strcasecmp(s, "tracking") == 0) {
+      add_open_command(REQ_TRACKING);
+    } else {
+      command_parse_error();
+    }
+  }
+}
+
+/* ================================================== */
+
 static void
 parse_allow_deny(char *line, ARR_Instance restrictions, int allow)
 {
@@ -2230,6 +2310,14 @@ CNF_GetManualEnabled(void)
 
 /* ================================================== */
 
+ARR_Instance
+CNF_GetOpenCommands(void)
+{
+  return open_commands;
+}
+
+/* ================================================== */
+
 int
 CNF_GetCommandPort(void) {
   return cmd_port;

diff --git a/conf.h b/conf.h
index e471f25fca33171b68cc46a4bd68ccc8b4080234..0eee0070474b54f1b267d897a9bdee56a2611eee 100644 (file)
--- a/conf.h
+++ b/conf.h
@@ -70,6 +70,7 @@ extern int CNF_GetLogTempComp(void);
 extern char *CNF_GetKeysFile(void);
 extern char *CNF_GetRtcFile(void);
 extern int CNF_GetManualEnabled(void);
+extern ARR_Instance CNF_GetOpenCommands(void);
 extern int CNF_GetCommandPort(void);
 extern int CNF_GetRtcOnUtc(void);
 extern int CNF_GetRtcSync(void);

diff --git a/doc/chrony.conf.adoc b/doc/chrony.conf.adoc
index 16967d5a60b90f1bfb9b878d64320a05535bb158..b1dbbf79651709d32671c3edf29958b55c946b16 100644 (file)
--- a/doc/chrony.conf.adoc
+++ b/doc/chrony.conf.adoc
@@ -2143,6 +2143,27 @@ An example of the use of the directive is:
 cmdratelimit interval 2
 ----
 
+[[opencommands]]*opencommands* [_command_]...::
+This directive specifies a list of monitoring commands to be enabled for the
+hosts allowed by the *cmdallow* directive. The following commands can be
+specified (the naming follows *chronyc*):
++
+*activity*+*+, *authdata*, *clients*, *manual*+*+, *ntpdata*, *rtcdata*+*+,
+*selectdata*, *serverstats*+*+, *smoothing*+*+, *sourcename*+*+, *sources*+*+,
+*sourcestats*, *tracking*+*+.
++
+The commands marked with +*+ are enabled by default. The protocol of these
+commands is considered stable and can be expected to work between different
+versions of *chronyc* and *chronyd*. The protocol of the other commands is not
+considered stable and different versions of *chronyc* and *chronyd* may not
+interoperate. When that happens, *chronyc* will print an '`Invalid command`' or
+'`Bad reply from daemon`' error.
++
+Note that some of the reported data can be potentially useful to attackers,
+enabling them to better observe and predict the internal state of *chronyd*.
+It is recommended to enable only commands that are actually needed for
+monitoring and limit the access to the hosts that need it.
+
 === Real-time clock (RTC)
 
 [[hwclockfile]]*hwclockfile* _file_::

diff --git a/doc/chronyc.adoc b/doc/chronyc.adoc
index dea93c9f567349aef582de969e94766bc5dbbf61..b3692f43ba6b94087cd8e2c1f1c1cad4d9990283 100644 (file)
--- a/doc/chronyc.adoc
+++ b/doc/chronyc.adoc
@@ -50,7 +50,7 @@ running under a non-root user), it will try to connect to 127.0.0.1 and then
 ::1.
 
 Only the following monitoring commands, which do not affect the behaviour of
-*chronyd*, are allowed from the network: *activity*, *manual list*,
+*chronyd*, are allowed from the network by default: *activity*, *manual list*,
 *rtcdata*, *smoothing*, *sourcename*, *sources*, *sourcestats*, *tracking*,
 *waitsync*. The
 set of hosts from which *chronyd* will accept these commands can be configured
@@ -58,7 +58,10 @@ with the <<chrony.conf.adoc#cmdallow,*cmdallow*>> directive in the *chronyd*'s
 configuration file or the <<cmdallow,*cmdallow*>> command in *chronyc*. By
 default, the commands are accepted only from localhost (127.0.0.1 or ::1).
 
-All other commands are allowed only through the Unix domain socket. When sent
+Other monitoring commands can be enabled for network access by the
+<<chrony.conf.adoc#opencommands,*opencommands*>> directive. Monitoring commands
+with disabled network access and commands that affect the behaviour of
+*chronyd* are allowed only through the Unix domain socket. If they are sent
 over the network, *chronyd* will respond with a '`Not authorised`' error, even
 if it is from localhost.
 

diff --git a/test/simulation/110-chronyc b/test/simulation/110-chronyc
index 5b3293169e91c3f4faeb3735c84e5e06c27c9e0d..e096cd03d92ebd07c29b8f48684aa4f49cfa0154 100755 (executable)
--- a/test/simulation/110-chronyc
+++ b/test/simulation/110-chronyc
@@ -195,6 +195,33 @@ do
        check_chronyc_output "501 Not authorised$" || test_fail
 done
 
+for chronyc_conf in \
+       "activity" \
+       "authdata" \
+       "clients" \
+       "manual list" \
+       "ntpdata" \
+       "rtcdata" \
+       "selectdata" \
+       "serverstats" \
+       "smoothing" \
+       "sourcename 192.168.123.1" \
+       "sources" \
+       "sourcestats" \
+       "tracking"
+do
+       server_conf="opencommands ${chronyc_conf% *}"
+       run_test || test_fail
+       check_chronyd_exit || test_fail
+       check_chronyc_output "501 Not authorised$" && test_fail
+
+       server_conf="opencommands"
+       run_test || test_fail
+       check_chronyd_exit || test_fail
+       check_chronyc_output "501 Not authorised$" || test_fail
+done
+
+server_conf="server 192.168.123.1"
 cmdmon_unix=1
 
 chronyc_conf="