Merge pull request #1417 from 3scale/cve-2023-44487-rapid-reset

THREESCALE-10224 CVE-2023-44487 http/2 rapid reset

Author: eguzki

.circleci/config.yml

@@ -105,7 +105,7 @@ executors:
   openresty:
     working_directory: /opt/app-root/apicast
     docker:
-      - image: quay.io/3scale/apicast-ci:openresty-1.19.3-pr1379
+      - image: quay.io/3scale/apicast-ci:openresty-1.19.3-23
       - image: redis:3.2.8-alpine
     environment:
       TEST_NGINX_BINARY: openresty

.codecov.yml

@@ -6,3 +6,14 @@ ignore:
   - t
   - bin/busted.lua
   - examples
+
+coverage:
+  status:
+    project:
+      default:
+        target: auto
+        threshold: 3%
+    patch:
+      default:
+        target: auto
+        threshold: 3%

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+### Fixed
+
+- 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
 
 - Detect number of CPU shares when running on Cgroups V2 [PR #1410](https://github.com/3scale/apicast/pull/1410) [THREESCALE-10167](https://issues.redhat.com/browse/THREESCALE-10167)

Dockerfile

@@ -1,6 +1,6 @@
 FROM registry.access.redhat.com/ubi8:8.5
 
-ARG OPENRESTY_RPM_VERSION="1.19.3-21.el8"
+ARG OPENRESTY_RPM_VERSION="1.19.3-23.el8"
 ARG LUAROCKS_VERSION="2.3.0"
 ARG JAEGERTRACING_CPP_CLIENT_RPM_VERSION="0.3.1-13.el8"
 

Dockerfile.devel

@@ -1,6 +1,6 @@
 FROM registry.access.redhat.com/ubi8:8.5
 
-ARG OPENRESTY_RPM_VERSION="1.19.3-21.el8"
+ARG OPENRESTY_RPM_VERSION="1.19.3-23.el8"
 ARG LUAROCKS_VERSION="2.3.0"
 ARG JAEGERTRACING_CPP_CLIENT_RPM_VERSION="0.3.1-13.el8"
 

Makefile

@@ -13,7 +13,7 @@ NPROC ?= $(firstword $(shell nproc 2>/dev/null) 1)
 
 SEPARATOR="\n=============================================\n"
 
-DEVEL_IMAGE ?= quay.io/3scale/apicast-ci:openresty-1.19.3-pr1379
+DEVEL_IMAGE ?= quay.io/3scale/apicast-ci:openresty-1.19.3-23
 DEVEL_DOCKERFILE ?= Dockerfile.devel
 
 RUNTIME_IMAGE ?= quay.io/3scale/apicast:latest
@@ -64,9 +64,9 @@ export COMPOSE_PROJECT_NAME
 # The development image is also used in CI (circleCI) as the 'openresty' executor
 # When the development image changes, make sure to:
 # * build a new development image:
-#     make dev-build IMAGE_NAME=quay.io/3scale/apicast-ci:openresty-1.19.3-pr{NUM}
+#     make dev-build IMAGE_NAME=quay.io/3scale/apicast-ci:openresty-X.Y.Z-{release_number}
 # * push to quay.io/3scale/apicast-ci with a fixed tag (avoid floating tags)
-#     docker push quay.io/3scale/apicast-ci:openresty-1.19.3-pr{NUM}
+#     docker push quay.io/3scale/apicast-ci:openresty-X.Y.Z-{release_number}
 # * update .circleci/config.yaml openresty executor with the image URL
 .PHONY: dev-build
 dev-build: export OPENRESTY_RPM_VERSION?=1.19.3

docker-compose-devel.yml

@@ -2,7 +2,7 @@
 version: '2.2'
 services:
   development:
-    image: ${IMAGE:-quay.io/3scale/apicast-ci:openresty-1.19.3-pr1379}
+    image: ${IMAGE:-quay.io/3scale/apicast-ci:openresty-1.19.3-23}
     platform: "linux/amd64"
     depends_on:
       - redis

gateway/conf.d/apicast.conf

@@ -3,6 +3,8 @@ set_by_lua_block $deployment {
   return require('apicast.user_agent').deployment()
 }
 
+lua_check_client_abort on;
+
 # TODO: enable in the future when we support SSL
 # ssl_certificate_by_lua_block { require('apicast.executor').call() }
 # ssl_session_fetch_by_lua_block { require('apicast.executor').call() }

gateway/src/apicast/policy/apicast/apicast.lua

@@ -30,11 +30,16 @@ end
 
 function _M.cleanup()
   -- now abort all the "light threads" running in the current request handler
+  ngx.log(ngx.INFO, "client closed the (downstream) connection prematurely.")
   ngx.exit(499)
 end
 
 function _M:rewrite(context)
-  ngx.on_abort(self.cleanup)
+  local ok, err = ngx.on_abort(self.cleanup)
+  if not ok then
+    ngx.log(ngx.ERR, "failed to register the on_abort callback: ", err)
+    ngx.exit(500)
+  end
 
   -- load configuration if not configured
   -- that is useful when lua_code_cache is off
@@ -87,6 +92,12 @@ function _M:post_action(context)
 end
 
 function _M:access(context)
+  local ok, err = ngx.on_abort(self.cleanup)
+  if not ok then
+    ngx.log(ngx.ERR, "failed to register the on_abort callback: ", err)
+    ngx.exit(500)
+  end
+
   if context.skip_apicast_access then return end
 
   -- Flag to run post_action() only when access() was executed.
@@ -108,6 +119,12 @@ function _M:access(context)
 end
 
 function _M:content(context)
+  local ok, err = ngx.on_abort(self.cleanup)
+  if not ok then
+    ngx.log(ngx.ERR, "failed to register the on_abort callback: ", err)
+    ngx.exit(500)
+  end
+
   if not context[self].upstream then
     ngx.log(ngx.WARN, "Upstream server not found for this request")
     return errors.upstream_not_found(context.service)

spec/policy/apicast/apicast_spec.lua

@@ -1,6 +1,16 @@
 local _M = require 'apicast.policy.apicast'
 
 describe('APIcast policy', function()
+  local ngx_on_abort_stub
+
+  before_each(function()
+      -- .access calls ngx.on_abort
+      -- busted tests are called in the context of ngx.timer
+      -- and that API ngx.on_abort is disabled in that context.
+      -- this stub is mocking the call
+      -- to prevent the internal error: API disabled in the context of ngx.timer
+      ngx_on_abort_stub = stub(ngx, 'on_abort')
+  end)
 
   it('has a name', function()
     assert.truthy(_M._NAME)