Merge pull request #4949 from BrianKopp/same-site

Add SameSite support - omit None for old browsers

docs/examples/affinity/cookie/README.md

@@ -12,6 +12,8 @@ Session affinity can be configured using the following annotations:
 |nginx.ingress.kubernetes.io/affinity-mode|The affinity mode defines how sticky a session is. Use `balanced` to redistribute some sessions when scaling pods or `persistent` for maximum stickyness.|`balanced` (default) or `persistent`|
 |nginx.ingress.kubernetes.io/session-cookie-name|Name of the cookie that will be created|string (defaults to `INGRESSCOOKIE`)|
 |nginx.ingress.kubernetes.io/session-cookie-path|Path that will be set on the cookie (required if your [Ingress paths][ingress-paths] use regular expressions)|string (defaults to the currently [matched path][ingress-paths])|
+|nginx.ingress.kubernetes.io/session-cookie-samesite|SameSite attribute to apply to the cookie|Browser accepted values are `None`, `Lax`, and `Strict`|
+|nginx.ingress.kubernetes.io/session-cookie-conditional-samesite-none|Will omit `SameSite=None` attribute for older browsers which reject the more-recently defined `SameSite=None` value|`"true"` or `"false"`
 |nginx.ingress.kubernetes.io/session-cookie-max-age|Time until the cookie expires, corresponds to the `Max-Age` cookie directive|number of seconds|
 |nginx.ingress.kubernetes.io/session-cookie-expires|Legacy version of the previous annotation for compatibility with older browsers, generates an `Expires` cookie directive by adding the seconds to the current date|number of seconds|
 |nginx.ingress.kubernetes.io/session-cookie-change-on-failure|When set to `false` nginx ingress will send request to upstream pointed by sticky cookie even if previous attempt failed. When set to `true` and previous attempt failed, sticky cookie will be changed to point to another upstream.|`true` or `false` (defaults to `false`)|

docs/examples/affinity/cookie/ingress-samesite.yaml

@@ -0,0 +1,40 @@
+apiVersion: networking.k8s.io/v1beta1
+kind: Ingress
+metadata:
+  name: cookie-samesite-none
+  annotations:
+    nginx.ingress.kubernetes.io/affinity: "cookie"
+    nginx.ingress.kubernetes.io/session-cookie-name: "SSNONE"
+    nginx.ingress.kubernetes.io/session-cookie-expires: "172800"
+    nginx.ingress.kubernetes.io/session-cookie-max-age: "172800"
+    nginx.ingress.kubernetes.io/session-cookie-samesite: "None"
+    nginx.ingress.kubernetes.io/session-cookie-conditional-samesite-none: "true" # omits SameSite=None for older browsers which reject cookies with SameSite=None
+spec:
+  rules:
+  - host: stickyingress-samesite-none.example.com
+    http:
+      paths:
+      - backend:
+          serviceName: http-svc
+          servicePort: 80
+        path: /
+---
+apiVersion: networking.k8s.io/v1beta1
+kind: Ingress
+metadata:
+  name: cookie-samesite-strict
+  annotations:
+    nginx.ingress.kubernetes.io/affinity: "cookie"
+    nginx.ingress.kubernetes.io/session-cookie-name: "STRICTCOOKIENAME"
+    nginx.ingress.kubernetes.io/session-cookie-expires: "172800"
+    nginx.ingress.kubernetes.io/session-cookie-max-age: "172800"
+    nginx.ingress.kubernetes.io/session-cookie-samesite: "Strict"
+spec:
+  rules:
+  - host: stickyingress-samesite-strict.example.com
+    http:
+      paths:
+      - backend:
+          serviceName: http-svc
+          servicePort: 80
+        path: /

docs/user-guide/nginx-configuration/annotations.md

