gluon
gluon copied to clipboard
freifunk-gluon
gluon-mesh-vpn-key-translate
This reencodes an existing fastd private key as WireGuards private (X25519). No key is migrated, when no fastd key exists.
We discussed whether it would be cleaner to put it in its own upgrade file, this would be one way of doing it, although writing to network-old appears bad to me. Open for discussion.
~~I have not tested the implementation of the upgrade yet.~~ Is tested. It updates missing or invalid WireGuard-keys based on the fastd secret key if it exists.
Other than the commit message which should contain a reference to gluon-mesh-vpn-key-translate instead of key-translate, @blocktrron and I are somewhat satisfied with this.
I will fix this upon rebase at the end.
@NeoRaider A review would be highly appreciated.
Just a thought, instead of operating on both UCI network_gluon-old and fastd, would it maybe make sense to have something like a generic gluon.mesh_vpn.secret? And derive both the fastd and wireguard secrets from it?
As the role based interface configuration is in /etc/config/gluon, too, maybe it would be a nice feature if for making a backup of your node configuration it would be sufficient to copy the high level /etc/config/gluon* files?
@T-X I think that's a great idea and something I'd gladly implement,
but I'd favor getting this migration functionality in the simplest possible way done before 2022.1,
when we'll first release WireGuard into the wild.
Storing the mesh_vpn_secret in config.gluon would be way cleaner though and something I could write for 2022.1.1.
but I'd favor getting this migration functionality in the simplest possible way done before
2022.1, when we'll first release WireGuard into the wild.Storing the mesh_vpn_secret in config.gluon would be way cleaner though and something I could write for
2022.1.1.
My preference/expectations for major vs. minor releases would be:
- Major: New features, new OpenWrt release base, refactoring/cleanups, new hardware support
- Minor: Bugfixes, new hardware revision support for an otherwise already supported device
So I'm wondering if it would be doable to do it in a clean(tm) way now? To avoid refactoring it in a minor release?
I think adding other big changes like where this secret goes to this PR would not help getting it merged before 2022.1.
Maybe @blocktrron and @NeoRaider could share their thoughts as well?
@CodeFetch thanks for your suggestions. Before I address
Some cosmetic suggestions...
though, I'll keep waiting for @NeoRaider to review the general idea, as @blocktrron suggested in the last developer meetup.
Leaving the code as is (just right now) has the benefit of it currently running successfully on ~~65~~ 132 devices in Hanover.
Once he answers, whether and if - in what form - this can be merged, I'm happy to come back to your suggestions for improvements.
@NeoRaider I think the c code is fine as it is.
I might give the lua code another shot, wrapping the migration into a function, to get rid of the countless nested ifs.
The Lua code is now reformatted, fixed (was missing unistd.read) and tested to be working again.
I'm not sure whether it is bad not to drain the pipe before closing it.
fixed the missing local, so the linter will be fine; rebased and squashed the commits.
@NeoRaider the migration works; what are the next steps and do I need to implement a drain of the pipe?
@NeoRaider question remains. Does sigpipe matter in this case? Or would draining the pipe be wasted effort?
After a discussion in todays meetup the c tool that translates keys will become a generic tool, that encodes strings of arbitrary length.
@NeoRaider: As requested on the last meetup, the c tool is no longer a tool to convert elliptic private keys from hex to b64, but a generic hex-to-b64 transcoder.
It now uses fread as you suggested earlier. As it does not end on carriage returns anymore, but waits for
Please let me know what parts of the code need refactoring, before I squash and rebase the commits down into two again.
While the tool seems to work as expected, this should be tested in the field again; the 600+ testresults from earlier do now hardly have more relevancy than a poc.
GitHubs JS is seriously broken. I cannot re-request given reviews from codefetch and mkg; the UI always drops one of you again. Sorry for the noise.
@CodeFetch Because there already is one perfectly fine implementation in libubox, and I didn't feel the urge to implement another one. I would not have learnt something from that and it would've wasted resources on the device having an unnecessary implementation.
Also I think I favor c code over lua, especially when it comes to things like encoding of arbitrary sized input.
If that resolves the question, I'm looking forward for your acknowledgement on your PR, as you gave a review before and hope I've met or discussed all relevant criteria to your changes requested :)
Just tested: This works great for stuff smaller than one buffer size. Key-migration worked from a 19.07 fastd release.
Everything bigger than one buffer size is currently broken though. (Terminates eraly after one buffer size and has therefore only converted a 10K char hex input to 128 base64 chars. Not sure how much time I can put into this today. Will report back later...
I hope that's it. Both checks were comparing out of bounds before.
Local build is running. Test result incoming.
I just reset Eilenriede-Tester to 19.07 (thanks @1977er),
and migrated it again to the latest commit. Still works.
I did a round-trip-test using 5K digits of PI. Worked as well.
@lemoer and I tested it using /dev/urandom and hexdump, which does now work as well.
- Include documentation for conversion.
- No conversion from wireguard to fastd possible
- Link to tool for infrastructure side
review evening: needs three lines of documentation with a reference to https://github.com/AiyionPrime/gluon-mesh-vpn-key-translate
Merging is blocked @NeoRaider requested changes
I'd say you merge this, if you're satisfied with the docs as we are?