Closed skeggse closed 2 years ago
Thanks for sending this in!
For platforms where using Python modules implemented in C is particularly difficult [...]
Are you running into issues when dealing with an architecture where netifaces
doesn't provide a binary wheel or something similar?
I'm curious why this functionality is enabled by default, given that (I assume) most hosts don't have direct public IP addresses.
That's generally the case, but advocate
is also used in several pieces of software that people tend to toss up on some totally ad-hoc EC2 instance with a public IPv4 address. They may have other services bound to 0.0.0.0
within the instance and they're essentially depending on security group rules or ufw
to block external connection attempts. The interface address blocking behaviour was added to advocate in reaction to me seeing such systems while red-teaming.
I don't know what environments people are going to use advocate in, or how embedders are going to expose configuration of advocate's filter behaviour, so it seemed prudent to go with stricter defaults to accommodate the slightly-less-than-sane environments advocate may need to operate in.
Additionally, I was concerned that if I made what was basically a "my network is weird, so please do some otherwise pointless checks to make me secure" option, nobody who should have that option on would turn it on.
Would you be open to a PR that disables this functionality if netifaces is not available?
I'd like to keep this functionality by default when someone does pip install advocate
for the reasons above, which would require keeping netifaces
in the package's install_requires
.
If you were just thinking along the lines of making those paths conditional on the import netifaces
not raise ImportError
ing, while keeping the install_requires
the same, I'd be fine making that change. You'd then have to use pip install --no-deps
to install advocate without netifaces, but it'd work.
Alternatively, if you're mainly having problems on Linux, I could look at making netifaces
only a hard requirement when platform_system != "Linux"
, and use socket.if_nameindex()
+ a fnclt.ioctl()
call with SIOCGIFADDR
to get the interface addr there. It looks simple enough to do that in pure Python 3.x now.
Are you running into issues when dealing with an architecture where
netifaces
doesn't provide a binary wheel or something similar?
In this case it's just build chain complexity in particular areas that make it more difficult than it ought to be.
That's generally the case, but
advocate
is also used in several pieces of software that people tend to toss up on some totally ad-hoc EC2 instance with a public IPv4 address.
Oh, interesting. Are there systems/platforms that configure the local interfaces with the public IP addresses? The EC2 instances I've looked at that have public IPs directly assigned to their ENIs don't report the IP address with ip addr
, and attempts to communicate with the public IP address seems to get the standard security group filtering treatment unlike attempts to communicate with private IPs on the same interfaces.
I don't know what environments people are going to use advocate in, or how embedders are going to expose configuration of advocate's filter behaviour, so it seemed prudent to go with stricter defaults to accommodate the slightly-less-than-sane environments advocate may need to operate in.
Yeah, this totally makes sense to me.
If you were just thinking along the lines of making those paths conditional on the
import netifaces
notraise ImportError
ing, while keeping theinstall_requires
the same, I'd be fine making that change.
Yeah this would be an improvement for some of my use-cases, and was roughly what I was thinking.
Alternatively, if you're mainly having problems on Linux, I could look at making
netifaces
only a hard requirement whenplatform_system != "Linux"
, and usesocket.if_nameindex()
+ afnclt.ioctl()
call withSIOCGIFADDR
to get the interface addr there. It looks simple enough to do that in pure Python 3.x now.
It is on Linux, yeah - I'd be quite happy with this change as well!
Are there systems/platforms that configure the local interfaces with the public IP addresses? The EC2 instances I've looked at that have public IPs directly assigned to their ENIs don't report the IP address with ip addr [...]
I checked, and you're right, of course. I probably should have used an example I'd actually tested :stuck_out_tongue:. It's at least the case on Digital Ocean's Ubuntu droplets, it's not the case on EC2 VPC instances.
Yeah this would be an improvement for some of my use-cases
Alright, in that case I'll do one of those two soon. I'm in the middle of dropping Python 2.x support which should get rid of all of the direct deps other than requests and netifaces. If working with the ioctl call won't be too hairy I'll do that, otherwise I'll just add the disable on ImportError logic.
Does https://github.com/JordanMilne/Advocate/commit/5aafc5f5a175516e68caa3d21a928281e190a56e work for what you were thinking? It defers import netifaces
errors until Advocate actually needs to fetch interface addrs, so if you install Advocate without dependencies and specify an AddrValidator
with autodetect_local_addresses=False
everything should work without netifaces
.
Ah yes, that's fantastic! Thanks, @JordanMilne :)
Great! I'll publish a release containing it this week.
How are we looking for a new release? Anything I can do to help?
Can we reopen this issue, since this hasn't technically been fixed until it's been shipped?
For platforms where using Python modules implemented in C is particularly difficult, it would be useful to be able to use Advocate without those features included.
In particular, it looks like
netifaces
exists solely to support detection of local addresses; in the vast majority of cases, I would expect all addresses reported bynetifaces
to be private addresses, and thus not pass the filter anyway. I'm not sure that it's worth a breaking change, but I'm curious why this functionality is enabled by default, given that (I assume) most hosts don't have direct public IP addresses.Would you be open to a PR that disables this functionality if
netifaces
is not available?