StreisandEffect / discussions

30 stars 3 forks source link

How to Submit Large Pull Requests? #98

Closed Frichetten closed 6 years ago

Frichetten commented 6 years ago

I apologize in advance if this has been covered before. I tried to find other issues in this repo or in the Contributing file, but didn't see anything.

My question: Is there a standard for how large pull requests are approved or rejected? By large, I mean implementing a new feature for example.

I'd like to submit a pull request that includes Pi-Hole (info here). I've personally been running it for a while and have been very happy. I think it is relevant to the Streisand project for a couple reasons including reducing the amount of bandwidth required to load a webpage (by cutting out ads), protecting against malvertising, and reducing trackers.

To the maintainers, would this be something you'd accept? Or do you think it would add unneeded complexity? There was a previous Issue #41 which requested it so there is some interest.

Theoretically it shouldn't increase the attack surface of the box (the process would be on an internal interface) and it could possibly help with dns leaking.

Thoughts?

cpu commented 6 years ago

Hi @Frichetten - great question!

My question: Is there a standard for how large pull requests are approved or rejected? By large, I mean implementing a new feature for example.

We should definitely address this in CONTRIBUTING.md :-) Opened https://github.com/StreisandEffect/streisand/issues/1192 to not forget.

To the maintainers, would this be something you'd accept? Or do you think it would add unneeded complexity? There was a previous Issue #41 which requested it so there is some interest.

I'm open to the idea of trying to integrate server side ad blocking as an optional service. In general I'm in favour of trying to trim down what is enabled by default and don't want to add new default services.

For Pi-Hole in particular, I'm not sure its the best approach. I have a few concerns about adopting it (Note, these are based on a skim of the README and not in-depth analysis, I could be wrong in many ways :laughing:):

1) It doesn't look to be in the Ubuntu main or universe repos. There aren't any packages at all :-( They only offer a Git based method with an installer script. We do include some software in a similar state today but I'm actively looking to fix that. One reason this is a big downside is there isn't any way we could auto-update PiHole on Streisand instances if there were a security vulnerability.

2) It looks pretty heavy weight with a fair amount of complexity beyond blocking ads. The web interface for example is a big feature.

3) PiHole says its based on a few core technologies: dnsmasq, curl, lighttpd, php, and AdminLTE Dashboard. We already ship dnsmasq so I'm concerned it may interfere with our dnsmasq playbook/configuration. That's a very sensitive service that most of the Streisand stack relies on. RE: lighttpd/php - that's a whole second webserver, we already install nginx. I'm also very hesitant to pull in a new PHP dependency.

I think there are probably better ways to try and address server-side ad blocking that won't involve so many tradeoffs/downsides.

There are a few other issues open about trying to deploy unbound as a local recursive resolver. That has some nice privacy/security features and would allow a much simpler ad blocking approach using something like https://github.com/jodrell/unbound-block-hosts - Personally I think that might be a better road forward. I believe Algo uses a similar approach with a small script to add dnsmasq configuration to block ads.

@jlund @nopdotcom @alimakki Do you have thoughts on this?

nopdotcom commented 6 years ago

At this point, it would need to be modularized, I think. Depending on the software, this can be a lot easier or harder than it looks.

If you have a fixed amount of time to work on this, I think building a Debian/Ubuntu package out of the modular parts would help a lot of people; it would also make this easier to install in Streisand. If that's too painful, cramming it all into a Docker image might help some people. (We're not going to ship Docker any time soon, but Docker might be a good add-on strategy for people with complex packages.)

I don't think we have any dynamic web content right now.

The good news about this software is that it's not going to exposed to unauthenticated users (except evil DNS query results!) but it looks like it gets involved in DNS munging for the client side, and that's on the critical path for users of multiple services. We haven't pen-tested with authenticated users with evil intent. To be fair, we try pretty hard to do avoid doing stupid things.

I'm not the best reviewer for this: I personally refuse to run web-exposed PHP software on any machine I care about, so I'd be selecting it out in the module list.

