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..76873a266ee81c6d94eea91f10c20817c33b0c83 100644
--- a/crypto/encryptor_unittest.cc
+++ b/crypto/encryptor_unittest.cc
@@ -35,6 +35,66 @@ TEST(EncryptorTest, EncryptDecrypt) {
   EXPECT_EQ(plaintext, decypted);
 }
 
+// ECB mode encryption is only implemented using NSS.

[wtc, 2011/06/24 18:06:06]

ECB => CTR

[Alpha Left Google, 2011/06/24 18:52:27]

Done.

+#if defined(USE_NSS)
+
+TEST(EncryptorTest, EncryptDecryptCTR) {
+  scoped_ptr<crypto::SymmetricKey> key(
+      crypto::SymmetricKey::GenerateRandomKey(
+          crypto::SymmetricKey::AES, 128));
+
+  EXPECT_TRUE(NULL != key.get());
+  const std::string kInitialCounter = "0000000000000000";

[Chris Palmer, 2011/10/03 19:50:09]

Since you make callers provide this value, chances are quite high that they will
get it wrong — such as by following your example here. (Unit tests are where I
would look for example calling code, especially since you don't provide
documentation in encryptor.h.)

In fact, people might feel required to set the counter to a constant starting
value (after all, it's a *counter*, not a nonce or an IV or a key). That would
be exactly wrong.

Normally in CTR mode we use a nonce distinct from a counter to clarify the
distinction between the nonce (random) and the counter (sequential and
predictable). The nonce should be random because, for security reasons, any
given (nonce, key) pair MUST only ever be used ONCE. (Same as any stream
cipher.)

Then, internally to CryptCTR, you concatenate the nonce and the counter.

If the caller is smart and sets the counter to be completely random, they would
get security but they might not be able to encrypt as many bytes as they had
hoped to: there is no particular guaranteed range of the counter anymore. With
distinct 64-bit nonce + 64-bit counter, the range is well-defined. (This is also
a security guarantee, since you can't ever encrypt more than 2**64 messages
(each with its own nonce) and each message can't be more than 2**64 blocks long.
Those are generous limits, and it's important for callers to know what the
limits are. In your scheme, the limits are not clear.

If the caller is REALLY smart, they can set the counter to have the high-order
64 bits be random (the nonce), and the low-order bits be 0s (the counter). Then
they could get the right behavior for themselves. But why require callers to be
that smart, and to know the internals of your implementation that well?

I would suggest using a 64-bit counter and a 64-bit nonce, and generating the
nonce for the caller or at least documenting that it MUST be random. Follow that
example in this unit test code.

It might also make sense to make the interface clearer. Instead of

    Encryptor::Init(key, mode, iv)

how about:

    Encryptor::InitCBC(key, iv)
    Encryptor::InitCTR(key, nonce)

However, it would be ok to use the existing Encryptor::Init(key, mode, iv)
interface and, in case of mode == CTR, expect nonce to be 64 random bytes (and
document that expectation). Then you could get rid of Encryptor::UpdateCounter.

+
+  crypto::Encryptor encryptor;
+  EXPECT_TRUE(encryptor.Init(key.get(), crypto::Encryptor::CTR, ""));
+  EXPECT_TRUE(encryptor.UpdateCounter(kInitialCounter));
+
+  std::string plaintext("normal plaintext of random length");
+  std::string ciphertext;
+  EXPECT_TRUE(encryptor.Encrypt(plaintext, &ciphertext));
+  EXPECT_LT(0U, ciphertext.size());

[Chris Palmer, 2011/10/03 19:50:09]

Check for the exact ciphertext size. As I mentioned earlier, the ciphertext can
be the exact same size as the plaintext; if you pad to an integer number of
blocks, you'll know the exact size then, too.

+
+  std::string decypted;
+  EXPECT_TRUE(encryptor.UpdateCounter(kInitialCounter));
+  EXPECT_TRUE(encryptor.Decrypt(ciphertext, &decypted));
+
+  EXPECT_EQ(plaintext, decypted);
+}
+
+TEST(EncryptorTest, CTRCounter) {
+  const int kCounterSize = 16;
+  const char kTest1[] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
+  uint8 buf[16];
+
+  // Increment 10 times.
+  crypto::Encryptor::Counter counter1(std::string(kTest1, kCounterSize));
+  for (int i = 0; i < 10; ++i)
+    counter1.Increment();
+  counter1.Write(buf);
+  EXPECT_EQ(0, memcmp(buf, kTest1, 15));
+  EXPECT_TRUE(buf[15] == 10);
+
+  // Check corner cases.
+  const char kTest2[] = {0, 0, 0, 0, 0, 0, 0, 0,
+                         0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+  const char kExpect2[] = {0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0};
+  crypto::Encryptor::Counter counter2(std::string(kTest2, kCounterSize));
+  counter2.Increment();
+  counter2.Write(buf);
+  EXPECT_EQ(0, memcmp(buf, kExpect2, kCounterSize));
+
+  const char kTest3[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+                         0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+  const char kExpect3[] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
+  crypto::Encryptor::Counter counter3(std::string(kTest3, kCounterSize));
+  counter3.Increment();
+  counter3.Write(buf);
+  EXPECT_EQ(0, memcmp(buf, kExpect3, kCounterSize));
+}
+
+#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