Ensure error messages don't leak private key

Since #8766, invalid base64 is rendered in errors, but we don't actually
want to show this in the case of an invalid private keys.

src/libstore/machines.cc

@@ -159,8 +159,9 @@ static Machine parseBuilderLine(const std::set<std::string> & defaultSystems, co
         const auto & str = tokens[fieldIndex];
         try {
             base64Decode(str);
-        } catch (const Error & e) {
+        } catch (FormatError & e) {
-            throw FormatError("bad machine specification: a column #%lu in a row: '%s' is not valid base64 string: %s", fieldIndex, line, e.what());
+            e.addTrace({}, "while parsing machine specification at a column #%lu in a row: '%s'", fieldIndex, line);
+            throw;
         }
         return str;
     };

src/libstore/ssh.cc

@@ -7,6 +7,16 @@
 
 namespace nix {
 
+static std::string parsePublicHostKey(std::string_view host, std::string_view sshPublicHostKey)
+{
+    try {
+        return base64Decode(sshPublicHostKey);
+    } catch (Error & e) {
+        e.addTrace({}, "while decoding ssh public host key for host '%s'", host);
+        throw;
+    }
+}
+
 SSHMaster::SSHMaster(
     std::string_view host,
     std::string_view keyFile,
@@ -15,7 +25,7 @@ SSHMaster::SSHMaster(
     : host(host)
     , fakeSSH(host == "localhost")
     , keyFile(keyFile)
-    , sshPublicHostKey(sshPublicHostKey)
+    , sshPublicHostKey(parsePublicHostKey(host, sshPublicHostKey))
     , useMaster(useMaster && !fakeSSH)
     , compress(compress)
     , logFD(logFD)
@@ -39,7 +49,7 @@ void SSHMaster::addCommonSSHOpts(Strings & args)
         std::filesystem::path fileName = state->tmpDir->path() / "host-key";
         auto p = host.rfind("@");
         std::string thost = p != std::string::npos ? std::string(host, p + 1) : host;
-        writeFile(fileName.string(), thost + " " + base64Decode(sshPublicHostKey) + "\n");
+        writeFile(fileName.string(), thost + " " + sshPublicHostKey + "\n");
         args.insert(args.end(), {"-oUserKnownHostsFile=" + fileName.string()});
     }
     if (compress)

src/libstore/ssh.hh

@@ -14,6 +14,9 @@ private:
     const std::string host;
     bool fakeSSH;
     const std::string keyFile;
+    /**
+     * Raw bytes, not Base64 encoding.
+     */
     const std::string sshPublicHostKey;
     const bool useMaster;
     const bool compress;

src/libutil/signature/local-keys.cc

@@ -14,17 +14,22 @@ BorrowedCryptoValue BorrowedCryptoValue::parse(std::string_view s)
     return {s.substr(0, colon), s.substr(colon + 1)};
 }
 
-Key::Key(std::string_view s)
+Key::Key(std::string_view s, bool sensitiveValue)
 {
     auto ss = BorrowedCryptoValue::parse(s);
 
     name = ss.name;
     key = ss.payload;
 
-    if (name == "" || key == "")
+    try {
-        throw Error("key is corrupt");
+        if (name == "" || key == "")
+            throw FormatError("key is corrupt");
 
-    key = base64Decode(key);
+        key = base64Decode(key);
+    } catch (Error & e) {
+        e.addTrace({}, "while decoding key named '%s'", name);
+        throw;
+    }
 }
 
 std::string Key::to_string() const
@@ -33,7 +38,7 @@ std::string Key::to_string() const
 }
 
 SecretKey::SecretKey(std::string_view s)
-    : Key(s)
+    : Key{s, true}
 {
     if (key.size() != crypto_sign_SECRETKEYBYTES)
         throw Error("secret key is not valid");
@@ -66,7 +71,7 @@ SecretKey SecretKey::generate(std::string_view name)
 }
 
 PublicKey::PublicKey(std::string_view s)
-    : Key(s)
+    : Key{s, false}
 {
     if (key.size() != crypto_sign_PUBLICKEYBYTES)
         throw Error("public key is not valid");

src/libutil/signature/local-keys.hh

@@ -31,15 +31,19 @@ struct Key
     std::string name;
     std::string key;
 
-    /**
-     * Construct Key from a string in the format
-     * ‘<name>:<key-in-base64>’.
-     */
-    Key(std::string_view s);
-
     std::string to_string() const;
 
 protected:
+
+    /**
+     * Construct Key from a string in the format
+     * ‘<name>:<key-in-base64>’.
+     *
+     * @param sensitiveValue Avoid displaying the raw Base64 in error
+     * messages to avoid leaking private keys.
+     */
+    Key(std::string_view s, bool sensitiveValue);
+
     Key(std::string_view name, std::string && key)
         : name(name), key(std::move(key)) { }
 };

src/libutil/util.cc

@@ -220,7 +220,7 @@ std::string base64Encode(std::string_view s)
 }
 
 
-std::string base64Decode(std::string_view s)
+std::string base64Decode(std::string_view s, bool sensitiveValue)
 {
     constexpr char npos = -1;
     constexpr std::array<char, 256> base64DecodeChars = [&] {
@@ -244,7 +244,10 @@ std::string base64Decode(std::string_view s)
 
         char digit = base64DecodeChars[(unsigned char) c];
         if (digit == npos) {
-            throw Error("invalid character in Base64 string: '%c' in '%s'", c, s.data());
+            auto msg = sensitiveValue
+                ? "<redacted>"
+                : fmt("'%s'", s.data());
+            throw FormatError("invalid character in Base64 string: '%c' in %s", c, msg);
         }
 
         bits += 6;

src/libutil/util.hh

@@ -172,10 +172,17 @@ constexpr char treeNull[] = "    ";
 
 
 /**
- * Base64 encoding/decoding.
+ * Encode arbitrary bytes as Base64.
  */
 std::string base64Encode(std::string_view s);
-std::string base64Decode(std::string_view s);
+
+/**
+ * Decode arbitrary bytes to Base64.
+ *
+ * @param sensitiveValue Avoid displaying the raw Base64 in error messages,
+ * e.g. to avoid leaking private keys.
+ */
+std::string base64Decode(std::string_view s, bool sensitiveValue = false);
 
 
 /**

tests/unit/libexpr/nix_api_expr.cc

@@ -8,7 +8,7 @@
 #include "tests/nix_api_expr.hh"
 #include "tests/string_callback.hh"
 
-#include "gmock/gmock.h"
+#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
 namespace nixC {

tests/unit/libutil/util.cc

@@ -6,6 +6,7 @@
 
 #include <limits.h>
 #include <gtest/gtest.h>
+#include <gmock/gmock.h>
 
 #include <numeric>
 
@@ -99,7 +100,19 @@ TEST(base64Decode, decodeAString)
 
 TEST(base64Decode, decodeThrowsOnInvalidChar)
 {
-    ASSERT_THROW(base64Decode("cXVvZCBlcm_0IGRlbW9uc3RyYW5kdW0="), Error);
+    ASSERT_THROW(base64Decode("cXVvZCBlcm_0IGRlbW9uc3RyYW5kdW0="), FormatError);
+}
+
+TEST(base64Decode, decodeThrowsNoLeak)
+{
+    std::string msg = "cXVvZCBlcm_0IGRlbW9uc3RyYW5kdW0=";
+    try {
+        base64Decode(msg, true);
+    } catch (FormatError & e) {
+        EXPECT_THAT(e.msg(), testing::Not(testing::HasSubstr(msg)));
+        return;
+    }
+    EXPECT_TRUE(false);
 }
 
 /* ----------------------------------------------------------------------------