Implement AES-CTR for NSS.

crypto/encryptor_unittest.cc

Index: crypto/encryptor_unittest.cc
diff --git a/crypto/encryptor_unittest.cc b/crypto/encryptor_unittest.cc
index b916854a9d81e9b71cc51457bc4512dcdf1ae602..44105bf93004bda9a0c1f8da433ca318845d574c 100644
--- a/crypto/encryptor_unittest.cc
+++ b/crypto/encryptor_unittest.cc
@@ -35,6 +35,50 @@ TEST(EncryptorTest, EncryptDecrypt) {
   EXPECT_EQ(plaintext, decypted);
 }
 
+// ECB mode encryption is only implemented using NSS.
+#if defined(OS_LINUX) && defined(USE_NSS)

[Ryan Sleevi, 2011/06/02 01:19:23]

nit: Just "defined(USE_NSS)" is sufficient.

+
+TEST(EncryptorTest, EncryptDecryptECB) {
+  scoped_ptr<crypto::SymmetricKey> key(
+      crypto::SymmetricKey::DeriveKeyFromPassword(

[Ryan Sleevi, 2011/06/02 01:19:23]

It would be better for the valgrind/heapchecker bots if this was just
GenerateRandomKey() instead. The reason being the number of iterations that
PBKDF2 requires to derive often cause the bots to timeout.

Same for line 64/65.

+          crypto::SymmetricKey::AES, "password", "saltiest", 1000, 128));
+  EXPECT_TRUE(NULL != key.get());

[Ryan Sleevi, 2011/06/02 01:19:23]

nit?: EXPECT_NE?

My own opinion is that it's fine as written, since the benefit of EXPECT_NE is
simply to print the values, and having it print NULL/0 is silly. That said,
there seems to have been a number of CLs lately towards the most-specific
EXPECT_ case, so I'm not sure if that's a new/emerging Chromium style.

+
+  crypto::Encryptor encryptor;
+  EXPECT_TRUE(encryptor.Init(key.get(), crypto::Encryptor::ECB, ""));
+
+  std::string plaintext("normal plaintext");

[Ryan Sleevi, 2011/06/02 01:19:23]

It's not clear from this string, compared with line 71 & 75, why this is valid
plaintext but line 71 is invalid. Please add a comment here about why "normal
plaintext" is ok, while "invalid plaintext" is magically not ok (16 bytes vs 17)

+  std::string ciphertext;
+  EXPECT_TRUE(encryptor.Encrypt(plaintext, &ciphertext));

[Ryan Sleevi, 2011/06/02 01:19:23]

nit: Since this is ECB mode, I think you may want to make sure to test the
property of no extra padding being added.

EXPECT_EQ(plaintext.size(), ciphertext.size())

+
+  EXPECT_LT(0U, ciphertext.size());
+
+  std::string decypted;
+  EXPECT_TRUE(encryptor.Decrypt(ciphertext, &decypted));

[Ryan Sleevi, 2011/06/02 01:19:23]

nit: Is the "normal" use case to use two Encryptors, one for writing, the other
for reading? Should that be tested here by initializing a new Decryptor before
decrypting?

The hypothetical edge case trying to be caught here is where Encrypt() modifies
some state of the underlying key that causes Decrypt() to behave differently
than if directly initialized and Decrypt() called (without calling Encrypt
first)

I don't feel strongly about this, since ECB should be stateless, but I wasn't
sure if it should be a common-case that is tested for regressions. May be
overkill.

+
+  EXPECT_EQ(plaintext, decypted);
+}
+
+TEST(EncryptorTest, ECBNoPadding) {
+  scoped_ptr<crypto::SymmetricKey> key(
+      crypto::SymmetricKey::DeriveKeyFromPassword(
+          crypto::SymmetricKey::AES, "password", "saltiest", 1000, 128));
+  EXPECT_TRUE(NULL != key.get());
+
+  crypto::Encryptor encryptor;
+  EXPECT_TRUE(encryptor.Init(key.get(), crypto::Encryptor::ECB, ""));
+
+  std::string plaintext("invalid plaintext");
+  std::string ciphertext;
+  EXPECT_FALSE(encryptor.Encrypt(plaintext, &ciphertext));
+
+  ciphertext = "invalid cipher text";
+  std::string decypted;
+  EXPECT_FALSE(encryptor.Decrypt(ciphertext, &decypted));
+}
+
+#endif
+
 // TODO(wtc): add more known-answer tests.  Test vectors are available from
 // http://www.ietf.org/rfc/rfc3602
 // http://csrc.nist.gov/publications/nistpubs/800-38a/sp800-38a.pdf