cilium · aanm · Oct 9, 2020 · Oct 6, 2020 · Oct 6, 2020 · Oct 6, 2020
diff --git a/Documentation/gettingstarted/http.rst b/Documentation/gettingstarted/http.rst
@@ -209,7 +209,7 @@ API call, but disallowing all other calls (including PUT /v1/exhaust-port).
 
 .. literalinclude:: ../../examples/minikube/sw_l3_l4_l7_policy.yaml
 
-Update the existing rule to apply L7-aware policy to protect *app1* using:
+Update the existing rule to apply L7-aware policy to protect *deathstar* using:
 
 .. parsed-literal::
 
@@ -232,6 +232,13 @@ and
     $ kubectl exec tiefighter -- curl -s -XPUT deathstar.default.svc.cluster.local/v1/exhaust-port
     Access denied
 
+As this rule builds on the identity-aware rule, traffic from pods without the label
+``org=empire`` will continue to be dropped causing the connection to time out:
+
+.. parsed-literal::
+    $ kubectl exec xwing -- curl -s -XPOST deathstar.default.svc.cluster.local/v1/request-landing
+
+
 As you can see, with Cilium L7 security policies, we are able to permit
 *tiefighter* to access only the required API resources on *deathstar*, thereby
 implementing a "least privilege" security approach for communication between

diff --git a/Documentation/gettingstarted/k8s-install-connectivity-test.rst b/Documentation/gettingstarted/k8s-install-connectivity-test.rst
@@ -1,11 +1,18 @@
 Deploy the connectivity test
 ----------------------------
 
-You can deploy the "connectivity-check" to test connectivity between pods.
+You can deploy the "connectivity-check" to test connectivity between pods. It is
+recommended to create a separate namespace for this.
 
 .. parsed-literal::
 
-    kubectl apply -f \ |SCM_WEB|\/examples/kubernetes/connectivity-check/connectivity-check.yaml
+    kubectl create ns cilium-test
+
+Deploy the check with:
+
+.. parsed-literal::
+
+    kubectl apply -n cilium-test -f \ |SCM_WEB|\/examples/kubernetes/connectivity-check/connectivity-check.yaml
 
 It will deploy a series of deployments which will use various connectivity
 paths to connect to each other. Connectivity paths include with and without

diff --git a/bpf/bpf_lxc.c b/bpf/bpf_lxc.c
@@ -46,7 +46,7 @@
 static __always_inline bool redirect_to_proxy(int verdict, __u8 dir)
 {
 	return is_defined(ENABLE_HOST_REDIRECT) && verdict > 0 &&
-	       (dir == CT_NEW || dir == CT_ESTABLISHED);
+	       (dir == CT_NEW || dir == CT_ESTABLISHED ||  dir == CT_REOPENED);
 }
 #endif
 
@@ -201,6 +201,10 @@ static __always_inline int ipv6_l3_from_lxc(struct __ctx_buff *ctx,
 		monitor = TRACE_PAYLOAD_LEN;
 		break;
 
+	case CT_REOPENED:
+		send_policy_verdict_notify(ctx, *dstID, tuple->dport,
+					   tuple->nexthdr, POLICY_EGRESS, 1,
+					   verdict, policy_match_type, audited);
 	case CT_ESTABLISHED:
 		/* Did we end up at a stale non-service entry? Recreate if so. */
 		if (unlikely(ct_state.rev_nat_index != ct_state_new.rev_nat_index))
@@ -575,6 +579,10 @@ static __always_inline int handle_ipv4_from_lxc(struct __ctx_buff *ctx,
 			return ret;
 		break;
 
+	case CT_REOPENED:
+		send_policy_verdict_notify(ctx, *dstID, tuple.dport,
+					   tuple.nexthdr, POLICY_EGRESS, 0,
+					   verdict, policy_match_type, audited);
 	case CT_ESTABLISHED:
 		/* Did we end up at a stale non-service entry? Recreate if so. */
 		if (unlikely(ct_state.rev_nat_index != ct_state_new.rev_nat_index))
@@ -933,10 +941,13 @@ ipv6_policy(struct __ctx_buff *ctx, int ifindex, __u32 src_label, __u8 *reason,
 	if (skip_ingress_proxy)
 		verdict = 0;
 
-	if (ret == CT_NEW) {
+	if (ret == CT_NEW || ret == CT_REOPENED) {
 		send_policy_verdict_notify(ctx, src_label, tuple.dport,
 					   tuple.nexthdr, POLICY_INGRESS, 1,
 					   verdict, policy_match_type, audited);
+	}
+
+	if (ret == CT_NEW) {
 #ifdef ENABLE_DSR
 	{
 		bool dsr = false;
@@ -1163,10 +1174,13 @@ ipv4_policy(struct __ctx_buff *ctx, int ifindex, __u32 src_label, __u8 *reason,
 	if (skip_ingress_proxy)
 		verdict = 0;
 
-	if (ret == CT_NEW) {
+	if (ret == CT_NEW || ret == CT_REOPENED) {
 		send_policy_verdict_notify(ctx, src_label, tuple.dport,
 					   tuple.nexthdr, POLICY_INGRESS, 0,
 					   verdict, policy_match_type, audited);
+	}
+
+	if (ret == CT_NEW) {
 #ifdef ENABLE_DSR
 	{
 		bool dsr = false;

diff --git a/bpf/lib/common.h b/bpf/lib/common.h
@@ -528,6 +528,7 @@ enum {
 	CT_ESTABLISHED,
 	CT_REPLY,
 	CT_RELATED,
+	CT_REOPENED,
 };
 
 /* Service flags (lb{4,6}_service->flags) */

diff --git a/bpf/lib/conntrack.h b/bpf/lib/conntrack.h
@@ -245,6 +245,7 @@ static __always_inline __u8 __ct_lookup(const void *map, struct __ctx_buff *ctx,
 			if (unlikely(reopen == (TCP_FLAG_SYN|0x1))) {
 				ct_reset_closing(entry);
 				*monitor = ct_update_timeout(entry, is_tcp, dir, seen_flags);
+				return CT_REOPENED;
 			}
 			break;
 		case ACTION_CLOSE:
@@ -399,7 +400,7 @@ static __always_inline int ct_lookup6(const void *map,
 	ret = __ct_lookup(map, ctx, tuple, action, dir, ct_state, is_tcp,
 			  tcp_flags, monitor);
 	if (ret != CT_NEW) {
-		if (likely(ret == CT_ESTABLISHED)) {
+		if (likely(ret == CT_ESTABLISHED || ret == CT_REOPENED)) {
 			if (unlikely(tuple->flags & TUPLE_F_RELATED))
 				ret = CT_RELATED;
 			else
@@ -581,7 +582,7 @@ static __always_inline int ct_lookup4(const void *map,
 	ret = __ct_lookup(map, ctx, tuple, action, dir, ct_state, is_tcp,
 			  tcp_flags, monitor);
 	if (ret != CT_NEW) {
-		if (likely(ret == CT_ESTABLISHED)) {
+		if (likely(ret == CT_ESTABLISHED || ret == CT_REOPENED)) {
 			if (unlikely(tuple->flags & TUPLE_F_RELATED))
 				ret = CT_RELATED;
 			else

diff --git a/bpf/lib/host_firewall.h b/bpf/lib/host_firewall.h
@@ -79,6 +79,10 @@ ipv6_host_policy_egress(struct __ctx_buff *ctx, __u32 src_id)
 			return ret;
 		break;
 
+	case CT_REOPENED:
+		send_policy_verdict_notify(ctx, dst_id, tuple.dport,
+					   tuple.nexthdr, POLICY_EGRESS, 1,
+					   verdict, policy_match_type, audited);
 	case CT_ESTABLISHED:
 	case CT_RELATED:
 	case CT_REPLY:
@@ -167,6 +171,10 @@ ipv6_host_policy_ingress(struct __ctx_buff *ctx, __u32 *src_id)
 		if (IS_ERR(ret))
 			return ret;
 
+	case CT_REOPENED:
+		send_policy_verdict_notify(ctx, *src_id, tuple.dport,
+					   tuple.nexthdr, POLICY_INGRESS, 1,
+					   verdict, policy_match_type, audited);
 	case CT_ESTABLISHED:
 	case CT_RELATED:
 	case CT_REPLY:
@@ -298,6 +306,10 @@ ipv4_host_policy_egress(struct __ctx_buff *ctx, __u32 src_id,
 			return ret;
 		break;
 
+	case CT_REOPENED:
+		send_policy_verdict_notify(ctx, dst_id, tuple.dport,
+					   tuple.nexthdr, POLICY_EGRESS, 0,
+					   verdict, policy_match_type, audited);
 	case CT_ESTABLISHED:
 	case CT_RELATED:
 	case CT_REPLY:
@@ -343,7 +355,7 @@ ipv4_host_policy_ingress(struct __ctx_buff *ctx, __u32 *src_id)
 	tuple.daddr = ip4->daddr;
 	tuple.saddr = ip4->saddr;
 	l4_off = l3_off + ipv4_hdrlen(ip4);
-#  ifndef IPV4_FRAGMENTS
+#  ifndef ENABLE_IPV4_FRAGMENTS
 	/* Indicate that this is a datagram fragment for which we cannot
 	 * retrieve L4 ports. Do not set flag if we support fragmentation.
 	 */
@@ -389,6 +401,10 @@ ipv4_host_policy_ingress(struct __ctx_buff *ctx, __u32 *src_id)
 		if (IS_ERR(ret))
 			return ret;
 
+	case CT_REOPENED:
+		send_policy_verdict_notify(ctx, *src_id, tuple.dport,
+					   tuple.nexthdr, POLICY_INGRESS, 0,
+					   verdict, policy_match_type, audited);
 	case CT_ESTABLISHED:
 	case CT_RELATED:
 	case CT_REPLY:

diff --git a/bpf/lib/lb.h b/bpf/lib/lb.h
@@ -748,6 +748,7 @@ static __always_inline int lb6_local(const void *map, struct __ctx_buff *ctx,
 		if (IS_ERR(ret))
 			goto drop_no_service;
 		goto update_state;
+	case CT_REOPENED:
 	case CT_ESTABLISHED:
 	case CT_RELATED:
 	case CT_REPLY:
@@ -1256,6 +1257,7 @@ static __always_inline int lb4_local(const void *map, struct __ctx_buff *ctx,
 		if (IS_ERR(ret))
 			goto drop_no_service;
 		goto update_state;
+	case CT_REOPENED:
 	case CT_ESTABLISHED:
 	case CT_RELATED:
 	case CT_REPLY:

diff --git a/bpf/lib/nodeport.h b/bpf/lib/nodeport.h
@@ -602,6 +602,7 @@ static __always_inline int nodeport_lb6(struct __ctx_buff *ctx,
 		}
 		break;
 
+	case CT_REOPENED:
 	case CT_ESTABLISHED:
 	case CT_REPLY:
 		if (unlikely(ct_state.rev_nat_index != svc->rev_nat_index))
@@ -1298,6 +1299,7 @@ static __always_inline int nodeport_lb4(struct __ctx_buff *ctx,
 		}
 		break;
 
+	case CT_REOPENED:
 	case CT_ESTABLISHED:
 	case CT_REPLY:
 		/* Recreate CT entries, as the existing one is stale and belongs

diff --git a/bpf/lib/trace.h b/bpf/lib/trace.h
@@ -40,6 +40,7 @@ enum {
 	TRACE_REASON_CT_ESTABLISHED = CT_ESTABLISHED,
 	TRACE_REASON_CT_REPLY = CT_REPLY,
 	TRACE_REASON_CT_RELATED = CT_RELATED,
+	TRACE_REASON_CT_REOPENED = CT_REOPENED,
 };
 
 #define TRACE_REASON_ENCRYPTED	    0x80

diff --git a/bpf/tests/conntrack_test.h b/bpf/tests/conntrack_test.h
@@ -166,6 +166,16 @@ static void test___ct_lookup(void)
 	assert(monitor == 0);
 	assert(timeout_in(entry, CT_CLOSE_TIMEOUT - 1));
 
+	/* A connection is reopened due to a newly seen SYN.*/
+	advance_time();
+	monitor = 0;
+	seen_flags.value = TCP_FLAG_SYN;
+	res = __ct_lookup(map, &ctx, tuple, ACTION_CREATE, CT_EGRESS,
+			  &ct_state, true, seen_flags, &monitor);
+	assert(res == CT_REOPENED);
+	assert(monitor == TRACE_PAYLOAD_LEN);
+	assert(timeout_in(entry, CT_CONNECTION_LIFETIME_TCP));
+
 	/* Label connection as new if the tuple wasn't previously tracked */
 	tuple = (void *)__TUPLE_NOEXIST;
 	seen_flags.value = TCP_FLAG_SYN;

diff --git a/pkg/contexthelpers/context.go b/pkg/contexthelpers/context.go
@@ -24,7 +24,7 @@ type SuccessChan chan bool
 // NewConditionalTimeoutContext returns a context which is cancelled when
 // success is not reported within the specified timeout
 func NewConditionalTimeoutContext(ctx context.Context, timeout time.Duration) (context.Context, context.CancelFunc, SuccessChan) {
-	ch := make(SuccessChan)
+	ch := make(SuccessChan, 1)
 	c, cancel := context.WithCancel(ctx)
 
 	go func() {

diff --git a/pkg/contexthelpers/context_test.go b/pkg/contexthelpers/context_test.go
@@ -58,6 +58,10 @@ func (b *ContextSuite) TestConditionalTimeoutContext(c *check.C) {
 		c.Errorf("context cancelled despite reporting success")
 	case <-time.After(100 * time.Millisecond):
 	}
-
 	cancel()
+
+	_, _, ch = NewConditionalTimeoutContext(context.Background(), 10*time.Millisecond)
+	time.Sleep(30 * time.Millisecond)
+	// validate that sending to success channel does not deadlock after the timeout
+	ch <- false
 }
diff --git a/pkg/monitor/datapath_trace.go b/pkg/monitor/datapath_trace.go
@@ -90,13 +90,15 @@ const (
 	TraceReasonCtEstablished
 	TraceReasonCtReply
 	TraceReasonCtRelated
+	TraceReasonCtReopened
 )
 
 var traceReasons = map[uint8]string{
 	TraceReasonPolicy:        "new",
 	TraceReasonCtEstablished: "established",
 	TraceReasonCtReply:       "reply",
 	TraceReasonCtRelated:     "related",
+	TraceReasonCtReopened:    "reopened",
 }
 
 func connState(reason uint8) string {