{"action":"opened","issue":{"url":"https://api.github.com/repos/cilium/cilium/issues/2795","repository_url":"https://api.github.com/repos/cilium/cilium","labels_url":"https://api.github.com/repos/cilium/cilium/issues/2795/labels{/name}","comments_url":"https://api.github.com/repos/cilium/cilium/issues/2795/comments","events_url":"https://api.github.com/repos/cilium/cilium/issues/2795/events","html_url":"https://github.com/cilium/cilium/issues/2795","id":296573704,"number":2795,"title":"Revisit whether ConnTrack should be a configurable for an endpoint","user":{"login":"ianvernon","id":3858271,"avatar_url":"https://avatars1.githubusercontent.com/u/3858271?v=4","gravatar_id":"","url":"https://api.github.com/users/ianvernon","html_url":"https://github.com/ianvernon","followers_url":"https://api.github.com/users/ianvernon/followers","following_url":"https://api.github.com/users/ianvernon/following{/other_user}","gists_url":"https://api.github.com/users/ianvernon/gists{/gist_id}","starred_url":"https://api.github.com/users/ianvernon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ianvernon/subscriptions","organizations_url":"https://api.github.com/users/ianvernon/orgs","repos_url":"https://api.github.com/users/ianvernon/repos","events_url":"https://api.github.com/users/ianvernon/events{/privacy}","received_events_url":"https://api.github.com/users/ianvernon/received_events","type":"User","site_admin":false},"labels":[{"id":721252768,"url":"https://api.github.com/repos/cilium/cilium/labels/area/datapath","name":"area/datapath","color":"bfdadc","default":false},{"id":746643464,"url":"https://api.github.com/repos/cilium/cilium/labels/area/policy","name":"area/policy","color":"c9f97a","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-02-13T00:31:18Z","updated_at":"2018-02-13T00:31:18Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"Example:\r\nIf we want to allow identity A to communicate to identity B, then without connection tracking enabled, we have to allow identity B to communicate to identity A - traffic must be allowed in both directions. \r\n\r\nIf ConnTrack is disabled for the endpoints, within the Consumables that applies to those endpoints, we have to find a way to specify which identities have been specified as being allowed to talk to each other. The current implementation in the Consumable structure is to have a map of security identities (ReverseRules) which specify which identities have allowed traffic it; as such, the Consumable must allow traffic in the reverse direction so the connection can be maintained. However, this has a variety of issues associated with it:\r\n\r\n* We enable ConnTrack for an endpoint dependent upon the policy:\r\n\r\n`pkg/endpoint/policy.go:regeneratePolicy`:\r\n```\r\n// Apply possible option changes before regenerating maps, as map regeneration\r\n\t// depends on the conntrack options\r\n\tif c.L4Policy != nil {\r\n\t\tif c.L4Policy.RequiresConntrack() {\r\n\t\t\topts[OptionConntrack] = optionEnabled\r\n\t\t}\r\n\t}\r\n```\r\n\r\nWe thus change something that the user may have configured based on the policy. In my opinion, we should not be changing options that are configurable unless the user specifies them. \r\n\r\n* This does not work in a multinode setup \r\n\r\nThe only way that the ReverseRules map is updated is based off of the `ConsumableCache`, which maps identities to Consumables on the local host. However, if the cache does not contain an entry for the identity for which the reverse rule is being added, as is the case when both identities are on separate nodes in the system, then no such reverse rule is added. This will always be the case when ConnTrack is disabled for two endpoints that are on separate nodes, as the ConsumableCache is only updated with identities local to the host on which the cilium-agent is running.\r\n\r\n* This hasn't been working anyway: \r\n\r\nin `pkg/policy/consumer.go:BanConsumerLocked`, which is called when an identity is no longer allowed ingress access for a Consumable, there is the following logic:\r\n```\r\nif consumer.Reverse != nil {\r\n\tc.deleteReverseRule(id, c.ID)\r\n}\r\n```\r\n\r\nHowever, the `Reverse` field for a Consumer is never populated anywhere in the code. Thus, in the case where endpoints are running on both nodes with ConnTrack disabled, the deletion of an identity from the `ReverseRules` of a Consumable wasn't happening when it should. \r\n\r\nI think this warrants us making this option immutable for an endpoint, or at least specifying clearly in our documentation in which cases this will work / not work (single node setup vs multinode setup). "}}
{"actor":{"display_login":"ianvernon"}}