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

Cleanup UAA Client Secret Setting and Ensure Authentication on Client Credentials Grant Flow #3232

Open
strehle opened this issue Jan 16, 2025 · 0 comments · May be fixed by #3233
Open

Cleanup UAA Client Secret Setting and Ensure Authentication on Client Credentials Grant Flow #3232

strehle opened this issue Jan 16, 2025 · 0 comments · May be fixed by #3233

Comments

@strehle
Copy link
Member

strehle commented Jan 16, 2025

The setup of clients in UAA can be done based on uaa.yml in a CF landscape, then there is no real validation if a client has no secret or an empty secret and you can do everything... also dangerous things.

Dangerous means: You setup a client with authorized grant type "client_credentials" and you create a client token with an empty secret, e.g. cf client could be upgraded via REST.
The normal case is, you create a client within UAA based on REST Endpoint
https://cloudfoundry.github.io/uaa/version/77.24.0/index.html#create-4

Here we have validation, done in class
ClientAdminEndpointsValidator

The validation means

The validation in the REST endpoint is a problem because it cannot check cover all parts, because with these calls

uaac client add uaa-client --name test --authorized_grant_types password --scope "openid test.read test.write" -s ""
uaac client update uaa-client --authorized_grant_types "client_credentials urn:ietf:params:oauth:grant-type:jwt-bearer"
uaac token client get uaa-client -t -s ""

You can setup a client with an empty secret and a JWT bearer grant with an empty secret.

JWT bearer with an empty secret (or without secret if allowpublic) is no issue - at least to me, because for JWT bearer you must bring a trusted assertion, so there is an authentication. Password grant is allowed with empty secret, and I dont see that much diferences between password grant and jwt bearer grant.

About client_credentials. This flow must not allow authentication without a secret or with an empty secret.

The issue should:

  • Start discussion about client authentication. To me it must be checked during the grant flows and not in REST . An authentication can be private_key_jwt and not only secret, but empty secret is an authentication we should not set on the same level as e.g. private_key_jwt
  • Track fixes about check of client authentication mechanism
strehle added a commit that referenced this issue Jan 16, 2025
Move the check for existing secrets to the
grant flow (runtime) and remove it from REST (configuration).

We support now private_key_jwt next to secret, so check all parts in REST is too less and will get complicated.
Fix a potential security issue because we must not create a token based on no authentication
@strehle strehle linked a pull request Jan 16, 2025 that will close this issue
@strehle strehle linked a pull request Jan 16, 2025 that will close this issue
strehle added a commit that referenced this issue Jan 16, 2025
Move the check for existing secrets to the
grant flow (runtime) and remove it from REST (configuration).

We support now private_key_jwt next to secret, so check all parts in REST is too less and will get complicated.
Fix a potential security issue because we must not create a token based on no authentication
strehle added a commit that referenced this issue Jan 16, 2025
Move the check for existing secrets to the
grant flow (runtime) and remove it from REST (configuration).

We support now private_key_jwt next to secret, so check all parts in REST is too less and will get complicated.
Fix a potential security issue because we must not create a token based on no authentication
strehle added a commit that referenced this issue Jan 16, 2025
Move the check for existing secrets to the
grant flow (runtime) and remove it from REST (configuration).

We support now private_key_jwt next to secret, so check all parts in REST is too less and will get complicated.
Fix a potential security issue because we must not create a token based on no authentication
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging a pull request may close this issue.

1 participant