feat(openid-connect): make session_secret support configurable by bzp2010 · Pull Request #8068 · apache/apisix

bzp2010 · 2022-10-11T04:53:01Z

Description

The current OIDC plugin for APISIX uses lua-resty-session, which requires encryption of the session, but we do not provide a default secret nor do we allow users to configure it directly.

Therefore, according to the implementation principle in lua-resty-session, if no secret configuration is provided, it will generate one at initialization, yes, one at each worker, and they are all different.

When a client uses a short connection or traffic passes through a load balancing component, it may request to a different worker each time, which causes decryption and hash verification failures.

I think if we can allow users to set session secret through plugin configuration, we can solve this problem.

Checklist

I have explained the need for this PR and the problem it solves
I have explained the changes or the new features added to this PR
I have added tests corresponding to this change
I have updated the documentation to reflect this change
I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

spacewander · 2022-10-14T05:28:42Z

 | set_userinfo_header                  | boolean | False    | true                  |              | When set to true and the UserInfo object is available, sets it in the `X-Userinfo` request header.                       |
 | set_refresh_token_header             | boolean | False    | false                 |              | When set to true and a refresh token object is available, sets it in the `X-Refresh-Token` request header.               |
+| session                              | object  | False    |                       |              | When bearer_only is set to false, openid-connect will use Authorization Code flow to authenticate on the IDP, so you need to set the session-related configuration. |
+| session.secret                       | string  | True     | Automatic generation  | 16 or more characters | The key used for session encrypt and HMAC operation. |


Should we apply the "16 or more characters" check in the schema?

Yes, I think it's best to do that. Let me modify it.

spacewander · 2022-10-14T11:48:56Z

+                    end,
+                },
+                {
+                    name = "sanity (bearer_only = false, user-set secret, less than 16 charactors)",


charactors

Typo?

Yes, I have fixed it.

…e#8068)

mkyc · 2022-12-15T14:20:22Z

Should this also be implemented in 2.15 versions since it's LTS? I can confirm that this problematic behaviour exists on 2.15.1 when gateway is behind AWS ALB Ingress.

tzssangglass · 2022-12-16T02:15:13Z

Should this also be implemented in 2.15 versions since it's LTS? I can confirm that this problematic behaviour exists on 2.15.1 when gateway is behind AWS ALB Ingress.

#6792 (comment) would works for you?

mkyc · 2022-12-16T16:56:12Z

@tzssangglass nice! Following workaround in APISIX helm chart values fixes problem in 2.15.1:

configurationSnippet:
  httpSrv: |
    set $session_secret 0123456789a5bac9bb3c868ec8b202e93;

MirtoBusico · 2022-12-24T13:05:14Z

@tzssangglass nice! Following workaround in APISIX helm chart values fixes problem in 2.15.1:
configurationSnippet:
  httpSrv: |
    set $session_secret 0123456789a5bac9bb3c868ec8b202e93;

Hi all,
I tried to use the workaround.

Now in my values.yaml for apisix helm chart I have:

# Custom configuration snippet.
configurationSnippet:
  main: |

  httpStart: |

  httpEnd: |

  httpSrv: |
    set $session_secret 0123456789a5bac9bb3c868ec8b202e93;

  httpAdmin: |

  stream: |

# Observability configuration.

Still I get a "openid-connect exits with http status code 500" error from the openid-connect plugin.

2022/12/24 12:44:45 [warn] 47#47: *581337 [lua] v3.lua:716: request_chunk(): http://apisix-etcd.apisix.svc.cluster.local:2379: failed to parse domain: failed to parse domain. Retrying, context: ngx.timer
2022/12/24 12:47:11 [warn] 47#47: *666008 [lua] v3.lua:716: request_chunk(): http://apisix-etcd.apisix.svc.cluster.local:2379: failed to parse domain: failed to parse domain. Retrying, context: ngx.timer
2022/12/24 12:48:22 [error] 50#50: *673065 [lua] openidc.lua:1100: authenticate(): state from argument: 75f1ea8eb72acd29e847e4afe36ca426 does not match state restored from session: a1072f4e1facdf0e90714bbd6163ea0e, client: 127.0.0.6, server: _, request: "GET /*?state=75f1ea8eb72acd29e847e4afe36ca426&session_state=aff4d9a9-1641-455d-b368-6f13c2925c32&code=9de7dc09-e3db-4507-a4be-51c4b57de1aa.aff4d9a9-1641-455d-b368-6f13c2925c32.84a0adb8-9534-4db9-9e55-7675c11e5b76 HTTP/1.0", host: "apisix.h.net"
2022/12/24 12:48:22 [error] 50#50: *673065 [lua] openid-connect.lua:315: phase_func(): OIDC authentication failed: state from argument does not match state restored from session, client: 127.0.0.6, server: _, request: "GET /*?state=75f1ea8eb72acd29e847e4afe36ca426&session_state=aff4d9a9-1641-455d-b368-6f13c2925c32&code=9de7dc09-e3db-4507-a4be-51c4b57de1aa.aff4d9a9-1641-455d-b368-6f13c2925c32.84a0adb8-9534-4db9-9e55-7675c11e5b76 HTTP/1.0", host: "apisix.h.net"
2022/12/24 12:48:22 [warn] 50#50: *673065 [lua] plugin.lua:934: run_plugin(): openid-connect exits with http status code 500, client: 127.0.0.6, server: _, request: "GET /*?state=75f1ea8eb72acd29e847e4afe36ca426&session_state=aff4d9a9-1641-455d-b368-6f13c2925c32&code=9de7dc09-e3db-4507-a4be-51c4b57de1aa.aff4d9a9-1641-455d-b368-6f13c2925c32.84a0adb8-9534-4db9-9e55-7675c11e5b76 HTTP/1.0", host: "apisix.h.net"

What I'm doing wrong?

mehmetcuneyit · 2023-06-06T10:10:20Z

@tzssangglass nice! Following workaround in APISIX helm chart values fixes problem in 2.15.1:
configurationSnippet:
  httpSrv: |
    set $session_secret 0123456789a5bac9bb3c868ec8b202e93;

Hi all, I tried to use the workaround.

Now in my values.yaml for apisix helm chart I have:

# Custom configuration snippet.
configurationSnippet:
  main: |

  httpStart: |

  httpEnd: |

  httpSrv: |
    set $session_secret 0123456789a5bac9bb3c868ec8b202e93;

  httpAdmin: |

  stream: |

# Observability configuration.

Still I get a "openid-connect exits with http status code 500" error from the openid-connect plugin.

2022/12/24 12:44:45 [warn] 47#47: *581337 [lua] v3.lua:716: request_chunk(): http://apisix-etcd.apisix.svc.cluster.local:2379: failed to parse domain: failed to parse domain. Retrying, context: ngx.timer
2022/12/24 12:47:11 [warn] 47#47: *666008 [lua] v3.lua:716: request_chunk(): http://apisix-etcd.apisix.svc.cluster.local:2379: failed to parse domain: failed to parse domain. Retrying, context: ngx.timer
2022/12/24 12:48:22 [error] 50#50: *673065 [lua] openidc.lua:1100: authenticate(): state from argument: 75f1ea8eb72acd29e847e4afe36ca426 does not match state restored from session: a1072f4e1facdf0e90714bbd6163ea0e, client: 127.0.0.6, server: _, request: "GET /*?state=75f1ea8eb72acd29e847e4afe36ca426&session_state=aff4d9a9-1641-455d-b368-6f13c2925c32&code=9de7dc09-e3db-4507-a4be-51c4b57de1aa.aff4d9a9-1641-455d-b368-6f13c2925c32.84a0adb8-9534-4db9-9e55-7675c11e5b76 HTTP/1.0", host: "apisix.h.net"
2022/12/24 12:48:22 [error] 50#50: *673065 [lua] openid-connect.lua:315: phase_func(): OIDC authentication failed: state from argument does not match state restored from session, client: 127.0.0.6, server: _, request: "GET /*?state=75f1ea8eb72acd29e847e4afe36ca426&session_state=aff4d9a9-1641-455d-b368-6f13c2925c32&code=9de7dc09-e3db-4507-a4be-51c4b57de1aa.aff4d9a9-1641-455d-b368-6f13c2925c32.84a0adb8-9534-4db9-9e55-7675c11e5b76 HTTP/1.0", host: "apisix.h.net"
2022/12/24 12:48:22 [warn] 50#50: *673065 [lua] plugin.lua:934: run_plugin(): openid-connect exits with http status code 500, client: 127.0.0.6, server: _, request: "GET /*?state=75f1ea8eb72acd29e847e4afe36ca426&session_state=aff4d9a9-1641-455d-b368-6f13c2925c32&code=9de7dc09-e3db-4507-a4be-51c4b57de1aa.aff4d9a9-1641-455d-b368-6f13c2925c32.84a0adb8-9534-4db9-9e55-7675c11e5b76 HTTP/1.0", host: "apisix.h.net"

What I'm doing wrong?

Change 'set $session_secret' -> 'set $session_redis_password' , the lua-resty-session lib updated this field

bzp2010 added 2 commits September 30, 2022 16:43

feat: add session config

6f20f30

test: add session secret cases

8a40188

bzp2010 self-assigned this Oct 11, 2022

bzp2010 changed the title ~~feat: make session_secret support configurable~~ feat(openid-connect): make session_secret support configurable Oct 11, 2022

bzp2010 added 5 commits October 11, 2022 16:34

test: fix cases

77fc0d2

fix: lint

2b4d7ec

chore: clean codes

ae4675e

docs: add attributes describe

747a7ab

docs: fix lint

51c8310

bzp2010 marked this pull request as ready for review October 13, 2022 06:41

bzp2010 requested review from soulbird, spacewander and tzssangglass October 13, 2022 06:41

tzssangglass previously approved these changes Oct 13, 2022

View reviewed changes

spacewander reviewed Oct 14, 2022

View reviewed changes

chore: add secret length limit

9aa0281

bzp2010 dismissed tzssangglass’s stale review via 9aa0281 October 14, 2022 07:42

bzp2010 requested review from spacewander and tzssangglass October 14, 2022 07:42

test: fix secret length cases

5b20c73

spacewander previously approved these changes Oct 14, 2022

View reviewed changes

spacewander reviewed Oct 14, 2022

View reviewed changes

fix: typo

08ea05b

bzp2010 dismissed spacewander’s stale review via 08ea05b October 16, 2022 15:18

bzp2010 requested a review from spacewander October 17, 2022 01:01

tzssangglass approved these changes Oct 17, 2022

View reviewed changes

spacewander approved these changes Oct 17, 2022

View reviewed changes

spacewander merged commit 8f39062 into apache:master Oct 17, 2022

Liu-Junlin pushed a commit to Liu-Junlin/apisix that referenced this pull request Nov 4, 2022

feat(openid-connect): make session_secret support configurable (apach…

7fbd8f4

…e#8068)

tzssangglass mentioned this pull request Dec 6, 2022

help request: Openid-connect - how to diagnose the error page "An error occurred. You can report issue to APISIX Faithfully yours, APISIX." #8452

Closed

MirtoBusico mentioned this pull request Dec 27, 2022

bug: apisix 2.15.1 do not accept private CA (worked in apisix 2.13.0) #8568

Closed

MirtoBusico mentioned this pull request Jun 6, 2023

help request: OpenID plugin error: "request to the redirect_uri path but there's no session state found" #9609

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(openid-connect): make session_secret support configurable#8068

feat(openid-connect): make session_secret support configurable#8068
spacewander merged 10 commits into
apache:masterfrom
bzp2010:fix-oidc-multi-worker

bzp2010 commented Oct 11, 2022

Uh oh!

spacewander Oct 14, 2022

Uh oh!

bzp2010 Oct 14, 2022

Uh oh!

bzp2010 Oct 14, 2022

Uh oh!

spacewander Oct 14, 2022

Uh oh!

bzp2010 Oct 16, 2022

Uh oh!

mkyc commented Dec 15, 2022

Uh oh!

tzssangglass commented Dec 16, 2022

Uh oh!

mkyc commented Dec 16, 2022

Uh oh!

MirtoBusico commented Dec 24, 2022

Uh oh!

mehmetcuneyit commented Jun 6, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

bzp2010 commented Oct 11, 2022

Description

Checklist

Uh oh!

spacewander Oct 14, 2022

Choose a reason for hiding this comment

Uh oh!

bzp2010 Oct 14, 2022

Choose a reason for hiding this comment

Uh oh!

bzp2010 Oct 14, 2022

Choose a reason for hiding this comment

Uh oh!

spacewander Oct 14, 2022

Choose a reason for hiding this comment

Uh oh!

bzp2010 Oct 16, 2022

Choose a reason for hiding this comment

Uh oh!

mkyc commented Dec 15, 2022

Uh oh!

tzssangglass commented Dec 16, 2022

Uh oh!

mkyc commented Dec 16, 2022

Uh oh!

MirtoBusico commented Dec 24, 2022

Uh oh!

mehmetcuneyit commented Jun 6, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants