openconnect_set_reported_os should reject illegal values

The OS values ('vpninfo->platname') that OpenConnect accepts are
historically derived from the Cisco AnyConnect protocol:
linux, linux-64, win, mac-intel, android, apple-ios.

In the Cisco AnyConnect protocol, the platname was sometimes sent verbatim
on the wire (see auth.c), and could perhaps be thought of as a potentially
free-form string.

However:
  (a) The platname value is now used in other protocols, requiring lookups to
      convert to the correct forms for those protocols (e.g., see
      auth-globalprotect.c).
  (b) Even in the Cisco AnyConnect protocol, the platname value has
      has to be handled with switch/if statements which can only correctly
      handle the limited set of known values.

Since only a limited set of values are actually *understood* by OpenConnect,
allowing arbitrary values to be provided—but silently ignored or
mishandled—leads to confusing errors, such as in
https://lists.infradead.org/pipermail/openconnect-devel/2021-December/005079.html.

In that case, a user specified '--os=windows' (incorrect) instead of
'--os=win' (correct).  This likely led to incorrect server behavior, but
OpenConnect silently accepted the incorrect value.

This patch modifies openconnect_set_reported_os to return -EINVAL if
any OS name other than one of the 6 legal values is specified.

Future improvements:

1. Replace the numerous repetitions of the literal OS values with something
   saner, like an enum type.
2. Consider retiring the Cisco-specific values altogether as part of the
   "Great Renaming"
   (https://gitlab.com/openconnect/openconnect/-/merge_requests/151).

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

diff --git a/library.c b/library.c
index 37b7f9bc80b38cbc768ed7e22fb61d80183d0190..d4f4683871b287cffa234eb278fa66773607b4f3 100644 (file)
--- a/library.c
+++ b/library.c
@@ -378,6 +378,8 @@ int openconnect_make_cstp_connection(struct openconnect_info *vpninfo)
 int openconnect_set_reported_os(struct openconnect_info *vpninfo,
                                const char *os)
 {
+       const char *allowed[] = {"linux", "linux-64", "win", "mac-intel", "android", "apple-ios"};
+
        if (!os) {
 #if defined(__APPLE__)
                os = "mac-intel";
@@ -388,8 +390,13 @@ int openconnect_set_reported_os(struct openconnect_info *vpninfo,
 #endif
        }
 
-       STRDUP(vpninfo->platname, os);
-       return 0;
+       for (int i = 0; i < ARRAY_SIZE(allowed); i++) {
+               if (!strcmp(os, allowed[i])) {
+                       STRDUP(vpninfo->platname, os);
+                       return 0;
+               }
+       }
+       return -EINVAL;
 }
 
 int openconnect_set_mobile_info(struct openconnect_info *vpninfo,

diff --git a/main.c b/main.c
index 9034e5c718a2542fa94e0ede9f9a688c21ea729d..67225b501dd46e555d0525db66179c49fd527fd0 100644 (file)
--- a/main.c
+++ b/main.c
@@ -948,7 +948,8 @@ static void usage(void)
        printf("\n%s:\n", _("Local system information"));
        printf("      --useragent=STRING          %s\n", _("HTTP header User-Agent: field"));
        printf("      --local-hostname=STRING     %s\n", _("Local hostname to advertise to server"));
-       printf("      --os=STRING                 %s\n", _("OS type (linux,linux-64,win,...) to report"));
+       printf("      --os=STRING                 %s\n", _("OS type to report. Allowed values are the following:"));
+       printf("                                  %s\n", _("linux, linux-64, win, mac-intel, android, apple-ios"));
        printf("      --version-string=STRING     %s\n", _("reported version string during authentication"));
        printf("                                  (%s %s)\n", _("default:"), openconnect_version_str);
 
@@ -2029,7 +2030,8 @@ int main(int argc, char **argv)
                case OPT_OS:
                        assert_nonnull_config_arg("os", config_arg);
                        if (openconnect_set_reported_os(vpninfo, config_arg)) {
-                               fprintf(stderr, _("Invalid OS identity \"%s\"\n"),
+                               fprintf(stderr, _("Invalid OS identity \"%s\"\n"
+                                                 "Allowed values: linux, linux-64, win, mac-intel, android, apple-ios\n"),
                                        config_arg);
                                exit(1);
                        }