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. |