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

http.send unix domain sockets - URL format #3661

Closed
kirk-patton opened this issue Jul 19, 2021 · 12 comments
Closed

http.send unix domain sockets - URL format #3661

kirk-patton opened this issue Jul 19, 2021 · 12 comments

Comments

@kirk-patton
Copy link
Contributor

I am not sure if OPA supports unix domain sockets when using the http.send method. If it does, then I am interested in the correct syntax to use when specifying the socket as part of the request.

Expected Behavior

http.send(unix://path/to/socket/path/to/url/endpoint) OR something similar

Actual Behavior

does not work.

Steps to Reproduce the Problem

socatSocket = response {
# In a terminal run
# (while true; do date; sleep 5; done) | socat UNIX-LISTEN:/tmp/http.sock -
# NOTE: works with _
# curl --unix-socket /tmp/http.sock http://localhost/
request := {
"url": "unix:///tmp/http.socket",
"method": "GET",
}
response := http.send(request)
trace(sprintf("DEBUG: **** %v",[response]))

}

Additional Info

@tsandall
Copy link
Member

tsandall commented Jul 19, 2021

Thanks @kirk-patton. The http.send built-in function does not currently support UNIX domain sockets however we could extend it relatively easily, I think.

This is where the HTTP client used for http.send calls is created: https://github.com/open-policy-agent/opa/blob/main/topdown/http.go#L372. We can update this function to parse the request URL and look for the unix:// scheme. If the UNIX domain socket scheme is used we can update the transport DialContext like in this example. I'd like to see an integration test in the http_test.go file that stands up a simple UNIX domain socket server in Go and responds to the request. We'll also need to update the docs for the http.send built-in function here to mention what URL schemes are supported.

@ashutosh-narkar did I miss anything here? I'm not sure if there's some special consideration given the caching stuff. etc.

@tsandall tsandall added this to TODO (Things That Should Be Done) in Open Policy Agent via automation Jul 19, 2021
@anderseknert
Copy link
Member

This seems like something that would need to be enabled with a config option, no? I don’t think OPA admins would expect policy authors to have that capability by default.

@tsandall
Copy link
Member

@anderseknert I'm not sure I follow. You're suggesting that admins are okay with users being able to execute arbitrary HTTP requests via TCP but not via UNIX domain socket? Can you elaborate a bit?

@anderseknert
Copy link
Member

anderseknert commented Jul 19, 2021

Yes, I think the function name implies as much. I would not expect policy authors to be able to send arbitrary IPC requests to the (however poorly configured) Docker daemon (or whatever else is) running on the same host on a Unix socket without explicit permission from the OPA admin. I think those are often poorly configured precisely because the expectation is they’re not exposed to untrusted processes.

@tsandall
Copy link
Member

That makes sense. I suggest we solve that in the same context as other security-related improvements to http.send. E.g., some folks have talked about adding a whitelist so you can restrict what hosts http.send could call out to. I think that your fears around unix domain sockets apply to localhost sockets as well.

@anderseknert
Copy link
Member

Sounds good! As for localhost sockets, yeah - I think there’s just more of an awareness what’s running on those among admins in general, but I agree, it would be good to be able to disallow localhost requests regardless of protocol.

@ashutosh-narkar
Copy link
Member

@ashutosh-narkar did I miss anything here? I'm not sure if there's some special consideration given the caching stuff. etc.

The implementation described by Torin in this comment sounds good to me. Regarding caching, the server would need to send a 200 or 304 response while evaluating whether the response is stale or not. That I think is the only contract http.send has with the server.

We could extend the http.send request parameters with a new security field to capture any security-related checks we want to add. The first one could be an allow_list sub-field.

@tsandall
Copy link
Member

We could extend the http.send request parameters with a new security field to capture any security-related checks we want to add. The first one could be an allow_list sub-field.

Let's look into that separately. The allow list needs to be set by something other than the policy; I don't think it makes sense to allow the policy to be able to define the allow list.

@ashutosh-narkar
Copy link
Member

Let's look into that separately. The allow list needs to be set by something other than the policy; I don't think it makes sense to allow the policy to be able to define the allow list.

You're right. It's not appropriate to have it in the built-in. Maybe the OPA config would be better ?

@kirk-patton
Copy link
Contributor Author

I will take a run at implementing @tsandall this

@kirk-patton
Copy link
Contributor Author

@tsandall #3667 That is for the http.send section. My desired use case is k8s + side car. Ex url unix:localhost/end/point?socket=%2Ftmp%2Fhttp.sock". I have not yet looked at the server side or how to restrict the call.

@tsandall
Copy link
Member

thanks @kirk-patton, I'll take a look hopefully today. Don't worry about the changes to restrict the call. That requires some design work and is outside the scope of this issue.

Open Policy Agent automation moved this from TODO (Things That Should Be Done) to Done Aug 17, 2021
jgchn pushed a commit to jgchn/opa that referenced this issue Aug 20, 2021
Fixes open-policy-agent#3661.

Signed-off-by: Kirk Patton <kpatton@verizonmedia.com>
dolevf pushed a commit to dolevf/opa that referenced this issue Nov 4, 2021
Fixes open-policy-agent#3661.

Signed-off-by: Kirk Patton <kpatton@verizonmedia.com>
Signed-off-by: Dolev Farhi <farhi.dolev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

4 participants