gluon-mesh-vpn-wireguard: add fastd key migration

[AiyionPrime]

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.

[AiyionPrime]

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.

[T-X]

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?

[AiyionPrime]

@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.

[AiyionPrime]

aligned the Makefile definition to the existing one

[T-X]

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?

[AiyionPrime]

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]

Some cosmetic suggestions...

[CodeFetch]

One call of fprintf would be better and use a macro instead of inline functions like:

#define error(msg, ...)                                            \
	do {                                                                   \
		fprintf(stderr, "Error: " msg "\n", ##__VA_ARGS__);            \
		exit(1);                                                    \
	} while (0)

I don't think it harms code readability to directly include the error message in the main function instead of hiding it away in a separate one.

[CodeFetch]

As "i" is only used in the for loop and is never going to reach size_t, just do
for (int i = 0; i < 32; i++)

[AiyionPrime]

@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.

[CodeFetch]

The braces are superfluous and don't contribute to code readability.

[AiyionPrime]

You are right, there are braces one could've left out regarding precedence;
but I'd disagree on the readability. Both does not really matter though, what does is, that that's not my code, but actually @NeoRaider's, as attributed above.
https://github.com/NeoRaider/fastd/blob/d9dd14045c647736566c959855f154302eeff444/src/protocols/ec25519_fhmqvc/ec25519_fhmqvc.c#L17-L27

But as I said earlier I'd really like to get some basic feedback from neoraider, before I address nitpicks.

Statistically speaking - looking at how my previous PR's went -there will likely be no line left unturned; really those braces will be the least of concerns, once he finds time, I'm afraid.

Not that I do not appreciate the engagement with the PR,
but beautifying is not the stage I'm currently at, yet,

[AiyionPrime]

@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.

[AiyionPrime]

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.

[AiyionPrime]

I hope that's it. Both checks were comparing out of bounds before.

Local build is running. Test result incoming.

[AiyionPrime]

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.

[AiyionPrime]

@NeoRaider I could provide you with an image from our community, which currently supports both fastd and WireGuard - so that you could watch the migration in action.
Let me know if that would help you reviewing this.

[blocktrron]

• Include documentation for conversion.
• No conversion from wireguard to fastd possible
• Link to tool for infrastructure side

[AiyionPrime]

review evening:
needs three lines of documentation with a reference to https://github.com/AiyionPrime/gluon-mesh-vpn-key-translate

[AiyionPrime]

Merging is blocked
@NeoRaider requested changes

I'd say you merge this, if you're satisfied with the docs as we are?

Discussed at meetup