Dan, Thanks for your review comments! I'll post webrev diffs shortly.
Mark Dan Anderson wrote: > Note: I didn't compare the GCM code with the NIST spec. > > DEA-1 > Nit: The webrev should have been generated with webrev -O so the link to > 6260053 works outside of Sun > done > DEA-2 kcf_cryptoadm.c > The new AES mechanism string (CKM_AES_GCM) will need to be added at the top > of kcf_cryptoadm.c (lines 47 and 126) if my kcf.conf changes are putback > before your changes: > http://dan.drydog.com/reviews/6414175-kcfconf/usr/src/uts/common/crypto/core/kcf_cryptoadm.c.html > I'll make this change after you putback. > DEA-3 modes.c crypto_free_mode_ctx() > If crypto_free_mode_ctx() is called frequently (my guess it's not_, consider > replacing the ever-growing cascading if with a switch statement on > (common_ctx->cc_flags & (ECB_MODE|CBC_MODE|CTR_MODE|CCM_MODE|GCM_MODE)) > done - looks cleaner too > DEA-4 aes.c > This: > 463 if (((aes_ctx->ac_flags & CTR_MODE) == 0) && > 464 ((aes_ctx->ac_flags & CCM_MODE) == 0) && > 465 ((aes_ctx->ac_flags & GCM_MODE) == 0) && > Could be replaced with this: > if (((aes_ctx->ac_flags & (CTR_MODE|CCM_MODE|GCM_MODE) == 0) && > > Also lines 581-583 and lines 1003-1005 > done > DEA-5 aes.c > 774 if (((aes_ctx->ac_flags & CCM_MODE) == 0) && > 775 ((aes_ctx->ac_flags & GCM_MODE) == 0)) { > COuld be replaced with: > if (((aes_ctx->ac_flags & (CCM_MODE|GCM_MODE) == 0) && > done > DEA-6 gcm.c > You're using ntohll()--yeah! > > DEA-7 gcm.c > Nit: lines 299-300: if statement indented 1 tab too much. > done > -- > This message posted from opensolaris.org > _______________________________________________ > crypto-discuss mailing list > crypto-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/crypto-discuss >
