NOTE that two critical files (tcp.c, and the new tcp_sig.c) were not
reviewed. I was mostly looking to see how the changes interact with the SPD
and SADB paths of IPsec and its utilities.
Sorry for not giving more time,
Dan
===================== (Cut up to and including here.) =====================
Reviewer Name: Dan McDonald
Document/Module Title: TCP signature option
Document/Module Author: Girish Moodalbail
Document/Module Version/Date: 25 July 2008
Reviewer Preparation Time: 2 hours
usr/src/lib/libipsecutil/common/ipsec_util.c
usr/src/uts/common/inet/ip.h
usr/src/uts/common/inet/ip/ip.c
usr/src/uts/common/inet/ip/spdsock.c
usr/src/uts/common/inet/ip_stack.h
usr/src/uts/common/inet/ipsec_impl.h
usr/src/uts/common/inet/tcp.h
usr/src/uts/common/net/pfkeyv2.h
usr/src/uts/common/net/pfpolicy.h
usr/src/uts/common/netinet/tcp.h
------- --------------- ------- -----------------------------------------------
No. Location Sev. Comment
------- --------------- ------- -----------------------------------------------
DM-0 Looks good, no comments needed.
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?
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?
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.
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.
DM-5 line 2460 E4 Clarify the comment some more (mention
tcpsig_sa() is called by keysock_passdown(),
e.g.).
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.
usr/src/uts/common/inet/tcp/tcp.c
usr/src/uts/common/inet/tcp/tcp_sig.c
------- --------------- ------- -----------------------------------------------
No. Location Sev. Comment
------- --------------- ------- -----------------------------------------------
DM-7 General I will defer these to someone who knows
the inner workings of TCP a little more.
I can say that it appears you accept the
results of a conn_t's IPsec latched actions,
which to me appears the right way to do
things.
