[S] Change in openvpn[master]: clinat: do not adjust UDP checksum if zero

Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1681?usp=email

to review the following change.


Change subject: clinat: do not adjust UDP checksum if zero
......................................................................

clinat: do not adjust UDP checksum if zero

As per RFC768, when the UDP checksum is zero, it means
it was not computed by the source, therefore any NAT
processing along the way should leave the checksum alone
and not update it.
Failing to do so would result in computing a bogus value.

At the same time, if the result of updating a non-zero
checksum ends up being zero, as per the same RFC, we must
store its one-complement (0xFFFF) as zero is reserved
for "checksum not computed", as mentioned above.

Ensure our Client NAT code follows both rules.

Github: closes OpenVPN/openvpn#1037
Reported-by: Jeff Salee <[email protected]>
Change-Id: I4068bf8175c23151298d142dc920ab89f861a411
Signed-off-by: Antonio Quartulli <[email protected]>
---
M src/openvpn/clinat.c
1 file changed, 17 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/81/1681/1

diff --git a/src/openvpn/clinat.c b/src/openvpn/clinat.c
index 32c1325..3724022 100644
--- a/src/openvpn/clinat.c
+++ b/src/openvpn/clinat.c
@@ -258,7 +258,23 @@
         {
             if (BLENZ(ipbuf) >= sizeof(struct openvpn_iphdr) + sizeof(struct openvpn_udphdr))
             {
-                ADJUST_CHECKSUM(accumulate, h->u.udp.check);
+                /* RFC 768: a UDP checksum of 0 means "no checksum computed".
+                 * Do not run the incremental adjustment over a non-checksum,
+                 * or we will write a bogus non-zero value into the field.
+                 */
+                if (h->u.udp.check)
+                {
+                    ADJUST_CHECKSUM(accumulate, h->u.udp.check);
+
+                    if (!h->u.udp.check)
+                    {
+                        /* RFC 768: a computed checksum of 0 must be transmitted
+                         * as 0xFFFF (one-complement), because 0 is reserved for
+                         * "no checksum computed"
+                         */
+                        h->u.udp.check = 0xFFFF;
+                    }
+                }
             }
         }
     }

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1681?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I4068bf8175c23151298d142dc920ab89f861a411
Gerrit-Change-Number: 1681
Gerrit-PatchSet: 1
Gerrit-Owner: ordex <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>

_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel