[ovs-dev] [PATCH 6/6] vswitch: Add support for IPsec certificate authentication.
Ben Pfaff
blp at nicira.com
Wed Dec 22 21:20:34 UTC 2010
[Reid, I am a blind man wandering in this Pythonic wilderness, so your
feedback on the original patch would be nice too.]
On Wed, Dec 22, 2010 at 12:04:34AM -0800, Justin Pettit wrote:
> Previously, it was possible to fake configuring the use of certificate
> authentication for IPsec, but it really just used a static pre-shared key
> behind the scenes. This commit publicly mentions certificate
> authentication and finally does the real work behind the scenes.
I noticed that there was some code in here that creates a file and
chmods it afterward to make it readable only by root. That is insecure,
because it leaves a race window in which an unprivileged process could
open the file (Unix checks permissions only at open time).
It seems to me undesirable that this daemon prevents modifying a host
in-place ("configuration changed for %s, need to delete interface
first"). In contrast, ovs-vswitchd is designed so that it can it can
transition from any current state to any new state. Otherwise, it makes
life difficult for the database client: it means that, to change
something, the client must commit a deletion, then commit an insertion,
in two separate commits. And if ovs-monitor-ipsec's connection to the
database goes down in the middle, so that it only sees the initial and
final states without observing the deletion itself, then it will never
update the interface at all, at least not until the next time it is
killed and restarted. In short, this doesn't look very robust.
In spd_add() and spd_del() (which you didn't change here) it seemed odd
to use a completely separate statement to add a newline.
I don't know Racoon, but I didn't see any other obvious problems.
Thanks,
Ben.
More information about the dev
mailing list