security(scrypt): Force Scrypt parameters to have all n, p, r values

src/Keystore/ScryptParameters.cpp

@@ -6,6 +6,7 @@
 
 #include <TrezorCrypto/rand.h>
 #include <limits>
+#include <sstream>
 
 using namespace TW;
 
@@ -21,6 +22,23 @@ Data randomSalt() {
 
 } // namespace internal
 
+std::string toString(const ScryptValidationError error) {
+    switch (error) {
+    case ScryptValidationError::desiredKeyLengthTooLarge:
+            return "Desired key length is too large";
+    case ScryptValidationError::invalidSaltLength:
+        return "Salt length is invalid";
+    case ScryptValidationError::blockSizeTooLarge:
+            return "Block size (r * p) is too large";
+    case ScryptValidationError::invalidCostFactor:
+            return "Cost factor n must be a power of 2 greater than 1";
+    case ScryptValidationError::overflow:
+            return "Parameters are too large and may cause overflow";
+    default:
+            return "Unknown error";
+    }
+}
+
 ScryptParameters ScryptParameters::minimal() {
     return { internal::randomSalt(), minimalN, defaultR, minimalP, defaultDesiredKeyLength };
 }
@@ -43,6 +61,9 @@ std::optional<ScryptValidationError> ScryptParameters::validate() const {
     if (desiredKeyLength > ((1ULL << 32) - 1) * 32) { // depending on size_t size on platform, may be always false
         return ScryptValidationError::desiredKeyLengthTooLarge;
     }
+    if (salt.size() < minSaltLength || salt.size() > maxSaltLength) {
+        return ScryptValidationError::invalidSaltLength;
+    }
     if (static_cast<uint64_t>(r) * static_cast<uint64_t>(p) >= (1 << 30)) {
         return ScryptValidationError::blockSizeTooLarge;
     }
@@ -71,14 +92,25 @@ static const auto r = "r";
 } // namespace CodingKeys::SP
 
 ScryptParameters::ScryptParameters(const nlohmann::json& json) {
+    if (json.count(CodingKeys::SP::n) == 0
+        || json.count(CodingKeys::SP::p) == 0
+        || json.count(CodingKeys::SP::r) == 0
+        || json.count(CodingKeys::SP::salt) == 0
+        || json.count(CodingKeys::SP::desiredKeyLength) == 0) {
+        throw std::invalid_argument("Missing required scrypt parameters n, p, r, salt, or dklen");
+    }
+
     salt = parse_hex(json[CodingKeys::SP::salt].get<std::string>());
     desiredKeyLength = json[CodingKeys::SP::desiredKeyLength];
-    if (json.count(CodingKeys::SP::n) != 0)
-        n = json[CodingKeys::SP::n];
-    if (json.count(CodingKeys::SP::n) != 0)
-        p = json[CodingKeys::SP::p];
-    if (json.count(CodingKeys::SP::n) != 0)
-        r = json[CodingKeys::SP::r];
+    n = json[CodingKeys::SP::n];
+    p = json[CodingKeys::SP::p];
+    r = json[CodingKeys::SP::r];
+
+    if (const auto error = validate()) {
+        std::stringstream ss;
+        ss << "Invalid scrypt parameters: " << toString(*error);
+        throw std::invalid_argument(ss.str());
+    }
 }
 
 /// Saves `this` as a JSON object.

src/Keystore/ScryptParameters.h

@@ -14,11 +14,14 @@ namespace TW::Keystore {
 
 enum class ScryptValidationError {
     desiredKeyLengthTooLarge,
+    invalidSaltLength,
     blockSizeTooLarge,
     invalidCostFactor,
     overflow,
 };
 
+std::string toString(ScryptValidationError error);
+
 /// Scrypt function parameters.
 struct ScryptParameters {
     /// The N and P parameters of Scrypt encryption algorithm, using 256MB memory and
@@ -40,6 +43,11 @@ struct ScryptParameters {
     /// Default desired key length of Scrypt encryption algorithm.
     static const std::size_t defaultDesiredKeyLength = 32;
 
+    /// Minimum and maximum salt length for Scrypt encryption algorithm.
+    /// https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-132.pdf
+    static const std::size_t minSaltLength = 16;
+    static const std::size_t maxSaltLength = 1024;
+
     /// Random salt.
     Data salt;