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

[Issue]Segfault right after OpenID succesful authentication with response_type=code when passing nonce parameter, but invalid request without nonce (nonce+required) #197

Open
ghost opened this issue Apr 20, 2022 · 15 comments

Comments

@ghost
Copy link

ghost commented Apr 20, 2022


name: Glewlwyd bug report
about: Create a report to help improve Glewlwyd
title: "[Issue]"
labels: ''
assignees: ''


Describe the issue
Segfault right after OpenID succesful authentication with response_type=code when passing nonce parameter, but invalid request without nonce (nonce+required)

To Reproduce
Configure a client with all the auth types added and a client secret
Configure the openid plugin accordingly with PEM keys

Expected behavior
The auth response should contain the authorization code since the response_type=code and according to the rfc the nonce should be optional for this grant type instead it is required and when passed, unfortunately Glewlwyd goes down due to a segfault (I am not sure in which response header the auth code would go though. Where should I expect it?).

Screenshots
If applicable, add screenshots to help explain your problem.

System (please complete the following information):

  • OS/Environment Ubuntu 20.04
  • Browser used [e.g. Mozilla Firefox 69, Chrome 77, lynx 2.9]
  • Glewlwyd Version 2.6.2
  • Source installation Docker image (Docker hub)

Additional context
I can ptovide more info if required

@babelouest
Copy link
Owner

Hello @timmotw ,

I need more information because I can't reproduce your issue. The nonce is required if you add the openid in the scope parameter, but if you omit the nonce parameter in the auth request, glewlwyd redirects to the redirect_uri with the error error=invalid_request&error_description=nonce+required, and there's no segfault.

Can you provide more information on your client configuration, and post the /auth url your client is using that causes the segfault, something like:

https://glewlwyd.tld/api/oidc/auth?redirect_uri=http://client.com/callback&response_type=code&client_id=the_client_id&state=theState&scope=openid

@ghost
Copy link
Author

ghost commented Apr 21, 2022

Yes here it is:

http://localhost:4593/api/timo/auth?nonce=nonce1234&response_type=code&client_id=timoui&client_secret=45678908-c0ee-11ec-9b30-cba422e192d1&scope=openid%20timo_a%20timo_b&g_admin&redirect_uri=http://localhost:4200

image

@babelouest
Copy link
Owner

I still can't reproduce your segfault with your parameters.

Nevertheless, you must not add the client_secret in the url, and I strongly recommend not to use the scopes g_profile and g_admin in client apps, instead you should use your own scopes to detach Glewlwyd and the resource you want to access via access tokens.

@ghost
Copy link
Author

ghost commented Apr 21, 2022

@babelouest I have it in mind about the g_admin, g_profile but thank you anyways; I have a lot of things to discover and learn. I will try to have a crash dump and attach it here. Concerning the client_secret, I will use the POST method in production but I am still testing the flows through the browser and since GET is supported it makes things easier for me.

Is there a way to make the Scope Grant full for a specific client app? Meaning no need for the user to select the scopes before Continuing to the client app and even continuing automatically? Also is it possible to use a wildcard as the scope value representing all scopes?

@babelouest
Copy link
Owner

The /auth endpoint doesn't authenticate clients via their client secret, wether in basic auth or post method. If you want to authenticate a client during an auth request you can use:

But either way, in your case it shouldn't be necessary, the client authentication is done in the /token request. The /auth request authentication is provided by the redirect_uri which is a parameter linked to the client.

If by make the Scope Grant full for a specific client app or use a wildcard as the scope value, you mean adding automatically all scopes available in the glewlwyd's server scope list, the short answer is no. It could possibly lead to privileges escalation or bigger issues. The correct way is to explicitly set the scopes you need, and only those scopes.

Also, I suggest you send me the dump file to my e-mail address instead of posting it here.

And finally, since you use the docker image, I'd like you to retry with the docker-source builder instance with the last commit, to see if you can reproduce the segfault with the last source.

To build the docker-source, you must do the following:

$ git clone https://github.com/babelouest/glewlwyd.git
$ cd glewlwyd
$ # build the docker-source image
$ make docker
$ # run the docker-source image
$ docker run --rm -it -p 4593:4593 -v /path/to/your/config:/etc/glewlwyd babelouest/glewlwyd:src

@ghost
Copy link
Author

ghost commented Apr 26, 2022

OK I lost my data so I hope I will be able to re-produce.

Coming back to my question about the scopes. I think showing to the user the complete list of scopes to choose from before being forwarded to our default client app. will be something they will not like. We want the list of scopes to be dictated by us.

Can we use another login page or must we go through the Glewlwyd login page?

@babelouest
Copy link
Owner

I think showing to the user the complete list of scopes to choose from before being forwarded to our default client app. will be something they will not like.

That's something I'm not willing to change in Glewlwyd.

The scope list required by the client must be open and complete. So the user is aware what are the levels requested by the client and must willingly agree with it, to make sure that the user knows what data can be accessed and/or changed by the client on its behalf.

But it's a free software, so you're allowed to change the login page and the login API to fit your needs:

@ghost
Copy link
Author

ghost commented May 2, 2022

Yes this is something we already started changing so we are 100% in sync.

Thank you very much @babelouest.

@ghost
Copy link
Author

ghost commented May 3, 2022

Hello @timmotw ,

I need more information because I can't reproduce your issue. The nonce is required if you add the openid in the scope parameter, but if you omit the nonce parameter in the auth request, glewlwyd redirects to the redirect_uri with the error error=invalid_request&error_description=nonce+required, and there's no segfault.

Can you provide more information on your client configuration, and post the /auth url your client is using that causes the segfault, something like:

https://glewlwyd.tld/api/oidc/auth?redirect_uri=http://client.com/callback&response_type=code&client_id=the_client_id&state=theState&scope=openid

@babelouest does a client really require the oidc scope in order to interact with the OID plugin or should I remove it? If I can avoid the nonce parameter I will. For example the following option shouldn't allow requests without nonce? It is a valid OAuth2 request missing the nonce which is an OIDC parameter:

image

@babelouest
Copy link
Owner

Yes, if you uncheck this option, then you can send /auth request without nonce value if you don't use the openid scope

@ghost
Copy link
Author

ghost commented May 3, 2022

@babelouest It complains though that the openid scope is not in the scope request parameter

@babelouest
Copy link
Owner

I'll run some tests to check what's wrong with that

@ghost
Copy link
Author

ghost commented May 3, 2022

@babelouest I ended up having it checked and removed oidc scope and it's not complaining for nonce

@babelouest
Copy link
Owner

My bad, that's what I meant in the first place...

@ghost
Copy link
Author

ghost commented May 3, 2022

My bad, that's what I meant in the first place...

I was a bit skeptical to argue 😄

Thank you man! Your help is really important!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant