Page MenuHomeFreeBSD

Import the WireGuard driver from zx2c4.com.
ClosedPublic

Authored by jhb on Oct 7 2022, 9:58 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 9, 10:28 AM
Unknown Object (File)
Sat, Nov 9, 10:28 AM
Unknown Object (File)
Thu, Nov 7, 1:39 PM
Unknown Object (File)
Thu, Nov 7, 1:38 PM
Unknown Object (File)
Thu, Nov 7, 1:36 PM
Unknown Object (File)
Thu, Nov 7, 1:25 PM
Unknown Object (File)
Thu, Nov 7, 11:33 AM
Unknown Object (File)
Mon, Oct 28, 6:43 AM

Details

Summary

This commit brings back the driver from commit
f187d6dfbf633665ba6740fe22742aec60ce02a2 plus subsequent fixes from
upstream.

Obtained from: git@git.zx2c4.com:wireguard-freebsd
Sponsored by: The FreeBSD Foundation

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jhb requested review of this revision.Oct 7 2022, 9:58 PM

IMHO, code should be cleaned up from __FreeBSD_version and only code that is for CURRENT shall end in git branch main.

IMHO, code should be cleaned up from __FreeBSD_version and only code that is for CURRENT shall end in git branch main.

The previous discussion on this lead to us keeping them for now- IIRC it's specifically that we don't want to maintain the out-of-tree repo just for compat shims and keeping those patches in ports is kind of a pain, so it's harmless to carry these here until if_wg ends up in all supported releases.

The previous discussion on this lead to us keeping them for now

Yeah, it seems reasonable to me to merge to stable/13 when appropriate, and then unifdef on main.

The previous discussion on this lead to us keeping them for now

Yeah, it seems reasonable to me to merge to stable/13 when appropriate, and then unifdef on main.

See also D36913. That trims a lot of the #ifdef's, but I wanted to have a starting point closer to the current driver (though even here I have some changes to fine LINT-NO*, etc. relative to upstream)

That seems to be missing a definition for sogetsockaddr():

/usr/src/sys/dev/wg/if_wg.c:825:14: error: implicit declaration of function 'sogetsockaddr' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
                        int ret = sogetsockaddr(so4, (struct sockaddr **)&bound_sin);
                                  ^
/usr/src/sys/dev/wg/if_wg.c:825:14: note: did you mean 'getsockaddr'?
/usr/src/sys/sys/socketvar.h:449:5: note: 'getsockaddr' declared here
int     getsockaddr(struct sockaddr **namp, const struct sockaddr *uaddr,
        ^
/usr/src/sys/dev/wg/if_wg.c:840:14: error: implicit declaration of function 'sogetsockaddr' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
                        int ret = sogetsockaddr(so6, (struct sockaddr **)&bound_sin6);
                                  ^

The patch that removed wireguard had this:

diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index eb748928cd91..7f06b51cf096 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -850,17 +850,6 @@ sopeeloff(struct socket *head)
 }
 #endif /* SCTP */

-int
-sogetsockaddr(struct socket *so, struct sockaddr **nam)
-{
-       int error;
-
-       CURVNET_SET(so->so_vnet);
-       error = (*so->so_proto->pr_usrreqs->pru_sockaddr)(so, nam);
-       CURVNET_RESTORE();
-       return (error);
-}
-
 int
 sobind(struct socket *so, struct sockaddr *nam, struct thread *td)
 {

Although it does have to be this, due to other refactoring in the mean time:

+int
+sogetsockaddr(struct socket *so, struct sockaddr **nam)
+{
+       int error;
+
+       CURVNET_SET(so->so_vnet);
+       error = (*so->so_proto->pr_sockaddr)(so, nam);
+       CURVNET_RESTORE();
+       return (error);
+}
+

That seems to be missing a definition for sogetsockaddr():

It's earlier in the stack, see D30087

  • Inline sogetsockaddr without CURVNET_SET/RESTORE.
  • A few other small fixes noted in child revisions.
sys/sys/priv.h
352

43f8c763cdeea29f95b6f0eebce3ad80dd210c7a introduced a conflict here.

IMO we might as well commit the priv.h change (after updating) and if_types.h now.

  • Rebase. Fix PRIV_* conflict.
This revision is now accepted and ready to land.Oct 24 2022, 7:29 PM
sys/dev/wg/if_wg.c
273

Oops, noticed these stats while prepping for commit. These were in my local branch for testing, but not needed in the tree. Will fix before commit.

gbe added a reviewer: manpages.
gbe added a subscriber: gbe.

LGTM from manpages

  • Remove debug counters not in upstream that snuck in by accident.
  • Correct license on if_wg.c.
This revision now requires review to proceed.Oct 26 2022, 8:49 PM
This revision is now accepted and ready to land.Oct 26 2022, 10:42 PM
pauamma_gundo.com added inline comments.
share/man/man4/wg.4
2

Unless I missed something, this matches the text at https://spdx.org/licenses/BSD-2-Clause.html, except that you wrote "author" instead of "copyright holders" but don't mention who the author is, only the copyright holder. (Not being an IP lawyer, I don't know what difference it makes.) If you meant (or decide to change to) "copyright holders", perhaps add

.\" SPDX-License-Identifier: BSD-2-Clause

at the top?

27

Bump.

48

US spelling

76

There was a recent report in Schneier on Security of a supposedly quantum-resistant protocol being vulnerable to a classical attack, so I wouldn't limit it.

79
85
99
103
107–108

I don't understand what you're trying to say here. Do you mean that because handshaking happens automatically whenever there's traffic to send, there's no need for explicit requests to establish or dismantle peer connections? If so, I'd make it more explicit.

143

Worth adding an IPv6 example?

164

I'd remove either "mutually" or "each other", leaning toward removing "mutually".

199

Will the manual page get MFC'd? If not, I'd mention it's 14-only.

203
This revision now requires changes to proceed.Oct 27 2022, 8:52 AM
share/man/man4/wg.4
2

This is the old manpage from before it was removed with only minor edits to reflect the use of wg(8) instead of ifconfig(8). For this commit restoring the oldmanpage is the goal. Subsequent edits to the manpage should be done as followup changes, not as part of this change to revert its removal.

It's also not mine to relicense and it would not be legal for me to change it without permission, that would be up to the copyright holder. We also do not require SPDX license tags.

199

It will get merged, but this part probably should be adjusted to be 14.0 for now.

I made an entry on my todo list for the man page changes. @jhb is right about, that this should be a separate differential.

In D36909#844179, @gbe wrote:

I made an entry on my todo list for the man page changes. @jhb is right about, that this should be a separate differential.

If you mean all the changes I suggested, this is fine with me as is.

This revision is now accepted and ready to land.Oct 28 2022, 9:46 AM
This revision was automatically updated to reflect the committed changes.