From e9ccfa7c620f232edc00f3d3fdf29c9e97b5d7fc Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Wed, 17 Jul 2024 22:24:27 +0545 Subject: [PATCH 1/7] fix: allow trailing dot in SNI for SSL --- apisix/ssl.lua | 3 + apisix/ssl/router/radixtree_sni.lua | 3 + t/certs/test-dot.crt | 18 ++ t/certs/test-dot.key | 28 +++ t/router/radixtree-sni3.t | 267 ++++++++++++++++++++++++++++ 5 files changed, 319 insertions(+) create mode 100644 t/certs/test-dot.crt create mode 100644 t/certs/test-dot.key create mode 100644 t/router/radixtree-sni3.t diff --git a/apisix/ssl.lua b/apisix/ssl.lua index ad820822c06e..9d0167f11c91 100644 --- a/apisix/ssl.lua +++ b/apisix/ssl.lua @@ -29,6 +29,7 @@ local str_byte = string.byte local assert = assert local type = type local ipairs = ipairs +local ngx_sub = ngx.re.sub ffi.cdef[[ unsigned long ERR_peek_error(void); @@ -66,6 +67,7 @@ function _M.server_name(clienthello) end end + sni = ngx_sub(sni, "\\.$", "", "jo") sni = str_lower(sni) return sni end @@ -212,6 +214,7 @@ end function _M.fetch_cert(sni, cert) + core.log.warn(debug.traceback()) local parsed_cert, err = cert_cache(cert, nil, parse_pem_cert, sni, cert) if not parsed_cert then return false, err diff --git a/apisix/ssl/router/radixtree_sni.lua b/apisix/ssl/router/radixtree_sni.lua index aab6aafe8819..49e252de3d34 100644 --- a/apisix/ssl/router/radixtree_sni.lua +++ b/apisix/ssl/router/radixtree_sni.lua @@ -240,6 +240,7 @@ function _M.set(matched_ssl, sni) local new_ssl_value = secret.fetch_secrets(matched_ssl.value, true, matched_ssl.value, "") or matched_ssl.value + core.log.warn("dibag: ", core.json.encode(new_ssl_value)) ok, err = _M.set_cert_and_key(sni, new_ssl_value) if not ok then @@ -285,9 +286,11 @@ local function ssl_filter(ssl) end if ssl.value.sni then + ssl.value.sni = ngx.re.sub(ssl.value.sni, "\\.$", "", "jo") ssl.value.sni = str_lower(ssl.value.sni) elseif ssl.value.snis then for i, v in ipairs(ssl.value.snis) do + v = ngx.re.sub(v, "\\.$", "", "jo") ssl.value.snis[i] = str_lower(v) end end diff --git a/t/certs/test-dot.crt b/t/certs/test-dot.crt new file mode 100644 index 000000000000..a9dfd3326221 --- /dev/null +++ b/t/certs/test-dot.crt @@ -0,0 +1,18 @@ +-----BEGIN CERTIFICATE----- +MIIC+zCCAeOgAwIBAgIUWUtIDbrU8QF90OXlMKyClPRNRcgwDQYJKoZIhvcNAQEL +BQAwETEPMA0GA1UEAwwGUk9PVENBMCAXDTI0MDcxNzE2MDcyM1oYDzIxMjQwNjIz +MTYwNzIzWjAYMRYwFAYDVQQDDA13d3cudGVzdC5jb20uMIIBIjANBgkqhkiG9w0B +AQEFAAOCAQ8AMIIBCgKCAQEApkX5NgwwEC/brmrUAfxSMGMaYOzjx+3BlCC23sLR +0uQ1+KMXt/Pd2QJVqREjEAiwXCMuHbB0qWD5985SfsjeRJJ8rc8CzJfcb7QESKfK +GdLaD8LsyAAg+Rxm0QyVFGrLJ82sjbEimLGCkLMpYsePxEDEifKPp3Z9bRUFT0zm +xcUEXojw5pzjrjIvfqVenWNP716s7bSdOFoc4RBlAdEI3pFUasLF9Lovz7BJLvtY +aoqgCNfb78C6zreDLswET5/338AVf9yPYc5HOthmygxkYTniK47/fOW64RQKXQ2X +EtBiIzN6dSXfTCXSpvow5XIR02rLoxsVEEwM9ODgUAJg6QIDAQABo0IwQDAdBgNV +HQ4EFgQUAHYNW6/hFM+Bqd2KNBXbLgJLaxcwHwYDVR0jBBgwFoAUjwSzlti+ag+f +BzoRa0wZbMaGh10wDQYJKoZIhvcNAQELBQADggEBAA1HfiDtHZV8sxJjasnNSM9f +6XTRCjT+DcABXm7k/Dmb8q5rpyqYwkUfadgAbmPx6T/dC4z7LblkcTkwD7azpkNE +fXY3Hx4qxSVSbSOHWnaSOX/8BRiPbSQNWGyTGh9AK/Vp/VJU2cDPqFbjQKHFq3ZI +w3GnRDerdA8vm5qzJ5/9wMF2ZsmnMiV3zX0Xisbzx/dponz6ktfygE3bk8Pb4wKt +D0EjbnLIXwyHv1czJrcRq0Y8irWaTY97vdff/J2aO9582zFNx6AnsU3+6fGsDyrO +ss+ggKDLK+aOBKroTNb3TgdPyPOgobUwLByFdKT/zTtWbkqyYMZzBme2SD4TWok= +-----END CERTIFICATE----- diff --git a/t/certs/test-dot.key b/t/certs/test-dot.key new file mode 100644 index 000000000000..5ed8aba21b13 --- /dev/null +++ b/t/certs/test-dot.key @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCmRfk2DDAQL9uu +atQB/FIwYxpg7OPH7cGUILbewtHS5DX4oxe3893ZAlWpESMQCLBcIy4dsHSpYPn3 +zlJ+yN5EknytzwLMl9xvtARIp8oZ0toPwuzIACD5HGbRDJUUassnzayNsSKYsYKQ +sylix4/EQMSJ8o+ndn1tFQVPTObFxQReiPDmnOOuMi9+pV6dY0/vXqzttJ04Whzh +EGUB0QjekVRqwsX0ui/PsEku+1hqiqAI19vvwLrOt4MuzARPn/ffwBV/3I9hzkc6 +2GbKDGRhOeIrjv985brhFApdDZcS0GIjM3p1Jd9MJdKm+jDlchHTasujGxUQTAz0 +4OBQAmDpAgMBAAECggEAD84ctm8h5fYApDOWJ8Kp9tzCwgYekE94vEmATIw5CPqF +qVbqbyNUmhdTWGzvN+vVhMqYzHxsmHmmBTDU7WWPYDYK+TQRbGx+iRUz54qghsQg +04j4PDor6DYTjWlMZfqRSV0u+vCErP5JnpLTOyckUrfD3ueCUX0tRsBN5wf0s0WD +7AiUIdVBesQwIuIin3MyhGFtQC0PNta3NdSBVbnUA69OL3QNxPoai5LACrAf1hkf +wPD/y6y2CswdER+j+obPChjTcnJFjRCkqqO+66QZWmMmVxq4ymCQg9IOgLRWtfhI +6Ts5RxVn12kEuPULk9oHHOjC+MVh3BmWFLb58G/gwQKBgQDUCSd/2uswTVlTYpw6 +XO3iVyoZVeo/BIiOm/kjmqmr5U/D7ZO27ElKBTe9CDQ4WB5PuisCy0/SnsJJsPpf +pWif2v0mVs3T9K7J1M1yQU2iMs+Z2stzLGe5AASImYpw9091v57A/1jI4VUoodOr +7sMo+9ROqx6dTG/tJgUa+VZaKQKBgQDIv8CZHv4LqvQEQrGoTcKOxQP47nsbfEPW +B0GQscykvRTWxlTfFdfFM4VG2ApERZDwjPFU84n4dH8J7P14iy2ty70krzHWNfjY +y52CXUb295HsdcQ0bP8wztuvM/Jfh1mKKynmezvAZlTSb+GMAAMrReuG2Ga1/gp1 +5daCd4IowQKBgG//md6eCybLZIh4CN+HIJwywGj7iazZvyvc1T9qPX8vs+9g+Wpg +6uFvWh6+S58LZI9mXbuvGq288BEuq0GERHxTlu3+YeA4WW8AubhFKDWpsyCogliG +tw7wJHTm7Up4R3+BxOBawFHzPCEnQYCKsIlgY6deGeCqdGCGeaHi3CrpAoGAdWam +xSW53qr4j/FNIqdvK72OaCtX9agDqAyQTIWer40gvcY5ZknI6TwLKnY38ttYO0XB +8TOIMbQ3g1+EkNWcPjKTh/upQqRHxsm1cMMKOG5qeYYZ26sOxsWC9oCDs1hdhg9e +LrtNI2T1IChsGEr9j3YRmse9sZtDFNX4UE6B4UECgYEAsuRRQK0tgvcsQxkX/bZb +VTKqI4ezGRLXuavBe42xWOBLFzEujGvbZMbxzD4F4H1dfVVor3ItAEoybC37jtHI +uEWLAQtZtNyDCOiq4UuwbmtIqtoJz556QUrwO0KdPPjg/jyZTxs5jdKRMk9bsfmR +80vnuQpr0CZe8EgHiMoysrs= +-----END PRIVATE KEY----- diff --git a/t/router/radixtree-sni3.t b/t/router/radixtree-sni3.t new file mode 100644 index 000000000000..4229879f6a5c --- /dev/null +++ b/t/router/radixtree-sni3.t @@ -0,0 +1,267 @@ +use t::APISIX 'no_plan'; + +log_level('debug'); +no_root_location(); + +BEGIN { + $ENV{TEST_NGINX_HTML_DIR} ||= html_dir(); +} + +add_block_preprocessor(sub { + my ($block) = @_; + + if (!$block->request) { + $block->set_value("request", "GET /t"); + } + +}); + + +run_tests; + +__DATA__ + +=== TEST 1: set sni with trailing period +--- config +location /t { + content_by_lua_block { + local core = require("apisix.core") + local t = require("lib.test_admin") + + local ssl_cert = t.read_file("t/certs/test2.crt") + local ssl_key = t.read_file("t/certs/test2.key") + local data = {cert = ssl_cert, key = ssl_key, sni = "*.test.com"} + + local code, body = t.test('/apisix/admin/ssls/1', + ngx.HTTP_PUT, + core.json.encode(data) + ) + + ngx.status = code + ngx.say(body) + } +} +--- request +GET /t +--- response_body +passed +--- error_code: 201 + + + +=== TEST 2: match against sni with no trailing period +--- config +listen unix:$TEST_NGINX_HTML_DIR/nginx.sock ssl; + +location /t { + content_by_lua_block { + do + local sock = ngx.socket.tcp() + + sock:settimeout(2000) + + local ok, err = sock:connect("unix:$TEST_NGINX_HTML_DIR/nginx.sock") + if not ok then + ngx.say("failed to connect: ", err) + return + end + + local sess, err = sock:sslhandshake(nil, "a.test.com.", false) + if not sess then + ngx.say("failed to do SSL handshake: ", err) + return + end + ngx.say("ssl handshake: ", sess ~= nil) + end -- do + -- collectgarbage() + } +} +--- request +GET /t +--- response_body +ssl handshake: true + + + +=== TEST 3: set snis with trailing period +--- config +location /t { + content_by_lua_block { + local core = require("apisix.core") + local t = require("lib.test_admin") + + local ssl_cert = t.read_file("t/certs/test2.crt") + local ssl_key = t.read_file("t/certs/test2.key") + local data = {cert = ssl_cert, key = ssl_key, snis = {"test2.com", "a.com"}} + + local code, body = t.test('/apisix/admin/ssls/1', + ngx.HTTP_PUT, + core.json.encode(data) + ) + + ngx.status = code + ngx.say(body) + } +} +--- request +GET /t +--- response_body +passed + + + +=== TEST 4: match agains sni with no trailing period +--- config +listen unix:$TEST_NGINX_HTML_DIR/nginx.sock ssl; + +location /t { + content_by_lua_block { + do + local sock = ngx.socket.tcp() + + sock:settimeout(2000) + + local ok, err = sock:connect("unix:$TEST_NGINX_HTML_DIR/nginx.sock") + if not ok then + ngx.say("failed to connect: ", err) + return + end + + local sess, err = sock:sslhandshake(nil, "test2.com.", false) + if not sess then + ngx.say("failed to do SSL handshake: ", err) + return + end + ngx.say("ssl handshake: ", sess ~= nil) + end -- do + -- collectgarbage() + } +} +--- request +GET /t +--- response_body +ssl handshake: true + + + +=== TEST 5: set ssl(sni: www.test.com.) +--- config +location /t { + content_by_lua_block { + local core = require("apisix.core") + local t = require("lib.test_admin") + local ssl_cert = t.read_file("t/certs/test-dot.crt") + local ssl_key = t.read_file("t/certs/test-dot.key") + local data = {cert = ssl_cert, key = ssl_key, sni = "www.test.com."} + local code, body = t.test('/apisix/admin/ssls/1', + ngx.HTTP_PUT, + core.json.encode(data), + [[{ + "value": { + "sni": "www.test.com." + }, + "key": "/apisix/ssls/1" + }]] + ) + ngx.status = code + ngx.say(body) + } +} +--- request +GET /t +--- response_body +passed + + + +=== TEST 6: set route(id: 1) +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed + + + +=== TEST 7: client request +--- config +listen unix:$TEST_NGINX_HTML_DIR/nginx.sock ssl; +location /t { + content_by_lua_block { + -- etcd sync + ngx.sleep(0.2) + do + local sock = ngx.socket.tcp() + sock:settimeout(2000) + local ok, err = sock:connect("unix:$TEST_NGINX_HTML_DIR/nginx.sock") + if not ok then + ngx.say("failed to connect: ", err) + return + end + ngx.say("connected: ", ok) + local sess, err = sock:sslhandshake(nil, "www.test.com", false) + if not sess then + ngx.say("failed to do SSL handshake: ", err) + return + end + ngx.say("ssl handshake: ", sess ~= nil) + local req = "GET /hello HTTP/1.0\r\nHost: www.test.com\r\nConnection: close\r\n\r\n" + local bytes, err = sock:send(req) + if not bytes then + ngx.say("failed to send http request: ", err) + return + end + ngx.say("sent http request: ", bytes, " bytes.") + while true do + local line, err = sock:receive() + if not line then + -- ngx.say("failed to receive response status line: ", err) + break + end + ngx.say("received: ", line) + end + local ok, err = sock:close() + ngx.say("close: ", ok, " ", err) + end -- do + -- collectgarbage() + } +} +--- request +GET /t +--- response_body eval +qr{connected: 1 +ssl handshake: true +sent http request: 62 bytes. +received: HTTP/1.1 200 OK +received: Content-Type: text/plain +received: Content-Length: 12 +received: Connection: close +received: Server: APISIX/\d\.\d+(\.\d+)? +received: \nreceived: hello world +close: 1 nil} +--- error_log +server name: "www.test.com" +--- no_error_log +[error] +[alert] From 934eb42f74fb69b9bd535416dd57682415e3a960 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Wed, 17 Jul 2024 22:30:47 +0545 Subject: [PATCH 2/7] remove dibag :P --- apisix/ssl/router/radixtree_sni.lua | 1 - 1 file changed, 1 deletion(-) diff --git a/apisix/ssl/router/radixtree_sni.lua b/apisix/ssl/router/radixtree_sni.lua index 49e252de3d34..caa69194e8a2 100644 --- a/apisix/ssl/router/radixtree_sni.lua +++ b/apisix/ssl/router/radixtree_sni.lua @@ -240,7 +240,6 @@ function _M.set(matched_ssl, sni) local new_ssl_value = secret.fetch_secrets(matched_ssl.value, true, matched_ssl.value, "") or matched_ssl.value - core.log.warn("dibag: ", core.json.encode(new_ssl_value)) ok, err = _M.set_cert_and_key(sni, new_ssl_value) if not ok then From e6fd39bcd1d733ebb1d97ce3d7b497b9e0caa3ed Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Wed, 17 Jul 2024 22:31:21 +0545 Subject: [PATCH 3/7] add license header --- t/router/radixtree-sni3.t | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/t/router/radixtree-sni3.t b/t/router/radixtree-sni3.t index 4229879f6a5c..a3bca0f80cda 100644 --- a/t/router/radixtree-sni3.t +++ b/t/router/radixtree-sni3.t @@ -1,3 +1,19 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# use t::APISIX 'no_plan'; log_level('debug'); From 4473a44bc7a472bb6567bf1b0cd59725856354fb Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Thu, 18 Jul 2024 07:19:46 +0545 Subject: [PATCH 4/7] remove debug log --- apisix/ssl.lua | 2 -- 1 file changed, 2 deletions(-) diff --git a/apisix/ssl.lua b/apisix/ssl.lua index 9d0167f11c91..5d0d8b6c02e9 100644 --- a/apisix/ssl.lua +++ b/apisix/ssl.lua @@ -225,8 +225,6 @@ end local function parse_pem_priv_key(sni, pkey) - core.log.debug("parsing priv key for sni: ", sni) - local key, err = aes_decrypt_pkey(pkey) if not key then core.log.error(err) From 55191646df30e41b8092768952d4e70b502f8882 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Thu, 18 Jul 2024 07:20:11 +0545 Subject: [PATCH 5/7] spell fix --- t/router/radixtree-sni3.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/router/radixtree-sni3.t b/t/router/radixtree-sni3.t index a3bca0f80cda..ff18bda7f7d3 100644 --- a/t/router/radixtree-sni3.t +++ b/t/router/radixtree-sni3.t @@ -126,7 +126,7 @@ passed -=== TEST 4: match agains sni with no trailing period +=== TEST 4: match against sni with no trailing period --- config listen unix:$TEST_NGINX_HTML_DIR/nginx.sock ssl; From 6c4e1b84336a6933e8eb462e82fcff0bd8c2d5d6 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Thu, 18 Jul 2024 09:47:03 +0545 Subject: [PATCH 6/7] remove traceback --- apisix/ssl.lua | 1 - 1 file changed, 1 deletion(-) diff --git a/apisix/ssl.lua b/apisix/ssl.lua index 5d0d8b6c02e9..749715b650ba 100644 --- a/apisix/ssl.lua +++ b/apisix/ssl.lua @@ -214,7 +214,6 @@ end function _M.fetch_cert(sni, cert) - core.log.warn(debug.traceback()) local parsed_cert, err = cert_cache(cert, nil, parse_pem_cert, sni, cert) if not parsed_cert then return false, err From d556af1fd5d9f984349f8df3474a30520cff14f0 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Thu, 18 Jul 2024 09:49:16 +0545 Subject: [PATCH 7/7] Revert "remove debug log" This reverts commit 4473a44bc7a472bb6567bf1b0cd59725856354fb. --- apisix/ssl.lua | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apisix/ssl.lua b/apisix/ssl.lua index 749715b650ba..412c9b86bc90 100644 --- a/apisix/ssl.lua +++ b/apisix/ssl.lua @@ -224,6 +224,8 @@ end local function parse_pem_priv_key(sni, pkey) + core.log.debug("parsing priv key for sni: ", sni) + local key, err = aes_decrypt_pkey(pkey) if not key then core.log.error(err)