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

X509 client certificate authentication enforces client_id without checking on client authentication method #1635

Closed
martinvisser opened this issue Jun 3, 2024 · 2 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@martinvisser
Copy link

We recently upgraded to Spring Boot 3.3 and Spring Authorization Server 1.3.0. We do an OAuth2 flow with the following configuration:

spring:
  security:
    oauth2:
      client:
        registration:
          local:
            authorization-grant-type: authorization_code
            client-id: client
            client-secret: secret
            redirect-uri: "{baseUrl}/{action}/oauth2/code/{registrationId}"
            scope: "openid,profile"
        provider:
          local:
            issuer-uri: http://localhost:8081
            user-name-attribute: sub

We have not set any client authentication method on this side, but the default is client_secret_basic, which we also use in our authorization server:

@Bean
fun registeredClientRepository(): RegisteredClientRepository {
    val registeredClient =
        RegisteredClient.withId(UUID.randomUUID().toString())
            .clientId("local")
            .clientSecret("{noop}secret")
            .clientAuthenticationMethod(CLIENT_SECRET_BASIC)
            .authorizationGrantType(AUTHORIZATION_CODE)
            .redirectUri("http://localhost:8080/login/oauth2/code/aad")
            .scope(OPENID)
            .scope(PROFILE)
            .clientSettings(ClientSettings.builder().requireAuthorizationConsent(false).build())
            .build()
    return InMemoryRegisteredClientRepository(registeredClient)
}

Locally this works all fine, because it runs on http. However, when we deploy our applications on Cloud Foundry, we connect via https. This then creates/adds a x509 certificate to the request coming in on the authorization server, which then triggers X509ClientCertificateAuthenticationConverter. This converter looks for the client_id, but that was never added by OAuth2AuthorizationCodeGrantRequestEntityConverter and thus throws an exception:

// client_id (REQUIRED)
String clientId = parameters.getFirst(OAuth2ParameterNames.CLIENT_ID);
if (!StringUtils.hasText(clientId) || parameters.get(OAuth2ParameterNames.CLIENT_ID).size() != 1) {
    throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_REQUEST);
}
if (!ClientAuthenticationMethod.CLIENT_SECRET_BASIC
        .equals(clientRegistration.getClientAuthenticationMethod())) {
    parameters.add(OAuth2ParameterNames.CLIENT_ID, clientRegistration.getClientId());
}

Maybe our set-up is wrong and we should not use client_secret_basic. Hoever, we also tried using client_secret_post, which gave a different error message, because X509ClientCertificateAuthenticationConverter changes the authentication method to tls_client_auth:

ClientAuthenticationMethod clientAuthenticationMethod = clientCertificateChain.length == 1
        ? ClientAuthenticationMethod.SELF_SIGNED_TLS_CLIENT_AUTH : ClientAuthenticationMethod.TLS_CLIENT_AUTH;

It seems like you cannot use another authentication method once you have a certificate and X509ClientCertificateAuthenticationConverter triggers.

We also found a somewhat nasty workaround where we remove the request attribute jakarta.servlet.request.X509Certificate via a filter, this just blocks the inner workings of X509ClientCertificateAuthenticationConverter and won't change the client authentication method.
But of course, we rather don't do that.
I would expect that if I set a client authentication method this would be used, not overwritten by the converter just because there is a (unrelated) certificate passed along.

(Side note, I asked on StackOverflow too, but got no interaction going there.)

@martinvisser martinvisser added the type: bug A general bug label Jun 3, 2024
@jgrandja jgrandja self-assigned this Jun 5, 2024
@jgrandja jgrandja added this to the 1.3.1 milestone Jun 5, 2024
@jgrandja jgrandja changed the title Spring Auth Server: x509 client certificate authentication enforces client_id without checking on client authentication method X509 client certificate authentication enforces client_id without checking on client authentication method Jun 5, 2024
@jgrandja jgrandja moved this to Prioritized in Spring Security Team Jun 5, 2024
@jgrandja
Copy link
Collaborator

jgrandja commented Jun 5, 2024

@martinvisser Thank you for the extra detail. I've confirmed this is a bug and I'll have a fix for you shortly.

@jgrandja jgrandja moved this from Prioritized to In Progress in Spring Security Team Jun 11, 2024
@jgrandja jgrandja moved this from In Progress to Done in Spring Security Team Jun 12, 2024
@jgrandja
Copy link
Collaborator

@martinvisser I just pushed the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
Status: Done
Development

No branches or pull requests

2 participants