Merge pull request #1419 from 3scale/dev-environment-grpc

THREESCALE-9976 Fix GRPC
3scale · Nov 8, 2023 · c7476bb · c7476bb
2 parents 292d738 + 800252f
commit c7476bb
Show file tree

Hide file tree

Showing 9 changed files with 290 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Fixed
 
+- Fix GRPC on HTTP2 POST method [PR #1419](https://github.com/3scale/apicast/pull/1419) [THREESCALE-10224](https://issues.redhat.com/browse/THREESCALE-9976)
+
 - Fixed CVE-2023-44487 (HTTP/2 Rapid Reset) [PR #1417](https://github.com/3scale/apicast/pull/1417) [THREESCALE-10224](https://issues.redhat.com/browse/THREESCALE-10224)
 
 ### Added
@@ -22,11 +24,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - In boot mode on `init_worker` check configuration expiration [PR #1399](https://github.com/3scale/APIcast/pull/1399) [THREESCALE-9003](https://issues.redhat.com/browse/THREESCALE-9003)
 - Removes the warning message at the bootstrap [PR #1398](https://github.com/3scale/APIcast/pull/1398) [THREESCALE-7942](https://issues.redhat.com/browse/THREESCALE-7942)
 - Set NGiNX variable variables_hash_max_size to 2048 to avoid startup warning [PR #1395](https://github.com/3scale/APIcast/pull/1395) [THREESCALE-7941](https://issues.redhat.com/browse/THREESCALE-7941)
-- Dev environment on aarch64 host [PR #1381](https://github.com/3scale/APIcast/pull/1381) 
+- Dev environment on aarch64 host [PR #1381](https://github.com/3scale/APIcast/pull/1381)
 
 ### Added
 
-- Doc: Policy Development Tutorial [PR #1384](https://github.com/3scale/APIcast/pull/1384) 
+- Doc: Policy Development Tutorial [PR #1384](https://github.com/3scale/APIcast/pull/1384)
 - Opentelemetry support. Opentracing is now deprecated [PR #1379](https://github.com/3scale/APIcast/pull/1379) [THREESCALE-7735](https://issues.redhat.com/browse/THREESCALE-7735)
 - `/admin/api/account/proxy_configs` endpoint for configuration loading [PR #1352](https://github.com/3scale/APIcast/pull/1352) [THREESCALE-8508](https://issues.redhat.com/browse/THREESCALE-8508)
 - Pagination of services and proxy config endpoints [PR #1397](https://github.com/3scale/APIcast/pull/1397) [THREESCALE-8373](https://issues.redhat.com/browse/THREESCALE-8373)

diff --git a/dev-environments/grpc/Makefile b/dev-environments/grpc/Makefile
@@ -0,0 +1,66 @@
+SHELL = /usr/bin/env bash -o pipefail
+.SHELLFLAGS = -ec
+.DEFAULT_GOAL := gateway
+MKFILE_PATH := $(abspath $(lastword $(MAKEFILE_LIST)))
+WORKDIR := $(patsubst %/,%,$(dir $(MKFILE_PATH)))
+DOCKER ?= $(shell which docker 2> /dev/null || echo "docker")
+
+gateway: ## run gateway configured to access upstream powered with TLS
+	$(DOCKER) compose -f docker-compose.yml run --service-ports gateway
+
+clean:
+	$(DOCKER) compose down --volumes --remove-orphans
+	$(DOCKER) compose -f docker-compose.yml down --volumes --remove-orphans
+	- rm -rf gateway-cert
+	- rm -rf upstream-cert
+	- rm -rf bin
+
+ca:
+	openssl genrsa -out rootCA.key 2048
+	openssl req -batch -new -x509 -nodes -key rootCA.key -sha256 -days 1024 -out rootCA.pem
+
+clientcerts:
+	openssl req -subj '/CN=$(DOMAIN)'  -newkey rsa:4096 -nodes \
+			-sha256 \
+			-days 3650 \
+			-keyout $(DOMAIN).key \
+			-out $(DOMAIN).csr
+	chmod +r $(DOMAIN).key
+	openssl x509 -req -in $(DOMAIN).csr -CA rootCA.pem -CAkey rootCA.key -CAcreateserial -out $(DOMAIN).crt -days 500 -sha256
+
+$(WORKDIR)/gateway-cert:
+	mkdir -p gateway-cert
+
+.PHONY: gateway-certs
+gateway-certs: $(WORKDIR)/gateway-cert
+	$(MAKE) ca -C $(WORKDIR)/gateway-cert -f $(WORKDIR)/Makefile
+	$(MAKE) clientcerts -C $(WORKDIR)/gateway-cert -f $(WORKDIR)/Makefile DOMAIN=gateway.example.com
+
+$(WORKDIR)/upstream-cert:
+	mkdir -p upstream-cert
+
+.PHONY: upstream-certs
+upstream-certs: $(WORKDIR)/upstream-cert
+	$(MAKE) ca -C $(WORKDIR)/upstream-cert -f $(WORKDIR)/Makefile
+	$(MAKE) clientcerts -C $(WORKDIR)/upstream-cert -f $(WORKDIR)/Makefile DOMAIN=upstream.example.com
+	cat $(WORKDIR)/upstream-cert/upstream.example.com.key $(WORKDIR)/upstream-cert/upstream.example.com.crt >$(WORKDIR)/upstream-cert/upstream.example.com.pem
+
+GRPCURL=$(WORKDIR)/bin/grpcurl
+$(GRPCURL):
+	$(call go-install-tool,$(GRPCURL),github.com/fullstorydev/grpcurl/cmd/grpcurl@v1.8.9)
+
+.PHONY: grpcurl
+grpcurl: $(GRPCURL) ## Download grpcurl locally if necessary.
+
+# go-install-tool will 'go install' any package $2 and install it to $1.
+define go-install-tool
+@[ -f $(1) ] || { \
+set -e ;\
+TMP_DIR=$$(mktemp -d) ;\
+cd $$TMP_DIR ;\
+go mod init tmp ;\
+echo "Downloading $(2)" ;\
+GOBIN=$(WORKDIR)/bin go install $(2) ;\
+rm -rf $$TMP_DIR ;\
+}
+endef
diff --git a/dev-environments/grpc/README.md b/dev-environments/grpc/README.md
@@ -0,0 +1,55 @@
+# APIcast GRPC endpoint
+
+## Create the SSL Certificates
+
+```sh
+make gateway-certs
+```
+
+```sh
+make upstream-certs
+```
+
+## Run the gateway
+
+Running local `apicast-test` docker image
+
+```sh
+make gateway
+```
+
+Running custom apicast image
+
+```sh
+make gateway IMAGE_NAME=quay.io/3scale/apicast:latest
+```
+
+Traffic between the gateway and upstream can be inspected looking at logs from `one.upstream` service
+
+```
+docker compose -p grpc logs -f one.upstream
+```
+
+## Testing
+
+
+
+Get `grpcurl`. You need [Go SDK](https://golang.org/doc/install) installed.
+
+Golang version >= 1.18 (from [fullstorydev/grpcurl](https://github.com/fullstorydev/grpcurl/blob/v1.8.9/go.mod#L3))
+
+```sh
+make grpcurl
+```
+
+Run request
+
+```sh
+bin/grpcurl -vv -insecure -H "app_id: abc123" -H "app_key: abc123" -authority gateway.example.com 127.0.0.1:8443 main.HelloWorld/Greeting
+```
+
+## Clean env
+
+```sh
+make clean
+```
diff --git a/dev-environments/grpc/apicast-config.json b/dev-environments/grpc/apicast-config.json
@@ -0,0 +1,35 @@
+{
+  "services": [
+    {
+      "id": "1",
+      "backend_version": "2",
+      "proxy": {
+        "hosts": ["gateway.example.com"],
+        "credentials_location": "headers",
+        "api_backend": "https://one.upstream:443",
+        "backend": {
+          "endpoint": "http://127.0.0.1:8081",
+          "host": "backend"
+        },
+        "policy_chain": [
+          {
+            "name": "apicast.policy.grpc"
+          },
+          {
+            "name": "apicast.policy.apicast"
+          }
+        ],
+        "proxy_rules": [
+          {
+            "http_method": "POST",
+            "pattern": "/",
+            "metric_system_name": "hits",
+            "delta": 1,
+            "parameters": [],
+            "querystring_parameters": {}
+          }
+        ]
+      }
+    }
+  ]
+}
diff --git a/dev-environments/grpc/docker-compose.yml b/dev-environments/grpc/docker-compose.yml
@@ -0,0 +1,40 @@
+---
+version: '3.8'
+services:
+  gateway:
+    image: ${IMAGE_NAME:-apicast-test}
+    depends_on:
+    - one.upstream
+    - two.upstream
+    environment:
+      APICAST_HTTPS_PORT: 8443
+      APICAST_HTTPS_CERTIFICATE: /var/run/secrets/apicast/gateway.example.com.crt
+      APICAST_HTTPS_CERTIFICATE_KEY: /var/run/secrets/apicast/gateway.example.com.key
+      THREESCALE_CONFIG_FILE: /tmp/config.json
+      THREESCALE_DEPLOYMENT_ENV: staging
+      APICAST_CONFIGURATION_LOADER: lazy
+      APICAST_WORKERS: 1
+      APICAST_LOG_LEVEL: debug
+      APICAST_CONFIGURATION_CACHE: "0"
+    expose:
+      - "8443"
+      - "8090"
+    ports:
+      - "8443:8443"
+      - "8090:8090"
+    volumes:
+      - ./apicast-config.json:/tmp/config.json
+      - ./gateway-cert:/var/run/secrets/apicast
+  one.upstream:
+    image: alpine/socat:1.7.4.4
+    container_name: one.upstream
+    command: "-v openssl-listen:443,reuseaddr,fork,cert=/etc/pki/upstream.example.com.pem,verify=0,openssl-max-proto-version=TLS1.3 ssl:two.upstream:8005,verify=0"
+    expose:
+      - "443"
+    restart: unless-stopped
+    volumes:
+      - ./upstream-cert/upstream.example.com.pem:/etc/pki/upstream.example.com.pem
+  two.upstream:
+    image: kalmhq/echoserver
+    expose:
+      - "8005"
diff --git a/gateway/src/apicast/configuration/service.lua b/gateway/src/apicast/configuration/service.lua
@@ -7,6 +7,9 @@ local tostring = tostring
 local rawget = rawget
 local lower = string.lower
 local gsub = string.gsub
+local str_find = string.find
+local type = type
+local tbl_concat = table.concat
 local select = select
 
 local re = require 'ngx.re'
@@ -30,13 +33,41 @@ local http_methods_with_body = {
   PATCH = true
 }
 
+local function is_http2_request()
+  return ngx.req.http_version() == 2.0
+end
+
+local function content_type_is_urlencoded(headers)
+    local ct = headers["Content-Type"]
+    if not ct then
+        return false
+    end
+
+    -- Handle duplicate headers
+    -- This shouldn't happen but can in the real world
+    if type(ct) ~= "string" then
+        ct = tbl_concat(ct, ",")
+    end
+
+    return str_find(lower(ct), "application/x-www-form-urlencoded", 1, true) ~= nil
+end
+
+
 local function read_body_args(...)
   local method = ngx.req.get_method()
 
+  if is_http2_request() then
+    return {}, 'not supported'
+  end
+
   if not http_methods_with_body[method] then
     return {}, 'not supported'
   end
 
+  if not content_type_is_urlencoded(ngx.req.get_headers()) then
+    return {}, 'not supported'
+  end
+
   ngx.req.read_body()
 
   local args, err = ngx.req.get_post_args()
@@ -174,9 +205,12 @@ end
 local function get_request_params(method)
   local params = ngx.req.get_uri_args()
 
-  if method == "GET" then
+  if is_http2_request() then
     return params
-  else
+  end
+
+    -- Only read request body when POST query arguments (of the MIME type application/x-www-form-urlencoded)
+  if http_methods_with_body[method] and content_type_is_urlencoded(ngx.req.get_headers()) then
     ngx.req.read_body()
     local body_params, err = ngx.req.get_post_args()
 
@@ -192,6 +226,8 @@ local function get_request_params(method)
 
     return body_params
   end
+
+  return params
 end
 
 -- This table can be used with `table.concat` to serialize

diff --git a/spec/configuration/service_spec.lua b/spec/configuration/service_spec.lua
@@ -2,6 +2,7 @@ local Service = require 'apicast.configuration.service'
 
 describe('Service object', function()
   describe(':credentials', function()
+    stub(ngx.req, 'http_version', function() return 1.1 end)
 
     describe('backend_version=1', function()
       it('returns only GET parameters', function()
@@ -23,6 +24,51 @@ describe('Service object', function()
 
         ngx.var = {}
         stub(ngx.req, 'get_method', function() return 'POST' end)
+        stub(ngx.req, 'get_headers', function() return {["Content-Type"] = 'application/x-www-form-urlencoded' } end)
+        stub(ngx.req, 'read_body')
+        stub(ngx.req, 'get_post_args', function() return { user_key = 'post' } end)
+
+        assert.same({ 'post', user_key = 'post' }, assert(service:extract_credentials()))
+      end)
+
+      it('unknown POST request returns empty', function()
+        local service = Service.new({
+          backend_version = 1,
+          credentials = { location = 'query' }
+        })
+
+        ngx.var = {}
+        stub(ngx.req, 'get_method', function() return 'POST' end)
+        -- No Content-Type header
+        stub(ngx.req, 'get_headers', function() return {} end)
+
+        assert.same({}, assert(service:extract_credentials()))
+      end)
+
+      it('urlencoded POST request without credentials', function()
+        local service = Service.new({
+          backend_version = 1,
+          credentials = { location = 'query' }
+        })
+
+        ngx.var = {}
+        stub(ngx.req, 'get_method', function() return 'POST' end)
+        stub(ngx.req, 'get_headers', function() return {["Content-Type"] = 'application/x-www-form-urlencoded' } end)
+        stub(ngx.req, 'read_body')
+        stub(ngx.req, 'get_post_args', function() return {} end)
+
+        assert.same({}, assert(service:extract_credentials()))
+      end)
+
+      it('urlencoded POST request with multiple Content-Type headers', function()
+        local service = Service.new({
+          backend_version = 1,
+          credentials = { location = 'query' }
+        })
+
+        ngx.var = {}
+        stub(ngx.req, 'get_method', function() return 'POST' end)
+        stub(ngx.req, 'get_headers', function() return {["Content-Type"] = {'other', 'application/x-www-form-urlencoded'} } end)
         stub(ngx.req, 'read_body')
         stub(ngx.req, 'get_post_args', function() return { user_key = 'post' } end)
 
@@ -74,6 +120,7 @@ describe('Service object', function()
 
         ngx.var = {}
         stub(ngx.req, 'get_method', function() return 'POST' end)
+        stub(ngx.req, 'get_headers', function() return {["Content-Type"] = 'application/x-www-form-urlencoded' } end)
         stub(ngx.req, 'read_body')
         stub(ngx.req, 'get_post_args', function() return { app_id = 'post' } end)
 
@@ -128,6 +175,7 @@ describe('Service object', function()
 
         ngx.var = {}
         stub(ngx.req, 'get_method', function() return 'POST' end)
+        stub(ngx.req, 'get_headers', function() return {["Content-Type"] = 'application/x-www-form-urlencoded' } end)
         stub(ngx.req, 'read_body')
         stub(ngx.req, 'get_post_args', function() return { access_token = 'post' } end)
 

diff --git a/t/apicast-mapping-rules.t b/t/apicast-mapping-rules.t
@@ -141,6 +141,7 @@ request body params in a POST call are taken into account when matching mapping
 POST /foo?user_key=somekey
 bar=baz
 --- more_headers
+Content-Type: application/x-www-form-urlencoded
 X-3scale-Debug: my-token
 --- response_body
 api response
@@ -193,6 +194,7 @@ precedence.
 POST /foo?a_param=val3&another_param=val2&user_key=somekey
 a_param=val1
 --- more_headers
+Content-Type: application/x-www-form-urlencoded
 X-3scale-Debug: my-token
 --- response_body
 api response

diff --git a/t/apicast-policy-upstream.t b/t/apicast-policy-upstream.t
@@ -277,6 +277,8 @@ GET /path_in_the_rule/some_path?user_key=uk&a_param=a_value HTTP/1.1
 --- request
 POST /a_path
 user_key=uk&a_param=a_value
+--- more_headers
+Content-Type: application/x-www-form-urlencoded
 --- response_body
 yay, api backend
 --- error_code: 200