Hai-May,
I have one question concerning
http://www.opensolaris.org/os/project/crypto/inprogress/fips/admin-policy-design/
>From section 3, I assume that if there's a hardware provider (such as ncp/0),
>that it is left enabled and only a warning message is printed.
Is this true?
Comments:
DEA-1 adm_kef.c lines 1392-1399
mech_table should be a local variable inside fips_sync_ulp() around line 1408,
as only fips_sync_ulp() uses the variable.
Static variables inside a function are still static and are not on the stack.
DEA-2 adm_kef.c lines 1464-1468
Same comment as DEA-1.
prov_table should be inside fips_sync_ksp() around line 1476.
DEA-3 adm_kef.c lines 1431 and 1434
You allocate memory and store it in argv[3] at line 1431, yet on line 1434 you
overwrite the pointer with a pointer to a string constant ("mechanism=").
As a result, the strcat() on line 1437 is overwriting memory that may be used
for something else.
To fix this, replace the pointer assignment on line 1434 with strlcpy() or
similar string copy function.
DEA-4 adm_kef.c lines 1498 and 1504.
Same problem as DEA-3.
You are overwriting a pointer allocated with calloc() on line 1498 at line
1504. Replace the pointer assignment on line 1505 with a strlcpy() or similar
string copy function.
DEA-5 adm_kef.c lines 1535-1537
You should have a I18N TRANSLATION_NOTE preceeding the error message.
Something like:
/*
* TRANSLATION_NOTE
* %1$s is "enable" or "disable" and is not to be translated.
* %2$s is "global" and is not to be translated.
*/
DEA-6 adm_kef_util.c line 307
Nit: you need a space before and after the + operators:
307 if ((rc = parse_fips(buf+strlen(token1)+1, pent)) != SUCCESS) {
DEA-7 (all files)
Nit: you need to update the copyright to 2009.
The rest looks good to me.
- Dan
--
This message posted from opensolaris.org