@@ -84,6 +84,8 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
 |[nginx.ingress.kubernetes.io/session-cookie-name](#cookie-affinity)|string|
 |[nginx.ingress.kubernetes.io/session-cookie-path](#cookie-affinity)|string|
 |[nginx.ingress.kubernetes.io/session-cookie-change-on-failure](#cookie-affinity)|"true" or "false"|
+|[nginx.ingress.kubernetes.io/session-cookie-samesite](#cookie-affinity)|string|
+|[nginx.ingress.kubernetes.io/session-cookie-conditional-samesite-none](#cookie-affinity)|"true" or "false"|
 |[nginx.ingress.kubernetes.io/ssl-redirect](#server-side-https-enforcement-through-redirect)|"true" or "false"|
 |[nginx.ingress.kubernetes.io/ssl-passthrough](#ssl-passthrough)|"true" or "false"|
 |[nginx.ingress.kubernetes.io/upstream-hash-by](#custom-nginx-upstream-hashing)|string|
@@ -169,6 +171,7 @@ If you use the ``cookie`` affinity type you can also specify the name of the coo
 
 The NGINX annotation `nginx.ingress.kubernetes.io/session-cookie-path` defines the path that will be set on the cookie. This is optional unless the annotation `nginx.ingress.kubernetes.io/use-regex` is set to true; Session cookie paths do not support regex.
 
+Use `nginx.ingress.kubernetes.io/session-cookie-samesite` to apply a `SameSite` attribute to the sticky cookie. Browser accepted values are `None`, `Lax`, and `Strict`. Some older browsers reject cookies with the more-recently-defined `SameSite=None`. To omit `SameSite=None` from these older browsers, add the annotation `nginx.ingress.kubernetes.io/session-cookie-conditional-samesite-none: "true"`.
 
 ### Authentication
 

internal/ingress/annotations/sessionaffinity/main.go

@@ -46,6 +46,12 @@ const (
 	// This is used to control the cookie path when use-regex is set to true
 	annotationAffinityCookiePath = "session-cookie-path"
 
+	// This is used to control the SameSite attribute of the cookie
+	annotationAffinityCookieSameSite = "session-cookie-samesite"
+
+	// This is used to control whether SameSite=None should be conditionally applied based on the User-Agent
+	annotationAffinityCookieConditionalSameSiteNone = "session-cookie-conditional-samesite-none"
+
 	// This is used to control the cookie change after request failure
 	annotationAffinityCookieChangeOnFailure = "session-cookie-change-on-failure"
 )
@@ -75,6 +81,10 @@ type Cookie struct {
 	Path string `json:"path"`
 	// Flag that allows cookie regeneration on request failure
 	ChangeOnFailure bool `json:"changeonfailure"`
+	// SameSite attribute value
+	SameSite string `json:"samesite"`
+	// Flag that conditionally applies SameSite=None attribute on cookie if user agent accepts it.
+	ConditionalSameSiteNone bool `json:"conditional-samesite-none"`
 }
 
 // cookieAffinityParse gets the annotation values related to Cookie Affinity
@@ -107,6 +117,16 @@ func (a affinity) cookieAffinityParse(ing *networking.Ingress) *Cookie {
 		klog.V(3).Infof("Invalid or no annotation value found in Ingress %v: %v. Ignoring it", ing.Name, annotationAffinityCookieMaxAge)
 	}
 
+	cookie.SameSite, err = parser.GetStringAnnotation(annotationAffinityCookieSameSite, ing)
+	if err != nil {
+		klog.V(3).Infof("Invalid or no annotation value found in Ingress %v: %v. Ignoring it", ing.Name, annotationAffinityCookieSameSite)
+	}
+
+	cookie.ConditionalSameSiteNone, err = parser.GetBoolAnnotation(annotationAffinityCookieConditionalSameSiteNone, ing)
+	if err != nil {
+		klog.V(3).Infof("Invalid or no annotation value found in Ingress %v: %v. Ignoring it", ing.Name, annotationAffinityCookieConditionalSameSiteNone)
+	}
+
 	cookie.ChangeOnFailure, err = parser.GetBoolAnnotation(annotationAffinityCookieChangeOnFailure, ing)
 	if err != nil {
 		klog.V(3).Infof("Invalid or no annotation value found in Ingress %v: %v. Ignoring it", ing.Name, annotationAffinityCookieChangeOnFailure)

internal/ingress/controller/controller.go

@@ -592,6 +592,8 @@ func (n *NGINXController) getBackendServers(ingresses []*ingress.Ingress) ([]*in
 					ups.SessionAffinity.CookieSessionAffinity.Expires = anns.SessionAffinity.Cookie.Expires
 					ups.SessionAffinity.CookieSessionAffinity.MaxAge = anns.SessionAffinity.Cookie.MaxAge
 					ups.SessionAffinity.CookieSessionAffinity.Path = cookiePath
+					ups.SessionAffinity.CookieSessionAffinity.SameSite = anns.SessionAffinity.Cookie.SameSite
+					ups.SessionAffinity.CookieSessionAffinity.ConditionalSameSiteNone = anns.SessionAffinity.Cookie.ConditionalSameSiteNone
 					ups.SessionAffinity.CookieSessionAffinity.ChangeOnFailure = anns.SessionAffinity.Cookie.ChangeOnFailure
 
 					locs := ups.SessionAffinity.CookieSessionAffinity.Locations

internal/ingress/types.go

@@ -144,12 +144,14 @@ type SessionAffinityConfig struct {
 // CookieSessionAffinity defines the structure used in Affinity configured by Cookies.
 // +k8s:deepcopy-gen=true
 type CookieSessionAffinity struct {
-	Name            string              `json:"name"`
+	Name                    string              `json:"name"`
-	Expires         string              `json:"expires,omitempty"`
+	Expires                 string              `json:"expires,omitempty"`
-	MaxAge          string              `json:"maxage,omitempty"`
+	MaxAge                  string              `json:"maxage,omitempty"`
-	Locations       map[string][]string `json:"locations,omitempty"`
+	Locations               map[string][]string `json:"locations,omitempty"`
-	Path            string              `json:"path,omitempty"`
+	Path                    string              `json:"path,omitempty"`
-	ChangeOnFailure bool                `json:"change_on_failure,omitempty"`
+	SameSite                string              `json:"samesite,omitempty"`
+	ConditionalSameSiteNone bool                `json:"conditional_samesite_none,omitempty"`
+	ChangeOnFailure         bool                `json:"change_on_failure,omitempty"`
 }
 
 // UpstreamHashByConfig described setting from the upstream-hash-by* annotations.

internal/ingress/types_equals.go

@@ -179,6 +179,12 @@ func (csa1 *CookieSessionAffinity) Equal(csa2 *CookieSessionAffinity) bool {
 	if csa1.MaxAge != csa2.MaxAge {
 		return false
 	}
+	if csa1.SameSite != csa2.SameSite {
+		return false
+	}
+	if csa1.ConditionalSameSiteNone != csa2.ConditionalSameSiteNone {
+		return false
+	}
 
 	return true
 }

rootfs/etc/nginx/lua/balancer/sticky.lua

@@ -2,6 +2,7 @@ local balancer_resty = require("balancer.resty")
 local ck = require("resty.cookie")
 local ngx_balancer = require("ngx.balancer")
 local split = require("util.split")
+local same_site = require("util.same_site")
 
 local _M = balancer_resty:new()
 local DEFAULT_COOKIE_NAME = "route"
@@ -43,6 +44,20 @@ function _M.set_cookie(self, value)
     cookie_path = ngx.var.location_path
   end
 
+  local cookie_samesite = self.cookie_session_affinity.samesite
+  if cookie_samesite then
+    local cookie_conditional_samesite_none = self.cookie_session_affinity.conditional_samesite_none
+    if cookie_conditional_samesite_none
+        and cookie_samesite == "None"
+        and not same_site.same_site_none_compatible(ngx.var.http_user_agent) then
+      cookie_samesite = nil
+    end
+  end
+
+  if cookie_samesite then
+    cookie_path = cookie_path .. "; SameSite=" .. cookie_samesite
+  end
+
   local cookie_data = {
     key = self:cookie_name(),
     value = value,

rootfs/etc/nginx/lua/test/balancer/sticky_test.lua

@@ -390,4 +390,70 @@ describe("Sticky", function()
     it("sets a cookie on the client", function() test(sticky_balanced) end)
     it("sets a cookie on the client", function() test(sticky_persistent) end)
   end)
+
+  describe("SameSite settings", function()
+    local mocked_cookie_new = cookie.new
+
+    before_each(function()
+      package.loaded["balancer.sticky_balanced"] = nil
+      package.loaded["balancer.sticky_persistent"] = nil
+      sticky_balanced = require("balancer.sticky_balanced")
+      sticky_persistent = require("balancer.sticky_persistent")
+    end)
+
+    after_each(function()
+      cookie.new = mocked_cookie_new
+    end)
+
+    local function test_set_cookie(sticky, samesite, conditional_samesite_none, expected_path)
+      local s = {}
+      cookie.new = function(self)
+        local cookie_instance = {
+          set = function(self, payload)
+            assert.equal(payload.key, test_backend.sessionAffinityConfig.cookieSessionAffinity.name)
+            assert.equal(payload.path, expected_path)
+            assert.equal(payload.domain, nil)
+            assert.equal(payload.httponly, true)
+            assert.equal(payload.secure, false)
+            return true, nil
+          end,
+          get = function(k) return false end,
+        }
+        s = spy.on(cookie_instance, "set")
+        return cookie_instance, false
+      end
+      local b = get_test_backend()
+      b.sessionAffinityConfig.cookieSessionAffinity.locations = {}
+      b.sessionAffinityConfig.cookieSessionAffinity.locations["test.com"] = {"/"}
+      b.sessionAffinityConfig.cookieSessionAffinity.samesite = samesite
+      b.sessionAffinityConfig.cookieSessionAffinity.conditional_samesite_none = conditional_samesite_none
+      local sticky_balancer_instance = sticky:new(b)
+      assert.has_no.errors(function() sticky_balancer_instance:balance() end)
+      assert.spy(s).was_called()
+    end
+
+    it("returns a cookie with SameSite=Strict when user specifies samesite strict", function()
+      test_set_cookie(sticky_balanced, "Strict", false, "/; SameSite=Strict")
+    end)
+    it("returns a cookie with SameSite=Strict when user specifies samesite strict and conditional samesite none", function()
+      test_set_cookie(sticky_balanced, "Strict", true, "/; SameSite=Strict")
+    end)
+    it("returns a cookie with SameSite=Lax when user specifies samesite lax", function()
+      test_set_cookie(sticky_balanced, "Lax", false, "/; SameSite=Lax")
+    end)
+    it("returns a cookie with SameSite=Lax when user specifies samesite lax and conditional samesite none", function()
+      test_set_cookie(sticky_balanced, "Lax", true, "/; SameSite=Lax")
+    end)
+    it("returns a cookie with SameSite=None when user specifies samesite None", function()
+      test_set_cookie(sticky_balanced, "None", false, "/; SameSite=None")
+    end)
+    it("returns a cookie with SameSite=None when user specifies samesite None and conditional samesite none with supported user agent", function()
+      mock_ngx({ var = { location_path = "/", host = "test.com" , http_user_agent = "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.2704.103 Safari/537.36"} })
+      test_set_cookie(sticky_balanced, "None", true, "/; SameSite=None")
+    end)
+    it("returns a cookie without SameSite=None when user specifies samesite None and conditional samesite none with unsupported user agent", function()
+      mock_ngx({ var = { location_path = "/", host = "test.com" , http_user_agent = "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36"} })
+      test_set_cookie(sticky_balanced, "None", true, "/")
+    end)
+  end)
 end)

rootfs/etc/nginx/lua/test/util/same_site_test.lua

@@ -0,0 +1,51 @@
+describe("same_site_compatible_test", function()
+  it("returns false for chrome 4", function()
+    local same_site = require("util.same_site")
+    assert.False(same_site.same_site_none_compatible("Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2704.103 Safari/537.36"))
+  end)
+  it("returns false for chrome 5", function()
+    local same_site = require("util.same_site")
+    assert.False(same_site.same_site_none_compatible("Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2704.103 Safari/537.36"))
+  end)
+  it("returns false for chrome 6", function()
+    local same_site = require("util.same_site")
+    assert.False(same_site.same_site_none_compatible("Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2704.103 Safari/537.36"))
+  end)
+  it("returns false for iPhone OS 12", function()
+    local same_site = require("util.same_site")
+    assert.False(same_site.same_site_none_compatible("Mozilla/5.0 (iPhone; CPU iPhone OS 12_0 like Mac OS X) AppleWebKit/602.1.50 (KHTML, like Gecko) CriOS/56.0.2924.75 Mobile/14E5239e Safari/602.1"))
+  end)
+  it("returns false for iPad OS 12", function()
+    local same_site = require("util.same_site")
+    assert.False(same_site.same_site_none_compatible("Mozilla/5.0 (iPad; CPU OS 12_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.0 Mobile/15E148 Safari/604.1"))
+  end)
+  it("returns false for Mac 10.14 Safari", function()
+    local same_site = require("util.same_site")
+    assert.False(same_site.same_site_none_compatible("Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.1.2 Safari/605.1.15"))
+  end)
+
+  it("returns true for chrome 7", function()
+    local same_site = require("util.same_site")
+    assert.True(same_site.same_site_none_compatible("Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.2704.103 Safari/537.36"))
+  end)
+  it("returns true for chrome 8", function()
+    local same_site = require("util.same_site")
+    assert.True(same_site.same_site_none_compatible("Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/80.0.2704.103 Safari/537.36"))
+  end)
+  it("returns true for iPhone OS 13", function()
+    local same_site = require("util.same_site")
+    assert.True(same_site.same_site_none_compatible("Mozilla/5.0 (iPhone; CPU iPhone OS 13_0 like Mac OS X) AppleWebKit/602.1.50 (KHTML, like Gecko) CriOS/56.0.2924.75 Mobile/14E5239e Safari/602.1"))
+  end)
+  it("returns true for iPad OS 13", function()
+    local same_site = require("util.same_site")
+    assert.True(same_site.same_site_none_compatible("Mozilla/5.0 (iPad; CPU OS 13_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.0 Mobile/15E148 Safari/604.1"))
+  end)
+  it("returns true for Mac 10.15 Safari", function()
+    local same_site = require("util.same_site")
+    assert.True(same_site.same_site_none_compatible("Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_2) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.0.4 Safari/605.1.15"))
+  end)
+  it("returns true for Mac 10.14 Chrome", function()
+    local same_site = require("util.same_site")
+    assert.True(same_site.same_site_none_compatible("Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/79.0.3945.117 Safari/537.36"))
+  end)
+end)

rootfs/etc/nginx/lua/util/same_site.lua

@@ -0,0 +1,36 @@
+local _M = {}
+
+-- determines whether to apply a SameSite=None attribute
+-- to a cookie, based on the user agent.
+-- returns: boolean
+--
+-- Chrome 80 treating third-party cookies as SameSite=Strict
+-- if SameSite is missing. Certain old browsers don't recognize
+-- SameSite=None and will reject cookies entirely bearing SameSite=None.
+-- This creates a situation where fixing things for
+-- Chrome >= 80 breaks things for old browsers.
+-- This function compares the user agent against known
+-- browsers which will reject SameSite=None cookies.
+-- reference: https://www.chromium.org/updates/same-site/incompatible-clients
+function _M.same_site_none_compatible(user_agent)
+  if string.match(user_agent, "Chrome/4") then
+    return false
+  elseif string.match(user_agent, "Chrome/5") then
+    return false
+  elseif string.match(user_agent, "Chrome/6") then
+    return false
+  elseif string.match(user_agent, "CPU iPhone OS 12") then
+    return false
+  elseif string.match(user_agent, "iPad; CPU OS 12") then
+    return false
+  elseif string.match(user_agent, "Macintosh")
+      and string.match(user_agent, "Intel Mac OS X 10_14")
+      and string.match(user_agent, "Safari")
+      and not string.match(user_agent, "Chrome") then
+    return false
+  end
+
+  return true
+end
+
+return _M