switch sshd_config parsing to argv_split()
authordjm <djm@openbsd.org>
Tue, 8 Jun 2021 07:09:42 +0000 (07:09 +0000)
committerdjm <djm@openbsd.org>
Tue, 8 Jun 2021 07:09:42 +0000 (07:09 +0000)
similar to the previous commit, this switches sshd_config parsing to
the newer tokeniser. Config parsing will be a little stricter wrt
quote correctness and directives appearing without arguments.

feedback and ok markus@

tested in snaps for the last five or so days - thanks Theo and those who
caught bugs

usr.bin/ssh/servconf.c

index 98c02d7..abdc6ef 100644 (file)
@@ -1,5 +1,5 @@
 
-/* $OpenBSD: servconf.c,v 1.379 2021/04/03 06:18:40 djm Exp $ */
+/* $OpenBSD: servconf.c,v 1.380 2021/06/08 07:09:42 djm Exp $ */
 /*
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
  *                    All rights reserved
@@ -997,18 +997,29 @@ match_cfg_line(char **condition, int line, struct connection_info *ci)
                    ci->laddress ? ci->laddress : "(null)", ci->lport);
 
        while ((attrib = strdelim(&cp)) && *attrib != '\0') {
+               /* Terminate on comment */
+               if (*attrib == '#') {
+                       cp = NULL; /* mark all arguments consumed */
+                       break;
+               }
+               arg = NULL;
                attributes++;
