SSH: sss_ssh_knownhosts must accept port numbers

sss_ssh_knownhosts was only accepting a hostname or IP address, but no
port number. Because token %H of ssh(1) could pass a port number, it
must be accepted.

The %H token can provide the hostname and port number in the
following format:

hostname
canonical.host.name
IP-address
[hostname]:port
[canonical.host.name]:port
[IP-address]:port

The port is specified only when a non-default port is used.

Identifiers without the brackets are also recognized in case a user
invokes the tool directly.

Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Tomáš Halman <thalman@redhat.com>

src/man/sss_ssh_knownhosts.1.xml

@@ -44,7 +44,7 @@
             key host authentication using the <quote>KnownHostsCommand</quote>
             option:
             <programlisting>
-              KnownHostsCommand /usr/bin/sss_ssh_knownhosts %H
+                KnownHostsCommand /usr/bin/sss_ssh_knownhosts %H
             </programlisting>
             Please refer to the <citerefentry>
             <refentrytitle>ssh_config</refentrytitle><manvolnum>5</manvolnum>
@@ -67,10 +67,48 @@
                     </para>
                 </listitem>
             </varlistentry>
+            <varlistentry>
+                <term>
+                    <option>-o</option>,<option>--only-host-name</option>
+                </term>
+                <listitem>
+                    <para>
+                        When the keys retrieved from the backend do not include
+                        the hostname, this tool will add the unmodified hostname
+                        as provided by the caller. If this flag is set, only the
+                        hostname (no port number) will be added to the keys.
+                    </para>
+                </listitem>
+            </varlistentry>
             <xi:include xmlns:xi="http://www.w3.org/2001/XInclude" href="include/param_help.xml" />
         </variablelist>
     </refsect1>
 
+    <refsect1 id='key_retrieval'>
+        <title>KEY RETRIEVAL</title>
+        <para>
+            The key lines retrieved from the backend are expected to respect the
+            key format as decribed in the <quote>SSH_KNOWN_HOSTS FILE FORMAT</quote>
+            section of <citerefentry><refentrytitle>sshd</refentrytitle>
+            <manvolnum>8</manvolnum></citerefentry>. However, returning only
+            the keytype and the key itself is tolerated, in which case, the
+            hostname received as parameter will be added before the keytype to
+            output a correctly formatted line. The hostname will be added
+            unmodified or just the hostname (no port number), depending on
+            whether the <option>-o</option>,<option>--only-host-name</option>
+            option was provided.
+        </para>
+        <para>
+            When the SSH server is listening on a non-default port, the
+            backend MUST provide the hostname including the port number in the
+            correct format and position as part of the key line. For example,
+            the minimal key line would be:
+            <programlisting>
+                [canonical.host.name]:2222 &lt;keytype&gt; &lt;base64-encoded key&gt;
+            </programlisting>
+        </para>
+    </refsect1>
+
     <refsect1 id='exit_status'>
         <title>EXIT STATUS</title>
         <para>

src/sss_client/ssh/sss_ssh_authorizedkeys.c

@@ -107,7 +107,7 @@ int main(int argc, const char **argv)
 
     /* print results */
     for (i = 0; i < ent->num_pubkeys; i++) {
-        ret = sss_ssh_print_pubkey(&ent->pubkeys[i], NULL);
+        ret = sss_ssh_print_pubkey(&ent->pubkeys[i], NULL, NULL);
         if (ret != EOK && ret != EINVAL) {
             DEBUG(SSSDBG_CRIT_FAILURE,
                   "ssh_ssh_print_pubkey() failed (%d): %s\n",

src/sss_client/ssh/sss_ssh_knownhosts.c

@@ -29,33 +29,92 @@
 #include "sss_client/sss_cli.h"
 #include "sss_client/ssh/sss_ssh_client.h"
 
+
+/*
+ * Parse the received hostname, which is expected in the format described in
+ * the “SSH_KNOWN_HOSTS FILE FORMAT” section of sshd(8). The parsed host name
+ * and port are returned as strings allocated with malloc(3), and not talloc(3),
+ * and must be freed by the caller.
+ *
+ * Some of the recognized formats are not expected from ssh, but it is easier
+ * to identify them and useful in the case the tool is launched manually by a
+ * user.
+ *
+ * If any of the expected values (host or port) is not found, their respective
+ * output arguments will NOT be modified.
+ */
+static errno_t parse_ssh_host(const char *ssh_host,
+                              const char **_host, const char **_port)
+{
+    int values;
+
+    /* Host name between brackets and with a port number.
+     * ssh can use this format.
+     */
+    values = sscanf(ssh_host, "[%m[^]]]:%ms", _host, _port);
+    if (values == 2) {
+        return EOK;
+    }
+    /* Just a host name enclosed between brackets.
+     * ssh is not expected to use this format but... who knows?
+     */
+    if (values == 1) {
+        return EOK;
+    }
+
+    /* A host name without brackets but with a port number.
+     * This is not expected from ssh, but users will certainly use it.
+     */
+    values = sscanf(ssh_host, "%m[^:]:%ms", _host, _port);
+    if (values == 2) {
+        return EOK;
+    }
+    /* A host name without brackets or port number.
+     * This is probably the most common case.
+     */
+    if (values == 1) {
+        return EOK;
+    }
+
+    return EINVAL;
+}
+
 static errno_t known_hosts(TALLOC_CTX *mem_ctx, const char *domain,
-                           const char *host, struct sss_ssh_ent **_ent)
+                           const char *ssh_host, int only_host_name)
 {
     errno_t ret;
     struct addrinfo ai_hint;
     struct addrinfo *ai = NULL;
     char canonhost[NI_MAXHOST];
+    const char *host = NULL;
+    const char *port = NULL;
     const char *canonname = NULL;
     struct sss_ssh_ent *ent = NULL;
+    size_t i;
 
-    if (_ent == NULL) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "NULL _ent received\n");
-        ERROR("Internal error\n");
-        return EINVAL;
+    /* WARNING:
+     * Memory for host and port is allocated with malloc(3) instead of talloc(3)
+     */
+    ret = parse_ssh_host(ssh_host, &host, &port);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE,
+              "Failed to parse the host name: %s\n", ssh_host);
+        goto done;
     }
