Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

THREESCALE-9976 Fix GRPC #1419

Merged
merged 5 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just a small thing but I think this is an unrelated change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just the IDE tool removing trailing spaces

- 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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just the IDE tool removing trailing spaces

- 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)
Expand Down
66 changes: 66 additions & 0 deletions dev-environments/grpc/Makefile
Original file line number Diff line number Diff line change
@@ -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 ;\
tkan145 marked this conversation as resolved.
Show resolved Hide resolved
}
endef
55 changes: 55 additions & 0 deletions dev-environments/grpc/README.md
Original file line number Diff line number Diff line change
@@ -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
```
35 changes: 35 additions & 0 deletions dev-environments/grpc/apicast-config.json
Original file line number Diff line number Diff line change
@@ -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": {}
}
]
}
}
]
}
40 changes: 40 additions & 0 deletions dev-environments/grpc/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -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"
40 changes: 38 additions & 2 deletions gateway/src/apicast/configuration/service.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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"]
tkan145 marked this conversation as resolved.
Show resolved Hide resolved
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()
Expand Down Expand Up @@ -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()

Expand All @@ -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
Expand Down
48 changes: 48 additions & 0 deletions spec/configuration/service_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down
2 changes: 2 additions & 0 deletions t/apicast-mapping-rules.t
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions t/apicast-policy-upstream.t
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading