Blame SOURCES/options_apply.patch

5fd609
From 11c0d687a081fe64501e21c95def7f893611d029 Mon Sep 17 00:00:00 2001
5fd609
From: Norbert Pocs <npocs@redhat.com>
5fd609
Date: Wed, 16 Nov 2022 10:40:38 +0100
5fd609
Subject: [PATCH 1/5] Add a placehohlder for non-expanded identities
5fd609
5fd609
Expanding a string twice could lead to unwanted behaviour.
5fd609
This solution creates a ssh_list (`opts.identites_non_exp`) to store the strings
5fd609
before expansion and by using ssh_apply it moves the string to the
5fd609
`opts.identities`. This way the expanded strings are separated.
5fd609
5fd609
Signed-off-by: Norbert Pocs <npocs@redhat.com>
5fd609
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
5fd609
---
5fd609
 include/libssh/session.h |  1 +
5fd609
 src/options.c            | 86 +++++++++++++++++++++++++---------------
5fd609
 src/session.c            | 23 +++++++++--
5fd609
 3 files changed, 75 insertions(+), 35 deletions(-)
5fd609
5fd609
diff --git a/include/libssh/session.h b/include/libssh/session.h
5fd609
index d3e5787c..e22b0d67 100644
5fd609
--- a/include/libssh/session.h
5fd609
+++ b/include/libssh/session.h
5fd609
@@ -209,6 +209,7 @@ struct ssh_session_struct {
5fd609
 #endif
5fd609
     struct {
5fd609
         struct ssh_list *identity;
5fd609
+        struct ssh_list *identity_non_exp;
5fd609
         char *username;
5fd609
         char *host;
5fd609
         char *bindaddr; /* bind the client to an ip addr */
5fd609
diff --git a/src/options.c b/src/options.c
5fd609
index 56e09c65..bb085384 100644
5fd609
--- a/src/options.c
5fd609
+++ b/src/options.c
5fd609
@@ -52,7 +52,7 @@
5fd609
  * @brief Duplicate the options of a session structure.
5fd609
  *
5fd609
  * If you make several sessions with the same options this is useful. You
5fd609
- * cannot use twice the same option structure in ssh_session_connect.
5fd609
+ * cannot use twice the same option structure in ssh_connect.
5fd609
  *
5fd609
  * @param src           The session to use to copy the options.
5fd609
  *
5fd609
@@ -61,13 +61,14 @@
5fd609
  *
5fd609
  * @returns             0 on success, -1 on error with errno set.
5fd609
  *
5fd609
- * @see ssh_session_connect()
5fd609
+ * @see ssh_connect()
5fd609
  * @see ssh_free()
5fd609
  */
5fd609
 int ssh_options_copy(ssh_session src, ssh_session *dest)
5fd609
 {
5fd609
     ssh_session new;
5fd609
     struct ssh_iterator *it = NULL;
5fd609
+    struct ssh_list *list = NULL;
5fd609
     char *id = NULL;
5fd609
     int i;
5fd609
5fd609
@@ -105,14 +106,15 @@ int ssh_options_copy(ssh_session src, ssh_session *dest)
5fd609
     }
5fd609
5fd609
     /* Remove the default identities */
5fd609
-    for (id = ssh_list_pop_head(char *, new->opts.identity);
5fd609
+    for (id = ssh_list_pop_head(char *, new->opts.identity_non_exp);
5fd609
          id != NULL;
5fd609
-         id = ssh_list_pop_head(char *, new->opts.identity)) {
5fd609
+         id = ssh_list_pop_head(char *, new->opts.identity_non_exp)) {
5fd609
         SAFE_FREE(id);
5fd609
     }
5fd609
     /* Copy the new identities from the source list */
5fd609
-    if (src->opts.identity != NULL) {
5fd609
-        it = ssh_list_get_iterator(src->opts.identity);
5fd609
+    list = new->opts.identity_non_exp;
5fd609
+    it = ssh_list_get_iterator(src->opts.identity_non_exp);
5fd609
+    for (i = 0; i < 2; i++) {
5fd609
         while (it) {
5fd609
             int rc;
5fd609
5fd609
@@ -122,7 +124,7 @@ int ssh_options_copy(ssh_session src, ssh_session *dest)
5fd609
                 return -1;
5fd609
             }
5fd609
5fd609
-            rc = ssh_list_append(new->opts.identity, id);
5fd609
+            rc = ssh_list_append(list, id);
5fd609
             if (rc < 0) {
5fd609
                 free(id);
5fd609
                 ssh_free(new);
5fd609
@@ -130,6 +132,10 @@ int ssh_options_copy(ssh_session src, ssh_session *dest)
5fd609
             }
5fd609
             it = it->next;
5fd609
         }
5fd609
+
5fd609
+        /* copy the identity list if there is any already */
5fd609
+        list = new->opts.identity;
5fd609
+        it = ssh_list_get_iterator(src->opts.identity);
5fd609
     }
5fd609
5fd609
     if (src->opts.sshdir != NULL) {
5fd609
@@ -331,7 +337,7 @@ int ssh_options_set_algo(ssh_session session,
5fd609
  *                Add a new identity file (const char *, format string) to
5fd609
  *                the identity list.\n
5fd609
  *                \n
5fd609
- *                By default identity, id_dsa and id_rsa are checked.\n
5fd609
+ *                By default id_rsa, id_ecdsa and id_ed25519 files are used.\n
5fd609
  *                \n
5fd609
  *                The identity used to authenticate with public key will be
5fd609
  *                prepended to the list.
5fd609
@@ -700,7 +706,11 @@ int ssh_options_set(ssh_session session, enum ssh_options_e type,
5fd609
             if (q == NULL) {
5fd609
                 return -1;
5fd609
             }
5fd609
-            rc = ssh_list_prepend(session->opts.identity, q);
5fd609
+            if (session->opts.exp_flags & SSH_OPT_EXP_FLAG_IDENTITY) {
5fd609
+                rc = ssh_list_append(session->opts.identity_non_exp, q);
5fd609
+            } else {
5fd609
+                rc = ssh_list_prepend(session->opts.identity_non_exp, q);
5fd609
+            }
5fd609
             if (rc < 0) {
5fd609
                 free(q);
5fd609
                 return -1;
5fd609
@@ -1202,7 +1212,7 @@ int ssh_options_get_port(ssh_session session, unsigned int* port_target) {
5fd609
  *              - SSH_OPTIONS_IDENTITY:
5fd609
  *                Get the first identity file name (const char *).\n
5fd609
  *                \n
5fd609
- *                By default identity, id_dsa and id_rsa are checked.
5fd609
+ *                By default id_rsa, id_ecdsa and id_ed25519 files are used.
5fd609
  *
5fd609
  *              - SSH_OPTIONS_PROXYCOMMAND:
5fd609
  *                Get the proxycommand necessary to log into the
5fd609
@@ -1246,7 +1256,11 @@ int ssh_options_get(ssh_session session, enum ssh_options_e type, char** value)
5fd609
             break;
5fd609
         }
5fd609
         case SSH_OPTIONS_IDENTITY: {
5fd609
-            struct ssh_iterator *it = ssh_list_get_iterator(session->opts.identity);
5fd609
+            struct ssh_iterator *it;
5fd609
+            it = ssh_list_get_iterator(session->opts.identity);
5fd609
+            if (it == NULL) {
5fd609
+                it = ssh_list_get_iterator(session->opts.identity_non_exp);
5fd609
+            }
5fd609
             if (it == NULL) {
5fd609
                 return SSH_ERROR;
5fd609
             }
5fd609
@@ -1541,7 +1555,6 @@ out:
5fd609
5fd609
 int ssh_options_apply(ssh_session session)
5fd609
 {
5fd609
-    struct ssh_iterator *it;
5fd609
     char *tmp;
5fd609
     int rc;
5fd609
5fd609
@@ -1586,15 +1599,17 @@ int ssh_options_apply(ssh_session session)
5fd609
         size_t plen = strlen(session->opts.ProxyCommand) +
5fd609
                       5 /* strlen("exec ") */;
5fd609
5fd609
-        p = malloc(plen + 1 /* \0 */);
5fd609
-        if (p == NULL) {
5fd609
-            return -1;
5fd609
-        }
5fd609
+        if (strncmp(session->opts.ProxyCommand, "exec ", 5) != 0) {
5fd609
+            p = malloc(plen + 1 /* \0 */);
5fd609
+            if (p == NULL) {
5fd609
+                return -1;
5fd609
+            }
5fd609
5fd609
-        rc = snprintf(p, plen + 1, "exec %s", session->opts.ProxyCommand);
5fd609
-        if ((size_t)rc != plen) {
5fd609
-            free(p);
5fd609
-            return -1;
5fd609
+            rc = snprintf(p, plen + 1, "exec %s", session->opts.ProxyCommand);
5fd609
+            if ((size_t)rc != plen) {
5fd609
+                free(p);
5fd609
+                return -1;
5fd609
+            }
5fd609
         }
5fd609
5fd609
         tmp = ssh_path_expand_escape(session, p);
5fd609
@@ -1606,24 +1621,33 @@ int ssh_options_apply(ssh_session session)
5fd609
         session->opts.ProxyCommand = tmp;
5fd609
     }
5fd609
5fd609
-    for (it = ssh_list_get_iterator(session->opts.identity);
5fd609
-         it != NULL;
5fd609
-         it = it->next) {
5fd609
-        char *id = (char *) it->data;
5fd609
-        if (strncmp(id, "pkcs11:", 6) == 0) {
5fd609
+    for (tmp = ssh_list_pop_head(char *, session->opts.identity_non_exp);
5fd609
+         tmp != NULL;
5fd609
+         tmp = ssh_list_pop_head(char *, session->opts.identity_non_exp)) {
5fd609
+        char *id = tmp;
5fd609
+        if (strncmp(id, "pkcs11:", 6) != 0) {
5fd609
             /* PKCS#11 URIs are using percent-encoding so we can not mix
5fd609
              * it with ssh expansion of ssh escape characters.
5fd609
-             * Skip these identities now, before we will have PKCS#11 support
5fd609
              */
5fd609
-             continue;
5fd609
+            tmp = ssh_path_expand_escape(session, id);
5fd609
+            if (tmp == NULL) {
5fd609
+                return -1;
5fd609
+            }
5fd609
+            free(id);
5fd609
         }
5fd609
-        tmp = ssh_path_expand_escape(session, id);
5fd609
-        if (tmp == NULL) {
5fd609
+
5fd609
+        /* use append to keep the order at first call and use prepend
5fd609
+         * to put anything that comes on the nth calls to the beginning */
5fd609
+        if (session->opts.exp_flags & SSH_OPT_EXP_FLAG_IDENTITY) {
5fd609
+            rc = ssh_list_prepend(session->opts.identity, tmp);
5fd609
+        } else {
5fd609
+            rc = ssh_list_append(session->opts.identity, tmp);
5fd609
+        }
5fd609
+        if (rc != SSH_OK) {
5fd609
             return -1;
5fd609
         }
5fd609
-        free(id);
5fd609
-        it->data = tmp;
5fd609
     }
5fd609
+    session->opts.exp_flags |= SSH_OPT_EXP_FLAG_IDENTITY;
5fd609
5fd609
     return 0;
5fd609
 }
5fd609
diff --git a/src/session.c b/src/session.c
5fd609
index 64e54957..34a492e4 100644
5fd609
--- a/src/session.c
5fd609
+++ b/src/session.c
5fd609
@@ -118,13 +118,17 @@ ssh_session ssh_new(void)
5fd609
     if (session->opts.identity == NULL) {
5fd609
         goto err;
5fd609
     }
5fd609
+    session->opts.identity_non_exp = ssh_list_new();
5fd609
+    if (session->opts.identity_non_exp == NULL) {
5fd609
+        goto err;
5fd609
+    }
5fd609
5fd609
     id = strdup("%d/id_ed25519");
5fd609
     if (id == NULL) {
5fd609
         goto err;
5fd609
     }
5fd609
5fd609
-    rc = ssh_list_append(session->opts.identity, id);
5fd609
+    rc = ssh_list_append(session->opts.identity_non_exp, id);
5fd609
     if (rc == SSH_ERROR) {
5fd609
         goto err;
5fd609
     }
5fd609
@@ -134,7 +138,7 @@ ssh_session ssh_new(void)
5fd609
     if (id == NULL) {
5fd609
         goto err;
5fd609
     }
5fd609
-    rc = ssh_list_append(session->opts.identity, id);
5fd609
+    rc = ssh_list_append(session->opts.identity_non_exp, id);
5fd609
     if (rc == SSH_ERROR) {
5fd609
         goto err;
5fd609
     }
5fd609
@@ -144,7 +148,7 @@ ssh_session ssh_new(void)
5fd609
     if (id == NULL) {
5fd609
         goto err;
5fd609
     }
5fd609
-    rc = ssh_list_append(session->opts.identity, id);
5fd609
+    rc = ssh_list_append(session->opts.identity_non_exp, id);
5fd609
     if (rc == SSH_ERROR) {
5fd609
         goto err;
5fd609
     }
5fd609
@@ -154,7 +158,7 @@ ssh_session ssh_new(void)
5fd609
     if (id == NULL) {
5fd609
         goto err;
5fd609
     }
5fd609
-    rc = ssh_list_append(session->opts.identity, id);
5fd609
+    rc = ssh_list_append(session->opts.identity_non_exp, id);
5fd609
     if (rc == SSH_ERROR) {
5fd609
         goto err;
5fd609
     }
5fd609
@@ -284,6 +288,17 @@ void ssh_free(ssh_session session)
5fd609
       ssh_list_free(session->opts.identity);
5fd609
   }
5fd609
5fd609
+  if (session->opts.identity_non_exp) {
5fd609
+      char *id;
5fd609
+
5fd609
+      for (id = ssh_list_pop_head(char *, session->opts.identity_non_exp);
5fd609
+           id != NULL;
5fd609
+           id = ssh_list_pop_head(char *, session->opts.identity_non_exp)) {
5fd609
+          SAFE_FREE(id);
5fd609
+      }
5fd609
+      ssh_list_free(session->opts.identity_non_exp);
5fd609
+  }
5fd609
+
5fd609
     while ((b = ssh_list_pop_head(struct ssh_buffer_struct *,
5fd609
                                   session->out_queue)) != NULL) {
5fd609
         SSH_BUFFER_FREE(b);
5fd609
--
5fd609
2.38.1
5fd609
5fd609
5fd609
From 4cb84b99fdb1ffd26c0241f5809e4f67ddd407c6 Mon Sep 17 00:00:00 2001
5fd609
From: Norbert Pocs <npocs@redhat.com>
5fd609
Date: Wed, 16 Nov 2022 11:03:30 +0100
5fd609
Subject: [PATCH 2/5] tests: Use opts.identites_non_exp not opts.identities
5fd609
5fd609
The configuration of identities are first saved to `opts.identities_non_exp`,
5fd609
then moved to `opts.identities` after calling ssh_options_apply and expanding
5fd609
the identity strings. These tests are testing against the proper configuration
5fd609
5fd609
Signed-off-by: Norbert Pocs <npocs@redhat.com>
5fd609
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
5fd609
---
5fd609
 tests/client/torture_auth_pkcs11.c |  2 +-
5fd609
 tests/unittests/torture_config.c   |  3 ++-
5fd609
 tests/unittests/torture_options.c  | 14 +++++++-------
5fd609
 3 files changed, 10 insertions(+), 9 deletions(-)
5fd609
5fd609
diff --git a/tests/client/torture_auth_pkcs11.c b/tests/client/torture_auth_pkcs11.c
5fd609
index ee97bff4..e75fea0e 100644
5fd609
--- a/tests/client/torture_auth_pkcs11.c
5fd609
+++ b/tests/client/torture_auth_pkcs11.c
5fd609
@@ -196,7 +196,7 @@ static void torture_auth_autopubkey(void **state, const char *obj_name, const ch
5fd609
5fd609
     rc = ssh_options_set(session, SSH_OPTIONS_IDENTITY, priv_uri);
5fd609
     assert_int_equal(rc, SSH_OK);
5fd609
-    assert_string_equal(session->opts.identity->root->data, priv_uri);
5fd609
+    assert_string_equal(session->opts.identity_non_exp->root->data, priv_uri);
5fd609
5fd609
     rc = ssh_connect(session);
5fd609
     assert_int_equal(rc, SSH_OK);
5fd609
diff --git a/tests/unittests/torture_config.c b/tests/unittests/torture_config.c
5fd609
index 354adc2f..100e68f6 100644
5fd609
--- a/tests/unittests/torture_config.c
5fd609
+++ b/tests/unittests/torture_config.c
5fd609
@@ -2078,7 +2078,8 @@ static void torture_config_identity(void **state)
5fd609
5fd609
     _parse_config(session, NULL, LIBSSH_TESTCONFIG_STRING13, SSH_OK);
5fd609
5fd609
-    it = ssh_list_get_iterator(session->opts.identity);
5fd609
+    /* The identities are first added to this temporary list before expanding */
5fd609
+    it = ssh_list_get_iterator(session->opts.identity_non_exp);
5fd609
     assert_non_null(it);
5fd609
     id = it->data;
5fd609
     /* The identities are prepended to the list so we start with second one */
5fd609
diff --git a/tests/unittests/torture_options.c b/tests/unittests/torture_options.c
5fd609
index dc4df383..3be2de8a 100644
5fd609
--- a/tests/unittests/torture_options.c
5fd609
+++ b/tests/unittests/torture_options.c
5fd609
@@ -406,12 +406,12 @@ static void torture_options_set_identity(void **state) {
5fd609
5fd609
     rc = ssh_options_set(session, SSH_OPTIONS_ADD_IDENTITY, "identity1");
5fd609
     assert_true(rc == 0);
5fd609
-    assert_string_equal(session->opts.identity->root->data, "identity1");
5fd609
+    assert_string_equal(session->opts.identity_non_exp->root->data, "identity1");
5fd609
5fd609
     rc = ssh_options_set(session, SSH_OPTIONS_IDENTITY, "identity2");
5fd609
     assert_true(rc == 0);
5fd609
-    assert_string_equal(session->opts.identity->root->data, "identity2");
5fd609
-    assert_string_equal(session->opts.identity->root->next->data, "identity1");
5fd609
+    assert_string_equal(session->opts.identity_non_exp->root->data, "identity2");
5fd609
+    assert_string_equal(session->opts.identity_non_exp->root->next->data, "identity1");
5fd609
 }
5fd609
5fd609
 static void torture_options_get_identity(void **state) {
5fd609
@@ -429,7 +429,7 @@ static void torture_options_get_identity(void **state) {
5fd609
5fd609
     rc = ssh_options_set(session, SSH_OPTIONS_IDENTITY, "identity2");
5fd609
     assert_int_equal(rc, SSH_OK);
5fd609
-    assert_string_equal(session->opts.identity->root->data, "identity2");
5fd609
+    assert_string_equal(session->opts.identity_non_exp->root->data, "identity2");
5fd609
     rc = ssh_options_get(session, SSH_OPTIONS_IDENTITY, &identity);
5fd609
     assert_int_equal(rc, SSH_OK);
5fd609
     assert_non_null(identity);
5fd609
@@ -867,9 +867,9 @@ static void torture_options_copy(void **state)
5fd609
     assert_non_null(new);
5fd609
5fd609
     /* Check the identities match */
5fd609
-    it = ssh_list_get_iterator(session->opts.identity);
5fd609
+    it = ssh_list_get_iterator(session->opts.identity_non_exp);
5fd609
     assert_non_null(it);
5fd609
-    it2 = ssh_list_get_iterator(new->opts.identity);
5fd609
+    it2 = ssh_list_get_iterator(new->opts.identity_non_exp);
5fd609
     assert_non_null(it2);
5fd609
     while (it != NULL && it2 != NULL) {
5fd609
         assert_string_equal(it->data, it2->data);
5fd609
@@ -956,7 +956,7 @@ static void torture_options_getopt(void **state)
5fd609
                         "aes128-ctr");
5fd609
     assert_string_equal(session->opts.wanted_methods[SSH_CRYPT_S_C],
5fd609
                         "aes128-ctr");
5fd609
-    assert_string_equal(session->opts.identity->root->data, "id_rsa");
5fd609
+    assert_string_equal(session->opts.identity_non_exp->root->data, "id_rsa");
5fd609
 #ifdef WITH_ZLIB
5fd609
     assert_string_equal(session->opts.wanted_methods[SSH_COMP_C_S],
5fd609
                         "zlib@openssh.com,zlib,none");
5fd609
--
5fd609
2.38.1
5fd609
5fd609
5fd609
From cd30217c9032419ebcf722c0bfc6b5ebfa3518d0 Mon Sep 17 00:00:00 2001
5fd609
From: Norbert Pocs <npocs@redhat.com>
5fd609
Date: Wed, 16 Nov 2022 16:51:02 +0100
5fd609
Subject: [PATCH 3/5] Add flags for escape expand operation
5fd609
5fd609
Calling `ssh_options_apply` more times can result in an unwanted behaviour of
5fd609
expanding the escape characters more times. Adding flags to check if the
5fd609
expansion was already done on the current string variables.
5fd609
5fd609
Signed-off-by: Norbert Pocs <npocs@redhat.com>
5fd609
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
5fd609
---
5fd609
 include/libssh/session.h |  7 ++++
5fd609
 src/options.c            | 91 ++++++++++++++++++++++++----------------
5fd609
 src/session.c            |  2 +
5fd609
 3 files changed, 63 insertions(+), 37 deletions(-)
5fd609
5fd609
diff --git a/include/libssh/session.h b/include/libssh/session.h
5fd609
index e22b0d67..cf219c2a 100644
5fd609
--- a/include/libssh/session.h
5fd609
+++ b/include/libssh/session.h
5fd609
@@ -93,6 +93,12 @@ enum ssh_pending_call_e {
5fd609
 #define SSH_OPT_FLAG_KBDINT_AUTH 0x4
5fd609
 #define SSH_OPT_FLAG_GSSAPI_AUTH 0x8
5fd609
5fd609
+/* Escape expansion of different variables */
5fd609
+#define SSH_OPT_EXP_FLAG_KNOWNHOSTS 0x1
5fd609
+#define SSH_OPT_EXP_FLAG_GLOBAL_KNOWNHOSTS 0x2
5fd609
+#define SSH_OPT_EXP_FLAG_PROXYCOMMAND 0x4
5fd609
+#define SSH_OPT_EXP_FLAG_IDENTITY 0x8
5fd609
+
5fd609
 /* extensions flags */
5fd609
 /* negotiation enabled */
5fd609
 #define SSH_EXT_NEGOTIATION     0x01
5fd609
@@ -232,6 +238,7 @@ struct ssh_session_struct {
5fd609
         char *gss_client_identity;
5fd609
         int gss_delegate_creds;
5fd609
         int flags;
5fd609
+        int exp_flags;
5fd609
         int nodelay;
5fd609
         bool config_processed;
5fd609
         uint8_t options_seen[SOC_MAX];
5fd609
diff --git a/src/options.c b/src/options.c
5fd609
index bb085384..c566244b 100644
5fd609
--- a/src/options.c
5fd609
+++ b/src/options.c
5fd609
@@ -730,6 +730,7 @@ int ssh_options_set(ssh_session session, enum ssh_options_e type,
5fd609
                     ssh_set_error_oom(session);
5fd609
                     return -1;
5fd609
                 }
5fd609
+                session->opts.exp_flags &= ~SSH_OPT_EXP_FLAG_KNOWNHOSTS;
5fd609
             }
5fd609
             break;
5fd609
         case SSH_OPTIONS_GLOBAL_KNOWNHOSTS:
5fd609
@@ -751,6 +752,7 @@ int ssh_options_set(ssh_session session, enum ssh_options_e type,
5fd609
                     ssh_set_error_oom(session);
5fd609
                     return -1;
5fd609
                 }
5fd609
+                session->opts.exp_flags &= ~SSH_OPT_EXP_FLAG_GLOBAL_KNOWNHOSTS;
5fd609
             }
5fd609
             break;
5fd609
         case SSH_OPTIONS_TIMEOUT:
5fd609
@@ -1014,6 +1016,7 @@ int ssh_options_set(ssh_session session, enum ssh_options_e type,
5fd609
                         return -1;
5fd609
                     }
5fd609
                     session->opts.ProxyCommand = q;
5fd609
+                    session->opts.exp_flags &= ~SSH_OPT_EXP_FLAG_PROXYCOMMAND;
5fd609
                 }
5fd609
             }
5fd609
             break;
5fd609
@@ -1572,53 +1575,67 @@ int ssh_options_apply(ssh_session session)
5fd609
         }
5fd609
     }
5fd609
5fd609
-    if (session->opts.knownhosts == NULL) {
5fd609
-        tmp = ssh_path_expand_escape(session, "%d/known_hosts");
5fd609
-    } else {
5fd609
-        tmp = ssh_path_expand_escape(session, session->opts.knownhosts);
5fd609
-    }
5fd609
-    if (tmp == NULL) {
5fd609
-        return -1;
5fd609
+    if ((session->opts.exp_flags & SSH_OPT_EXP_FLAG_KNOWNHOSTS) == 0) {
5fd609
+        if (session->opts.knownhosts == NULL) {
5fd609
+            tmp = ssh_path_expand_escape(session, "%d/known_hosts");
5fd609
+        } else {
5fd609
+            tmp = ssh_path_expand_escape(session, session->opts.knownhosts);
5fd609
+        }
5fd609
+        if (tmp == NULL) {
5fd609
+            return -1;
5fd609
+        }
5fd609
+        free(session->opts.knownhosts);
5fd609
+        session->opts.knownhosts = tmp;
5fd609
+        session->opts.exp_flags |= SSH_OPT_EXP_FLAG_KNOWNHOSTS;
5fd609
     }
5fd609
-    free(session->opts.knownhosts);
5fd609
-    session->opts.knownhosts = tmp;
5fd609
5fd609
-    if (session->opts.global_knownhosts == NULL) {
5fd609
-        tmp = strdup("/etc/ssh/ssh_known_hosts");
5fd609
-    } else {
5fd609
-        tmp = ssh_path_expand_escape(session, session->opts.global_knownhosts);
5fd609
-    }
5fd609
-    if (tmp == NULL) {
5fd609
-        return -1;
5fd609
+    if ((session->opts.exp_flags & SSH_OPT_EXP_FLAG_GLOBAL_KNOWNHOSTS) == 0) {
5fd609
+        if (session->opts.global_knownhosts == NULL) {
5fd609
+            tmp = strdup("/etc/ssh/ssh_known_hosts");
5fd609
+        } else {
5fd609
+            tmp = ssh_path_expand_escape(session,
5fd609
+                                         session->opts.global_knownhosts);
5fd609
+        }
5fd609
+        if (tmp == NULL) {
5fd609
+            return -1;
5fd609
+        }
5fd609
+        free(session->opts.global_knownhosts);
5fd609
+        session->opts.global_knownhosts = tmp;
5fd609
+        session->opts.exp_flags |= SSH_OPT_EXP_FLAG_GLOBAL_KNOWNHOSTS;
5fd609
     }
5fd609
-    free(session->opts.global_knownhosts);
5fd609
-    session->opts.global_knownhosts = tmp;
5fd609
5fd609
-    if (session->opts.ProxyCommand != NULL) {
5fd609
-        char *p = NULL;
5fd609
-        size_t plen = strlen(session->opts.ProxyCommand) +
5fd609
-                      5 /* strlen("exec ") */;
5fd609
5fd609
-        if (strncmp(session->opts.ProxyCommand, "exec ", 5) != 0) {
5fd609
-            p = malloc(plen + 1 /* \0 */);
5fd609
-            if (p == NULL) {
5fd609
-                return -1;
5fd609
-            }
5fd609
+    if ((session->opts.exp_flags & SSH_OPT_EXP_FLAG_PROXYCOMMAND) == 0) {
5fd609
+        if (session->opts.ProxyCommand != NULL) {
5fd609
+            char *p = NULL;
5fd609
+            size_t plen = strlen(session->opts.ProxyCommand) +
5fd609
+                          5 /* strlen("exec ") */;
5fd609
+
5fd609
+            if (strncmp(session->opts.ProxyCommand, "exec ", 5) != 0) {
5fd609
+                p = malloc(plen + 1 /* \0 */);
5fd609
+                if (p == NULL) {
5fd609
+                    return -1;
5fd609
+                }
5fd609
5fd609
-            rc = snprintf(p, plen + 1, "exec %s", session->opts.ProxyCommand);
5fd609
-            if ((size_t)rc != plen) {
5fd609
+                rc = snprintf(p, plen + 1, "exec %s", session->opts.ProxyCommand);
5fd609
+                if ((size_t)rc != plen) {
5fd609
+                    free(p);
5fd609
+                    return -1;
5fd609
+                }
5fd609
+                tmp = ssh_path_expand_escape(session, p);
5fd609
                 free(p);
5fd609
-                return -1;
5fd609
+            } else {
5fd609
+                tmp = ssh_path_expand_escape(session,
5fd609
+                                             session->opts.ProxyCommand);
5fd609
             }
5fd609
-        }
5fd609
5fd609
-        tmp = ssh_path_expand_escape(session, p);
5fd609
-        free(p);
5fd609
-        if (tmp == NULL) {
5fd609
-            return -1;
5fd609
+            if (tmp == NULL) {
5fd609
+                return -1;
5fd609
+            }
5fd609
+            free(session->opts.ProxyCommand);
5fd609
+            session->opts.ProxyCommand = tmp;
5fd609
+            session->opts.exp_flags |= SSH_OPT_EXP_FLAG_PROXYCOMMAND;
5fd609
         }
5fd609
-        free(session->opts.ProxyCommand);
5fd609
-        session->opts.ProxyCommand = tmp;
5fd609
     }
5fd609
5fd609
     for (tmp = ssh_list_pop_head(char *, session->opts.identity_non_exp);
5fd609
diff --git a/src/session.c b/src/session.c
5fd609
index 34a492e4..06f6a26f 100644
5fd609
--- a/src/session.c
5fd609
+++ b/src/session.c
5fd609
@@ -114,6 +114,8 @@ ssh_session ssh_new(void)
5fd609
                           SSH_OPT_FLAG_KBDINT_AUTH |
5fd609
                           SSH_OPT_FLAG_GSSAPI_AUTH;
5fd609
5fd609
+    session->opts.exp_flags = 0;
5fd609
+
5fd609
     session->opts.identity = ssh_list_new();
5fd609
     if (session->opts.identity == NULL) {
5fd609
         goto err;
5fd609
--
5fd609
2.38.1
5fd609
5fd609
5fd609
From ed58082f9706f2ab3bdeca24f632356b9bc325e6 Mon Sep 17 00:00:00 2001
5fd609
From: Norbert Pocs <npocs@redhat.com>
5fd609
Date: Wed, 16 Nov 2022 17:17:14 +0100
5fd609
Subject: [PATCH 4/5] torture_options.c: Add identity test for ssh_options_copy
5fd609
5fd609
Test if the ssh_options_apply is called on session before ssh_options_copy,
5fd609
then `opts.identity` ssh_list will be copied
5fd609
5fd609
Signed-off-by: Norbert Pocs <npocs@redhat.com>
5fd609
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
5fd609
---
5fd609
 tests/unittests/torture_options.c | 28 ++++++++++++++++++++++++++++
5fd609
 1 file changed, 28 insertions(+)
5fd609
5fd609
diff --git a/tests/unittests/torture_options.c b/tests/unittests/torture_options.c
5fd609
index 3be2de8a..907cc8df 100644
5fd609
--- a/tests/unittests/torture_options.c
5fd609
+++ b/tests/unittests/torture_options.c
5fd609
@@ -918,6 +918,34 @@ static void torture_options_copy(void **state)
5fd609
                         sizeof(session->opts.options_seen));
5fd609
5fd609
     ssh_free(new);
5fd609
+
5fd609
+    /* test if ssh_options_apply was called before ssh_options_copy
5fd609
+     * the opts.identity list gets copied (percent expanded list) */
5fd609
+    rv = ssh_options_apply(session);
5fd609
+    assert_ssh_return_code(session, rv);
5fd609
+
5fd609
+    rv = ssh_options_copy(session, &new;;
5fd609
+    assert_ssh_return_code(session, rv);
5fd609
+    assert_non_null(new);
5fd609
+
5fd609
+    it = ssh_list_get_iterator(session->opts.identity_non_exp);
5fd609
+    assert_null(it);
5fd609
+    it2 = ssh_list_get_iterator(new->opts.identity_non_exp);
5fd609
+    assert_null(it2);
5fd609
+
5fd609
+    it = ssh_list_get_iterator(session->opts.identity);
5fd609
+    assert_non_null(it);
5fd609
+    it2 = ssh_list_get_iterator(new->opts.identity);
5fd609
+    assert_non_null(it2);
5fd609
+    while (it != NULL && it2 != NULL) {
5fd609
+        assert_string_equal(it->data, it2->data);
5fd609
+        it = it->next;
5fd609
+        it2 = it2->next;
5fd609
+    }
5fd609
+    assert_null(it);
5fd609
+    assert_null(it2);
5fd609
+
5fd609
+    ssh_free(new);
5fd609
 }
5fd609
5fd609
 #define EXECUTABLE_NAME "test-exec"
5fd609
--
5fd609
2.38.1
5fd609
5fd609
5fd609
From 89dd4a927b946d4df5c48073ca25cd843e0acde0 Mon Sep 17 00:00:00 2001
5fd609
From: Norbert Pocs <npocs@redhat.com>
5fd609
Date: Wed, 16 Nov 2022 17:18:49 +0100
5fd609
Subject: [PATCH 5/5] torture_options.c: Add test for ssh_options_apply
5fd609
5fd609
Test that ssh_options_apply can be called multiple times without expanding
5fd609
escape characters more than once. If the options are updated after calling
5fd609
ssh_options_apply keep the last options.
5fd609
5fd609
Signed-off-by: Norbert Pocs <npocs@redhat.com>
5fd609
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
5fd609
---
5fd609
 tests/unittests/torture_options.c | 165 ++++++++++++++++++++++++++++++
5fd609
 1 file changed, 165 insertions(+)
5fd609
5fd609
diff --git a/tests/unittests/torture_options.c b/tests/unittests/torture_options.c
5fd609
index 907cc8df..ea63b45e 100644
5fd609
--- a/tests/unittests/torture_options.c
5fd609
+++ b/tests/unittests/torture_options.c
5fd609
@@ -1332,6 +1332,170 @@ static void torture_options_caret_sign(void **state)
5fd609
     free(awaited);
5fd609
 }
5fd609
5fd609
+static void torture_options_apply (void **state) {
5fd609
+    ssh_session session = *state;
5fd609
+    struct ssh_list *awaited_list = NULL;
5fd609
+    struct ssh_iterator *it1 = NULL, *it2 = NULL;
5fd609
+    char *id = NULL;
5fd609
+    int rc;
5fd609
+
5fd609
+    rc = ssh_options_set(session,
5fd609
+                         SSH_OPTIONS_KNOWNHOSTS,
5fd609
+                         "%%d/.ssh/known_hosts");
5fd609
+    assert_ssh_return_code(session, rc);
5fd609
+
5fd609
+    rc = ssh_options_set(session,
5fd609
+                         SSH_OPTIONS_GLOBAL_KNOWNHOSTS,
5fd609
+                         "/etc/%%u/libssh/known_hosts");
5fd609
+    assert_ssh_return_code(session, rc);
5fd609
+
5fd609
+    rc = ssh_options_set(session,
5fd609
+                         SSH_OPTIONS_PROXYCOMMAND,
5fd609
+                         "exec echo \"Hello libssh %%d!\"");
5fd609
+    assert_ssh_return_code(session, rc);
5fd609
+
5fd609
+    rc = ssh_options_set(session,
5fd609
+                         SSH_OPTIONS_ADD_IDENTITY,
5fd609
+                         "%%d/do_not_expand");
5fd609
+    assert_ssh_return_code(session, rc);
5fd609
+
5fd609
+    rc = ssh_options_apply(session);
5fd609
+    assert_ssh_return_code(session, rc);
5fd609
+
5fd609
+    /* check that the values got expanded */
5fd609
+    assert_true(session->opts.exp_flags & SSH_OPT_EXP_FLAG_KNOWNHOSTS);
5fd609
+    assert_true(session->opts.exp_flags & SSH_OPT_EXP_FLAG_GLOBAL_KNOWNHOSTS);
5fd609
+    assert_true(session->opts.exp_flags & SSH_OPT_EXP_FLAG_PROXYCOMMAND);
5fd609
+    assert_true(ssh_list_count(session->opts.identity_non_exp) == 0);
5fd609
+    assert_true(ssh_list_count(session->opts.identity) > 0);
5fd609
+
5fd609
+    /* should not change anything calling it again */
5fd609
+    rc = ssh_options_apply(session);
5fd609
+    assert_ssh_return_code(session, rc);
5fd609
+
5fd609
+    /* check that the expansion was done only once */
5fd609
+    assert_string_equal(session->opts.knownhosts, "%d/.ssh/known_hosts");
5fd609
+    assert_string_equal(session->opts.global_knownhosts,
5fd609
+                        "/etc/%u/libssh/known_hosts");
5fd609
+    /* no exec should be added if there already is one */
5fd609
+    assert_string_equal(session->opts.ProxyCommand,
5fd609
+                        "exec echo \"Hello libssh %d!\"");
5fd609
+    assert_string_equal(session->opts.identity->root->data,
5fd609
+                        "%d/do_not_expand");
5fd609
+
5fd609
+    /* apply should keep the freshest setting */
5fd609
+    rc = ssh_options_set(session,
5fd609
+                         SSH_OPTIONS_KNOWNHOSTS,
5fd609
+                         "hello there");
5fd609
+    assert_ssh_return_code(session, rc);
5fd609
+
5fd609
+    rc = ssh_options_set(session,
5fd609
+                         SSH_OPTIONS_GLOBAL_KNOWNHOSTS,
5fd609
+                         "lorem ipsum");
5fd609
+    assert_ssh_return_code(session, rc);
5fd609
+
5fd609
+    rc = ssh_options_set(session,
5fd609
+                         SSH_OPTIONS_PROXYCOMMAND,
5fd609
+                         "mission_impossible");
5fd609
+    assert_ssh_return_code(session, rc);
5fd609
+
5fd609
+    rc = ssh_options_set(session,
5fd609
+                         SSH_OPTIONS_ADD_IDENTITY,
5fd609
+                         "007");
5fd609
+    assert_ssh_return_code(session, rc);
5fd609
+
5fd609
+    rc = ssh_options_set(session,
5fd609
+                         SSH_OPTIONS_ADD_IDENTITY,
5fd609
+                         "3");
5fd609
+    assert_ssh_return_code(session, rc);
5fd609
+
5fd609
+    rc = ssh_options_set(session,
5fd609
+                         SSH_OPTIONS_ADD_IDENTITY,
5fd609
+                         "2");
5fd609
+    assert_ssh_return_code(session, rc);
5fd609
+
5fd609
+    rc = ssh_options_set(session,
5fd609
+                         SSH_OPTIONS_ADD_IDENTITY,
5fd609
+                         "1");
5fd609
+    assert_ssh_return_code(session, rc);
5fd609
+
5fd609
+    /* check that flags show need of escape expansion */
5fd609
+    assert_false(session->opts.exp_flags & SSH_OPT_EXP_FLAG_KNOWNHOSTS);
5fd609
+    assert_false(session->opts.exp_flags & SSH_OPT_EXP_FLAG_GLOBAL_KNOWNHOSTS);
5fd609
+    assert_false(session->opts.exp_flags & SSH_OPT_EXP_FLAG_PROXYCOMMAND);
5fd609
+    assert_false(ssh_list_count(session->opts.identity_non_exp) == 0);
5fd609
+
5fd609
+    rc = ssh_options_apply(session);
5fd609
+    assert_ssh_return_code(session, rc);
5fd609
+
5fd609
+    /* check that the values got expanded */
5fd609
+    assert_true(session->opts.exp_flags & SSH_OPT_EXP_FLAG_KNOWNHOSTS);
5fd609
+    assert_true(session->opts.exp_flags & SSH_OPT_EXP_FLAG_GLOBAL_KNOWNHOSTS);
5fd609
+    assert_true(session->opts.exp_flags & SSH_OPT_EXP_FLAG_PROXYCOMMAND);
5fd609
+    assert_true(ssh_list_count(session->opts.identity_non_exp) == 0);
5fd609
+
5fd609
+    assert_string_equal(session->opts.knownhosts, "hello there");
5fd609
+    assert_string_equal(session->opts.global_knownhosts, "lorem ipsum");
5fd609
+    /* check that the "exec " was added at the beginning */
5fd609
+    assert_string_equal(session->opts.ProxyCommand, "exec mission_impossible");
5fd609
+    assert_string_equal(session->opts.identity->root->data, "1");
5fd609
+
5fd609
+    /* check the order of the identity files after double expansion */
5fd609
+    awaited_list = ssh_list_new();
5fd609
+    /* append the new data in order */
5fd609
+    id = strdup("1");
5fd609
+    rc = ssh_list_append(awaited_list, id);
5fd609
+    assert_int_equal(rc, SSH_OK);
5fd609
+    id = strdup("2");
5fd609
+    rc = ssh_list_append(awaited_list, id);
5fd609
+    assert_int_equal(rc, SSH_OK);
5fd609
+    id = strdup("3");
5fd609
+    rc = ssh_list_append(awaited_list, id);
5fd609
+    assert_int_equal(rc, SSH_OK);
5fd609
+    id = strdup("007");
5fd609
+    rc = ssh_list_append(awaited_list, id);
5fd609
+    assert_int_equal(rc, SSH_OK);
5fd609
+    id = strdup("%d/do_not_expand");
5fd609
+    rc = ssh_list_append(awaited_list, id);
5fd609
+    assert_int_equal(rc, SSH_OK);
5fd609
+    /* append the defaults; this list is copied from ssh_new@src/session.c */
5fd609
+    id = ssh_path_expand_escape(session, "%d/id_ed25519");
5fd609
+    rc = ssh_list_append(awaited_list, id);
5fd609
+    assert_int_equal(rc, SSH_OK);
5fd609
+#ifdef HAVE_ECC
5fd609
+    id = ssh_path_expand_escape(session, "%d/id_ecdsa");
5fd609
+    rc = ssh_list_append(awaited_list, id);
5fd609
+    assert_int_equal(rc, SSH_OK);
5fd609
+#endif
5fd609
+    id = ssh_path_expand_escape(session, "%d/id_rsa");
5fd609
+    rc = ssh_list_append(awaited_list, id);
5fd609
+    assert_int_equal(rc, SSH_OK);
5fd609
+#ifdef HAVE_DSA
5fd609
+    id = ssh_path_expand_escape(session, "%d/id_dsa");
5fd609
+    rc = ssh_list_append(awaited_list, id);
5fd609
+    assert_int_equal(rc, SSH_OK);
5fd609
+#endif
5fd609
+
5fd609
+    assert_int_equal(ssh_list_count(awaited_list),
5fd609
+                     ssh_list_count(session->opts.identity));
5fd609
+
5fd609
+    it1 = ssh_list_get_iterator(awaited_list);
5fd609
+    assert_non_null(it1);
5fd609
+    it2 = ssh_list_get_iterator(session->opts.identity);
5fd609
+    assert_non_null(it2);
5fd609
+    while (it1 != NULL && it2 != NULL) {
5fd609
+        assert_string_equal(it1->data, it2->data);
5fd609
+
5fd609
+        free((void*)it1->data);
5fd609
+        it1 = it1->next;
5fd609
+        it2 = it2->next;
5fd609
+    }
5fd609
+    assert_null(it1);
5fd609
+    assert_null(it2);
5fd609
+
5fd609
+    ssh_list_free(awaited_list);
5fd609
+}
5fd609
+
5fd609
 #ifdef WITH_SERVER
5fd609
 const char template[] = "temp_dir_XXXXXX";
5fd609
5fd609
@@ -2132,6 +2296,7 @@ int torture_run_tests(void) {
5fd609
                                         setup, teardown),
5fd609
         cmocka_unit_test_setup_teardown(torture_options_caret_sign,
5fd609
                                         setup, teardown),
5fd609
+        cmocka_unit_test_setup_teardown(torture_options_apply, setup, teardown),
5fd609
     };
5fd609
5fd609
 #ifdef WITH_SERVER
5fd609
--
5fd609
2.38.1
5fd609