From 1967fbf1f2e8f3534f325a6c9e2e708a534d8f63 Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Tue, 11 Feb 2025 12:27:23 +0100 Subject: [PATCH] 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. --- cmdmon.c | 19 +++----- conf.c | 90 ++++++++++++++++++++++++++++++++++++- conf.h | 1 + doc/chrony.conf.adoc | 21 +++++++++ doc/chronyc.adoc | 7 ++- test/simulation/110-chronyc | 27 +++++++++++ 6 files changed, 148 insertions(+), 17 deletions(-) diff --git a/cmdmon.c b/cmdmon.c index ea562fc1..57867c7d 100644 --- 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 8d207492..5f600480 100644 --- 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 e471f25f..0eee0070 100644 --- 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 16967d5a..b1dbbf79 100644 --- 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 dea93c9f..b3692f43 100644 --- 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 <> directive in the *chronyd*'s configuration file or the <> 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 +<> 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 5b329316..e096cd03 100755 --- 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=" -- 2.47.2