Repository: randombit/botan
Pull request #2960 (2022-04-19 08:53:44)

TLS Client Authentication for the CLI tool
It does what it says on the box. Adding `Command::get_arg_maybe()` for convenience.

Usage:

```
./botan tls_client --client-cert=client.pem --client-cert-key=key.pem --port=443 client-auth.somedomain.com
```


Patch:
diff --git a/doc/cli.rst b/doc/cli.rst
index 8eafece33..b0c64feae 100644
--- a/doc/cli.rst
+++ b/doc/cli.rst
@@ -215,7 +215,7 @@ from which a policy description will be read.
   Prints the list of ciphersuites that will be offered under a particular
   policy/version.
 
-``tls_client host --port=443 --print-certs --policy=default --tls1.0 --tls1.1 --tls1.2 --skip-system-cert-store --trusted-cas= --session-db= --session-db-pass= --next-protocols= --type=tcp``
+``tls_client host --port=443 --print-certs --policy=default --tls1.0 --tls1.1 --tls1.2 --skip-system-cert-store --trusted-cas= --session-db= --session-db-pass= --next-protocols= --type=tcp --client-cert= --client-cert-key=``
   Implements a testing TLS client, which connects to *host* via TCP or UDP on
   port *port*. The TLS version can be set with the flags *tls1.0*, *tls1.1* and
   *tls1.2* of which the lowest specified version is automatically chosen.  If
@@ -224,6 +224,8 @@ from which a policy description will be read.
   prints all certificates in the chain, if *print-certs* is passed.
   *next-protocols* is a comma separated list and specifies the protocols to
   advertise with Application-Layer Protocol Negotiation (ALPN).
+  Pass a path to a client certificate PEM and unencrypted PKCS8 encoded private
+  key if client authentication is required.
 
 ``tls_server cert key --port=443 --type=tcp --policy=default --dump-traces= --max-clients=0 --socket-id=0``
   Implements a testing TLS server, which allows TLS clients to connect and which
diff --git a/src/cli/cli.cpp b/src/cli/cli.cpp
index e58f1a68d..8cf2de99e 100644
--- a/src/cli/cli.cpp
+++ b/src/cli/cli.cpp
@@ -128,6 +128,15 @@ std::string Command::get_arg_or(const std::string& opt_name, const std::string&
    return m_args->get_arg_or(opt_name, otherwise);
    }
 
+std::optional<std::string> Command::get_arg_maybe(const std::string& opt_name) const
+   {
+   auto arg = m_args->get_arg(opt_name);
+   if(arg.empty())
+      return std::nullopt;
+   else
+      return arg;
+   }
+
 size_t Command::get_arg_sz(const std::string& opt_name) const
    {
    return m_args->get_arg_sz(opt_name);
diff --git a/src/cli/cli.h b/src/cli/cli.h
index b7f81277d..2f2080ea9 100644
--- a/src/cli/cli.h
+++ b/src/cli/cli.h
@@ -10,6 +10,7 @@
 #include <botan/build.h>
 #include <functional>
 #include <ostream>
+#include <optional>
 #include <map>
 #include <memory>
 #include <string>
@@ -146,6 +147,11 @@ class Command
       */
       std::string get_arg_or(const std::string& opt_name, const std::string& otherwise) const;
 
+      /*
+      * Like get_arg() but if the argument was not specified or is empty, returns std::nullopt
+      */
+      std::optional<std::string> get_arg_maybe(const std::string& opt_name) const;
+
       size_t get_arg_sz(const std::string& opt_name) const;
 
       uint16_t get_arg_u16(const std::string& opt_name) const;
diff --git a/src/cli/tls_client.cpp b/src/cli/tls_client.cpp
index 279549a9a..177b6d522 100644
--- a/src/cli/tls_client.cpp
+++ b/src/cli/tls_client.cpp
@@ -35,7 +35,8 @@ class TLS_Client final : public Command, public Botan::TLS::Callbacks
       TLS_Client()
          : Command("tls_client host --port=443 --print-certs --policy=default "
                    "--skip-system-cert-store --trusted-cas= "
-                   "--session-db= --session-db-pass= --next-protocols= --type=tcp")
+                   "--session-db= --session-db-pass= --next-protocols= --type=tcp "
+                   "--client-cert= --client-cert-key=")
          {
          init_sockets();
          }
@@ -62,8 +63,6 @@ class TLS_Client final : public Command, public Botan::TLS::Callbacks
 
       void go() override
          {
-         // TODO client cert auth
-
          std::unique_ptr<Botan::TLS::Session_Manager> session_mgr;
 
          const std::string sessions_db = get_arg("session-db");
@@ -124,7 +123,10 @@ class TLS_Client final : public Command, public Botan::TLS::Callbacks
 
          m_sockfd = connect_to_host(host, port, use_tcp);
 
-         Basic_Credentials_Manager creds(use_system_cert_store, trusted_CAs);
+         const auto client_crt_path = get_arg_maybe("client-cert");
+         const auto client_key_path = get_arg_maybe("client-cert-key");
+
+         Basic_Credentials_Manager creds(use_system_cert_store, trusted_CAs, client_crt_path, client_key_path);
 
          Botan::TLS::Client client(*this, *session_mgr, creds, *policy, rng(),
                                    Botan::TLS::Server_Information(hostname, port),
@@ -386,6 +388,17 @@ class TLS_Client final : public Command, public Botan::TLS::Callbacks
             }
          }
 
+      std::vector<uint8_t> tls_sign_message(
+         const Botan::Private_Key& key,
+         Botan::RandomNumberGenerator& rng,
+         const std::string& emsa,
+         Botan::Signature_Format format,
+         const std::vector<uint8_t>& msg) override
+         {
+         output() << "Performing client authentication\n";
+         return Botan::TLS::Callbacks::tls_sign_message(key, rng, emsa, format, msg);
+         }
+
       socket_type m_sockfd = invalid_socket();
    };
 
