Thanks Dan for the review. Please see in-line. > usr/src/uts/common/net/pfkeyv2.h > Since you are the author of RFC 2367, wanted to cross check the assignment of value "1" to *MD5* algorithm.
#define SADB_AALG_MD5 1 The openbsd header file for instance has it defined to be #define SADB_AALG_MD5 249 (http://fxr.watson.org/fxr/source/net/pfkeyv2.h?v=OPENBSD#L275) > usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ipsecconf.c > > ------- --------------- ------- > ----------------------------------------------- > No. Location Sev. Comment > ------- --------------- ------- > ----------------------------------------------- > > DM-1 Lines 839-43 T2 What if I specify TCP_SIG w/o MD5? Does the > known_algs[] matrix handle this off-the-map > case properly now? > It's mandatory to specify the algorithm type *MD5* when using ipsecconf(1M). If it's not specified no TCP signature policy will be applied and an error message will be displayed > usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ipseckey.c > > ------- --------------- ------- > ----------------------------------------------- > No. Location Sev. Comment > ------- --------------- ------- > ----------------------------------------------- > > DM-2 parseauthstr() T4 Do you handle escape characters or anything > else bizarre? Or does ipseckey(1M)'s parser > do that trick for you? > Only printable ascii characters are allowed as part of the keyword *authstring*. Code valuse from 33 to 126, and no control characters are allowed. Further *add* subcommand of ipseckey can be used only in an interactive mode. In an interactive mode none of the special characters need to be escaped ($, \, &, !, |, et al) as shell would not interpret them. If these characters need to be part of the authstring they can be entered as is in interactive mode. > usr/src/uts/common/Makefile.files > > ------- --------------- ------- > ----------------------------------------------- > No. Location Sev. Comment > ------- --------------- ------- > ----------------------------------------------- > > DM-3 General T1 You're not up to date -- I see that Mark > Powers's putback isn't reflected in your own > gate code yet. > Yes. I just did a bringover and there were no conflicts. > > usr/src/uts/common/inet/ip/keysock.c > > ------- --------------- ------- > ----------------------------------------------- > No. Location Sev. Comment > ------- --------------- ------- > ----------------------------------------------- > > DM-4 keysock_strip() T2 Can you make sadb_strip() subsume this > function or vice-versa? It'll involve making > either keysock dependent on ipsecah or > vice-versa, however. I can be convinced why > it's not a good idea to collapse the two, but > it will take some convincing. > Yes. Made sadb_strip() global. Removed keysock_strip() from keysock.c. Used sadb_strip() in keysock_pfkey_reply() in keysock.c. For this I had to make *keysock* dependent on *ipsecah* as *ipsecah* module contains sadb functions. > DM-5 line 2460 E4 Clarify the comment some more (mention > tcpsig_sa() is called by keysock_passdown(), > e.g.). > Added. > > usr/src/uts/common/inet/ip/spd.c > > ------- --------------- ------- > ----------------------------------------------- > No. Location Sev. Comment > ------- --------------- ------- > ----------------------------------------------- > > DM-6 Line 3203 E3 self-encap implies AH or ESP, so don't > check for it here and clarify the comment to > be more authoritative than an XXX one. > Have added the following comment. /* * If only TCP signature protection is requested, then the * incoming/outgoing packets will not be encrypted so * allow_clear should be B_TRUE else it should be B_FALSE */ regards ~Girish
