Skip to content

Fix UDP checksum handling in clinat.c #1038

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
laviniuc wants to merge 1 commit into
base: master
Choose a base branch
from
+16 −1
Open

Fix UDP checksum handling in clinat.c #1038

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/openvpn/clinat.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,22 @@ client_nat_transform(const struct client_nat_option_list *list, struct buffer *i
{
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 != 0)
{
ADJUST_CHECKSUM(accumulate, h->u.udp.check);

/* RFC 768: a computed checksum of 0 must be transmitted
* as 0xFFFF, because 0 is reserved to mean "no checksum". */

if (h->u.udp.check == 0)
{
h->u.udp.check = 0xFFFF;
Copy link
Copy Markdown
Member

@ordex ordex May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You quoted the RFC saying: "NAT implementations rewriting IP addresses must either leave checksum=0 unchanged or fully recompute the checksum from scratch."

Why setting it to 0xFFFF?

Copy link
Copy Markdown
Author

@laviniuc laviniuc May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's the check after it's been re-adjusted / recomputed. if it happens to be 0 it should be sent as FFFF

Copy link
Copy Markdown
Member

@ordex ordex May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you have a reference about this rule that we could add as a comment here?
I was looking up the RFC, but I couldn't find anything about checksum=0.

Copy link
Copy Markdown
Author

@laviniuc laviniuc May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.rfc-editor.org/rfc/rfc768.html page 2
and explained here https://people.na.infn.it/~garufi/didattica/CorsoAcq/Trasp/Lezione9/tcpip_ill/udp_user.htm section 11.3

Copy link
Copy Markdown
Member

@ordex ordex May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, my bad. For whatever reason I mixed up your patch with another change and got off-track.

}
}
}
}
}
Expand Down