+    DEBUG(SSSDBG_FUNC_DATA, "Parsed hostname: %s, port: %s\n",
+          host, port == NULL ? "default" : port);
 
+    /* Canonicalize the host name in case the user used an alias or IP address */
     memset(&ai_hint, 0, sizeof(struct addrinfo));
     ai_hint.ai_family = AF_UNSPEC;
     ai_hint.ai_socktype = SOCK_STREAM;
     ai_hint.ai_protocol = IPPROTO_TCP;
     ai_hint.ai_flags = AI_NUMERICHOST;
 
-    DEBUG(SSSDBG_FUNC_DATA, "Looking up canonical name for: %s\n", host);
-    ret = getaddrinfo(host, NULL, &ai_hint, &ai);
+    ret = getaddrinfo(host, port, &ai_hint, &ai);
     if (ret != EOK) {
         ai_hint.ai_flags = AI_CANONNAME;
-        ret = getaddrinfo(host, NULL, &ai_hint, &ai);
+        ret = getaddrinfo(host, port, &ai_hint, &ai);
         if (ret != EOK) {
             DEBUG(SSSDBG_OP_FAILURE,
                   "getaddrinfo() failed (%d): %s\n", ret, gai_strerror(ret));
@@ -65,7 +124,7 @@ static errno_t known_hosts(TALLOC_CTX *mem_ctx, const char *domain,
         }
     } else {
         ret = getnameinfo(ai->ai_addr, ai->ai_addrlen,
-                          canonhost, NI_MAXHOST, NULL, 0, NI_NAMEREQD);
+                          canonhost, sizeof(canonhost), NULL, 0, NI_NAMEREQD);
         if (ret != EOK) {
             DEBUG(SSSDBG_OP_FAILURE,
                   "getnameinfo() failed (%d): %s\n", ret, gai_strerror(ret));
@@ -89,10 +148,28 @@ static errno_t known_hosts(TALLOC_CTX *mem_ctx, const char *domain,
         goto done;
     }
 
-    *_ent = ent;
+    /* Print the results.
+     * We pass the host name to handle the case when the key doesn't include
+     * the host name */
+    for (i = 0; i < ent->num_pubkeys; i++) {
+        ret = sss_ssh_print_pubkey(&ent->pubkeys[i],
+                                   only_host_name ? host : ssh_host,
+                                   host);
+        if (ret != EOK && ret != EINVAL) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  "ssh_ssh_print_pubkey() failed (%d): %s\n",
+                  ret, strerror(ret));
+            goto done;
+        }
+    }
+
     ret = EOK;
 
 done:
+    talloc_free(ent);
+    /* These two strings were allocated with malloc() */
+    free(discard_const(host));
+    free(discard_const(port));
     if (ai != NULL) {
         freeaddrinfo(ai);
     }
@@ -103,19 +180,20 @@ int main(int argc, const char **argv)
 {
     TALLOC_CTX *mem_ctx = NULL;
     int pc_debug = SSSDBG_TOOLS_DEFAULT;
+    int pc_only_host_name = false;
     const char *pc_domain = NULL;
     const char *pc_host = NULL;
     struct poptOption long_options[] = {
         POPT_AUTOHELP
         { "debug", '\0', POPT_ARG_INT | POPT_ARGFLAG_DOC_HIDDEN, &pc_debug, 0,
           _("The debug level to run with"), NULL },
         { "domain", 'd', POPT_ARG_STRING, &pc_domain, 0,
-          _("The SSSD domain to use"), NULL },
+          _("The SSSD domain to use"), _("domain name") },
+        { "only-host-name", 'o', POPT_ARG_VAL, &pc_only_host_name, true,
+          _("When the key has no host name, add only the host name"), NULL },
         POPT_TABLEEND
     };
     poptContext pc = NULL;