diff --git a/src/cli/tls_helpers.h b/src/cli/tls_helpers.h
index f93c14719..2772e9fb4 100644
--- a/src/cli/tls_helpers.h
+++ b/src/cli/tls_helpers.h
@@ -36,15 +36,49 @@ inline bool value_exists(const std::vector<std::string>& vec,
 
 class Basic_Credentials_Manager : public Botan::Credentials_Manager
    {
+   protected:
+      void load_credentials(const std::string& crt, const std::string& key)
+         {
+         Certificate_Info cert;
+
+         Botan::DataSource_Stream key_in(key);
+         cert.key = Botan::PKCS8::load_key(key_in);
+
+         Botan::DataSource_Stream in(crt);
+         while(!in.end_of_data())
+            {
+            try
+               {
+               cert.certs.push_back(Botan::X509_Certificate(in));
+               }
+            catch(std::exception&)
+               {
+               }
+            }
+
+         // TODO: attempt to validate chain ourselves
+
+         m_creds.push_back(cert);
+         }
+
    public:
       Basic_Credentials_Manager(bool use_system_store,
-                                const std::string& ca_path)
+                                std::string ca_path,
+                                std::optional<std::string> client_crt = std::nullopt,
+                                std::optional<std::string> client_key = std::nullopt)
          {
          if(ca_path.empty() == false)
             {
             m_certstores.push_back(std::make_shared<Botan::Certificate_Store_In_Memory>(ca_path));
             }
 
+         BOTAN_ARG_CHECK(client_crt.has_value() == client_key.has_value(), "either provide both client certificate and key or neither");
+
+         if(client_crt.has_value() && client_key.has_value())
+            {
+            load_credentials(client_crt.value(), client_key.value());
+            }
+
 #if defined(BOTAN_HAS_CERTSTOR_SYSTEM)
          if(use_system_store)
             {
@@ -58,26 +92,7 @@ class Basic_Credentials_Manager : public Botan::Credentials_Manager
       Basic_Credentials_Manager(const std::string& server_crt,
                                 const std::string& server_key)
          {
-         Certificate_Info cert;
-
-         Botan::DataSource_Stream key_in(server_key);
-         cert.key = Botan::PKCS8::load_key(key_in);
-
-         Botan::DataSource_Stream in(server_crt);
-         while(!in.end_of_data())
-            {
-            try
-               {
-               cert.certs.push_back(Botan::X509_Certificate(in));
-               }
-            catch(std::exception&)
-               {
-               }
-            }
-
-         // TODO: attempt to validate chain ourselves
-
-         m_creds.push_back(cert);
+         load_credentials(server_crt, server_key);
          }
 
       std::vector<Botan::Certificate_Store*>
@@ -100,29 +115,42 @@ class Basic_Credentials_Manager : public Botan::Credentials_Manager
          return v;
          }
 
-      std::vector<Botan::X509_Certificate> cert_chain(
+      std::vector<Botan::X509_Certificate> find_cert_chain(
          const std::vector<std::string>& algos,
+         const std::vector<Botan::X509_DN>& acceptable_cas,
          const std::string& type,
          const std::string& hostname) override
          {
-         BOTAN_UNUSED(type);
-
-         for(auto const& i : m_creds)
+         if(type == "tls-client")
             {
-            if(std::find(algos.begin(), algos.end(), i.key->algo_name()) == algos.end())
+            for(const auto& dn : acceptable_cas)
                {
-               continue;
+               for(const auto &cred : m_creds)
+                  {
+                  if(dn == cred.certs[0].issuer_dn())
+                     return cred.certs;
+                  }
                }
-
-            if(hostname != "" && !i.certs[0].matches_dns_name(hostname))
+            }
+         else if(type == "tls-server")
+            {
+            for(auto const& i : m_creds)
                {
-               continue;
-               }
+               if(std::find(algos.begin(), algos.end(), i.key->algo_name()) == algos.end())
+                  {
+                  continue;
+                  }
+
+               if(hostname != "" && !i.certs[0].matches_dns_name(hostname))
+                  {
+                  continue;
+                  }
 
-            return i.certs;
+               return i.certs;
+               }
             }
 
-         return std::vector<Botan::X509_Certificate>();
+         return {};
          }
 
       Botan::Private_Key* private_key_for(const Botan::X509_Certificate& cert,


Test patch:
diff --git a/src/scripts/test_cli.py b/src/scripts/test_cli.py
index 25001664b..d6a288716 100755
--- a/src/scripts/test_cli.py
+++ b/src/scripts/test_cli.py
@@ -892,8 +892,12 @@ def cli_tls_socket_tests(tmp_dir):
 
     time.sleep(wait_time)
 
-    tls_client.stdin.write(client_msg)
-    tls_client.stdin.flush()
+    try:
+        tls_client.stdin.write(client_msg)
+        tls_client.stdin.flush()
+    except BrokenPipeError:
+        pass # On handshake failure, the stdin pipe of is already closed here
+             # and error reporting below would not be reached.
 
     time.sleep(wait_time)
 
@@ -908,7 +912,10 @@ def cli_tls_socket_tests(tmp_dir):
     if client_msg not in stdout:
         logging.error("Missing client message from stdout %s", stdout)
 
-    tls_server.communicate()
+    (srv_stdout, srv_stderr) = tls_server.communicate()
+    if srv_stderr:
+        logging.error("server said (stdout): %s", srv_stdout)
+        logging.error("server said (stderr): %s", srv_stderr)
 
 def cli_tls_http_server_tests(tmp_dir):
     if not run_socket_tests() or not check_for_command("tls_http_server"):