-
Notifications
You must be signed in to change notification settings - Fork 5.7k
fix(permissions): check before and after dns resolution #30085
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
base: main
Are you sure you want to change the base?
Conversation
@@ -3345,6 +3401,20 @@ impl PermissionsContainer { | |||
Ok(()) | |||
} | |||
|
|||
pub fn check_net_resolved_addr_is_not_denied( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
workaround the fact that if you do --allow-net=localhost
, after dns resolution we'd try to look for 127.0.0.1
and deny.
maybe a better fix would be to expand --allow-net=localhost
to localhost
and 127.0.0.1
on startup. we'd have to think about how to avoid hurting startup perf too badly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about it more, I don't think expanding it on startup is safe, because for instance what if example.com
resolves to 123.123.0.1
, and then that IP gets reallocated to some other (malicious) service. Then, a fetch request of that IP would still be allowed through even though it doesn't belong to example.com
anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes a lot of sense that if an address is in allow, only a matching --deny-net=... should take precedence, and another --allow-net shouldn't be necessary:
--deny-net=127.0.0.1 --allow-net=example.com
Would imply:
example.com -> 1.1.1.1 (pass)
example.com -> 127.0.0.1 (fail)
# This rbndr.us randomly swaps between 127.0.0.1 and 192.168.0.1
--deny-net=192.168.0.1/24 --allow-net=7f000001.c0a80001.rbndr.us
Would imply:
7f000001.c0a80001.rbndr.us -> 127.0.0.1 (pass)
7f000001.c0a80001.rbndr.us -> 192.168.0.1 (fail)
--allow-net=example.com
Would imply:
example.com -> [any IP] (pass)
3a36108
to
28b7f3f
Compare
TODO: