Fix issue with connection resetting every hour when using otp. #208
Conversation
…s being used to avoid connection resetting every hour. Edit docs to make clear that a more secure cipher needs to be selected to use with otp to avoid the connection being reset every 64 MB of data
|
Can you add a test to make sure Pending that it looks good to merge. Thanks! |
|
Big 👍 for this PR. Promoting the use of OTP seems the obvious step forward wrt client auth. Playing devils advocate: disabling |
|
@pieterlange I agree there is a security risk with setting reneg-sec to 0. This is to a degree mitigated by OpenVPN 2.3+ setting reneg-bytes to 64MB to protect against sweet32 attacks when an insecure cipher is used. My updates to otp documentation advise choosing a different cipher when using otp unless you want your connection reset every 64MB of data. In future auth-gen-token would work better but we'd have to either strand any vpn clients that use openvpn earlier than 2.4 or deal with legacy clients differently. @kylemanna writing tests now. |
…onnection will be ask for a otp every hour. Tests added to make sure it's there when otp is enabled
|
Tests look good, can you address the two comments from the review? Thanks! |
|
@kylemanna do you mean the comments by @pieterlange? If so as above: I agree there is a security risk with setting reneg-sec to 0. This is to a degree mitigated by OpenVPN 2.3+ setting reneg-bytes to 64MB to protect against sweet32 attacks when an insecure cipher is used. My updates to otp documentation advise choosing a different cipher when using otp unless you want your connection reset every 64MB of data. In future auth-gen-token would work better but we'd have to either strand any vpn clients that use openvpn earlier than 2.4 or deal with legacy clients differently. |
|
@kylemanna Has your code review been submitted or is it still pending? I can't see the comments and clicking the links doesn't take me anywhere. |
|
Hmm, perhaps the repository is configured wrong, here's a screenshot of what I linked to. I assumed it was working since you should be able to see and comment on them according to Github docs:
If you can't see them then there must be a problem on my end somewhere. |
|
Screenshot shows reviews as pending. I believe you have to click on Review Changes in the Files changed tab and Submit Review. Made the first change. For the second are you sure about this? I thought negotiated ciphers were only part of OpenVPN 2.4. |
| @@ -94,6 +94,10 @@ $OVPN_ADDITIONAL_CLIENT_CONFIG | |||
| if [ -n "$OVPN_COMP_LZO" ]; then | |||
| echo "comp-lzo" | |||
| fi | |||
|
|
|||
| if [ "$OVPN_OTP_AUTH" = "1" ]; then | |||
There was a problem hiding this comment.
Can you change this to -n "$OVPN_COMP_LZO" like the other tests?
| @@ -11,9 +11,11 @@ and use this image to generate user configuration. | |||
|
|
|||
| In order to enable two factor authentication the following steps are required. | |||
|
|
|||
| * Generate server configuration with `-2` option | |||
| * Choose a more secure [cipher](https://community.openvpn.net/openvpn/wiki/SWEET32) to use because since [OpenVPN 2.3.13](https://community.openvpn.net/openvpn/wiki/ChangesInOpenvpn23#OpenVPN2.3.13) the default openvpn cipher BF-CBC will cause a renegotiated connection every 64 MB of data | |||
There was a problem hiding this comment.
This should really say that if your client is broken or outdated, and fails to negotiate a more reasonable cipher, only then should override it. Many modern OpenVPN clients work just fine.
I think you're right, I must have skipped a step. I thought I used this review process before, but apparently not. Do you see them now?
I guess my concern is more that it adds a step that will ultimately be obsoleted by cipher negotiation in OpenVPN 2.4. It's probably a moot problem though and people running OTP should be paying some kind of attention, so it's probably not a big deal for now. |
Dealing with this issue
#185
Also updated documentation to make it clear that using the default openvpn cipher will cause issues with otp.