-    struct sss_ssh_ent *ent;
-    size_t i;
     int ret;
     errno_t res;
 
@@ -154,30 +232,19 @@ int main(int argc, const char **argv)
         BAD_POPT_PARAMS(pc, _("Host not specified\n"), ret, fini);
     }
 
-    /* look up the public keys */
-    res = known_hosts(mem_ctx, pc_domain, pc_host, &ent);
-    if (res != EOK) {
-        /* On a successful execution, even if no key was found,
-         * ssh expects EXIT_SUCCESS. */
-        ret = (res == ENOENT ? EXIT_SUCCESS : EXIT_FAILURE);
-        goto fini;
-    }
-
     /* If the other side closes its end of the pipe, we don't want this tool
      * to exit abruptly, but to finish gracefully instead because the valid
      * key can be present in the data already written
      */
     signal(SIGPIPE, SIG_IGN);
 
-    /* print results */
-    for (i = 0; i < ent->num_pubkeys; i++) {
-        ret = sss_ssh_print_pubkey(&ent->pubkeys[i], pc_host);
-        if (ret != EOK && ret != EINVAL) {
-            DEBUG(SSSDBG_CRIT_FAILURE,
-                  "ssh_ssh_print_pubkey() failed (%d): %s\n",
-                  ret, strerror(ret));
-            goto fini;
-        }
+    /* look up the public keys */
+    res = known_hosts(mem_ctx, pc_domain, pc_host, pc_only_host_name);
+    if (res != EOK) {
+        /* On a successful execution, even if no key was found,
+         * ssh expects EXIT_SUCCESS. */
+        ret = (res == ENOENT ? EXIT_SUCCESS : EXIT_FAILURE);
+        goto fini;
     }
 
     ret = EXIT_SUCCESS;

src/sss_client/ssh/sss_ssh_knownhostsproxy.c

@@ -310,7 +310,7 @@ int main(int argc, const char **argv)
         /* print results */
         if (ent != NULL) {
             for (size_t i = 0; i < ent->num_pubkeys; i++) {
-                ret = sss_ssh_print_pubkey(&ent->pubkeys[i], NULL);
+                ret = sss_ssh_print_pubkey(&ent->pubkeys[i], NULL, NULL);
                 if (ret != EOK && ret != EINVAL) {
                     DEBUG(SSSDBG_CRIT_FAILURE,
                           "ssh_ssh_print_pubkey() failed (%d): %s\n",

src/tests/system/tests/test_ipa.py

@@ -51,7 +51,7 @@ def test_ipa__hostpublickeys_by_name(client: Client, ipa: IPA, public_keys: list
         1. All public keys were printed
     :customerscenario: False
     """
-    hostname = f"ssh.{ipa.domain}"
+    hostname = f"ssh-host.{ipa.domain}"
     ip = "10.255.251.10"
 
     ipa.host_account(hostname).add(ip=ip, sshpubkey=public_keys)
@@ -76,24 +76,25 @@ def test_ipa__hostpublickeys_by_shortname(client: Client, ipa: IPA, public_keys:
         2. Configure SSSD with SSH responder
         3. Start SSSD
     :steps:
-        1. Lookup SSH key by running "sss_ssh_knownhosts ssh"
+        1. Lookup SSH key by running "sss_ssh_knownhosts ssh-host"
     :expectedresults:
         1. All public keys were printed
     :customerscenario: False
     """
-    hostname = f"ssh.{ipa.domain}"
+    shortname = "ssh-host"
+    hostname = f"{shortname}.{ipa.domain}"
     ip = "10.255.251.10"
     ipa.host_account(hostname).add(ip=ip, sshpubkey=public_keys)
 
     client.fs.append("/etc/resolv.conf", f"search {ipa.domain}")
     client.sssd.enable_responder("ssh")
     client.sssd.start()
 
-    result = client.sss_ssh_knownhosts("ssh")
+    result = client.sss_ssh_knownhosts(shortname)
     assert result.rc == 0, "Did not get OpenSSH known hosts public keys!"
     assert len(public_keys) == len(result.stdout_lines), "Did not get expected number of public keys!"
     for key in public_keys:
-        assert f"ssh {key}" in result.stdout_lines, "Did not get expected public keys!"
+        assert f"{shortname} {key}" in result.stdout_lines, "Did not get expected public keys!"
 
 
 @pytest.mark.ticket(gh=5518)
@@ -112,7 +113,7 @@ def test_ipa__hostpublickeys_by_ip(client: Client, ipa: IPA, public_keys: list[s
         1. All public keys were printed
     :customerscenario: False
     """
-    hostname = f"ssh.{ipa.domain}"
+    hostname = f"ssh-host.{ipa.domain}"
     ip = "10.255.251.10"
     ipa.host_account(hostname).add(ip=ip, sshpubkey=public_keys)