Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2033)

Unified Diff: nss/mozilla/security/nss/lib/freebl/ReviewComments

Issue 10919163: Add GCM, CTR, and CTS modes to AES. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/deps/third_party/
Patch Set: Created 8 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: nss/mozilla/security/nss/lib/freebl/ReviewComments
===================================================================
--- nss/mozilla/security/nss/lib/freebl/ReviewComments (revision 0)
+++ nss/mozilla/security/nss/lib/freebl/ReviewComments (revision 0)
@@ -0,0 +1,63 @@
+Issues:
+
+1. The |param| input parameter to CTR_CreateContext, CTR_InitContext, etc.
+should be a const void * or a pointer to a concrete type, rather than
+const unsigned char *. Should we change the type to const void * or a
+a pointer to a concrete type, such as const CK_AES_CTR_PARAMS * ?
+
+2. lib/freebl should not use PKCS #11 types defined in pkcs11t.h, such as
+CK_AES_CTR_PARAMS.
+
+3. ctr_GetNextCtr does not handle counter overflow.
+
+4. Only single-part operations are supported for CTS and GCM.
+CTS_EncryptUpdate can only be called once on each operation context.
+
+5. Why is the XOR operation defined as a function (ctr_xor) in CTR but as
+a macro (XOR_BLOCK and GCM_XOR) in CTS and GCM?
+
+======================================================================
+
+Patch set 1:
+
+Original patch by rrelyea, except manifest.mn.
+
+======================================================================
+
+Patch set 2:
+
+Move lib/util/pkcs11t.h to this patch because lib/freebl needs the
+pkcs11t.h changes.
+
+======================================================================
+
+Patch set 3:
+
+Remove the RCS Id lines from new files. I seem to remember we decided to
+remove RCS Id lines from NSS source files.
+
+Add comments. Remove blank lines, spaces at the end of lines. Fix typos.
+
+Change freeblDestroyFunc to return void instead of SECStatus.
+
+Check that blocksize is <= the sizes of various block buffers.
+
+Change BITS_PER_BYTE (deprecated NSPR macro name) to PR_BITS_PER_BYTE.
+
+Fix a bug in ctr_GetNextCtr: *counterPtr should be pre-incremented.
+
+CTR_Update does not need the local cipherblock buffer. It can just use
+ctr->buffer when ctr->buffer is empty.
+
+Mark internal functions as static.
+
+Change some variables of type int to unsigned int.
+
+Use NSS_SecureMemcmp instead of PORT_Memcmp to check the GCM authentication
+tag.
+
+Make AES_InitContext support the three new modes (CTS, CTR, and GCM),
+so that AES_CreateContext and AES_InitContext differ only in whether
+the AESContext structure is allocated.
+
+AES_DestroyContext should still call cx->destroy when freeit is false.
« no previous file with comments | « no previous file | nss/mozilla/security/nss/lib/freebl/blapii.h » ('j') | nss/mozilla/security/nss/lib/freebl/ctr.h » ('J')

Powered by Google App Engine
This is Rietveld 408576698