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

fix(runloop): upstream ssl failure when plugins use response handler #11502

Merged

Conversation

catbro666
Copy link
Contributor

@catbro666 catbro666 commented Aug 31, 2023

Summary

If a plugin has response() handler, in Kong.response it will emits a subrequest by calling ngx.location.capture("/kong_buffered_http", options). ngx.location.capture will create a new nginx request, so the overwritten ssl info (client key & cert etc.) get lost in the new nginx request.

To fix this, those ssl info need to be re-set in the new request context. We choose to do this in the early rewrite phase of the new request before Kong.balancer() getting executed.

The normal flow is as below:

Kong.access.before            Kong.access.after             Kong.balancer(first try)    Kong.balancer(retry)
       v                             v                                     |                 v
balancer_prepare              balancer.execute(try_count=0)                |          balancer.execute(try_count~=0)
       v                             v                                     |                 |
set service ssl config        set upstream ssl config                      v                 v
                 \                /                                  init upstream ssl connection
                  \              /                                              |
                   v            v                                               v
                   ---------------                   set onto              ------------
                   | downstream r|        --------------------------->     |upstream c|
                   ---------------                                         ------------

For the buffered_proxying case, ngx.location.capture will create a new request, so the flow diagram become:

Kong.access.before            Kong.access.after             Kong.balancer(first try)    Kong.balancer(retry)
       v                             v                                     |                 v
balancer_prepare              balancer.execute(try_count=0)                |          balancer.execute(try_count~=0)
       v                             v                                     |                 |
set service ssl config        set upstream ssl config                      v                 v
                 \                /                                  init upstream ssl connection
                  \              /                                              |
                   v            v                                               v
                   ---------------                   set onto              ------------
                   | downstream r|        --------------------------->     |upstream c|
                   ---------------                                         ------------
ngx.location.capture     |
.........................+.............................................................................................
                         | ctx
                         | vars
                         v
                   ---------------                   set onto              ------------
                   |    sub r    |        --------------------------->     |upstream c|
                   ---------------                                         ------------
                         ^                                                      ^
                         |                                                      |
            -----------------------------                               ------------------
            |       Kong.rewrite        |                               |  Kong.balancer |
            |  set service ssl config   |                               |  same as above |
            |  set upstream ssl config  |                               ------------------
            -----------------------------

FTI-5347

If a plugin has response() handler, in `Kong.response` it will emits
a subrequest by calling `ngx.location.capture("/kong_buffered_http", options)`.
`ngx.location.capture` will create a new nginx request, so the overwritten
ssl info (client key & cert etc.) get lost in the new nginx request.

To fix this, those ssl info need to be re-set in the new request
context. We choose to do this in the early rewrite phase of the new
request before `Kong.balancer()` getting executed.

[FTI-5347](https://konghq.atlassian.net/browse/FTI-5347)
@catbro666 catbro666 force-pushed the fix-FTI-5347-kong-response-subrequest-lost-client-ssl-info branch from fede3ca to 8c43536 Compare August 31, 2023 08:40
@pull-request-size pull-request-size bot added size/XL and removed size/L labels Sep 6, 2023
@catbro666 catbro666 marked this pull request as ready for review September 6, 2023 16:14
Copy link
Member

@oowl oowl left a comment

Choose a reason for hiding this comment

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

LGTM

@windmgc windmgc merged commit 54468c4 into master Sep 19, 2023
5 checks passed
@windmgc windmgc deleted the fix-FTI-5347-kong-response-subrequest-lost-client-ssl-info branch September 19, 2023 05:48
catbro666 added a commit that referenced this pull request Sep 21, 2023
…11502)

* fix(runloop): upstream ssl failure when plugins use response handler

If a plugin has response() handler, in `Kong.response` it will emits
a subrequest by calling `ngx.location.capture("/kong_buffered_http", options)`.
`ngx.location.capture` will create a new nginx request, so the overwritten
ssl info (client key & cert etc.) get lost in the new nginx request.

To fix this, those ssl info need to be re-set in the new request
context. We choose to do this in the early rewrite phase of the new
request before `Kong.balancer()` getting executed.

[FTI-5347](https://konghq.atlassian.net/browse/FTI-5347)

(cherry picked from commit 54468c4)
windmgc pushed a commit that referenced this pull request Sep 21, 2023
Only some style clean for #11502, applying early return.
windmgc pushed a commit that referenced this pull request Sep 22, 2023
…11600)

* fix(runloop): upstream ssl failure when plugins use response handler (#11502)

* fix(runloop): upstream ssl failure when plugins use response handler

If a plugin has response() handler, in `Kong.response` it will emits
a subrequest by calling `ngx.location.capture("/kong_buffered_http", options)`.
`ngx.location.capture` will create a new nginx request, so the overwritten
ssl info (client key & cert etc.) get lost in the new nginx request.

To fix this, those ssl info need to be re-set in the new request
context. We choose to do this in the early rewrite phase of the new
request before `Kong.balancer()` getting executed.

[FTI-5347](https://konghq.atlassian.net/browse/FTI-5347)

(cherry picked from commit 54468c4)

* update nginx template in spec

---------

Co-authored-by: Zhefeng C <[email protected]>
Co-authored-by: Zhefeng Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants