Skip to content
This repository has been archived by the owner on Apr 16, 2020. It is now read-only.

Pass TLS connection context into Request #67

Open
aep opened this issue Jun 13, 2018 · 14 comments
Open

Pass TLS connection context into Request #67

aep opened this issue Jun 13, 2018 · 14 comments

Comments

@aep
Copy link

aep commented Jun 13, 2018

currently in the connection handler we just call tower_h2::Server::serve(socket)
it would be useful if there was a way to pass metadata to that call that is later available in Request
specifically i'd like to give it the client certificate

@aep
Copy link
Author

aep commented Jun 19, 2018

any consensus on how that should be implemented?

i would prefer a service wouldn't be Cloned but instead reconstructed on every incoming connection with the connection passed as argument.

impl server::Greeter<C> for Greet<C> {
    pub fn new(c: C) -> Self

//..
let c: C = whateverYouWant();
let serve = h2.serve(sock, c);

@carllerche
Copy link
Member

In general, this is what the extensions field on http::Request is for.

While manual for now, you should be able to do what you want by calling serve_modified instead: https://tower-rs.github.io/tower-h2/tower_h2/server/struct.Server.html#method.serve_modified. You then stick the certificate into the request's extension slot: https://docs.rs/http/0.1.6/http/request/struct.Request.html#method.extensions_mut

Does this work?

@olix0r
Copy link
Contributor

olix0r commented Jun 20, 2018

@carllerche
Copy link
Member

I'm going to close this, but feel free to post additional questions or let me know if you think there are remaining actionable items (and I can open it again).

@aep
Copy link
Author

aep commented Jun 20, 2018

this sounds great, but tower_grpc::Request does not contain the http::Request so there's no way to get the extensions out

@carllerche
Copy link
Member

I think it would be fine to include the extensions as part of the grpc request type.

@olix0r @seanmonstar Thoughts?

@carllerche carllerche reopened this Jun 20, 2018
@seanmonstar
Copy link
Contributor

Yep, seems simple enough to stick them inside tower_grpc::Request!

@olix0r
Copy link
Contributor

olix0r commented Jun 20, 2018

So, we're suggesting that grpc::Request::from_http steals http::Request's extension map? SGTM.

@carllerche
Copy link
Member

@aep is this something you want to contribute?

@aep
Copy link
Author

aep commented Jun 21, 2018

can do later :)

@olix0r
Copy link
Contributor

olix0r commented Jul 7, 2018

possibly related: hyperium/hyper#1594

@carllerche
Copy link
Member

Relates to: tower-rs/tower#108

@ian-p-cooke
Copy link

@carllerche @aep is this something being worked on? It's not clear to me if the recommended implementation is supposed to get the http::Request's extensions into the tower_grpc::Request or somehow wraps up the MakeService functionality in hyper.

I was using grpc-rs for some prototyping but given the recent blog post about a possible switch to tower-grpc I'm interested in experimenting with tower. I too need the TLS client certificate information available to my grpc services. An older implementation using Cap'n Proto is here. I'm still wrapping my head around tower-grpc/h2's design but I believe I could contribute the changes necessary to support my use case; I just need some guidance.

@ian-p-cooke
Copy link

I implemented an example of what I wanted to do but I didn't need to modify tower-grpc to do it. However, there's a significant difference in how I create a Server per-connection instead of using one server for all connections. Maybe that's what I'm supposed to be doing here though? It does make sense to me to put the client session information into the Server instance instead of passing it into each request as was suggested. I can clean up my TLS example if you think it's a good starting point for showing how to get TLS session information into the GRPC request handing.

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

No branches or pull requests

5 participants