Optional encryption #1758

awh · 2015-12-07T16:47:28Z

This PR adds support for optional encryption via the --trusted-subnets argument to weave launch. Whilst the control plane is always secured when a password is specified, peers will forego authenticated encryption on the overlay (data plane) connection between them only if both consider the other to be located on a trusted subnet.

I recommend that the commits are reviewed individually in sequence; most commit messages contain further explanatory text.

Fixes #82, #1788.

prog/weaver/http.go

@@ -27,7 +27,7 @@ var rootTemplate = template.New("root").Funcs(map[string]interface{}{
 		}
 		return count
 	},
-	"upstreamServers": func(servers []string) string {
+	"printList": func(servers []string) string {


rade · 2015-12-07T17:08:50Z

I highly recommend "?w=1" for viewing the diff.

awh · 2015-12-07T17:14:56Z

I highly recommend "?w=1" for viewing the diff.

This is why I submitted the whitespace change in a separate commit and recommended reviewing commits individually...

bboreham · 2015-12-09T11:42:33Z

Why did we want to "restore leading whitespace"?

awh · 2015-12-09T17:49:01Z

Why did we want to "restore leading whitespace"?

weave status output was modelled on systemctl status, which has a single space margin down the left hand side; the length of the new TrustedSubnets field required everything to be moved to the right to maintain it.

awh · 2015-12-09T17:51:39Z

Why do we run this on the remote host? Do we expect the resolution of the address to be different there?

The weave routers in GCE connect to each other's private (GCE internal) IP addresses, so we need to resolve their hostnames on the VMs themselves to get the correct trusted subnets.

If so, maybe a comment explaining why would be useful to the next maintainer.

Ah go on then 😄

awh · 2015-12-09T17:59:30Z

I believe I have addressed all comments except the one about trailing curly braces, which I think merits discussion outside of this PR. Will autosquash when you're ready to merge.

bboreham · 2015-12-10T16:25:17Z

LGTM; please squash commits.

awh · 2015-12-11T09:08:05Z

Don't merge - I've found an issue with the overlay switch change.

awh · 2015-12-11T12:35:36Z

I've found an issue with the overlay switch change.

There was actually no need to stop any forwarders at all - now we just record the overlay name in the subforwarder structure (no actual need to do this, but it seemed neater) and continue; PTAL. Will squash again when ready.

bboreham · 2015-12-11T13:25:27Z

LGTM

mesh/connection.go

+	finished        <-chan struct{} // closed to signal that actorLoop has finished
+	OverlayConn     OverlayConnection
+	TrustRemote     bool // is remote on a trusted subnet?
+	TrustedByRemote bool // does remote trust us?


Add a 'Trusted' feature to the protocol exchange which indicates whether the remote considers us to be reachable via a secure network; overlay encryption will be disabled in the event that both peers agree. For backwards compatibility purposes a default value of untrusted is assumed if the feature is missing.

If overlay encryption is required between peers, the fast datapath forwarder returns an error rather than aborting the process, allowing the overlay switch to fall back gracefully to an encrypted sleeve overlay connection.

`weave status` now displays the list of specified trusted subnets, and `weave staus connections` shows the encryption state of individual connections.

Update features and troubleshooting markdown.

awh · 2015-12-15T13:52:12Z

I have added #1788 to the 'fixes' list for this PR, as it is addressed by the commit that disables the pcap optimisation.

Prior to optional encryption support, enabling encryption disabled the fast datapath overlay (because it doesn't support encryption) but left the OVS datapath netdev in place for bridging (so that users could switch encryption on and off without resetting the bridge). In this situation the ODP miss handler is guaranteed to be invoked for every packet, so as an optimisation the weave script configured the router to use pcap to capture packets from the bridge as that is slightly more efficient. The introduction of optional encryption means the guarantee no longer holds, and so the optimisation must be removed.

Make encryption optional; fixes #82, #1788. LGTM.

As of e12dbb4 (#1758) we no longer force the usage of pcap when encryption is enabled. Hence... - we no longer need to figure out whether the router was started with a password when re-attaching it from the proxy. We therefore remove the code for that. - there is no longer any mode in which we pass both `--datapath` and `--iface` options to the router. Previously this would only happen when encryption was enabled and the only overlay used for connections was sleeve. It is not clear what the behaviour is when these conditions do not hold, so, to be safe, we prevent the specification of both flags.

awh added this to the 1.4.0 milestone Dec 7, 2015

awh self-assigned this Dec 7, 2015

rade reviewed Dec 7, 2015
View reviewed changes

prog/weaver/http.go

@@ -27,7 +27,7 @@ var rootTemplate = template.New("root").Funcs(map[string]interface{}{

}

return count

},

"upstreamServers": func(servers []string) string {

"printList": func(servers []string) string {

This comment was marked as abuse.

Sign in to view

awh force-pushed the issues/82-optional-encryption branch from 89af6cd to d6d36d7 Compare December 7, 2015 16:58

awh force-pushed the issues/82-optional-encryption branch 2 times, most recently from f509aff to a910bdc Compare December 9, 2015 10:42

awh removed their assignment Dec 9, 2015

bboreham assigned awh Dec 9, 2015

awh assigned bboreham and unassigned awh Dec 9, 2015

bboreham assigned awh and unassigned bboreham Dec 10, 2015

awh force-pushed the issues/82-optional-encryption branch from 4f1c362 to b1d790f Compare December 10, 2015 17:28

awh assigned bboreham and unassigned awh Dec 11, 2015

bboreham assigned awh and unassigned bboreham Dec 11, 2015

awh force-pushed the issues/82-optional-encryption branch from 760b831 to 629a77e Compare December 11, 2015 13:34

awh removed their assignment Dec 11, 2015

awh assigned bboreham Dec 11, 2015

rade reviewed Dec 11, 2015
View reviewed changes

mesh/connection.go

finished <-chan struct{} // closed to signal that actorLoop has finished

OverlayConn OverlayConnection

TrustRemote bool // is remote on a trusted subnet?

TrustedByRemote bool // does remote trust us?

This comment was marked as abuse.

Sign in to view

bboreham assigned awh and unassigned bboreham Dec 11, 2015

awh added 6 commits December 14, 2015 12:12

Fallback to encrypted sleeve on a per-connection basis

059cc65

If overlay encryption is required between peers, the fast datapath forwarder returns an error rather than aborting the process, allowing the overlay switch to fall back gracefully to an encrypted sleeve overlay connection.

Add trusted subnets to weave status

4e41d4d

`weave status` now displays the list of specified trusted subnets, and `weave staus connections` shows the encryption state of individual connections.

Restore leading whitespace to weave status

e0cb9cf

Document trusted subnets

b9792f8

Update features and troubleshooting markdown.

Smoke test trusted subnets

85d4b1f

awh force-pushed the issues/82-optional-encryption branch from 629a77e to 85d4b1f Compare December 14, 2015 12:14

awh force-pushed the issues/82-optional-encryption branch from 7d43ecc to ba9db8e Compare December 15, 2015 14:07

awh assigned awh and bboreham and unassigned awh Dec 15, 2015

awh force-pushed the issues/82-optional-encryption branch from ba9db8e to e12dbb4 Compare December 15, 2015 14:56

awh mentioned this pull request Dec 15, 2015

weave launch|launch-router tickles NetworkManager #1783

Closed

bboreham added a commit that referenced this pull request Dec 15, 2015

Merge pull request #1758 from /issues/82-optional-encryption

c973808

Make encryption optional; fixes #82, #1788. LGTM.

bboreham merged commit c973808 into master Dec 15, 2015

This was referenced Dec 15, 2015

Prefix interface names with 'veth' so NetworkManager ignores them #1790

Merged

Encrypted overlay performance regression affecting 1.2.0 and 1.3.0 #1788

Closed

awh deleted the issues/82-optional-encryption branch December 17, 2015 10:47

bboreham mentioned this pull request Dec 18, 2015

Authenticated peering #1649

Closed

bboreham mentioned this pull request Jan 11, 2016

prefer private over public IPs for peer connectivty #646

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optional encryption #1758

Optional encryption #1758

awh commented Dec 7, 2015

This comment was marked as abuse.

rade commented Dec 7, 2015

awh commented Dec 7, 2015

bboreham commented Dec 9, 2015

awh commented Dec 9, 2015

awh commented Dec 9, 2015

awh commented Dec 9, 2015

bboreham commented Dec 10, 2015

awh commented Dec 11, 2015

awh commented Dec 11, 2015

bboreham commented Dec 11, 2015

This comment was marked as abuse.

awh commented Dec 15, 2015

Optional encryption #1758

Optional encryption #1758

Conversation

awh commented Dec 7, 2015

This comment was marked as abuse.

rade commented Dec 7, 2015

awh commented Dec 7, 2015

bboreham commented Dec 9, 2015

awh commented Dec 9, 2015

awh commented Dec 9, 2015

awh commented Dec 9, 2015

bboreham commented Dec 10, 2015

awh commented Dec 11, 2015

awh commented Dec 11, 2015

bboreham commented Dec 11, 2015

This comment was marked as abuse.

awh commented Dec 15, 2015