Implement AES-CTR for NSS.

crypto/encryptor.h

Index: crypto/encryptor.h
diff --git a/crypto/encryptor.h b/crypto/encryptor.h
index d8250f6bdf48f65ab10f9fb64d400444d1863a9d..804da85bb9360417728dc623db846ef6fdf62f59 100644
--- a/crypto/encryptor.h
+++ b/crypto/encryptor.h
@@ -8,8 +8,8 @@
 
 #include <string>
 
+#include "base/scoped_ptr.h"
 #include "build/build_config.h"

[Ryan Sleevi, 2011/06/08 01:29:23]

IWYU: #include "base/basictypes.h" for uint64/uint8.

[Alpha Left Google, 2011/06/13 23:32:45]

Done.

-
 #if defined(USE_NSS)
 #include "crypto/scoped_nss_types.h"
 #elif defined(OS_WIN)
@@ -23,13 +23,36 @@ class SymmetricKey;
 class Encryptor {
  public:
   enum Mode {
-    CBC
+    CBC,
+    CTR,
+  };
+
+  class Counter {

[Ryan Sleevi, 2011/06/08 01:29:23]

It's not clear why you added virtual methods and made this part of the public
interface, when there are no public methods that an Encryptor takes a Counter,
so there is no way for clients of this API to override with custom behaviour.

Note that if clients did need to derive, I think wtc's comment about
CK_AES_CTR_PARAM is sufficient for the API needs, and wouldn't require any
custom-derived classes to implement.

[Alpha Left Google, 2011/06/13 23:32:45]

I made it non virtual now. We could implement them in Counter later for
different number of bits.

+   public:
+    Counter(const std::string& counter);
+    virtual ~Counter();
+
+    // Increment the counter value.
+    virtual void Increment();
+
+    // Write the content of the counter to |buf|.

[Ryan Sleevi, 2011/06/08 01:29:23]

Documentation nit: Should specify that |buf| should be sized to store
|GetLengthInBytes()| bytes.

API nit?: void* instead of uint8*

[Alpha Left Google, 2011/06/13 23:32:45]

Done.

+    virtual void Write(uint8* buf);
+
+    // Return the length of this counter.
+    virtual const int GetLengthInBytes() const;
+
+   private:
+    uint64 high_num_;
+    uint64 low_num_;
   };
+
   Encryptor();
   virtual ~Encryptor();
 
   // Initializes the encryptor using |key| and |iv|. Returns false if either the
   // key or the initialization vector cannot be used.
+  //
+  // When |mode| is CTR then |iv| should be empty.
   bool Init(SymmetricKey* key, Mode mode, const std::string& iv);
 
   // Encrypts |plaintext| into |ciphertext|.
@@ -38,11 +61,33 @@ class Encryptor {
   // Decrypts |ciphertext| into |plaintext|.
   bool Decrypt(const std::string& ciphertext, std::string* plaintext);
 
+  // Update the counter value when in CTR mode. Currently only 128-bits
+  // counter value is supported.
+  //
+  // Return true only if update was successful.
+  bool UpdateCounter(const std::string& counter);
+
   // TODO(albertb): Support streaming encryption.
 
  private:
+  // Generate a mask using |counter_| to be used for encryption in CTR mode.
+  // Resulting mask will be written to |mask| with |mask_len| bytes.
+  //
+  // The generated mask will always have at least |plaintext_len| bytes and
+  // will be a multiple of the counter length.
+  //
+  // This method is used only in CTR mode.
+  void GenerateCounterMask(int plaintext_len, scoped_array<uint8>* mask,
+                           int* mask_len);

[Ryan Sleevi, 2011/06/08 01:29:23]

style nit: I believe the style guide preference is to use size_t for
|plaintext_len| and |mask_len| (also below)

http://www.chromium.org/developers/coding-style#TOC-Types

style nit: My understanding of
http://www.chromium.org/developers/coding-style#TOC-Code-formatting was that
each argument should be on it's own line for the declaration. I'm not sure if
|plaintext_len| and |mask| have the strict relationship to justify combining.

[Alpha Left Google, 2011/06/13 23:32:45]

Done.

+
+  // Mask the |plaintext| message using |mask|. The output will be written to
+  // |ciphertext|.
+  void MaskMessage(const uint8* plaintext, int plaintext_len,
+                   const uint8* mask, uint8* ciphertext) const;

[Ryan Sleevi, 2011/06/08 01:29:23]

Documentation nit: |ciphertext| must be at least |plaintext_len| bytes.

API nit: Should |plaintext|, |mask|, and |ciphertext| be void* instead of
uint8*? I don't see how from a caller's perspective the type should matter...

[Alpha Left Google, 2011/06/13 23:32:45]

Done.

+
   SymmetricKey* key_;
   Mode mode_;
+  scoped_ptr<Counter> counter_;
 
 #if defined(USE_OPENSSL)
   bool Crypt(bool encrypt,  // Pass true to encrypt, false to decrypt.