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

Access token signature lookup bug #3485

Closed
5 of 6 tasks
rauanmayemir opened this issue Apr 10, 2023 · 2 comments · Fixed by #3486
Closed
5 of 6 tasks

Access token signature lookup bug #3485

rauanmayemir opened this issue Apr 10, 2023 · 2 comments · Fixed by #3486
Labels
bug Something is not working.

Comments

@rauanmayemir
Copy link

Preflight checklist

Describe the bug

After upgrading to 2.1.0 all the previously issued access tokens stop working due to a recent change in persistence.

Say I have the following generated access token (test env, not to worry):

ory_at_NQhlySvX3uyK8ThT9uX9yebQ8Qt3fAWY5o6mlCKX084.Y3JSKXSGRIsoxMEfHdAEbweH-rE5p-qajxSAcoAdmxI

Previously, signature (PK) column would contain Y3JSKXSGRIsoxMEfHdAEbweH-rE5p-qajxSAcoAdmxI. But after the upgrade it will be stored as a SHA384 hash, something like 54dcf7602fc6e6abf7df20f49f5cd445ee0ac431497638d69cdb013d924df5da9dff9b8040c45d40a48b3b2c8102b30d.

I believe, there is a bug here:
image

The raw signature is being double hashed for access tokens:
image

So the lookup sql query will look for signature in (sha384_of_rawsignature, sha384_of_sha384_of_rawsignature) instead of signature in (rawsignature, sha384_of_rawsignature).

@aeneasr please acknowledge. This is a critical issue that broke the previous major upgrade, and now it's breaking the minor upgrade.

Reproducing the bug

Generate an access token in 2.0.3, upgrade hydra to 2.1.0 - previously generated access token will no longer be valid - hydra fails to retrieve the record from the hydra_oauth2_access due to borked signature value.

In 2.0.3, it only hashed the raw signature if the config was set to use JWT.

In 2.1.0, it changed to hashing signature in any case. But then the lookup is borked to querying for hash and double hash (omitting raw signatures altogether).

Same issue will happen if you repeat the steps for 1.9.x -> 2.0.3.

Except in 1.9.x, it never hashed anything. Thus, 2.0.3 broke the querying, 2.1.0 broke querying again.

Relevant log output

