config: Get rid of the dynamic seen array

* This makes the array constant in the session structure, avoiding
   allocations and frees while parsing the file
 * It also drops passing the seen array to all the functions,
   because it is already part of the passed session
 * The test cases are adjusted to match these changes

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>

include/libssh/config.h

@@ -0,0 +1,66 @@
+/*
+ * config.h - parse the ssh config file
+ *
+ * This file is part of the SSH Library
+ *
+ * Copyright (c) 2009-2018    by Andreas Schneider <asn@cryptomilk.org>
+ *
+ * The SSH Library is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or (at your
+ * option) any later version.
+ *
+ * The SSH Library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public
+ * License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with the SSH Library; see the file COPYING.  If not, write to
+ * the Free Software Foundation, Inc., 59 Temple Place - Suite 330, Boston,
+ * MA 02111-1307, USA.
+ */
+
+#ifndef LIBSSH_CONFIG_H_
+#define LIBSSH_CONFIG_H_
+
+
+enum ssh_config_opcode_e {
+    /* Unknown opcode */
+    SOC_UNKNOWN = -3,
+    /* Known and not applicable to libssh */
+    SOC_NA = -2,
+    /* Known but not supported by current libssh version */
+    SOC_UNSUPPORTED = -1,
+    SOC_HOST,
+    SOC_MATCH,
+    SOC_HOSTNAME,
+    SOC_PORT,
+    SOC_USERNAME,
+    SOC_IDENTITY,
+    SOC_CIPHERS,
+    SOC_MACS,
+    SOC_COMPRESSION,
+    SOC_TIMEOUT,
+    SOC_PROTOCOL,
+    SOC_STRICTHOSTKEYCHECK,
+    SOC_KNOWNHOSTS,
+    SOC_PROXYCOMMAND,
+    SOC_GSSAPISERVERIDENTITY,
+    SOC_GSSAPICLIENTIDENTITY,
+    SOC_GSSAPIDELEGATECREDENTIALS,
+    SOC_INCLUDE,
+    SOC_BINDADDRESS,
+    SOC_GLOBALKNOWNHOSTSFILE,
+    SOC_LOGLEVEL,
+    SOC_HOSTKEYALGORITHMS,
+    SOC_KEXALGORITHMS,
+    SOC_GSSAPIAUTHENTICATION,
+    SOC_KBDINTERACTIVEAUTHENTICATION,
+    SOC_PASSWORDAUTHENTICATION,
+    SOC_PUBKEYAUTHENTICATION,
+    SOC_PUBKEYACCEPTEDTYPES,
+
+    SOC_MAX /* Keep this one last in the list */
+};
+#endif /* LIBSSH_CONFIG_H_ */

include/libssh/session.h

@@ -29,6 +29,7 @@
 #include "libssh/auth.h"
 #include "libssh/channels.h"
 #include "libssh/poll.h"
+#include "libssh/config.h"
 
 /* These are the different states a SSH session can be into its life */
 enum ssh_session_state_e {
@@ -220,7 +221,7 @@ struct ssh_session_struct {
         int flags;
         int nodelay;
         bool config_processed;
-        uint8_t *options_seen;
+        uint8_t options_seen[SOC_MAX];
     } opts;
     /* counters */
     ssh_counter socket_counter;

src/config.c

@@ -32,6 +32,7 @@
 #endif
 #include <stdbool.h>
 
+#include "libssh/config.h"
 #include "libssh/priv.h"
 #include "libssh/session.h"
 #include "libssh/misc.h"
@@ -39,45 +40,6 @@
 
 #define MAX_LINE_SIZE 1024
 
-enum ssh_config_opcode_e {
-  /* Unknown opcode */
-  SOC_UNKNOWN = -3,
-  /* Known and not applicable to libssh */
-  SOC_NA = -2,
-  /* Known but not supported by current libssh version */
-  SOC_UNSUPPORTED = -1,
-  SOC_HOST,
-  SOC_MATCH,
-  SOC_HOSTNAME,
-  SOC_PORT,
-  SOC_USERNAME,
-  SOC_IDENTITY,
-  SOC_CIPHERS,
-  SOC_MACS,
-  SOC_COMPRESSION,
-  SOC_TIMEOUT,
-  SOC_PROTOCOL,
-  SOC_STRICTHOSTKEYCHECK,
-  SOC_KNOWNHOSTS,
-  SOC_PROXYCOMMAND,
-  SOC_GSSAPISERVERIDENTITY,
-  SOC_GSSAPICLIENTIDENTITY,
-  SOC_GSSAPIDELEGATECREDENTIALS,
-  SOC_INCLUDE,
-  SOC_BINDADDRESS,
-  SOC_GLOBALKNOWNHOSTSFILE,
-  SOC_LOGLEVEL,
-  SOC_HOSTKEYALGORITHMS,
-  SOC_KEXALGORITHMS,
-  SOC_GSSAPIAUTHENTICATION,
-  SOC_KBDINTERACTIVEAUTHENTICATION,
-  SOC_PASSWORDAUTHENTICATION,
-  SOC_PUBKEYAUTHENTICATION,
-  SOC_PUBKEYACCEPTEDTYPES,
-
-  SOC_END /* Keep this one last in the list */
-};
-
 struct ssh_config_keyword_table_s {
   const char *name;
   enum ssh_config_opcode_e opcode;
@@ -213,7 +175,7 @@ static struct ssh_config_match_keyword_table_s ssh_config_match_keyword_table[]
 };
 
 static int ssh_config_parse_line(ssh_session session, const char *line,
-    unsigned int count, int *parsing, uint8_t *seen);
+    unsigned int count, int *parsing);
 
 static enum ssh_config_opcode_e ssh_config_get_opcode(char *keyword) {
   int i;
@@ -326,8 +288,7 @@ static int ssh_config_get_yesno(char **str, int notfound) {
 static void
 local_parse_file(ssh_session session,
                  const char *filename,
-                 int *parsing,
+                 int *parsing)
-                 uint8_t *seen)
 {
     FILE *f;
     char line[MAX_LINE_SIZE] = {0};
@@ -344,7 +305,7 @@ local_parse_file(ssh_session session,
     SSH_LOG(SSH_LOG_PACKET, "Reading additional configuration data from %s", filename);
     while (fgets(line, sizeof(line), f)) {
         count++;
-        rv = ssh_config_parse_line(session, line, count, parsing, seen);
+        rv = ssh_config_parse_line(session, line, count, parsing);
         if (rv < 0) {
             fclose(f);
             return;
@@ -358,8 +319,7 @@ local_parse_file(ssh_session session,
 #if defined(HAVE_GLOB) && defined(HAVE_GLOB_GL_FLAGS_MEMBER)
 static void local_parse_glob(ssh_session session,
                              const char *fileglob,
-                             int *parsing,
+                             int *parsing)
-                             uint8_t *seen)
 {
     glob_t globbuf = {
         .gl_flags = 0,
@@ -379,7 +339,7 @@ static void local_parse_glob(ssh_session session,
     }
 
     for (i = 0; i < globbuf.gl_pathc; i++) {
-        local_parse_file(session, globbuf.gl_pathv[i], parsing, seen);
+        local_parse_file(session, globbuf.gl_pathv[i], parsing);
     }
 
     globfree(&globbuf);
@@ -420,8 +380,11 @@ ssh_config_match(char *value, const char *pattern, bool negate)
     return result;
 }
 
-static int ssh_config_parse_line(ssh_session session, const char *line,
+static int
-    unsigned int count, int *parsing, uint8_t *seen)
+ssh_config_parse_line(ssh_session session,
+                      const char *line,
+                      unsigned int count,
+                      int *parsing)
 {
   enum ssh_config_opcode_e opcode;
   const char *p;
@@ -430,6 +393,7 @@ static int ssh_config_parse_line(ssh_session session, const char *line,
   char *lowerhost;
   size_t len;
   int i;
+  uint8_t *seen = session->opts.options_seen;
   long l;
 
   x = s = strdup(line);
@@ -473,9 +437,9 @@ static int ssh_config_parse_line(ssh_session session, const char *line,
       p = ssh_config_get_str_tok(&s, NULL);
       if (p && *parsing) {
 #if defined(HAVE_GLOB) && defined(HAVE_GLOB_GL_FLAGS_MEMBER)
-        local_parse_glob(session, p, parsing, seen);
+        local_parse_glob(session, p, parsing);
 #else
-        local_parse_file(session, p, parsing, seen);
+        local_parse_file(session, p, parsing);
 #endif /* HAVE_GLOB */
       }
       break;
@@ -844,7 +808,6 @@ int ssh_config_parse_file(ssh_session session, const char *filename)
     unsigned int count = 0;
     FILE *f;
     int parsing, rv;
-    uint8_t *seen = NULL;
 
     f = fopen(filename, "r");
     if (f == NULL) {
@@ -853,23 +816,10 @@ int ssh_config_parse_file(ssh_session session, const char *filename)
 
     SSH_LOG(SSH_LOG_PACKET, "Reading configuration data from %s", filename);
 
-    /* Preserve the seen array among invocations throughout the session */
-    if (session->opts.options_seen == NULL) {
-        seen = calloc(SOC_END - SOC_UNSUPPORTED, sizeof(uint8_t));
-        if (seen == NULL) {
-            fclose(f);
-            ssh_set_error_oom(session);
-            return -1;
-        }
-        session->opts.options_seen = seen;
-    } else {
-        seen = session->opts.options_seen;
-    }
-
     parsing = 1;
     while (fgets(line, sizeof(line), f)) {
         count++;
-        rv = ssh_config_parse_line(session, line, count, &parsing, seen);
+        rv = ssh_config_parse_line(session, line, count, &parsing);
         if (rv < 0) {
             fclose(f);
             return -1;

src/session.c

@@ -283,7 +283,6 @@ void ssh_free(ssh_session session) {
   SAFE_FREE(session->opts.gss_server_identity);
   SAFE_FREE(session->opts.gss_client_identity);
   SAFE_FREE(session->opts.pubkey_accepted_types);
-  SAFE_FREE(session->opts.options_seen);
 
   for (i = 0; i < 10; i++) {
       if (session->opts.wanted_methods[i]) {

tests/client/torture_client_config.c

@@ -92,8 +92,6 @@ static void torture_client_config_system(void **state)
     struct torture_state *s = *state;
     int ret = 0;
 
-    assert_true(s->ssh.session->opts.options_seen == NULL);
-
     /* The first tests assumes there is system-wide configuration file
      * setting Ciphers to some non-default value. We do not have any control
      * of that in this test case.
@@ -121,8 +119,6 @@ static void torture_client_config_emulate(void **state)
     char *filename = NULL;
     int ret = 0;
 
-    assert_true(s->ssh.session->opts.options_seen == NULL);
-
     /* The first tests assumes there is system-wide configuration file
      * setting Ciphers to some non-default value. We do not have any control
      * of that in this test case
@@ -151,8 +147,6 @@ static void torture_client_config_autoparse(void **state)
     struct torture_state *s = *state;
     int ret = 0;
 
-    assert_true(s->ssh.session->opts.options_seen == NULL);
-
     ret = ssh_connect(s->ssh.session);
     assert_ssh_return_code(s->ssh.session, ret);
 
@@ -169,8 +163,6 @@ static void torture_client_config_suppress(void **state)
     bool b = false;
     int ret = 0;
 
-    assert_true(s->ssh.session->opts.options_seen == NULL);
-
     ret = ssh_options_set(s->ssh.session, SSH_OPTIONS_PROCESS_CONFIG, &b);
     assert_ssh_return_code(s->ssh.session, ret);
 

tests/torture.c

@@ -1163,6 +1163,10 @@ void torture_write_file(const char *filename, const char *data){
     close(fd);
 }
 
+void torture_reset_config(ssh_session session)
+{
+    memset(session->opts.options_seen, 0, sizeof(session->opts.options_seen));
+}
 
 int main(int argc, char **argv) {
     struct argument_s arguments;

tests/torture.h

@@ -120,6 +120,8 @@ void torture_setup_sshd_server(void **state);
 void torture_teardown_socket_dir(void **state);
 void torture_teardown_sshd_server(void **state);
 
+void torture_reset_config(ssh_session session);
+
 /*
  * This function must be defined in every unit test file.
  */

tests/unittests/torture_config.c

@@ -156,7 +156,6 @@ static int teardown(void **state)
     return 0;
 }
 
-
 /**
  * @brief tests ssh_config_parse_file with Include directives
  */
@@ -300,7 +299,7 @@ static void torture_config_auth_methods(void **state) {
     assert_int_equal(session->opts.flags, 0);
 
     /* gradually enable them again */
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     ssh_options_set(session, SSH_OPTIONS_HOST, "gss");
     ret = ssh_config_parse_file(session, LIBSSH_TESTCONFIG8);
     assert_true(ret == 0);
@@ -355,34 +354,34 @@ static void torture_config_match(void **state)
     assert_string_equal(session->opts.host, "all-matched.com");
 
     /* Hostname example does simple hostname matching */
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     ssh_options_set(session, SSH_OPTIONS_HOST, "example");
     ret = ssh_config_parse_file(session, LIBSSH_TESTCONFIG10);
     assert_true(ret == 0);
     assert_string_equal(session->opts.host, "example.com");
 
     /* We can match also both hosts from a comma separated list */
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     ssh_options_set(session, SSH_OPTIONS_HOST, "example1");
     ret = ssh_config_parse_file(session, LIBSSH_TESTCONFIG10);
     assert_true(ret == 0);
     assert_string_equal(session->opts.host, "exampleN");
 
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     ssh_options_set(session, SSH_OPTIONS_HOST, "example2");
     ret = ssh_config_parse_file(session, LIBSSH_TESTCONFIG10);
     assert_true(ret == 0);
     assert_string_equal(session->opts.host, "exampleN");
 
     /* We can match by user */
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     ssh_options_set(session, SSH_OPTIONS_USER, "guest");
     ret = ssh_config_parse_file(session, LIBSSH_TESTCONFIG10);
     assert_true(ret == 0);
     assert_string_equal(session->opts.host, "guest.com");
 
     /* We can combine two options on a single line to match both of them */
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     ssh_options_set(session, SSH_OPTIONS_USER, "tester");
     ssh_options_set(session, SSH_OPTIONS_HOST, "testhost");
     ret = ssh_config_parse_file(session, LIBSSH_TESTCONFIG10);
@@ -390,7 +389,7 @@ static void torture_config_match(void **state)
     assert_string_equal(session->opts.host, "testhost.com");
 
     /* We can also negate conditions */
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     ssh_options_set(session, SSH_OPTIONS_USER, "not-tester");
     ssh_options_set(session, SSH_OPTIONS_HOST, "testhost");
     ret = ssh_config_parse_file(session, LIBSSH_TESTCONFIG10);

tests/unittests/torture_options.c

@@ -462,26 +462,26 @@ static void torture_options_config_host(void **state) {
 
     assert_int_equal(session->opts.port, 42);
 
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     ssh_options_set(session, SSH_OPTIONS_HOST, "testhost2");
     ssh_options_parse_config(session, "test_config");
     assert_int_equal(session->opts.port, 43);
 
     session->opts.port = 0;
 
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     ssh_options_set(session, SSH_OPTIONS_HOST, "testhost3");
     ssh_options_parse_config(session, "test_config");
     assert_int_equal(session->opts.port, 43);
 
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     ssh_options_set(session, SSH_OPTIONS_HOST, "testhost4");
     ssh_options_parse_config(session, "test_config");
     assert_int_equal(session->opts.port, 44);
 
     session->opts.port = 0;
 
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     ssh_options_set(session, SSH_OPTIONS_HOST, "testhost5");
     ssh_options_parse_config(session, "test_config");
     assert_int_equal(session->opts.port, 44);
@@ -509,7 +509,7 @@ static void torture_options_config_match(void **state)
     assert_ssh_return_code_equal(session, rv, SSH_ERROR);
 
     /* The Match all keyword needs to be the only one (start) */
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     config = fopen("test_config", "w");
     assert_non_null(config);
     fputs("Match all host local\n",
@@ -520,7 +520,7 @@ static void torture_options_config_match(void **state)
     assert_ssh_return_code_equal(session, rv, SSH_ERROR);
 
     /* The Match all keyword needs to be the only one (end) */
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     config = fopen("test_config", "w");
     assert_non_null(config);
     fputs("Match host local all\n",
@@ -531,7 +531,7 @@ static void torture_options_config_match(void **state)
     assert_ssh_return_code_equal(session, rv, SSH_ERROR);
 
     /* The Match host keyword requires an argument */
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     config = fopen("test_config", "w");
     assert_non_null(config);
     fputs("Match host\n",
@@ -542,7 +542,7 @@ static void torture_options_config_match(void **state)
     assert_ssh_return_code_equal(session, rv, SSH_ERROR);
 
     /* The Match user keyword requires an argument */
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     config = fopen("test_config", "w");
     assert_non_null(config);
     fputs("Match user\n",
@@ -553,7 +553,7 @@ static void torture_options_config_match(void **state)
     assert_ssh_return_code_equal(session, rv, SSH_ERROR);
 
     /* The Match canonical keyword is ignored */
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     config = fopen("test_config", "w");
     assert_non_null(config);
     fputs("Match canonical\n"
@@ -570,7 +570,7 @@ static void torture_options_config_match(void **state)
     session->opts.port = 0;
 
     /* The Match originalhost keyword is ignored */
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     config = fopen("test_config", "w");
     assert_non_null(config);
     fputs("Match originalhost origin\n"
@@ -587,7 +587,7 @@ static void torture_options_config_match(void **state)
     session->opts.port = 0;
 
     /* The Match localuser keyword is ignored */
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     config = fopen("test_config", "w");
     assert_non_null(config);
     fputs("Match originalhost origin\n"
@@ -604,7 +604,7 @@ static void torture_options_config_match(void **state)
     session->opts.port = 0;
 
     /* The Match exec keyword is ignored */
-    SAFE_FREE(session->opts.options_seen);
+    torture_reset_config(session);
     config = fopen("test_config", "w");
     assert_non_null(config);
     fputs("Match exec /bin/true\n"