Make netifaces dependency optional

[skeggse]

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 by netifaces 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?

[JordanMilne]

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 ImportErroring, 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.

[skeggse]

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 not raise ImportErroring, while keeping the install_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 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.

It is on Linux, yeah - I'd be quite happy with this change as well!

[JordanMilne]

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 😛. 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.

[JordanMilne]

Does 5aafc5f 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.

[skeggse]

Ah yes, that's fantastic! Thanks, @JordanMilne :)

[JordanMilne]

Great! I'll publish a release containing it this week.

[skeggse]

How are we looking for a new release? Anything I can do to help?

[skeggse]

Can we reopen this issue, since this hasn't technically been fixed until it's been shipped?