{"audience":"application","error":{"debug":"not_found","message":"request_unauthorized","reason":"Check that you provided valid credentials in the right format.","stack_trace":"\ngithub.com/ory/x/errorsx.WithStack\n\t/go/pkg/mod/github.com/ory/x@v0.0.534/errorsx/errors.go:41\ngithub.com/ory/hydra/v2/persistence/sql.(*Persister).findSessionBySignature.func1\n\t/project/persistence/sql/persister_oauth2.go:265\ngithub.com/ory/x/popx.Transaction.func2\n\t/go/pkg/mod/github.com/ory/x@v0.0.534/popx/transaction.go:45\ngithub.com/gobuffalo/pop/v6.(*Connection).Transaction.func1\n\t/go/pkg/mod/github.com/gobuffalo/pop/v6@v6.0.8/connection.go:176\ngithub.com/gobuffalo/pop/v6.commonDialect.Lock\n\t/go/pkg/mod/github.com/gobuffalo/pop/v6@v6.0.8/dialect_common.go:29\ngithub.com/gobuffalo/pop/v6.(*Connection).Transaction\n\t/go/pkg/mod/github.com/gobuffalo/pop/v6@v6.0.8/connection.go:156\ngithub.com/ory/x/popx.Transaction\n\t/go/pkg/mod/github.com/ory/x@v0.0.534/popx/transaction.go:44\ngithub.com/ory/hydra/v2/persistence/sql.(*Persister).transaction\n\t/project/persistence/sql/persister.go:181\ngithub.com/ory/hydra/v2/persistence/sql.(*Persister).findSessionBySignature\n\t/project/persistence/sql/persister_oauth2.go:256\ngithub.com/ory/hydra/v2/persistence/sql.(*Persister).GetAccessTokenSession\n\t/project/persistence/sql/persister_oauth2.go:382\ngithub.com/ory/fosite/handler/oauth2.(*CoreValidator).introspectAccessToken\n\t/go/pkg/mod/github.com/ory/fosite@v0.44.0/handler/oauth2/introspector.go:71\ngithub.com/ory/fosite/handler/oauth2.(*CoreValidator).IntrospectToken\n\t/go/pkg/mod/github.com/ory/fosite@v0.44.0/handler/oauth2/introspector.go:46\ngithub.com/ory/fosite.(*Fosite).IntrospectToken\n\t/go/pkg/mod/github.com/ory/fosite@v0.44.0/introspect.go:45\ngithub.com/ory/hydra/v2/oauth2.(*Handler).introspectOAuth2Token\n\t/project/oauth2/handler.go:734\ngithub.com/ory/x/httprouterx.NoCacheHandle.func1\n\t/go/pkg/mod/github.com/ory/x@v0.0.534/httprouterx/nocache.go:21\ngithub.com/julienschmidt/httprouter.(*Router).ServeHTTP\n\t/go/pkg/mod/github.com/julienschmidt/httprouter@v1.3.0/router.go:387\ngithub.com/urfave/negroni.Wrap.func1\n\t/go/pkg/mod/github.com/urfave/negroni@v1.0.0/negroni.go:46\ngithub.com/urfave/negroni.HandlerFunc.ServeHTTP\n\t/go/pkg/mod/github.com/urfave/negroni@v1.0.0/negroni.go:29\ngithub.com/urfave/negroni.middleware.ServeHTTP\n\t/go/pkg/mod/github.com/urfave/negroni@v1.0.0/negroni.go:38\nnet/http.HandlerFunc.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2109\ngithub.com/ory/hydra/v2/x.RejectInsecureRequests.func1\n\t/project/x/tls_termination.go:65\ngithub.com/urfave/negroni.HandlerFunc.ServeHTTP\n\t/go/pkg/mod/github.com/urfave/negroni@v1.0.0/negroni.go:29\ngithub.com/urfave/negroni.middleware.ServeHTTP\n\t/go/pkg/mod/github.com/urfave/negroni@v1.0.0/negroni.go:38\ngithub.com/ory/x/metricsx.(*Service).ServeHTTP\n\t/go/pkg/mod/github.com/ory/x@v0.0.534/metricsx/middleware.go:258\ngithub.com/urfave/negroni.middleware.ServeHTTP\n\t/go/pkg/mod/github.com/urfave/negroni@v1.0.0/negroni.go:38\nnet/http.HandlerFunc.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2109\ngithub.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerResponseSize.func1\n\t/go/pkg/mod/github.com/prometheus/client_golang@v1.13.0/prometheus/promhttp/instrument_server.go:284\nnet/http.HandlerFunc.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2109\ngithub.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerCounter.func1\n\t/go/pkg/mod/github.com/prometheus/client_golang@v1.13.0/prometheus/promhttp/instrument_server.go:142\nnet/http.HandlerFunc.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2109\ngithub.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerDuration.func1\n\t/go/pkg/mod/github.com/prometheus/client_golang@v1.13.0/prometheus/promhttp/instrument_server.go:92\nnet/http.HandlerFunc.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2109","status":"Unauthorized","status_code":401},"http_request":{"headers":{"accept-encoding":"gzip","content-length":"93","content-type":"application/x-www-form-urlencoded","user-agent":"Go-http-client/1.1","x-b3-parentspanid":"9cc4d93e5ee209f5","x-b3-sampled":"0","x-b3-spanid":"0c43246f38db3c97","x-b3-traceid":"d74c010831b4dbc29cc4d93e5ee209f5","x-envoy-attempt-count":"1","x-forwarded-client-cert":"By=spiffe://cluster.local/ns/idp/sa/default;Hash=e6b987bee4008c346fa7b4b6b35b9f6387949c2af2ca891912afeab83222e55a;Subject=\"\";URI=spiffe://cluster.local/ns/idp/sa/default","x-forwarded-proto":"https","x-request-id":"44253d08-bc98-45a9-a56f-426fd009c6ea"},"host":"hydra-admin.idp.svc.cluster.local","method":"POST","path":"/admin/oauth2/introspect","query":null,"remote":"127.0.0.6:48105","scheme":"http"},"level":"info","msg":"access denied","otel":{"span_id":"517260162c0fd4b3","trace_id":"a1732b8d06c5ec1e6de13b3d37b12b5b"},"service_name":"Ory Hydra","service_version":"v2.1.0","time":"2023-04-10T12:52:28.70243505Z"}

Relevant configuration

No response

Version

2.1.0

On which operating system are you observing this issue?

None

In which environment are you deploying?

None

Additional Context

No response

@rauanmayemir rauanmayemir added the bug Something is not working. label Apr 10, 2023
@gou-fi
Copy link

gou-fi commented Apr 10, 2023

+1. Same issue noticed when testing an update from 2.0.3 to 2.1.0 today.

@rauanmayemir rauanmayemir changed the title Access token signature change in 2.1.0 is a breaking change Access token signature lookup bug Apr 11, 2023
hperl added a commit that referenced this issue Apr 11, 2023
hperl added a commit that referenced this issue Apr 11, 2023
aeneasr pushed a commit that referenced this issue Apr 11, 2023
@rauanmayemir
Copy link
Author

@hperl Many thanks! 🙌

harnash pushed a commit to Wikia/ory-hydra that referenced this issue Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants