Progress on Zinc (thus WireGuard)
When last we looked at the WireGuard VPN code and its progress toward mainline inclusion, said progress was impeded by disagreements about the new "Zinc" cryptographic library that is added by the WireGuard patches. Since that August look, several more versions of WireGuard and Zinc have been posted; it would seem that Zinc is getting closer to being accepted. Once that happens, the networking developers are poised to review that portion of the code, which likely will lead to WireGuard in the kernel some time in the next development cycle or two.
Jason Donenfeld posted Zinc v3 as part of an updated WireGuard posting on September 10. Of the versions he has posted since our article (up to v6 as of this writing), v3 has gotten most of the comments. One of the main complaints about Zinc is that it creates a new crypto API in the kernel without really addressing why the existing one would not work for WireGuard. As Ard Biesheuvel put it:
As I pointed out in a previous version, I don't think we need a separate crypto API/library in the kernel, and I don't think you have convinced anyone else yet either.
Perhaps you can devote /your/ rare talent and energy to improving what we already have for everybody's sake, rather than providing a completely separate crypto stack that only benefits WireGuard (unless you yourself port the existing crypto API software algorithms to this crypto stack first and present *that* work as a convincing case in itself)
But Greg Kroah-Hartman said
that the current crypto API is too hard to use for many parts of the kernel, which leads to
simpler, private implementations of crypto primitives sprinkled all over
the kernel tree. He suggested that the existing crypto API be switched
over time to use the Zinc primitives where that is possible. But Eric
Biggers was concerned
that no conversions of that sort have been done, which means there could be
undiscovered problems in the Zinc API that will make it difficult to do so:
"I don't think it makes sense to
merge all this stuff without doing the conversions, or at the very least
demonstrating how they will be done
".
Donenfeld said
that he is willing to do those conversions, but wants to get the series
merged first. "I'd really prefer to land this
series through net-next, and then after we can turn our attention to
integrating this into the existing crypto API
". But, as Andrew Lunn
pointed
out, that may be putting the cart ahead of the horse. He noted that
the networking developers have not had a serious look at the WireGuard
patches and won't "until the controversial part
of the code, Zinc, has been sorted out
". He also predicted that
networking maintainer David Miller would not take the code into his tree
without an Acked-by from the crypto maintainers.
Miller confirmed
that assessment and clarified
that even though he is listed as one of the two crypto maintainers, he
would be looking for an ack from the other maintainer, Herbert Xu, as "I haven't done a serious
review of crypto code in ages
". Xu has been quiet, so far, on the
Zinc patches, with one exception.
Donenfeld feels that Biesheuvel is not pleased with the
existence of Zinc. When Biesheuvel listed
the
additional
items that he thinks need to be addressed in Zinc, Donenfeld's response
is prefaced by a number of worries about Biesheuvel: that he is
"generally hostile to this whole initiative
", is trying to
"stall it indefinitely
", and perhaps will just keep bikeshedding "until
Zinc copies lots of the same design decisions from the present crypto
API
". Donenfeld did also say that he hoped these were all just fears and
did not truly reflect what was happening. But that was when Xu stepped in
to make it clear that he
values the review that Biesheuvel has been doing:
Please keep in mind that this is a large project that has to support multiple users on one hand (not just WireGuard) and complex hardware acceleration drivers on the other. Ard has been one of the most prolific contributors to the crypto code and his review should be taken seriously.
For his part, Biesheuvel tried to make his intentions clear to Donenfeld in another part of the thread:
But the main technical objections that Biggers and Biesheuvel have raised were still being hashed out in the thread. Andy Lutomirski suggested that Donenfeld add a conversion of one of the algorithms in the existing crypto API to use Zinc as part of the patch set. After a bit of resistance, Donenfeld agreed. Conversions of the Poly1305 hash and ChaCha20 cipher (which are what WireGuard uses) in the existing crypto subsystem were part of the WireGuard v4 patch set.
Along the way, there have also been discussions about the OpenSSL
implementations of some of the primitives (i.e. CRYPTOGAMS) that were
incorporated into
Zinc. These are written in assembly, but are actually generated from Perl
scripts. Donenfeld has modified the assembly output in order to make it
comply with kernel coding standards but has also made some other adjustments.
That makes it difficult to keep the implementation in sync with any changes
that OpenSSL might make, Biesheuvel noted. "Dumping 10,000s of lines of
generated assembler in the kernel tree like that is really
unacceptable IMO.
" Donenfeld said
that he disagrees with the characterization of the code, but that getting
his changes into the OpenSSL upstream is desirable.
Beyond that, there were some minor licensing concerns (and the resulting SPDX identifiers) with some files, which have seemingly been resolved. Similarly, some performance problems were noted and addressed. In short, Zinc is starting to look like something that could be merged. Donenfeld posted v6 of the WireGuard patch set on September 25.
Zinc is still awaiting an ack from Xu, though it is not clear how much he has scrutinized the code at this point. Once that happens, though, the networking side of the patch set can be reviewed by Miller and other networking developers. If all goes well, it will end up in the mainline before too long—but that still means at least four, or more likely seven, months from now. Whenever it comes, it is clear that WireGuard is eagerly anticipated by many.
Index entries for this article | |
---|---|
Kernel | Cryptography |
Kernel | Network/Virtual private networks |
Security | Cryptography |
Security | Network/Virtual private network (VPN) |
(Log in to post comments)
Progress on Zinc (thus WireGuard)
Posted Sep 26, 2018 17:35 UTC (Wed) by zx2c4 (subscriber, #82519) [Link]
Donenfeld said that he is willing to do those conversions, but wants to get the series merged first. "I'd really prefer to land this series through net-next, and then after we can turn our attention to integrating this into the existing crypto API".For what it's worth, I wound up doing those conversions anyway, and they're a part of v6. This pattern applies for most other things -- I didn't want to do them, in order to keep the Zinc patchset small, but Ard has insisted, and I've relented. So hopefully piece by piece things are being done as needed. So it goes.
but that getting his changes into the OpenSSL upstream is desirable.And indeed Andy Polyakov and I have already started talking about this stuff and things are moving nicely on this front.
Progress on Zinc (thus WireGuard)
Posted Sep 26, 2018 17:39 UTC (Wed) by GennaroReinger (guest, #127208) [Link]
https://lore.kernel.org/lkml/CAHmME9p5b=L0FSL72gCszhvut-k...
[0] Ad hominem, short for argumentum ad hominem, is a fallacious argumentative strategy whereby genuine discussion of the topic at hand is avoided by instead attacking the character, motive, or other attribute of the person making the argument, or persons associated with the argument, rather than attacking the substance of the argument itself. https://en.wikipedia.org/wiki/Ad_hominem
Progress on Zinc (thus WireGuard)
Posted Sep 26, 2018 18:48 UTC (Wed) by johannbg (subscriber, #65743) [Link]
It can also be a good practise in communities for other reviewers to chime in once certain number of reviews from the same reviewer is exceeded ( like 3 or something ), to prevent the submitter getting the feeling that no matter how many times he rewrites things at the reviewer behest, it never is or will be good enough.
Then there are legit case to what he is implying is taking place ( the I own the code syndrom which is similar when administrators self claim ownership of servers ) but as you point out there are better ways to deal with this instead of starting ad hominen attack, like for example just saying it's starting to feel like i'm swimming upstream, can I get a second opinion from someone else ( or something along those lines ).
Progress on Zinc (thus WireGuard)
Posted Sep 26, 2018 18:54 UTC (Wed) by zx2c4 (subscriber, #82519) [Link]
The patchset is very much benefitting from the reviews of many, who have all sorts of interesting perspectives to offer. In many areas it's improved tremendously thanks to those. However, I'm pretty wary of directions that lead to bloating the core concepts of Zinc and making the same mistakes as the current crypto API. On those matters, I've been particularly standoffish, especially when I've seen many repeated instances of this. But indeed that should be handled more diplomatically.
Progress on Zinc (thus WireGuard)
Posted Jun 7, 2020 8:10 UTC (Sun) by scientes (guest, #83068) [Link]
"If, now, your right eye is making you stumble, tear it out and throw it away from you. For it is better for you to lose one of your members than for your whole body to be pitched into Gehenna. Also, if your right hand is making you stumble, cut it off and throw it away from you. For it is better for you to lose one of your members than for your whole body to land in Gehenna."--Jesus, Matthew 5:29-30
(Yes, I know this is an old thread)
Progress on Zinc (thus WireGuard)
Posted Sep 26, 2018 19:24 UTC (Wed) by rweikusat2 (subscriber, #117920) [Link]
Progress on Zinc (thus WireGuard)
Posted Sep 27, 2018 0:55 UTC (Thu) by pabs (subscriber, #43278) [Link]
Progress on Zinc (thus WireGuard)
Posted Sep 27, 2018 9:09 UTC (Thu) by moltonel (guest, #45207) [Link]
Progress on Zinc (thus WireGuard)
Posted Sep 27, 2018 9:14 UTC (Thu) by epa (subscriber, #39769) [Link]
Progress on Zinc (thus WireGuard)
Posted Sep 27, 2018 11:34 UTC (Thu) by ardbiesheuvel (subscriber, #89747) [Link]
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/...
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/...
Progress on Zinc (thus WireGuard)
Posted Sep 27, 2018 18:13 UTC (Thu) by JoeBuck (subscriber, #2330) [Link]