On Mon, Jun 17, 2019 at 5:56 PM Jonathan Dowland <j...@debian.org> wrote: > > On Mon, Jun 17, 2019 at 04:22:30AM +0800, Shengjing Zhu wrote: > >Control: severity -1 normal > > > >On Tue, Jun 11, 2019 at 6:09 PM Shengjing Zhu <z...@debian.org> wrote: > >> I checked more carefully on https://github.com/moby/moby/pull/28257 > >> and https://github.com/moby/moby/issues/14041 > >> Then I concluded that docker does nothing wrong in this case. > >> > >[...] > > > >With the reason I explained last week, I would downgrade this issue. > > Sorry for not replying sooner, I had a very busy week. > > Your code review and conclusions from it are reasonable. I read them > last week (although I did not have time to reply) and my thoughts were: > is that the code in the current Debian package? >
Yes, you can check it here https://sources.debian.org/src/docker.io/18.09.1+dfsg1-7/libnetwork/drivers/bridge/setup_ip_forwarding.go/#L31 The code is only called if ip_forward is not enabled. > I ask because the reason I filed this in the first place is because I > hit it on my main Debian system. My kvm/libvirt-based VMs lost their > networking due to docker changing the forward iptables policy. I had > not manually modified the ip_forward /proc setting, so either > > 1) this was not necessary for libvirt/kvm's networking to function, > and so is not a perfect test as to whether changing the forwarding > iptables chain will break something > > or > > 2) libvirt/kvm's networking set the ip_forward proc setting it required, > and the packaged docker does not correctly check the proc value before > making the change, despite the logic in the code linked above (or > perhaps that is not the code corresponding to the docker version that > is presently packaged) > > (Or I suppose 3), a race is possible between the /proc check and the iptables > change, but I don't think that's likely what has happened in my case.) > > So IMHO the severity drop is not appropriate because it's predicated on a > theoretical reasoning of the situation rather than a practical one, i.e., I > actually hit this on a real machine in real circumstances. But I will attempt > to reproduce it a second time on my real machine before challenging the > severity again. Please do think more about this issue. And understand why docker does this for the security reason. > > To respond to some of your specific points: > > > So as for your VM scenario, why didn't you set ip_forward manually? > > Because I did not need to. Either it was not necessary for the specific > networking setup for my libvirt/kvm VMs, or libvirt/kvm set it explicitly > itself. I do not manually need to tweak stuff in /proc. As part of > reproducing this I will check the specific network setup that the libvirt > VMs are using, but it's the package/debian default. > Because libvirt enables ip_forward itself. https://sources.debian.org/src/libvirt/5.0.0-3/src/network/bridge_driver.c/#L2207 And I would argue this is a security issue too, if libvirt enables ip_forward and does nothing else. > > How docker know it's not a vulnerability if it didn't set FORWARD > > chain to DROP when it enables ip_forward. > > They could add a "-j DROP" rule that was scoped specifically to the docker > subnet, after their other (-j ACCEPT) rules. That's just one way that this > could be done less disruptively. No, because they enabled ip_forward setting. -- Shengjing Zhu