Frichetten commented 6 years ago

@cpu Thanks for the feedback! You're right, as far as I know their simplest installation procedure revolves around piping a script from their website into Bash.

@nopdotcom Just out of professional curiosity, has Streisand ever been pen-tested by a professional that you know about? I would think a Streisand instance would have a relatively low attack surface, unless the web server or the VPN services listening get an RCE bug.

nopdotcom commented 6 years ago

I think we're currently looking for pros to do pen-testing. There are some leads, but while we're waiting, we've gotta get our heads straight about the threat models we care about. More importantly, defining the threat models we don't care about as much.

My opinion is that attacks by authenticated users on the host-only net is a considerably lower priority than attacks on unauthenticated Internet-facing services. Any non-trivial vulns from the Internet side are showstoppers. (The unattended-upgrades package has saved us (and all our users) several times so far when critical bugs were discovered upstream; the next time the cron job hit, the system just pulled signed packages from main or a PPA; bug resolved.)

Just for laughs, from a slightly old server:

$ sudo netstat -nlp -A inet | grep LISTEN
tcp        0      0 192.0.2.1:8530       0.0.0.0:*               LISTEN      1314/ss-server
tcp        0      0 127.0.0.1:8181          0.0.0.0:*               LISTEN      1788/nginx.conf
tcp        0      0 0.0.0.0:53              0.0.0.0:*               LISTEN      1497/dnsmasq
tcp        0      0 0.0.0.0:22              0.0.0.0:*               LISTEN      1347/sshd
tcp        0      0 127.0.0.1:8888          0.0.0.0:*               LISTEN      1685/tinyproxy
tcp        0      0 127.0.0.1:443           0.0.0.0:*               LISTEN      1788/nginx.conf
tcp        0      0 0.0.0.0:4443            0.0.0.0:*               LISTEN      1375/ocserv-main
tcp        0      0 192.0.2.1:443        0.0.0.0:*               LISTEN      1278/sslh
tcp        0      0 127.0.0.1:2812          0.0.0.0:*               LISTEN      1661/monit
tcp        0      0 0.0.0.0:636             0.0.0.0:*               LISTEN      1406/openvpn
tcp        0      0 0.0.0.0:993             0.0.0.0:*               LISTEN      1676/stunnel4
tcp6       0      0 :::53                   :::*                    LISTEN      1497/dnsmasq
tcp6       0      0 :::22                   :::*                    LISTEN      1347/sshd
tcp6       0      0 :::4443                 :::*                    LISTEN      1375/ocserv-main

ETA: part of why I'm ok with the host-only net is that there aren't many services running on it, and I think most are handed off from sslh.

Frichetten commented 6 years ago

@nopdotcom: I agree with your assessment. Internet facing services are a much greater risk. From an attackers perspective though, there likely isn't much to mess with. Like you mentioned, most of those services are being updated regularly which solves a lot of possible issues.

As @cpu mentioned though, there is some software that isn't in a PPA. Do you happen to know if any of that is for an external service (I.E one of the VPN options)?

cpu commented 6 years ago

It sounds like we have rough consensus that:

  1. optional server-side adblocking would be a great addition
  2. PiHole is not a good fit for accomplishing server-side adblocking for Streisand.

@nopdotcom Just out of professional curiosity, has Streisand ever been pen-tested by a professional that you know about?

It hasn't, but we're in the scheduling phase with the Open Tech Fund's Red Team Lab to have an audit performed.

I think the scope of this thread is ballooning so I'm going to close it. More discussion about threat modelling would be best in a separate issue for that purpose. I'll take a crack at https://github.com/StreisandEffect/streisand/issues/1192 this weekend to resolve future questions about contributing large PRs :-) Thanks all!

alimakki commented 6 years ago

@Frichetten

As @cpu mentioned though, there is some software that isn't in a PPA. Do you happen to know if any of that is for an external service (I.E one of the VPN options)?

ocserv (openconnect), Libreswan, and Shadowsocks are compiled from source and thus do not currently benefit from unattended upgrades.