feat(aes): fix per review

trustwallet · Nov 18, 2022 · 3398f9c · 3398f9c
1 parent ea37c1d
commit 3398f9c
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 40 deletions.
diff --git a/include/TrustWalletCore/TWStoredKey.h b/include/TrustWalletCore/TWStoredKey.h
@@ -84,13 +84,14 @@ struct TWStoredKey* _Nullable TWStoredKeyImportHDWalletWithEncryption(TWString*
 TW_EXPORT_STATIC_METHOD
 struct TWStoredKey* _Nullable TWStoredKeyImportJSON(TWData* _Nonnull json);
 
-/// Creates a new key, with given encryption strength level.  Returned object needs to be deleted.
+/// Creates a new key, with given encryption strength level. Returned object needs to be deleted.
 ///
 /// \param name The name of the key to be stored
 /// \param password Non-null block of data, password of the stored key
 /// \param encryptionLevel The level of encryption, see \TWStoredKeyEncryptionLevel
 /// \note Returned object needs to be deleted with \TWStoredKeyDelete
 /// \return The stored key as a non-null pointer
+[[deprecated("Use TWStoredKeyCreateLevelAndEncryption instead")]]
 TW_EXPORT_STATIC_METHOD
 struct TWStoredKey* _Nonnull TWStoredKeyCreateLevel(TWString* _Nonnull name, TWData* _Nonnull password, enum TWStoredKeyEncryptionLevel encryptionLevel);
 

diff --git a/src/Keystore/AESParameters.cpp b/src/Keystore/AESParameters.cpp
@@ -14,35 +14,28 @@ using namespace TW;
 
 namespace {
 
-constexpr std::size_t gBlockSize{16};
-
-Data generateIv(std::size_t blockSize = gBlockSize) {
-    // https://www.reddit.com/r/crypto/comments/30x5xg/what_length_should_the_iv_be_for_aes256ctr/
-    // First off, AES 128 uses a 128 bit key.
-    // So if you're using AES 256, you're using a 256 bit key.
-    // Let's not confuse the block length with key length here.
-    // For AES, your block length is always going to be 128 bits/16 bytes regardless of the key length used
+Data generateIv(std::size_t blockSize = TW::Keystore::gBlockSize) {
     auto iv = Data(blockSize, 0);
     random_buffer(iv.data(), blockSize);
     return iv;
 }
 
 static TWStoredKeyEncryption getCipher(const std::string& cipher) {
-    if (cipher == "aes-128-ctr") {
+    if (cipher == Keystore::gAes128Ctr) {
         return TWStoredKeyEncryption::TWStoredKeyEncryptionAes128Ctr;
-    } else if (cipher == "aes-192-ctr") {
+    } else if (cipher == Keystore::gAes192Ctr) {
         return TWStoredKeyEncryption::TWStoredKeyEncryptionAes192Ctr;
-    } else if (cipher == "aes-256-ctr") {
+    } else if (cipher == Keystore::gAes256Ctr) {
         return TWStoredKeyEncryption::TWStoredKeyEncryptionAes256Ctr;
     }
     return TWStoredKeyEncryptionAes128Ctr;
 }
 
 const std::unordered_map<TWStoredKeyEncryption, Keystore::AESParameters> gEncryptionRegistry{
-    {TWStoredKeyEncryptionAes128Ctr, Keystore::AESParameters{.mKeyLength = Keystore::A128, .mCipher = "aes-128-ctr", .mCipherEncryption = TWStoredKeyEncryptionAes128Ctr}},
-    {TWStoredKeyEncryptionAes128Cbc, Keystore::AESParameters{.mKeyLength = Keystore::A128, .mCipher = "aes-128-cbc", .mCipherEncryption = TWStoredKeyEncryptionAes128Cbc}},
-    {TWStoredKeyEncryptionAes192Ctr, Keystore::AESParameters{.mKeyLength = Keystore::A192, .mCipher = "aes-192-ctr", .mCipherEncryption = TWStoredKeyEncryptionAes192Ctr}},
-    {TWStoredKeyEncryptionAes256Ctr, Keystore::AESParameters{.mKeyLength = Keystore::A256, .mCipher = "aes-256-ctr", .mCipherEncryption = TWStoredKeyEncryptionAes256Ctr}}
+    {TWStoredKeyEncryptionAes128Ctr, Keystore::AESParameters{.mKeyLength = Keystore::A128, .mCipher = Keystore::gAes128Ctr, .mCipherEncryption = TWStoredKeyEncryptionAes128Ctr}},
+    {TWStoredKeyEncryptionAes128Cbc, Keystore::AESParameters{.mKeyLength = Keystore::A128, .mCipher = Keystore::gAes128Cbc, .mCipherEncryption = TWStoredKeyEncryptionAes128Cbc}},
+    {TWStoredKeyEncryptionAes192Ctr, Keystore::AESParameters{.mKeyLength = Keystore::A192, .mCipher = Keystore::gAes192Ctr, .mCipherEncryption = TWStoredKeyEncryptionAes192Ctr}},
+    {TWStoredKeyEncryptionAes256Ctr, Keystore::AESParameters{.mKeyLength = Keystore::A256, .mCipher = Keystore::gAes256Ctr, .mCipherEncryption = TWStoredKeyEncryptionAes256Ctr}}
 };
 } // namespace
 

diff --git a/src/Keystore/AESParameters.h b/src/Keystore/AESParameters.h
@@ -13,19 +13,25 @@
 
 namespace TW::Keystore {
 
-enum AESSize: std::int32_t {
+enum AESKeySize : std::int32_t {
     Uninitialized = 0,
     A128 = 16,
     A192 = 24,
     A256 = 32,
 };
 
+inline constexpr std::size_t gBlockSize{16};
+inline constexpr const char* gAes128Ctr{"aes-128-ctr"};
+inline constexpr const char* gAes128Cbc{"aes-128-cbc"};
+inline constexpr const char* gAes192Ctr{"aes-192-ctr"};
+inline constexpr const char* gAes256Ctr{"aes-256-ctr"};
+
 // AES128/192/256 parameters.
 struct AESParameters {
     // For AES, your block length is always going to be 128 bits/16 bytes
-    std::int32_t mBlockSize{16};
+    std::int32_t mBlockSize{gBlockSize};
     std::int32_t mKeyLength{A128};
-    std::string  mCipher{"aes-128-ctr"};
+    std::string  mCipher{gAes128Ctr};
     TWStoredKeyEncryption mCipherEncryption{TWStoredKeyEncryptionAes128Ctr};
     Data iv;
 

diff --git a/src/interface/TWStoredKey.cpp b/src/interface/TWStoredKey.cpp
@@ -40,7 +40,7 @@ struct TWStoredKey* _Nonnull TWStoredKeyCreateEncryption(TWString* _Nonnull name
 }
 
 struct TWStoredKey* _Nonnull TWStoredKeyCreate(TWString* _Nonnull name, TWData* _Nonnull password) {
-    return TWStoredKeyCreateLevel(name, password, TWStoredKeyEncryptionLevelDefault);
+    return TWStoredKeyCreateLevelAndEncryption(name, password, TWStoredKeyEncryptionLevelDefault, TWStoredKeyEncryptionAes128Ctr);
 }
 
 struct TWStoredKey* _Nullable TWStoredKeyImportPrivateKey(TWData* _Nonnull privateKey, TWString* _Nonnull name, TWData* _Nonnull password, enum TWCoinType coin) {

diff --git a/tests/chains/XRP/TWAnySignerTests.cpp b/tests/chains/XRP/TWAnySignerTests.cpp
@@ -84,25 +84,20 @@ TEST(TWAnySignerRipple, SignTrustSetPayment) {
 
 TEST(TWAnySignerRipple, SignTokenPayment0) {
     // https://testnet.xrpl.org/transactions/8F7820892294598B58CFA2E1101D15ED98C179B25A2BA6DAEB4F5B727CB00D4E
-    for (auto value : std::vector({"10", "10e0", "10.0", "10.0e0", "1e1", ".1e2", "0.1e2", "100e-1", "10000000000000000e-15", "0.0000000000000001e17"})) {
-        auto key = parse_hex("4ba5fd2ebf0f5d7e579b3c354c263ebb39cda4093845125786a280301af14e21");
-        Proto::SigningInput input;
-
-        input.mutable_op_payment()->mutable_currency_amount()->set_currency("USD");
-        input.mutable_op_payment()->mutable_currency_amount()->set_value("10");
-        input.mutable_op_payment()->mutable_currency_amount()->set_issuer("rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn");
-        input.set_fee(10);
-        input.set_sequence(32268645);
-        input.set_last_ledger_sequence(32268666);
-        input.set_account("raPAA61ca99bdwNiZs5JJukR5rvkHWvkBX");
-        input.mutable_op_payment()->set_destination("rU893viamSnsfP3zjzM2KPxjqZjXSXK6VF");
-        input.set_private_key(key.data(), key.size());
-
-        Proto::SigningOutput output;
-        ANY_SIGN(input, TWCoinTypeXRP);
-
-        EXPECT_EQ(hex(output.encoded()), "12000022000000002401ec6165201b01ec617a61d4c38d7ea4c6800000000000000000000000000055534400000000004b4e9c06f24296074f7bc48f92a97916c6dc5ea968400000000000000a7321020652a477b0cca8b74d6e68a6a386a836b226101617481b95180eaffbe841b3227446304402203e925caeb05006afb135254e9ae4e46de2019db6c6f68614ef969885063a777602206af110fc29775256fcad8b14974c6a838141d82193192d3b57324fe1079afa1781143b2fa4f36553e5b7a4f54ff9e6883e44b4b0dbb383148132e4e20aecf29090ac428a9c43f230a829220d");
-    }
+    auto key = parse_hex("4ba5fd2ebf0f5d7e579b3c354c263ebb39cda4093845125786a280301af14e21");
+    Proto::SigningInput input;
+    input.mutable_op_payment()->mutable_currency_amount()->set_currency("USD");
+    input.mutable_op_payment()->mutable_currency_amount()->set_value("10");
+    input.mutable_op_payment()->mutable_currency_amount()->set_issuer("rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn");
+    input.set_fee(10);
+    input.set_sequence(32268645);
+    input.set_last_ledger_sequence(32268666);
+    input.set_account("raPAA61ca99bdwNiZs5JJukR5rvkHWvkBX");
+    input.mutable_op_payment()->set_destination("rU893viamSnsfP3zjzM2KPxjqZjXSXK6VF");
+    input.set_private_key(key.data(), key.size());
+    Proto::SigningOutput output;
+    ANY_SIGN(input, TWCoinTypeXRP);
+    EXPECT_EQ(hex(output.encoded()), "12000022000000002401ec6165201b01ec617a61d4c38d7ea4c6800000000000000000000000000055534400000000004b4e9c06f24296074f7bc48f92a97916c6dc5ea968400000000000000a7321020652a477b0cca8b74d6e68a6a386a836b226101617481b95180eaffbe841b3227446304402203e925caeb05006afb135254e9ae4e46de2019db6c6f68614ef969885063a777602206af110fc29775256fcad8b14974c6a838141d82193192d3b57324fe1079afa1781143b2fa4f36553e5b7a4f54ff9e6883e44b4b0dbb383148132e4e20aecf29090ac428a9c43f230a829220d");
 }
 
 TEST(TWAnySignerRipple, SignTokenPayment1) {

diff --git a/tests/interface/TWStoredKeyTests.cpp b/tests/interface/TWStoredKeyTests.cpp
@@ -57,7 +57,7 @@ TEST(TWStoredKey, createWallet) {
     const auto name = WRAPS(TWStringCreateWithUTF8Bytes("name"));
     const auto passwordString = WRAPS(TWStringCreateWithUTF8Bytes("password"));
     const auto password = WRAPD(TWDataCreateWithBytes(reinterpret_cast<const uint8_t *>(TWStringUTF8Bytes(passwordString.get())), TWStringSize(passwordString.get())));
-    const auto key = WRAP(TWStoredKey, TWStoredKeyCreateLevel(name.get(), password.get(), TWStoredKeyEncryptionLevelDefault));
+    const auto key = WRAP(TWStoredKey, TWStoredKeyCreateLevelAndEncryption(name.get(), password.get(), TWStoredKeyEncryptionLevelDefault, TWStoredKeyEncryptionAes128Ctr));
     const auto name2 = WRAPS(TWStoredKeyName(key.get()));
     EXPECT_EQ(string(TWStringUTF8Bytes(name2.get())), "name");
     const auto mnemonic = WRAPS(TWStoredKeyDecryptMnemonic(key.get(), password.get()));