+               /* Criterion "all" has no argument and must appear alone */
                if (strcasecmp(attrib, "all") == 0) {
-                       if (attributes != 1 ||
-                           ((arg = strdelim(&cp)) != NULL && *arg != '\0')) {
+                       if (attributes > 1 || ((arg = strdelim(&cp)) != NULL &&
+                           *arg != '\0' && *arg != '#')) {
                                error("'all' cannot be combined with other "
                                    "Match attributes");
                                return -1;
                        }
+                       if (arg != NULL && *arg == '#')
+                               cp = NULL; /* mark all arguments consumed */
                        *condition = cp;
                        return 1;
                }
-               if ((arg = strdelim(&cp)) == NULL || *arg == '\0') {
+               /* All other criteria require an argument */
+               if ((arg = strdelim(&cp)) == NULL ||
+                   *arg == '\0' || *arg == '#') {
                        error("Missing Match criteria for %s", attrib);
                        return -1;
                }
@@ -1203,7 +1214,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
     struct connection_info *connectinfo, int *inc_flags, int depth,
     struct include_list *includes)
 {
-       char ch, *cp, ***chararrayptr, **charptr, *arg, *arg2, *p;
+       char ch, *str, ***chararrayptr, **charptr, *arg, *arg2, *p, *keyword;
        int cmdline = 0, *intptr, value, value2, n, port, oactive, r, found;
        SyslogFacility *log_facility_ptr;
        LogLevel *log_level_ptr;
@@ -1215,6 +1226,9 @@ process_server_config_line_depth(ServerOptions *options, char *line,
        const char *errstr;
        struct include_item *item;
        glob_t gbuf;
+       char **oav = NULL, **av;
+       int oac = 0, ac;
+       int ret = -1;
 
        /* Strip trailing whitespace. Allow \f (form feed) at EOL only */
        if ((len = strlen(line)) == 0)
@@ -1225,46 +1239,59 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                line[len] = '\0';
        }
 
-       cp = line;
-       if ((arg = strdelim(&cp)) == NULL)
+       str = line;
+       if ((keyword = strdelim(&str)) == NULL)
                return 0;
        /* Ignore leading whitespace */
-       if (*arg == '\0')
-               arg = strdelim(&cp);
-       if (!arg || !*arg || *arg == '#')
+       if (*keyword == '\0')
+               keyword = strdelim(&str);
+       if (!keyword || !*keyword || *keyword == '#')
                return 0;
+       if (str == NULL || *str == '\0') {
+               error("%s line %d: no argument after keyword \"%s\"",
+                   filename, linenum, keyword);
+               return -1;
+       }
        intptr = NULL;
        charptr = NULL;
-       opcode = parse_token(arg, filename, linenum, &flags);
+       opcode = parse_token(keyword, filename, linenum, &flags);
+
+       if (argv_split(str, &oac, &oav, 1) != 0) {
+               error("%s line %d: invalid quotes", filename, linenum);
+               return -1;
+       }
+       ac = oac;
+       av = oav;
 
        if (activep == NULL) { /* We are processing a command line directive */
                cmdline = 1;
                activep = &cmdline;
        }
        if (*activep && opcode != sMatch && opcode != sInclude)
-               debug3("%s:%d setting %s %s", filename, linenum, arg, cp);
+               debug3("%s:%d setting %s %s", filename, linenum, keyword, str);
        if (*activep == 0 && !(flags & SSHCFG_MATCH)) {
                if (connectinfo == NULL) {
                        fatal("%s line %d: Directive '%s' is not allowed "
-                           "within a Match block", filename, linenum, arg);
+                           "within a Match block", filename, linenum, keyword);
                } else { /* this is a directive we have already processed */
-                       while (arg)
-                               arg = strdelim(&cp);
-                       return 0;
+                       ret = 0;
+                       goto out;
                }
        }
 
        switch (opcode) {
        case sBadOption:
-               return -1;
+               goto out;
        case sPort:
                /* ignore ports from configfile if cmdline specifies ports */
-               if (options->ports_from_cmdline)
-                       return 0;
+               if (options->ports_from_cmdline) {
+                       argv_consume(&ac);
+                       break;
+               }
                if (options->num_ports >= MAX_PORTS)
                        fatal("%s line %d: too many ports.",
                            filename, linenum);
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
                        fatal("%s line %d: missing port number.",
                            filename, linenum);
@@ -1277,7 +1304,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
        case sLoginGraceTime:
                intptr = &options->login_grace_time;
  parse_time:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
                        fatal("%s line %d: missing time value.",
                            filename, linenum);
@@ -1289,7 +1316,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                break;
 
        case sListenAddress:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (arg == NULL || *arg == '\0')
                        fatal("%s line %d: missing address",
                            filename, linenum);
@@ -1314,16 +1341,15 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                }
                /* Optional routing table */
                arg2 = NULL;
-               if ((arg = strdelim(&cp)) != NULL) {
+               if ((arg = argv_next(&ac, &av)) != NULL) {
                        if (strcmp(arg, "rdomain") != 0 ||
-                           (arg2 = strdelim(&cp)) == NULL)
+                           (arg2 = argv_next(&ac, &av)) == NULL)
                                fatal("%s line %d: bad ListenAddress syntax",
                                    filename, linenum);
                        if (!valid_rdomain(arg2))
                                fatal("%s line %d: bad routing domain",
                                    filename, linenum);
                }
-
                queue_listen_addr(options, p, arg2, port);
 
                break;
@@ -1332,7 +1358,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                intptr = &options->address_family;
                multistate_ptr = multistate_addressfamily;
  parse_multistate:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
                        fatal("%s line %d: missing argument.",
                            filename, linenum);
@@ -1351,7 +1377,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                break;
 
        case sHostKeyFile:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
                        fatal("%s line %d: missing file name.",
                            filename, linenum);
@@ -1363,7 +1389,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 
        case sHostKeyAgent:
                charptr = &options->host_key_agent;
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
                        fatal("%s line %d: missing socket name.",
                            filename, linenum);
@@ -1373,7 +1399,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                break;
 
        case sHostCertificate:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
                        fatal("%s line %d: missing file name.",
                            filename, linenum);
@@ -1384,7 +1410,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
        case sPidFile:
                charptr = &options->pid_file;
  parse_filename:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
                        fatal("%s line %d: missing file name.",
                            filename, linenum);
@@ -1427,7 +1453,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
        case sHostbasedAcceptedAlgorithms:
                charptr = &options->hostbased_accepted_algos;
  parse_pubkey_algos:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
                        fatal("%s line %d: Missing argument.",
                            filename, linenum);
@@ -1459,7 +1485,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
        case sPubkeyAuthOptions:
                intptr = &options->pubkey_auth_options;
                value = 0;
-               while ((arg = strdelim(&cp)) && *arg != '\0') {
+               while ((arg = argv_next(&ac, &av)) != NULL) {
                        if (strcasecmp(arg, "none") == 0)
                                continue;
                        if (strcasecmp(arg, "touch-required") == 0)
@@ -1467,9 +1493,9 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                        else if (strcasecmp(arg, "verify-required") == 0)
                                value |= PUBKEYAUTH_VERIFY_REQUIRED;
                        else {
-                               fatal("%s line %d: unsupported "
-                                   "PubkeyAuthOptions option %s",
-                                   filename, linenum, arg);
+                               error("%s line %d: unsupported %s option %s",
+                                   filename, linenum, keyword, arg);
+                               goto out;
                        }
                }
                if (*activep && *intptr == -1)
@@ -1531,10 +1557,10 @@ process_server_config_line_depth(ServerOptions *options, char *line,
        case sX11DisplayOffset:
                intptr = &options->x11_display_offset;
  parse_int:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if ((errstr = atoi_err(arg, &value)) != NULL)
-                       fatal("%s line %d: integer value %s.",
-                           filename, linenum, errstr);
+                       fatal("%s line %d: %s integer value %s.",
+                           filename, linenum, keyword, errstr);
                if (*activep && *intptr == -1)
                        *intptr = value;
                break;
@@ -1570,10 +1596,10 @@ process_server_config_line_depth(ServerOptions *options, char *line,
        case sPermitUserEnvironment:
                intptr = &options->permit_user_env;
                charptr = &options->permit_user_env_allowlist;
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
-                       fatal("%s line %d: missing argument.",
-                           filename, linenum);
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                value = 0;
                p = NULL;
                if (strcmp(arg, "yes") == 0)
@@ -1599,25 +1625,26 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                goto parse_multistate;
 
        case sRekeyLimit:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
-                       fatal("%.200s line %d: Missing argument.", filename,
-                           linenum);
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                if (strcmp(arg, "default") == 0) {
                        val64 = 0;
                } else {
                        if (scan_scaled(arg, &val64) == -1)
-                               fatal("%.200s line %d: Bad number '%s': %s",
-                                   filename, linenum, arg, strerror(errno));
+                               fatal("%.200s line %d: Bad %s number '%s': %s",
+                                   filename, linenum, keyword,
+                                   arg, strerror(errno));
                        if (val64 != 0 && val64 < 16)
-                               fatal("%.200s line %d: RekeyLimit too small",
-                                   filename, linenum);
+                               fatal("%.200s line %d: %s too small",
+                                   filename, linenum, keyword);
                }
                if (*activep && options->rekey_limit == -1)
                        options->rekey_limit = val64;
-               if (cp != NULL) { /* optional rekey interval present */
-                       if (strcmp(cp, "none") == 0) {
-                               (void)strdelim(&cp);    /* discard */
+               if (ac != 0) { /* optional rekey interval present */
+                       if (strcmp(av[0], "none") == 0) {
+                               (void)argv_next(&ac, &av);      /* discard */
                                break;
                        }
                        intptr = &options->rekey_interval;
@@ -1636,7 +1663,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 
        case sLogFacility:
                log_facility_ptr = &options->log_facility;
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                value = log_facility_number(arg);
                if (value == SYSLOG_FACILITY_NOT_SET)
                        fatal("%.200s line %d: unsupported log facility '%s'",
@@ -1647,7 +1674,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 
        case sLogLevel:
                log_level_ptr = &options->log_level;
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                value = log_level_number(arg);
                if (value == SYSLOG_LEVEL_NOT_SET)
                        fatal("%.200s line %d: unsupported log level '%s'",
@@ -1657,10 +1684,27 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                break;
 
        case sLogVerbose:
-               while ((arg = strdelim(&cp)) && *arg != '\0') {
-                       if (!*activep)
+               found = options->num_log_verbose == 0;
+               i = 0;
+               while ((arg = argv_next(&ac, &av)) != NULL) {
+                       if (*arg == '\0') {
+                               error("%s line %d: keyword %s empty argument",
+                                   filename, linenum, keyword);
+                               goto out;
+                       }
+                       /* Allow "none" only in first position */
+                       if (strcasecmp(arg, "none") == 0) {
+                               if (i > 0 || ac > 0) {
+                                       error("%s line %d: keyword %s \"none\" "
+                                           "argument must appear alone.",
+                                           filename, linenum, keyword);
+                                       goto out;
+                               }
+                       }
+                       i++;
+                       if (!found || !*activep)
                                continue;
-                       opt_array_append(filename, linenum, "oLogVerbose",
+                       opt_array_append(filename, linenum, keyword,
                            &options->log_verbose, &options->num_log_verbose,
                            arg);
                }
@@ -1685,55 +1729,51 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                goto parse_flag;
 
        case sAllowUsers:
-               while ((arg = strdelim(&cp)) && *arg != '\0') {
-                       if (match_user(NULL, NULL, NULL, arg) == -1)
-                               fatal("%s line %d: invalid AllowUsers pattern: "
-                                   "\"%.100s\"", filename, linenum, arg);
+               chararrayptr = &options->allow_users;
+               uintptr = &options->num_allow_users;
+ parse_allowdenyusers:
+               while ((arg = argv_next(&ac, &av)) != NULL) {
+                       if (*arg == '\0' ||
+                           match_user(NULL, NULL, NULL, arg) == -1)
+                               fatal("%s line %d: invalid %s pattern: \"%s\"",
+                                   filename, linenum, keyword, arg);
                        if (!*activep)
                                continue;
-                       opt_array_append(filename, linenum, "AllowUsers",
-                           &options->allow_users, &options->num_allow_users,
-                           arg);
+                       opt_array_append(filename, linenum, keyword,
+                           chararrayptr, uintptr, arg);
                }
                break;
 
        case sDenyUsers:
-               while ((arg = strdelim(&cp)) && *arg != '\0') {
-                       if (match_user(NULL, NULL, NULL, arg) == -1)
-                               fatal("%s line %d: invalid DenyUsers pattern: "
-                                   "\"%.100s\"", filename, linenum, arg);
-                       if (!*activep)
-                               continue;
-                       opt_array_append(filename, linenum, "DenyUsers",
-                           &options->deny_users, &options->num_deny_users,
-                           arg);
-               }
-               break;
+               chararrayptr = &options->deny_users;
+               uintptr = &options->num_deny_users;
+               goto parse_allowdenyusers;
 
        case sAllowGroups:
-               while ((arg = strdelim(&cp)) && *arg != '\0') {
+               chararrayptr = &options->allow_groups;
+               uintptr = &options->num_allow_groups;
+ parse_allowdenygroups:
+               while ((arg = argv_next(&ac, &av)) != NULL) {
+                       if (*arg == '\0')
+                               fatal("%s line %d: empty %s pattern",
+                                   filename, linenum, keyword);
                        if (!*activep)
                                continue;
-                       opt_array_append(filename, linenum, "AllowGroups",
-                           &options->allow_groups, &options->num_allow_groups,
-                           arg);
+                       opt_array_append(filename, linenum, keyword,
+                           chararrayptr, uintptr, arg);
                }
                break;
 
        case sDenyGroups:
-               while ((arg = strdelim(&cp)) && *arg != '\0') {
-                       if (!*activep)
-                               continue;
-                       opt_array_append(filename, linenum, "DenyGroups",
-                           &options->deny_groups, &options->num_deny_groups,
-                           arg);
-               }
-               break;
+               chararrayptr = &options->deny_groups;
+               uintptr = &options->num_deny_groups;
+               goto parse_allowdenygroups;
 
        case sCiphers:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
-                       fatal("%s line %d: Missing argument.", filename, linenum);
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                if (*arg != '-' &&
                    !ciphers_valid(*arg == '+' || *arg == '^' ? arg + 1 : arg))
                        fatal("%s line %d: Bad SSH2 cipher spec '%s'.",
@@ -1743,9 +1783,10 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                break;
 
        case sMacs:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
-                       fatal("%s line %d: Missing argument.", filename, linenum);
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                if (*arg != '-' &&
                    !mac_valid(*arg == '+' || *arg == '^' ? arg + 1 : arg))
                        fatal("%s line %d: Bad SSH2 mac spec '%s'.",
@@ -1755,10 +1796,10 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                break;
 
        case sKexAlgorithms:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
-                       fatal("%s line %d: Missing argument.",
-                           filename, linenum);
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                if (*arg != '-' &&
                    !kex_names_valid(*arg == '+' || *arg == '^' ?
                    arg + 1 : arg))
@@ -1773,20 +1814,20 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                        fatal("%s line %d: too many subsystems defined.",
                            filename, linenum);
                }
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
-                       fatal("%s line %d: Missing subsystem name.",
-                           filename, linenum);
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                if (!*activep) {
-                       arg = strdelim(&cp);
+                       arg = argv_next(&ac, &av);
                        break;
                }
                for (i = 0; i < options->num_subsystems; i++)
                        if (strcmp(arg, options->subsystem_name[i]) == 0)
-                               fatal("%s line %d: Subsystem '%s' already defined.",
-                                   filename, linenum, arg);
+                               fatal("%s line %d: Subsystem '%s' "
+                                   "already defined.", filename, linenum, arg);
                options->subsystem_name[options->num_subsystems] = xstrdup(arg);
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
                        fatal("%s line %d: Missing subsystem command.",
                            filename, linenum);
@@ -1795,7 +1836,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                /* Collect arguments (separate to executable) */
                p = xstrdup(arg);
                len = strlen(p) + 1;
-               while ((arg = strdelim(&cp)) != NULL && *arg != '\0') {
+               while ((arg = argv_next(&ac, &av)) != NULL) {
                        len += 1 + strlen(arg);
                        p = xreallocarray(p, 1, len);
                        strlcat(p, " ", len);
@@ -1806,10 +1847,10 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                break;
 
        case sMaxStartups:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
-                       fatal("%s line %d: Missing MaxStartups spec.",
-                           filename, linenum);
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                if ((n = sscanf(arg, "%d:%d:%d",
                    &options->max_startups_begin,
                    &options->max_startups_rate,
@@ -1818,20 +1859,20 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                            options->max_startups ||
                            options->max_startups_rate > 100 ||
                            options->max_startups_rate < 1)
-                               fatal("%s line %d: Illegal MaxStartups spec.",
-                                   filename, linenum);
+                               fatal("%s line %d: Invalid %s spec.",
+                                   filename, linenum, keyword);
                } else if (n != 1)
-                       fatal("%s line %d: Illegal MaxStartups spec.",
-                           filename, linenum);
+                       fatal("%s line %d: Invalid %s spec.",
+                           filename, linenum, keyword);
                else
                        options->max_startups = options->max_startups_begin;
                break;
 
        case sPerSourceNetBlockSize:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
-                       fatal("%s line %d: Missing PerSourceNetBlockSize spec.",
-                           filename, linenum);
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                switch (n = sscanf(arg, "%d:%d", &value, &value2)) {
                case 2:
                        if (value2 < 0 || value2 > 128)
@@ -1842,8 +1883,8 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                                n = -1;
                }
                if (n != 1 && n != 2)
-                       fatal("%s line %d: Invalid PerSourceNetBlockSize"
-                           " spec.", filename, linenum);
+                       fatal("%s line %d: Invalid %s spec.",
+                           filename, linenum, keyword);
                if (*activep) {
                        options->per_source_masklen_ipv4 = value;
                        options->per_source_masklen_ipv6 = value2;
@@ -1851,16 +1892,16 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                break;
 
        case sPerSourceMaxStartups:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
-                       fatal("%s line %d: Missing PerSourceMaxStartups spec.",
-                           filename, linenum);
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                if (strcmp(arg, "none") == 0) { /* no limit */
                        value = INT_MAX;
                } else {
                        if ((errstr = atoi_err(arg, &value)) != NULL)
-                               fatal("%s line %d: integer value %s.",
-                                   filename, linenum, errstr);
+                               fatal("%s line %d: %s integer value %s.",
+                                   filename, linenum, keyword, errstr);
                }
                if (*activep)
                        options->per_source_max_startups = value;
@@ -1885,24 +1926,29 @@ process_server_config_line_depth(ServerOptions *options, char *line,
         * AuthorizedKeysFile   /etc/ssh_keys/%u
         */
        case sAuthorizedKeysFile:
-               if (*activep && options->num_authkeys_files == 0) {
-                       while ((arg = strdelim(&cp)) && *arg != '\0') {
-                               arg = tilde_expand_filename(arg, getuid());
-                               opt_array_append(filename, linenum,
-                                   "AuthorizedKeysFile",
+               uvalue = options->num_authkeys_files;
+               while ((arg = argv_next(&ac, &av)) != NULL) {
+                       if (*arg == '\0') {
+                               error("%s line %d: keyword %s empty argument",
+                                   filename, linenum, keyword);
+                               goto out;
+                       }
+                       arg2 = tilde_expand_filename(arg, getuid());
+                       if (*activep && uvalue == 0) {
+                               opt_array_append(filename, linenum, keyword,
                                    &options->authorized_keys_files,
-                                   &options->num_authkeys_files, arg);
-                               free(arg);
+                                   &options->num_authkeys_files, arg2);
                        }
+                       free(arg2);
                }
-               return 0;
+               break;
 
        case sAuthorizedPrincipalsFile:
                charptr = &options->authorized_principals_file;
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
-                       fatal("%s line %d: missing file name.",
-                           filename, linenum);
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                if (*activep && *charptr == NULL) {
                        *charptr = tilde_expand_filename(arg, getuid());
                        /* increase optional counter */
@@ -1920,13 +1966,13 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                goto parse_int;
 
        case sAcceptEnv:
-               while ((arg = strdelim(&cp)) && *arg != '\0') {
-                       if (strchr(arg, '=') != NULL)
+               while ((arg = argv_next(&ac, &av)) != NULL) {
+                       if (*arg == '\0' || strchr(arg, '=') != NULL)
                                fatal("%s line %d: Invalid environment name.",
                                    filename, linenum);
                        if (!*activep)
                                continue;
-                       opt_array_append(filename, linenum, "AcceptEnv",
+                       opt_array_append(filename, linenum, keyword,
                            &options->accept_env, &options->num_accept_env,
                            arg);
                }
@@ -1934,23 +1980,23 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 
        case sSetEnv:
                uvalue = options->num_setenv;
-               while ((arg = strdelimw(&cp)) && *arg != '\0') {
-                       if (strchr(arg, '=') == NULL)
+               while ((arg = argv_next(&ac, &av)) != NULL) {
+                       if (*arg == '\0' || strchr(arg, '=') == NULL)
                                fatal("%s line %d: Invalid environment.",
                                    filename, linenum);
                        if (!*activep || uvalue != 0)
                                continue;
-                       opt_array_append(filename, linenum, "SetEnv",
+                       opt_array_append(filename, linenum, keyword,
                            &options->setenv, &options->num_setenv, arg);
                }
                break;
 
        case sPermitTunnel:
                intptr = &options->permit_tun;
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
-                       fatal("%s line %d: Missing yes/point-to-point/"
-                           "ethernet/no argument.", filename, linenum);
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                value = -1;
                for (i = 0; tunmode_desc[i].val != -1; i++)
                        if (strcmp(tunmode_desc[i].text, arg) == 0) {
@@ -1958,8 +2004,8 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                                break;
                        }
                if (value == -1)
-                       fatal("%s line %d: Bad yes/point-to-point/ethernet/"
-                           "no argument: %s", filename, linenum, arg);
+                       fatal("%s line %d: bad %s argument %s",
+                           filename, linenum, keyword, arg);
                if (*activep && *intptr == -1)
                        *intptr = value;
                break;
@@ -1970,7 +2016,12 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                            "command-line option");
                }
                value = 0;
-               while ((arg2 = strdelim(&cp)) != NULL && *arg2 != '\0') {
+               while ((arg2 = argv_next(&ac, &av)) != NULL) {
+                       if (*arg2 == '\0') {
+                               error("%s line %d: keyword %s empty argument",
+                                   filename, linenum, keyword);
+                               goto out;
+                       }
                        value++;
                        found = 0;
                        if (*arg2 != '/' && *arg2 != '~') {
@@ -2010,9 +2061,8 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                            filename, linenum, arg);
                        if ((r = glob(arg, 0, NULL, &gbuf)) != 0) {
                                if (r != GLOB_NOMATCH) {
-                                       fatal("%s line %d: include \"%s\" "
-                                           "glob failed", filename,
-                                           linenum, arg);
+                                       fatal("%s line %d: include \"%s\" glob "
+                                           "failed", filename, linenum, arg);
                                }
                                /*
                                 * If no entry matched then record a
@@ -2051,8 +2101,8 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                        free(arg);
                }
                if (value == 0) {
-                       fatal("%s line %d: Include missing filename argument",
-                           filename, linenum);
+                       fatal("%s line %d: %s missing filename argument",
+                           filename, linenum, keyword);
                }
                break;
 
@@ -2060,14 +2110,23 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                if (cmdline)
                        fatal("Match directive not supported as a command-line "
                            "option");
-               value = match_cfg_line(&cp, linenum,
+               value = match_cfg_line(&str, linenum,
                    (*inc_flags & SSHCFG_NEVERMATCH ? NULL : connectinfo));
                if (value < 0)
                        fatal("%s line %d: Bad Match condition", filename,
                            linenum);
                *activep = (*inc_flags & SSHCFG_NEVERMATCH) ? 0 : value;
-               /* The MATCH_ONLY is applicable only until the first match block */
+               /*
+                * The MATCH_ONLY flag is applicable only until the first
+                * match block.
+                */
                *inc_flags &= ~SSHCFG_MATCH_ONLY;
+               /*
+                * If match_cfg_line() didn't consume all its arguments then
+                * arrange for the extra arguments check below to fail.
+                */
+               if (str == NULL || *str == '\0')
+                       argv_consume(&ac);
                break;
 
        case sPermitListen:
@@ -2079,10 +2138,10 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                        uintptr = &options->num_permitted_opens;
                        chararrayptr = &options->permitted_opens;
                }
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
-                       fatal("%s line %d: missing %s specification",
-                           filename, linenum, lookup_opcode_name(opcode));
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                uvalue = *uintptr;      /* modified later */
                if (strcmp(arg, "any") == 0 || strcmp(arg, "none") == 0) {
                        if (*activep && uvalue == 0) {
@@ -2093,7 +2152,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                        }
                        break;
                }
-               for (; arg != NULL && *arg != '\0'; arg = strdelim(&cp)) {
+               for (; arg != NULL && *arg != '\0'; arg = argv_next(&ac, &av)) {
                        if (opcode == sPermitListen &&
                            strchr(arg, ':') == NULL) {
                                /*
@@ -2106,21 +2165,18 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                                ch = '\0';
                                p = hpdelim2(&arg, &ch);
                                if (p == NULL || ch == '/') {
-                                       fatal("%s line %d: missing host in %s",
-                                           filename, linenum,
-                                           lookup_opcode_name(opcode));
+                                       fatal("%s line %d: %s missing host",
+                                           filename, linenum, keyword);
                                }
                                p = cleanhostname(p);
                        }
                        if (arg == NULL ||
                            ((port = permitopen_port(arg)) < 0)) {
-                               fatal("%s line %d: bad port number in %s",
-                                   filename, linenum,
-                                   lookup_opcode_name(opcode));
+                               fatal("%s line %d: %s bad port number",
+                                   filename, linenum, keyword);
                        }
                        if (*activep && uvalue == 0) {
-                               opt_array_append(filename, linenum,
-                                   lookup_opcode_name(opcode),
+                               opt_array_append(filename, linenum, keyword,
                                    chararrayptr, uintptr, arg2);
                        }
                        free(arg2);
@@ -2128,21 +2184,22 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                break;
 
        case sForceCommand:
-               if (cp == NULL || *cp == '\0')
-                       fatal("%.200s line %d: Missing argument.", filename,
-                           linenum);
-               len = strspn(cp, WHITESPACE);
+               if (str == NULL || *str == '\0')
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
+               len = strspn(str, WHITESPACE);
                if (*activep && options->adm_forced_command == NULL)
-                       options->adm_forced_command = xstrdup(cp + len);
-               return 0;
+                       options->adm_forced_command = xstrdup(str + len);
+               argv_consume(&ac);
+               break;
 
        case sChrootDirectory:
                charptr = &options->chroot_directory;
 
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
-                       fatal("%s line %d: missing file name.",
-                           filename, linenum);
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                if (*activep && *charptr == NULL)
                        *charptr = xstrdup(arg);
                break;
@@ -2157,10 +2214,10 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 
        case sSecurityKeyProvider:
                charptr = &options->sk_provider;
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
-                       fatal("%s line %d: missing file name.",
-                           filename, linenum);
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                if (*activep && *charptr == NULL) {
                        *charptr = strcasecmp(arg, "internal") == 0 ?
                            xstrdup(arg) : derelativise_path(arg);
@@ -2171,16 +2228,19 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                break;
 
        case sIPQoS:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
+               if (!arg || *arg == '\0')
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                if ((value = parse_ipqos(arg)) == -1)
-                       fatal("%s line %d: Bad IPQoS value: %s",
-                           filename, linenum, arg);
-               arg = strdelim(&cp);
+                       fatal("%s line %d: Bad %s value: %s",
+                           filename, linenum, keyword, arg);
+               arg = argv_next(&ac, &av);
                if (arg == NULL)
                        value2 = value;
                else if ((value2 = parse_ipqos(arg)) == -1)
-                       fatal("%s line %d: Bad IPQoS value: %s",
-                           filename, linenum, arg);
+                       fatal("%s line %d: Bad %s value: %s",
+                           filename, linenum, keyword, arg);
                if (*activep) {
                        options->ip_qos_interactive = value;
                        options->ip_qos_bulk = value2;
@@ -2188,120 +2248,102 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                break;
 
        case sVersionAddendum:
-               if (cp == NULL || *cp == '\0')
-                       fatal("%.200s line %d: Missing argument.", filename,
-                           linenum);
-               len = strspn(cp, WHITESPACE);
+               if (str == NULL || *str == '\0')
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
+               len = strspn(str, WHITESPACE);
+               if (strchr(str + len, '\r') != NULL) {
+                       fatal("%.200s line %d: Invalid %s argument",
+                           filename, linenum, keyword);
+               }
+               if ((arg = strchr(line, '#')) != NULL) {
+                       *arg = '\0';
+                       rtrim(line);
+               }
                if (*activep && options->version_addendum == NULL) {
-                       if (strcasecmp(cp + len, "none") == 0)
+                       if (strcasecmp(str + len, "none") == 0)
                                options->version_addendum = xstrdup("");
-                       else if (strchr(cp + len, '\r') != NULL)
-                               fatal("%.200s line %d: Invalid argument",
-                                   filename, linenum);
                        else
-                               options->version_addendum = xstrdup(cp + len);
+                               options->version_addendum = xstrdup(str + len);
                }
-               return 0;
+               argv_consume(&ac);
+               break;
 
        case sAuthorizedKeysCommand:
-               if (cp == NULL)
-                       fatal("%.200s line %d: Missing argument.", filename,
-                           linenum);
-               len = strspn(cp, WHITESPACE);
-               if (*activep && options->authorized_keys_command == NULL) {
-                       if (cp[len] != '/' && strcasecmp(cp + len, "none") != 0)
-                               fatal("%.200s line %d: AuthorizedKeysCommand "
-                                   "must be an absolute path",
-                                   filename, linenum);
-                       options->authorized_keys_command = xstrdup(cp + len);
+               charptr = &options->authorized_keys_command;
+ parse_command:
+               len = strspn(str, WHITESPACE);
+               if (str[len] != '/' && strcasecmp(str + len, "none") != 0) {
+                       fatal("%.200s line %d: %s must be an absolute path",
+                           filename, linenum, keyword);
                }
-               return 0;
+               if (*activep && options->authorized_keys_command == NULL)
+                       *charptr = xstrdup(str + len);
+               argv_consume(&ac);
+               break;
 
        case sAuthorizedKeysCommandUser:
                charptr = &options->authorized_keys_command_user;
-
-               arg = strdelim(&cp);
-               if (!arg || *arg == '\0')
-                       fatal("%s line %d: missing AuthorizedKeysCommandUser "
-                           "argument.", filename, linenum);
+ parse_localuser:
+               arg = argv_next(&ac, &av);
+               if (!arg || *arg == '\0') {
+                       fatal("%s line %d: missing %s argument.",
+                           filename, linenum, keyword);
+               }
                if (*activep && *charptr == NULL)
                        *charptr = xstrdup(arg);
                break;
 
        case sAuthorizedPrincipalsCommand:
-               if (cp == NULL)
-                       fatal("%.200s line %d: Missing argument.", filename,
-                           linenum);
-               len = strspn(cp, WHITESPACE);
-               if (*activep &&
-                   options->authorized_principals_command == NULL) {
-                       if (cp[len] != '/' && strcasecmp(cp + len, "none") != 0)
-                               fatal("%.200s line %d: "
-                                   "AuthorizedPrincipalsCommand must be "
-                                   "an absolute path", filename, linenum);
-                       options->authorized_principals_command =
-                           xstrdup(cp + len);
-               }
-               return 0;
+               charptr = &options->authorized_principals_command;
+               goto parse_command;
 
        case sAuthorizedPrincipalsCommandUser:
                charptr = &options->authorized_principals_command_user;
-
-               arg = strdelim(&cp);
-               if (!arg || *arg == '\0')
-                       fatal("%s line %d: missing "
-                           "AuthorizedPrincipalsCommandUser argument.",
-                           filename, linenum);
-               if (*activep && *charptr == NULL)
-                       *charptr = xstrdup(arg);
-               break;
+               goto parse_localuser;
 
        case sAuthenticationMethods:
-               if (options->num_auth_methods == 0) {
-                       value = 0; /* seen "any" pseudo-method */
-                       value2 = 0; /* successfully parsed any method */
-                       while ((arg = strdelim(&cp)) && *arg != '\0') {
-                               if (strcmp(arg, "any") == 0) {
-                                       if (options->num_auth_methods > 0) {
-                                               fatal("%s line %d: \"any\" "
-                                                   "must appear alone in "
-                                                   "AuthenticationMethods",
-                                                   filename, linenum);
-                                       }
-                                       value = 1;
-                               } else if (value) {
-                                       fatal("%s line %d: \"any\" must appear "
-                                           "alone in AuthenticationMethods",
-                                           filename, linenum);
-                               } else if (auth2_methods_valid(arg, 0) != 0) {
-                                       fatal("%s line %d: invalid "
-                                           "authentication method list.",
-                                           filename, linenum);
+               found = options->num_auth_methods == 0;
+               value = 0; /* seen "any" pseudo-method */
+               value2 = 0; /* successfully parsed any method */
+               while ((arg = argv_next(&ac, &av)) != NULL) {
+                       if (strcmp(arg, "any") == 0) {
+                               if (options->num_auth_methods > 0) {
+                                       fatal("%s line %d: \"any\" must "
+                                           "appear alone in %s",
+                                           filename, linenum, keyword);
                                }
-                               value2 = 1;
-                               if (!*activep)
-                                       continue;
-                               opt_array_append(filename, linenum,
-                                   "AuthenticationMethods",
-                                   &options->auth_methods,
-                                   &options->num_auth_methods, arg);
-                       }
-                       if (value2 == 0) {
-                               fatal("%s line %d: no AuthenticationMethods "
-                                   "specified", filename, linenum);
+                               value = 1;
+                       } else if (value) {
+                               fatal("%s line %d: \"any\" must appear "
+                                   "alone in %s", filename, linenum, keyword);
+                       } else if (auth2_methods_valid(arg, 0) != 0) {
+                               fatal("%s line %d: invalid %s method list.",
+                                   filename, linenum, keyword);
                        }
+                       value2 = 1;
+                       if (!found || !*activep)
+                               continue;
+                       opt_array_append(filename, linenum, keyword,
+                           &options->auth_methods,
+                           &options->num_auth_methods, arg);
                }
-               return 0;
+               if (value2 == 0) {
+                       fatal("%s line %d: no %s specified",
+                           filename, linenum, keyword);
+               }
+               break;
 
        case sStreamLocalBindMask:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
-                       fatal("%s line %d: missing StreamLocalBindMask "
-                           "argument.", filename, linenum);
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                /* Parse mode in octal format */
                value = strtol(arg, &p, 8);
                if (arg == p || value < 0 || value > 0777)
-                       fatal("%s line %d: Bad mask.", filename, linenum);
+                       fatal("%s line %d: Invalid %s.",
+                           filename, linenum, keyword);
                if (*activep)
                        options->fwd_opts.streamlocal_bind_mask = (mode_t)value;
                break;
@@ -2311,13 +2353,13 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                goto parse_flag;
 
        case sFingerprintHash:
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
-                       fatal("%.200s line %d: Missing argument.",
-                           filename, linenum);
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                if ((value = ssh_digest_alg_by_name(arg)) == -1)
-                       fatal("%.200s line %d: Invalid hash algorithm \"%s\".",
-                           filename, linenum, arg);
+                       fatal("%.200s line %d: Invalid %s algorithm \"%s\".",
+                           filename, linenum, keyword, arg);
                if (*activep)
                        options->fingerprint_hash = value;
                break;
@@ -2328,13 +2370,13 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 
        case sRDomain:
                charptr = &options->routing_domain;
-               arg = strdelim(&cp);
+               arg = argv_next(&ac, &av);
                if (!arg || *arg == '\0')
-                       fatal("%.200s line %d: Missing argument.",
-                           filename, linenum);
+                       fatal("%s line %d: %s missing argument.",
+                           filename, linenum, keyword);
                if (strcasecmp(arg, "none") != 0 && strcmp(arg, "%D") != 0 &&
                    !valid_rdomain(arg))
-                       fatal("%s line %d: bad routing domain",
+                       fatal("%s line %d: invalid routing domain",
                            filename, linenum);
                if (*activep && *charptr == NULL)
                        *charptr = xstrdup(arg);
@@ -2346,19 +2388,27 @@ process_server_config_line_depth(ServerOptions *options, char *line,
                do_log2(opcode == sIgnore ?
                    SYSLOG_LEVEL_DEBUG2 : SYSLOG_LEVEL_INFO,
                    "%s line %d: %s option %s", filename, linenum,
-                   opcode == sUnsupported ? "Unsupported" : "Deprecated", arg);
-               while (arg)
-                   arg = strdelim(&cp);
+                   opcode == sUnsupported ? "Unsupported" : "Deprecated",
+                   keyword);
+               argv_consume(&ac);
                break;
 
        default:
                fatal("%s line %d: Missing handler for opcode %s (%d)",
-                   filename, linenum, arg, opcode);
+                   filename, linenum, keyword, opcode);
        }
-       if ((arg = strdelim(&cp)) != NULL && *arg != '\0')
-               fatal("%s line %d: garbage at end of line; \"%.200s\".",
-                   filename, linenum, arg);
-       return 0;
+       /* Check that there is no garbage at end of line. */
+       if (ac > 0) {
+               error("%.200s line %d: keyword %s extra arguments "
+                   "at end of line", filename, linenum, keyword);
+               goto out;
+       }
+
+       /* success */
+       ret = 0;
+ out:
+       argv_free(oav, oac);
+       return ret;
 }
 
 int
@@ -2397,12 +2447,10 @@ load_server_config(const char *filename, struct sshbuf *conf)
        while (getline(&line, &linesize, f) != -1) {
                lineno++;
                /*
-                * Trim out comments and strip whitespace
+                * Strip whitespace
                 * NB - preserve newlines, they are needed to reproduce
                 * line numbers later for error messages
                 */
-               if ((cp = strchr(line, '#')) != NULL)
-                       memcpy(cp, "\n", 2);
                cp = line + strspn(line, " \t\r");
                if ((r = sshbuf_put(conf, cp, strlen(cp))) != 0)
                        fatal_fr(r, "sshbuf_put");