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 Reactive Support #5038

Closed
rwinch opened this issue Feb 23, 2018 · 10 comments
Closed

X509 Reactive Support #5038

rwinch opened this issue Feb 23, 2018 · 10 comments
Assignees
Labels
in: web An issue in web modules (web, webmvc) status: duplicate A duplicate of another issue

Comments

@rwinch
Copy link
Member

rwinch commented Feb 23, 2018

Summary

Now that Spring Framework provides X509 information, we should provide authentication mechanism using it. See https://jira.spring.io/browse/SPR-15964

@rwinch rwinch added in: web An issue in web modules (web, webmvc) Reactive labels Feb 23, 2018
@rwinch rwinch added this to the 5.1.0.M1 milestone Feb 23, 2018
@rwinch rwinch modified the milestones: 5.1.0.M1, 5.1.0.M2 May 11, 2018
@rwinch rwinch modified the milestones: 5.1.0.M2, 5.1.0.RC1 Jul 26, 2018
@alek-sys
Copy link
Contributor

alek-sys commented Dec 7, 2018

Hey @rwinch, are there any updates on that issue? Is seems it was scheduler for 5.1 but I cannot find it there. WebFlux seems to be supporting that now https://jira.spring.io/browse/SPR-15964

@rwinch
Copy link
Member Author

rwinch commented Dec 7, 2018

@alek-sys Thanks for the nudge. We just haven't had time to add the support. Would you be interested in contributing this support?

@rwinch rwinch added the status: ideal-for-contribution An issue that we actively are looking for someone to help us with label Dec 7, 2018
@alek-sys
Copy link
Contributor

alek-sys commented Dec 8, 2018

I'm happy to! If there is any guidance you'd like to provide it is much appreciated. Should it be just an implementation of ServerAuthenticationConverter or do you see that as some sort of preauth mechanism similar to original X509AuthenticationFilter?

@rwinch
Copy link
Member Author

rwinch commented Dec 11, 2018

Thanks! I think that it could be done by creating a ServerAuthenticationConverter (as you suggested) plus a ReactiveAuthenticationManager and injecting those into the existing AuthenticationWebFilter.

@rwinch rwinch removed the status: ideal-for-contribution An issue that we actively are looking for someone to help us with label Jan 16, 2019
@rwinch rwinch self-assigned this Jan 16, 2019
@rwinch rwinch added this to the 5.2.0.M1 milestone Jan 16, 2019
@rwinch rwinch closed this as completed Jan 16, 2019
@rwinch rwinch removed this from the 5.2.0.M1 milestone Jan 16, 2019
@rwinch rwinch reopened this Jan 16, 2019
alek-sys added a commit to alek-sys/spring-security that referenced this issue Apr 26, 2019
alek-sys added a commit to alek-sys/spring-security that referenced this issue Apr 26, 2019
alek-sys added a commit to alek-sys/spring-security that referenced this issue Apr 26, 2019
rwinch pushed a commit that referenced this issue Apr 26, 2019
rwinch pushed a commit that referenced this issue Apr 26, 2019
@rwinch rwinch removed the Reactive label May 6, 2019
@rwinch rwinch removed their assignment Jul 29, 2019
@samhaque
Copy link

samhaque commented Apr 29, 2020

@rwinch I followed the documentation for x509 auth for reactive applications(my app is a spring cloud gateway application) as mentioned here:
https://docs.spring.io/spring-security/site/docs/current/reference/html5/#reactive-x509

However I noticed some weird behaviour, where even after creating a chain like this:

 http
        .x509(x509 -> x509
            .principalExtractor(principalExtractor)
            .authenticationManager(authenticationManager)
        )
        .authorizeExchange(exchanges -> exchanges
            .anyExchange().authenticated()
        );

and setting the authenticationManager to accept certs with the common name I want:

    SubjectDnX509PrincipalExtractor principalExtractor =
            new SubjectDnX509PrincipalExtractor();

    ReactiveAuthenticationManager authenticationManager = authentication -> {
        authentication.setAuthenticated("MY_TRUSTED_CN".equals(authentication.getName()));
        return Mono.just(authentication);
    };

it still defaults to http basic auth. Also since there is no debug logging in spring security for reactive apps( #5758), it is especially hard to debug inside PCF as the java buildpack takes the client cert from the go routers as a header and injects it into the keystore, and Spring Security just logs the GET/POST event but does not log any logger.debug lines even though I set the log level for spring security in my application.yml to DEBUG.

Let me know if you want this as a separate issue, I am pretty sure I am doing something wrong here as the implementation does make sense.

@rwinch
Copy link
Member Author

rwinch commented Apr 29, 2020

@samhaque Please create a separate issue and provide a complete sample/directions to reproduce.

@alek-sys
Copy link
Contributor

alek-sys commented Apr 30, 2020

Hey @samhaque, keep in mind mTLS via XFCC header on PCF is tricky. There are two important things to consider:

  • If you have HAproxy in your deployment, XFCC header gets stripped out from the request so you won't be able to use mTLS security
  • For reactive stack, client certificate is not correctly mapped to request properties, there is an open PR to JBP Client Certificate Mapper

@samhaque
Copy link

Hey @samhaque, keep in mind mTLS via XFCC header on PCF is tricky. There are two important things to consider:

  • If you have HAproxy in your deployment, XFCC header gets stripped out from the request so you won't be able to use mTLS security

  • For reactive stack, client certificate is not correctly mapped to request properties, there is an open PR to JBP Client Certificate Mapper

We have a isolated segment for mTLS, so it's not a HAproxy issue.

Is it possible to review that PR so the certificate mapping issue can be fixed in the next release?

@alek-sys
Copy link
Contributor

alek-sys commented Apr 30, 2020

Is it possible to review that PR so the certificate mapping issue can be fixed in the next release?

I'm not sure. Feel free to comment on the PR to show your interest, but even when it is merged it'll take some time to update and release JBP. For now you can just bring this filter to your codebase, unitl JBP is updated.

@rwinch
Copy link
Member Author

rwinch commented May 1, 2020

Closing this as a duplicate of the merged PR gh-6336

@rwinch rwinch closed this as completed May 1, 2020
@rwinch rwinch self-assigned this May 1, 2020
@rwinch rwinch added the status: duplicate A duplicate of another issue label May 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

3 participants