OLD | NEW |
---|---|
(Empty) | |
1 Issues: | |
2 | |
3 1. The |param| input parameter to CTR_CreateContext, CTR_InitContext, etc. | |
4 should be a const void * or a pointer to a concrete type, rather than | |
5 const unsigned char *. Should we change the type to const void * or a | |
6 a pointer to a concrete type, such as const CK_AES_CTR_PARAMS * ? | |
7 | |
8 2. lib/freebl should not use PKCS #11 types defined in pkcs11t.h, such as | |
9 CK_AES_CTR_PARAMS. In addition, ctr.c was written to support any suitable | |
10 block cipher, but CK_AES_CTR_PARAMS is AES-specific. | |
11 | |
12 3. ctr_GetNextCtr does not handle counter rollover. | |
13 | |
14 4. Only single-part operations are supported for CTS and GCM. | |
15 CTS_EncryptUpdate can only be called once on each operation context. | |
16 | |
17 5. Why is the XOR operation defined as a function (ctr_xor) in CTR but as | |
18 a macro (XOR_BLOCK and GCM_XOR) in CTS and GCM? | |
19 | |
20 6. The GCM code for 64-bit block ciphers is not tested. I recommend removing | |
21 it. | |
22 | |
23 7. CK_AES_GCM_PARAMS and CK_AES_CCM_PARAMS were renamed CK_GCM_PARAMS and | |
24 CK_CCM_PARAMS in PKCS #11 v2.30 draft. | |
25 | |
26 8. In ctr.c, ctr->bufPtr satisfies the following invariant: | |
27 0 < ctr->bufPtr <= blocksize | |
28 I can make ctr->bufPtr behave like ghash->bufLen in gcm.c. Want me to do | |
29 that? | |
30 | |
31 9. I found two bugs in CTS_DecryptUpdate where you convert the input buffer | |
32 from CS-1 to CS-2. Since you didn't provide tests for CTS, I didn't try to | |
33 fix these bugs. | |
34 | |
35 ====================================================================== | |
36 | |
37 Patch set 1: | |
38 | |
39 Original patch by rrelyea, except manifest.mn. | |
40 | |
41 ====================================================================== | |
42 | |
43 Patch set 2: | |
44 | |
45 Move lib/util/pkcs11t.h to this patch because lib/freebl needs the | |
46 pkcs11t.h changes. | |
47 | |
48 ====================================================================== | |
49 | |
50 Patch set 3: | |
51 | |
52 Remove the RCS Id lines from new files. I seem to remember we decided to | |
53 remove RCS Id lines from NSS source files. | |
54 | |
55 Add comments. Remove blank lines, spaces at the end of lines. Fix typos. | |
56 | |
57 Change freeblDestroyFunc to return void instead of SECStatus. | |
58 | |
59 Check that blocksize is <= the sizes of various block buffers. | |
60 | |
61 Change BITS_PER_BYTE (deprecated NSPR macro name) to PR_BITS_PER_BYTE. | |
62 | |
63 Fix a bug in ctr_GetNextCtr: *counterPtr should be pre-incremented. | |
64 | |
65 CTR_Update does not need the local cipherblock buffer. It can just use | |
66 ctr->buffer when ctr->buffer is empty. | |
67 | |
68 Mark internal functions as static. | |
69 | |
70 Change some variables of type int to unsigned int. | |
71 | |
72 Use NSS_SecureMemcmp instead of PORT_Memcmp to check the GCM authentication | |
73 tag. | |
74 | |
75 Make AES_InitContext support the three new modes (CTS, CTR, and GCM), | |
76 so that AES_CreateContext and AES_InitContext differ only in whether | |
77 the AESContext structure is allocated. | |
rjrejyea
2012/09/19 22:52:48
OK, so this is a bad idea, but it's not obvious wh
wtc
2012/09/20 23:27:23
Bob: thanks for the info. The reason for the asymm
| |
78 | |
79 AES_DestroyContext should still call cx->destroy when freeit is false. | |
80 | |
81 ====================================================================== | |
82 | |
83 Patch set 3: | |
wtc
2012/09/18 01:03:40
This is a typo. This should say "Patch set 4".
| |
84 | |
85 Add comments. Fix typos. | |
86 | |
87 Change some variables of type int to unsigned int. | |
88 | |
89 Improve the input validation in CTR_InitContext. | |
90 | |
91 Fix a bug in the generation of pre-counter block for 64-bit block ciphers | |
92 in GCM_CreateContext. Note that the spec is gcm-revised-spec.pdf, dated | |
93 May 31, 2005. | |
94 | |
95 Declare poly_128 and poly_64 as const. | |
96 | |
97 Fix a 32-bit bug in GCM code. cLen must be a 64-bit unsigned integer, so | |
98 declaring cLen as unsigned long is not portable. | |
OLD | NEW |