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

MTLS client certificate propagation #2752

Closed
Verdent opened this issue Feb 10, 2021 · 4 comments · Fixed by #4185
Closed

MTLS client certificate propagation #2752

Verdent opened this issue Feb 10, 2021 · 4 comments · Fixed by #4185
Assignees
Labels
2.x Issues for 2.x version branch enhancement New feature or request OCI P3 security webserver
Milestone

Comments

@Verdent
Copy link
Member

Verdent commented Feb 10, 2021

When MTLS is performed it would be useful for users to have access to the used client certificate over the context.

@Verdent Verdent added enhancement New feature or request security webserver 2.x Issues for 2.x version branch labels Feb 10, 2021
@Verdent Verdent self-assigned this Feb 10, 2021
@Verdent Verdent changed the title Client certificate propagation MTLS client certificate propagation Feb 10, 2021
@m0mus m0mus added the P3 label Feb 11, 2021
@astromechza
Copy link
Contributor

I'm going to put up a PR for this as I'm facing just this issue.

@astromechza
Copy link
Contributor

astromechza commented May 8, 2022

@Verdent / @m0mus Looking at ways to do this I see 2 different approaches:

  1. Adding a Getter to the ServerRequest that returns an Optional<X509Certificate>
  2. OR, having the ForwardingHandler registering a context key on the request Scope
        // If the client x509 certificate is present on the channel, also add it to the context scope of the ongoing
        // request so that helidon handlers can inspect and react to this.
        Optional.ofNullable(ctx.channel().attr(CLIENT_CERTIFICATE).get())
                .ifPresent(cert -> requestScope.register(WebServerTls.CLIENT_X509_CERTIFICATE, cert));

The latter seems much cleaner, but I'm not sure what the precedent is for registering context items.

Any advice on this?

@trentjeff
Copy link
Member

@Verdent - fyi: #4185 - does this satisfy this issue?

@Verdent
Copy link
Member Author

Verdent commented May 12, 2022

@trentjeff from my point of view it does. @tomas-langer are you OK with not having it in headers?

@barchetta barchetta added this to the 2.5.1 milestone May 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x Issues for 2.x version branch enhancement New feature or request OCI P3 security webserver
Projects
Archived in project
6 participants