security audit fixes

[ainghazal]

in progress: address security fixes found by 7asecurity.

[hellais]

MIV-01-006

It is my understanding that a fix for this has not been implemented. Yet according to the audit report it seems like this is a flaw in the protocol design of OpenVPN and the issue is present in the reference implementation as well. Apparently the issue is not so severe in there, because a retry mechanism is in place.

While the retry mechanism might be effective as a protection against bad network conditions, I don't think it's really effective as a protection against a motivated attacker (such as a state actor or an ISP). In that case since they control the traffic flows, they can just pre all traffic that looks like OpenVPN (or that doesn't look like something they would recognise) with a bad payload leading to the connection never establishing.

In light of the fact that our primary goal for this client is measurement, we actually care to record these kinds of connectivity issues and while we might want to add retries eventually to understand the persistence of the attacker and rule out potential transient network conditions, I don't think it's necessarily critical to solve this in order to land this.

[hellais]

MIV-01-009

This seems very hard to do. I think we ought to document this as future work.

[ainghazal]

thanks for the review @hellais

re. MIV-01-009, the only outstanding pattern is the first of the three reported: https://github.com/ooni/minivpn/issues/34 - P_ACK2 is addressed in this PR, and the multiple reset packets was a side-effect of not reusing properly the same underlying vpn connection for the long-lived socks5 proxy (which, incidentally, can be used for an implementation of a ooni tunnel).

I am going to go ahead and fix the issue with blockSize and merge this series, so that we can rebase the remaining PRs.

[ainghazal]

MIV-01-006

It is my understanding that a fix for this has not been implemented. Yet according to the audit report it seems like this is a flaw in the protocol design of OpenVPN and the issue is present in the reference implementation as well. Apparently the issue is not so severe in there, because a retry mechanism is in place.

While the retry mechanism might be effective as a protection against bad network conditions, I don't think it's really effective as a protection against a motivated attacker (such as a state actor or an ISP). In that case since they control the traffic flows, they can just pre all traffic that looks like OpenVPN (or that doesn't look like something they would recognise) with a bad payload leading to the connection never establishing.

In light of the fact that our primary goal for this client is measurement, we actually care to record these kinds of connectivity issues and while we might want to add retries eventually to understand the persistence of the attacker and rule out potential transient network conditions, I don't think it's necessarily critical to solve this in order to land this.

yes, I think this is a wontfix for the moment. however, since I wrote this patch series I've revisited the reliability layer implementation and I think it's quite close to be in an usable state (I keep debugging it a bit, and is missing proper tests). In my priority lists for further work in minivpn, I think implementing proper DoS protection (--tls-auth and --tls-crypt) is more urgent. I documented a bit how the DoS currently compares between this and the ref implementation in here - it's interesting that, if the injection rate is kept low, the handshake manages to complete successfully.

on the other hand, we want to be able to measure DoS happening - I think the trick is to be as sensitive as openvpn itself.