#732 Add message body to response-ratelimiting plugin, block requests… #733
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ests via access.lua, not header_filter.lua
| for limit_name, v in pairs(usage) do | ||
| for period_name, lv in pairs(usage[limit_name]) do | ||
| if lv.remaining <= 0 then | ||
| stop = true -- No more |
There was a problem hiding this comment.
We could break here, or even return the 429directly.
|
LGTM. Tbh I have no idea why that plugin was not already implemented like so, it sounds utterly stupid that the actual limiting is happening in This also makes me think: rate-limiting/quota plugins should ideally return which limit has been reached in the response body, I think that could be helpful, and values stored in Thanks for spotting this. |
thibaultcha
added
the
pr/ready (but hold merge)
For two reasons:
Bulk requestsThis plugin has a substantial problem with bulk requests that can change the state of multiple objects in the upstream service: when sending a bulk request that exceeds the limits, there is no way for Kong to prevent the request from going through and possibly change the state of the objects. For example an SMS service allows to send up to 10 sms messages per day, but also allows to send a bulk request, so that one single In both implementations (the previous one, and the one in the PR) this is unavoidable and a known limitation of the plugin. But there is a difference: in the previous implementation, when going over the limits on a bulk request, I would send a This behavior is useful also for bulk requests that don't change the state of the objects in the upstream service. Example: A video API only allows to retrieve 100 videos per day using a Multiple quotasThe reason why the previous implementation doesn't block the request in the |
|
Fair enough, but the most simple, very common use case which is the one describe by @nricklin is still broken in the old implementation. A request (not bulked) must not hit upstream if the quota is exceeded. |
thibaultcha
added
status/do not merge
and removed
pr/ready (but hold merge)
|
To improve the previous implementation, we could have this check in The rest of the logic needs to be in |
|
In my opinion handling bulk requests correctly is up to the API creator. But you're right that handling multiple quotas is a problem. I think it's an impossible problem. How can Kong ever know whether to block a request up front, when it only finds out it should have been blocked after the response comes back? A solution could be to tie a set of request paths & verbs to a particular quota. Example:
Then depending on the path and verb you check to see whether you should block based on a limit. |
kikito
pushed a commit
that referenced
this pull request
Apr 23, 2024
### Summary
#### 2.6.0
```
Release 2.6.0 Tue February 6 2024
Security fixes:
#789 #814 CVE-2023-52425 -- Fix quadratic runtime issues with big tokens
that can cause denial of service, in partial where
dealing with compressed XML input. Applications
that parsed a document in one go -- a single call to
functions XML_Parse or XML_ParseBuffer -- were not affected.
The smaller the chunks/buffers you use for parsing
previously, the bigger the problem prior to the fix.
Backporters should be careful to no omit parts of
pull request #789 and to include earlier pull request #771,
in order to not break the fix.
#777 CVE-2023-52426 -- Fix billion laughs attacks for users
compiling *without* XML_DTD defined (which is not common).
Users with XML_DTD defined have been protected since
Expat >=2.4.0 (and that was CVE-2013-0340 back then).
Bug fixes:
#753 Fix parse-size-dependent "invalid token" error for
external entities that start with a byte order mark
#780 Fix NULL pointer dereference in setContext via
XML_ExternalEntityParserCreate for compilation with
XML_DTD undefined
#812 #813 Protect against closing entities out of order
Other changes:
#723 Improve support for arc4random/arc4random_buf
#771 #788 Improve buffer growth in XML_GetBuffer and XML_Parse
#761 #770 xmlwf: Support --help and --version
#759 #770 xmlwf: Support custom buffer size for XML_GetBuffer and read
#744 xmlwf: Improve language and URL clickability in help output
#673 examples: Add new example "element_declarations.c"
#764 Be stricter about macro XML_CONTEXT_BYTES at build time
#765 Make inclusion to expat_config.h consistent
#726 #727 Autotools: configure.ac: Support --disable-maintainer-mode
#678 #705 ..
#706 #733 #792 Autotools: Sync CMake templates with CMake 3.26
#795 Autotools: Make installation of shipped man page doc/xmlwf.1
independent of docbook2man availability
#815 Autotools|CMake: Add missing -DXML_STATIC to pkg-config file
section "Cflags.private" in order to fix compilation
against static libexpat using pkg-config on Windows
#724 #751 Autotools|CMake: Require a C99 compiler
(a de-facto requirement already since Expat 2.2.2 of 2017)
#793 Autotools|CMake: Fix PACKAGE_BUGREPORT variable
#750 #786 Autotools|CMake: Make test suite require a C++11 compiler
#749 CMake: Require CMake >=3.5.0
#672 CMake: Lowercase off_t and size_t to help a bug in Meson
#746 CMake: Sort xmlwf sources alphabetically
#785 CMake|Windows: Fix generation of DLL file version info
#790 CMake: Build tests/benchmark/benchmark.c as well for
a build with -DEXPAT_BUILD_TESTS=ON
#745 #757 docs: Document the importance of isFinal + adjust tests
accordingly
#736 docs: Improve use of "NULL" and "null"
#713 docs: Be specific about version of XML (XML 1.0r4)
and version of C (C99); (XML 1.0r5 will need a sponsor.)
#762 docs: reference.html: Promote function XML_ParseBuffer more
#779 docs: reference.html: Add HTML anchors to XML_* macros
#760 docs: reference.html: Upgrade to OK.css 1.2.0
#763 #739 docs: Fix typos
#696 docs|CI: Use HTTPS URLs instead of HTTP at various places
#669 #670 ..
#692 #703 ..
#733 #772 Address compiler warnings
#798 #800 Address clang-tidy warnings
#775 #776 Version info bumped from 9:10:8 (libexpat*.so.1.8.10)
to 10:0:9 (libexpat*.so.1.9.0); see https://verbump.de/
for what these numbers do
Infrastructure:
#700 #701 docs: Document security policy in file SECURITY.md
#766 docs: Improve parse buffer variables in-code documentation
#674 #738 ..
#740 #747 ..
#748 #781 #782 Refactor coverage and conformance tests
#714 #716 Refactor debug level variables to unsigned long
#671 Improve handling of empty environment variable value
in function getDebugLevel (without visible user effect)
#755 #774 ..
#758 #783 ..
#784 #787 tests: Improve test coverage with regard to parse chunk size
#660 #797 #801 Fuzzing: Improve fuzzing coverage
#367 #799 Fuzzing|CI: Start running OSS-Fuzz fuzzing regression tests
#698 #721 CI: Resolve some Travis CI leftovers
#669 CI: Be robust towards absence of Git tags
#693 #694 CI: Set permissions to "contents: read" for security
#709 CI: Pin all GitHub Actions to specific commits for security
#739 CI: Reject spelling errors using codespell
#798 CI: Enforce clang-tidy clean code
#773 #808 ..
#809 #810 CI: Upgrade Clang from 15 to 18
#796 CI: Start using Clang's Control Flow Integrity sanitizer
#675 #720 #722 CI: Adapt to breaking changes in GitHub Actions Ubuntu images
#689 CI: Adapt to breaking changes in Clang/LLVM Debian packaging
#763 CI: Adapt to breaking changes in codespell
#803 CI: Adapt to breaking changes in Cppcheck
Special thanks to:
Ivan Galkin
Joyce Brum
Philippe Antoine
Rhodri James
Snild Dolkow
spookyahell
Steven Garske
and
Clang AddressSanitizer
Clang UndefinedBehaviorSanitizer
codespell
GCC Farm Project
OSS-Fuzz
Sony Mobile
```
#### 2.6.1
```
Release 2.6.1 Thu February 29 2024
Bug fixes:
#817 Make tests independent of CPU speed, and thus more robust
#828 #836 Expose billion laughs API with XML_DTD defined and
XML_GE undefined, regression from 2.6.0
Other changes:
#829 Hide test-only code behind new internal macro
#833 Autotools: Reject expat_config.h.in defining SIZEOF_VOID_P
#819 Address compiler warnings
#832 #834 Version info bumped from 10:0:9 (libexpat*.so.1.9.0)
to 10:1:9 (libexpat*.so.1.9.1); see https://verbump.de/
for what these numbers do
Infrastructure:
#818 CI: Adapt to breaking changes in clang-format
Special thanks to:
David Hall
Snild Dolkow
```
#### 2.6.2
```
Release 2.6.2 Wed March 13 2024
Security fixes:
#839 #842 CVE-2024-28757 -- Prevent billion laughs attacks with
isolated use of external parsers. Please see the commit
message of commit 1d50b80cf31de87750103656f6eb693746854aa8
for details.
Bug fixes:
#839 #841 Reject direct parameter entity recursion
and avoid the related undefined behavior
Other changes:
#847 Autotools: Fix build for DOCBOOK_TO_MAN containing spaces
#837 Add missing #821 and #824 to 2.6.1 change log
#838 #843 Version info bumped from 10:1:9 (libexpat*.so.1.9.1)
to 10:2:9 (libexpat*.so.1.9.2); see https://verbump.de/
for what these numbers do
Special thanks to:
Philippe Antoine
Tomas Korbar
and
Clang UndefinedBehaviorSanitizer
OSS-Fuzz / ClusterFuzz
```
Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>
bungle
added a commit
that referenced
this pull request
Apr 23, 2024
### Summary
#### 2.6.0
```
Release 2.6.0 Tue February 6 2024
Security fixes:
#789 #814 CVE-2023-52425 -- Fix quadratic runtime issues with big tokens
that can cause denial of service, in partial where
dealing with compressed XML input. Applications
that parsed a document in one go -- a single call to
functions XML_Parse or XML_ParseBuffer -- were not affected.
The smaller the chunks/buffers you use for parsing
previously, the bigger the problem prior to the fix.
Backporters should be careful to no omit parts of
pull request #789 and to include earlier pull request #771,
in order to not break the fix.
#777 CVE-2023-52426 -- Fix billion laughs attacks for users
compiling *without* XML_DTD defined (which is not common).
Users with XML_DTD defined have been protected since
Expat >=2.4.0 (and that was CVE-2013-0340 back then).
Bug fixes:
#753 Fix parse-size-dependent "invalid token" error for
external entities that start with a byte order mark
#780 Fix NULL pointer dereference in setContext via
XML_ExternalEntityParserCreate for compilation with
XML_DTD undefined
#812 #813 Protect against closing entities out of order
Other changes:
#723 Improve support for arc4random/arc4random_buf
#771 #788 Improve buffer growth in XML_GetBuffer and XML_Parse
#761 #770 xmlwf: Support --help and --version
#759 #770 xmlwf: Support custom buffer size for XML_GetBuffer and read
#744 xmlwf: Improve language and URL clickability in help output
#673 examples: Add new example "element_declarations.c"
#764 Be stricter about macro XML_CONTEXT_BYTES at build time
#765 Make inclusion to expat_config.h consistent
#726 #727 Autotools: configure.ac: Support --disable-maintainer-mode
#678 #705 ..
#706 #733 #792 Autotools: Sync CMake templates with CMake 3.26
#795 Autotools: Make installation of shipped man page doc/xmlwf.1
independent of docbook2man availability
#815 Autotools|CMake: Add missing -DXML_STATIC to pkg-config file
section "Cflags.private" in order to fix compilation
against static libexpat using pkg-config on Windows
#724 #751 Autotools|CMake: Require a C99 compiler
(a de-facto requirement already since Expat 2.2.2 of 2017)
#793 Autotools|CMake: Fix PACKAGE_BUGREPORT variable
#750 #786 Autotools|CMake: Make test suite require a C++11 compiler
#749 CMake: Require CMake >=3.5.0
#672 CMake: Lowercase off_t and size_t to help a bug in Meson
#746 CMake: Sort xmlwf sources alphabetically
#785 CMake|Windows: Fix generation of DLL file version info
#790 CMake: Build tests/benchmark/benchmark.c as well for
a build with -DEXPAT_BUILD_TESTS=ON
#745 #757 docs: Document the importance of isFinal + adjust tests
accordingly
#736 docs: Improve use of "NULL" and "null"
#713 docs: Be specific about version of XML (XML 1.0r4)
and version of C (C99); (XML 1.0r5 will need a sponsor.)
#762 docs: reference.html: Promote function XML_ParseBuffer more
#779 docs: reference.html: Add HTML anchors to XML_* macros
#760 docs: reference.html: Upgrade to OK.css 1.2.0
#763 #739 docs: Fix typos
#696 docs|CI: Use HTTPS URLs instead of HTTP at various places
#669 #670 ..
#692 #703 ..
#733 #772 Address compiler warnings
#798 #800 Address clang-tidy warnings
#775 #776 Version info bumped from 9:10:8 (libexpat*.so.1.8.10)
to 10:0:9 (libexpat*.so.1.9.0); see https://verbump.de/
for what these numbers do
Infrastructure:
#700 #701 docs: Document security policy in file SECURITY.md
#766 docs: Improve parse buffer variables in-code documentation
#674 #738 ..
#740 #747 ..
#748 #781 #782 Refactor coverage and conformance tests
#714 #716 Refactor debug level variables to unsigned long
#671 Improve handling of empty environment variable value
in function getDebugLevel (without visible user effect)
#755 #774 ..
#758 #783 ..
#784 #787 tests: Improve test coverage with regard to parse chunk size
#660 #797 #801 Fuzzing: Improve fuzzing coverage
#367 #799 Fuzzing|CI: Start running OSS-Fuzz fuzzing regression tests
#698 #721 CI: Resolve some Travis CI leftovers
#669 CI: Be robust towards absence of Git tags
#693 #694 CI: Set permissions to "contents: read" for security
#709 CI: Pin all GitHub Actions to specific commits for security
#739 CI: Reject spelling errors using codespell
#798 CI: Enforce clang-tidy clean code
#773 #808 ..
#809 #810 CI: Upgrade Clang from 15 to 18
#796 CI: Start using Clang's Control Flow Integrity sanitizer
#675 #720 #722 CI: Adapt to breaking changes in GitHub Actions Ubuntu images
#689 CI: Adapt to breaking changes in Clang/LLVM Debian packaging
#763 CI: Adapt to breaking changes in codespell
#803 CI: Adapt to breaking changes in Cppcheck
Special thanks to:
Ivan Galkin
Joyce Brum
Philippe Antoine
Rhodri James
Snild Dolkow
spookyahell
Steven Garske
and
Clang AddressSanitizer
Clang UndefinedBehaviorSanitizer
codespell
GCC Farm Project
OSS-Fuzz
Sony Mobile
```
#### 2.6.1
```
Release 2.6.1 Thu February 29 2024
Bug fixes:
#817 Make tests independent of CPU speed, and thus more robust
#828 #836 Expose billion laughs API with XML_DTD defined and
XML_GE undefined, regression from 2.6.0
Other changes:
#829 Hide test-only code behind new internal macro
#833 Autotools: Reject expat_config.h.in defining SIZEOF_VOID_P
#819 Address compiler warnings
#832 #834 Version info bumped from 10:0:9 (libexpat*.so.1.9.0)
to 10:1:9 (libexpat*.so.1.9.1); see https://verbump.de/
for what these numbers do
Infrastructure:
#818 CI: Adapt to breaking changes in clang-format
Special thanks to:
David Hall
Snild Dolkow
```
#### 2.6.2
```
Release 2.6.2 Wed March 13 2024
Security fixes:
#839 #842 CVE-2024-28757 -- Prevent billion laughs attacks with
isolated use of external parsers. Please see the commit
message of commit 1d50b80cf31de87750103656f6eb693746854aa8
for details.
Bug fixes:
#839 #841 Reject direct parameter entity recursion
and avoid the related undefined behavior
Other changes:
#847 Autotools: Fix build for DOCBOOK_TO_MAN containing spaces
#837 Add missing #821 and #824 to 2.6.1 change log
#838 #843 Version info bumped from 10:1:9 (libexpat*.so.1.9.1)
to 10:2:9 (libexpat*.so.1.9.2); see https://verbump.de/
for what these numbers do
Special thanks to:
Philippe Antoine
Tomas Korbar
and
Clang UndefinedBehaviorSanitizer
OSS-Fuzz / ClusterFuzz
```
Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>
tysoekong
pushed a commit
that referenced
this pull request
Apr 26, 2024
```
Release 2.6.0 Tue February 6 2024
Security fixes:
#789 #814 CVE-2023-52425 -- Fix quadratic runtime issues with big tokens
that can cause denial of service, in partial where
dealing with compressed XML input. Applications
that parsed a document in one go -- a single call to
functions XML_Parse or XML_ParseBuffer -- were not affected.
The smaller the chunks/buffers you use for parsing
previously, the bigger the problem prior to the fix.
Backporters should be careful to no omit parts of
pull request #789 and to include earlier pull request #771,
in order to not break the fix.
#777 CVE-2023-52426 -- Fix billion laughs attacks for users
compiling *without* XML_DTD defined (which is not common).
Users with XML_DTD defined have been protected since
Expat >=2.4.0 (and that was CVE-2013-0340 back then).
Bug fixes:
#753 Fix parse-size-dependent "invalid token" error for
external entities that start with a byte order mark
#780 Fix NULL pointer dereference in setContext via
XML_ExternalEntityParserCreate for compilation with
XML_DTD undefined
#812 #813 Protect against closing entities out of order
Other changes:
#723 Improve support for arc4random/arc4random_buf
#771 #788 Improve buffer growth in XML_GetBuffer and XML_Parse
#761 #770 xmlwf: Support --help and --version
#759 #770 xmlwf: Support custom buffer size for XML_GetBuffer and read
#744 xmlwf: Improve language and URL clickability in help output
#673 examples: Add new example "element_declarations.c"
#764 Be stricter about macro XML_CONTEXT_BYTES at build time
#765 Make inclusion to expat_config.h consistent
#726 #727 Autotools: configure.ac: Support --disable-maintainer-mode
#678 #705 ..
#706 #733 #792 Autotools: Sync CMake templates with CMake 3.26
#795 Autotools: Make installation of shipped man page doc/xmlwf.1
independent of docbook2man availability
#815 Autotools|CMake: Add missing -DXML_STATIC to pkg-config file
section "Cflags.private" in order to fix compilation
against static libexpat using pkg-config on Windows
#724 #751 Autotools|CMake: Require a C99 compiler
(a de-facto requirement already since Expat 2.2.2 of 2017)
#793 Autotools|CMake: Fix PACKAGE_BUGREPORT variable
#750 #786 Autotools|CMake: Make test suite require a C++11 compiler
#749 CMake: Require CMake >=3.5.0
#672 CMake: Lowercase off_t and size_t to help a bug in Meson
#746 CMake: Sort xmlwf sources alphabetically
#785 CMake|Windows: Fix generation of DLL file version info
#790 CMake: Build tests/benchmark/benchmark.c as well for
a build with -DEXPAT_BUILD_TESTS=ON
#745 #757 docs: Document the importance of isFinal + adjust tests
accordingly
#736 docs: Improve use of "NULL" and "null"
#713 docs: Be specific about version of XML (XML 1.0r4)
and version of C (C99); (XML 1.0r5 will need a sponsor.)
#762 docs: reference.html: Promote function XML_ParseBuffer more
#779 docs: reference.html: Add HTML anchors to XML_* macros
#760 docs: reference.html: Upgrade to OK.css 1.2.0
#763 #739 docs: Fix typos
#696 docs|CI: Use HTTPS URLs instead of HTTP at various places
#669 #670 ..
#692 #703 ..
#733 #772 Address compiler warnings
#798 #800 Address clang-tidy warnings
#775 #776 Version info bumped from 9:10:8 (libexpat*.so.1.8.10)
to 10:0:9 (libexpat*.so.1.9.0); see https://verbump.de/
for what these numbers do
Infrastructure:
#700 #701 docs: Document security policy in file SECURITY.md
#766 docs: Improve parse buffer variables in-code documentation
#674 #738 ..
#740 #747 ..
#748 #781 #782 Refactor coverage and conformance tests
#714 #716 Refactor debug level variables to unsigned long
#671 Improve handling of empty environment variable value
in function getDebugLevel (without visible user effect)
#755 #774 ..
#758 #783 ..
#784 #787 tests: Improve test coverage with regard to parse chunk size
#660 #797 #801 Fuzzing: Improve fuzzing coverage
#367 #799 Fuzzing|CI: Start running OSS-Fuzz fuzzing regression tests
#698 #721 CI: Resolve some Travis CI leftovers
#669 CI: Be robust towards absence of Git tags
#693 #694 CI: Set permissions to "contents: read" for security
#709 CI: Pin all GitHub Actions to specific commits for security
#739 CI: Reject spelling errors using codespell
#798 CI: Enforce clang-tidy clean code
#773 #808 ..
#809 #810 CI: Upgrade Clang from 15 to 18
#796 CI: Start using Clang's Control Flow Integrity sanitizer
#675 #720 #722 CI: Adapt to breaking changes in GitHub Actions Ubuntu images
#689 CI: Adapt to breaking changes in Clang/LLVM Debian packaging
#763 CI: Adapt to breaking changes in codespell
#803 CI: Adapt to breaking changes in Cppcheck
Special thanks to:
Ivan Galkin
Joyce Brum
Philippe Antoine
Rhodri James
Snild Dolkow
spookyahell
Steven Garske
and
Clang AddressSanitizer
Clang UndefinedBehaviorSanitizer
codespell
GCC Farm Project
OSS-Fuzz
Sony Mobile
```
```
Release 2.6.1 Thu February 29 2024
Bug fixes:
#817 Make tests independent of CPU speed, and thus more robust
#828 #836 Expose billion laughs API with XML_DTD defined and
XML_GE undefined, regression from 2.6.0
Other changes:
#829 Hide test-only code behind new internal macro
#833 Autotools: Reject expat_config.h.in defining SIZEOF_VOID_P
#819 Address compiler warnings
#832 #834 Version info bumped from 10:0:9 (libexpat*.so.1.9.0)
to 10:1:9 (libexpat*.so.1.9.1); see https://verbump.de/
for what these numbers do
Infrastructure:
#818 CI: Adapt to breaking changes in clang-format
Special thanks to:
David Hall
Snild Dolkow
```
```
Release 2.6.2 Wed March 13 2024
Security fixes:
#839 #842 CVE-2024-28757 -- Prevent billion laughs attacks with
isolated use of external parsers. Please see the commit
message of commit 1d50b80cf31de87750103656f6eb693746854aa8
for details.
Bug fixes:
#839 #841 Reject direct parameter entity recursion
and avoid the related undefined behavior
Other changes:
#847 Autotools: Fix build for DOCBOOK_TO_MAN containing spaces
#837 Add missing #821 and #824 to 2.6.1 change log
#838 #843 Version info bumped from 10:1:9 (libexpat*.so.1.9.1)
to 10:2:9 (libexpat*.so.1.9.2); see https://verbump.de/
for what these numbers do
Special thanks to:
Philippe Antoine
Tomas Korbar
and
Clang UndefinedBehaviorSanitizer
OSS-Fuzz / ClusterFuzz
```
KAG-4331
Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
… via access.lua, not header_filter.lua
I modified the plugin to block requests from access.lua rather than dropping a 429 in the status via the header_filter.lua component. Seems like header filters were receiving the response from the upstream server and THEN issuing a 429 from Kong, which is really bad. You may have created an object or something but got a 429.
Now access is blocked on the front-end.