Add explicit trust establishment #383

roosterfish · 2024-08-26T13:48:08Z

This PR adds the explicit trust establishment to MicroCloud according to the spec in https://discourse.ubuntu.com/t/explicit-trust-establishment-mechanism-for-microcloud/44261.

The PR adds the EFF wordlist with over 1300 lines so please consider this for the review.
The license information is based on https://www.eff.org/copyright.

It has the following dependencies which have to be met before it can be marked as ready:

LXD: Shared: Add HMAC and cert utils lxd#13969
MicroCluster: v2: Backport websocket client functions microcluster#239

In addition we are currently waiting on MicroCeph to merge the v2 of MicroCluster which is also are requirement for this PR.

Open TODO's:

Align test suite to work with updated questions
Release minor version of MicroCluster to allow pulling the new websocket client functions in (we will track the commit for now)
Update this PR's go.mod file if the dependencies are met
Check for additional unit testing capabilities for the changes in this PR
Remove the TODO (regarding https://github.com/canonical/microcluster/blob/a4c5273899ac294750d63089f81f78fd1894df67/internal/rest/resources/api_1.0.go#L21-L25) after importing MicroCluster v2
Update the docs (e.g. preseed)

cmd/microcloud/session.go

roosterfish · 2024-08-29T11:42:24Z

Added some changes as microcloud service add was broken due to the wrong certificate being used.
Functions like RemoteClusterMembers are used both during pre-init and after bootstrap/join but the latter has to use the cluster certificate.

I also made a mistake whilst rebasing #358. As the remote token creation/deletion always happens after the MicroCloud's microcluster is formed, we neither need a secret nor a custom certificate as we can already use full mTLS.

masnax

This is awesome, and intuitive to follow, nice job @roosterfish! :)

I mostly have nits to report, primarily structuring of the messages that are printed to the user.

Couple things that stand out:

During a snap mismatch, the error seems incorrect now because the multicast lookup behaviour is different: Error: Failed to read join confirmation: Failed to send join intent: Skipping peer "micro01" due to missing services (MicroOVN)
During preseed, it seems we require listen_address to be set, but that defeats the purpose of multicast lookup
the --add flag for preseed ~~seems redundant~~ can be improved
The biggest one (but also not a blocker) is the behaviour if a joiner's session ends. On the initiator there's no feedback, so as you are flipping through your joiners and typing the password, if one of them dies in the background, by the time you get back to the initiator and confirm the systems in the table, you have no way of knowing that a problem occurred, and the initiation fails. It would be nice if we could have a column for "session status" in the table so that we know when a joiner has closed its session, and can work around it. I don't believe our current table package allows for easy updates though, so we can deal with this later.

api/session.go

client/websocket.go

api/session_join.go

client/websocket.go

service/lxd.go

cmd/microcloud/preseed.go

cmd/microcloud/session.go

api/session_join.go

cmd/microcloud/session.go

masnax

Just a handful of more nits, and some validation suggestions :)

masnax · 2024-09-04T07:20:32Z

cmd/microcloud/session.go

+	if !c.autoSetup && target == "" {
+		fmt.Println("Searching an eligible system ...")
+	}


Should be "Searching for an eligible system ..." right?

Also maybe s/system/initiator/?

api/session.go

cmd/microcloud/preseed.go

cmd/microcloud/session.go

This PR adds another `QueryRaw` function to the client which allows accessing the raw `*http.Response`. That is required for the canonical/microcloud#383 in order to access the remotes certificate used in the TLS connection. Furthermore the parsing of API responses is moved into the public facing package to allow any user of `QueryRaw` to both access the raw response and parsing it as usual.

Signed-off-by: Julian Pelizäus <[email protected]>

roosterfish · 2024-10-04T16:10:21Z

@masnax preseed test succeeded after resolving the conflict, so I would carefully say it's ready for review again :)

The test run before showed green for all jobs.

Signed-off-by: Julian Pelizäus <[email protected]>

This new func allows extracting the passphrase from the stdout of 'microcloud init' and running multiple 'microcloud join' using this passphrase in interactive mode. Signed-off-by: Julian Pelizäus <[email protected]>

Signed-off-by: Julian Pelizäus <[email protected]>

Also move the join message out of waitForJoin to not indicate a join if not all of the peers clusters are joined. Signed-off-by: Julian Pelizäus <[email protected]>

This ensures the mDNS multicast traffic flowing between the nodes isn't interrupted as this behavior was observed on the GitHub runners. Signed-off-by: Julian Pelizäus <[email protected]>

roosterfish · 2024-10-07T10:07:24Z

Did another small push as I saw a slice was initialized with the wrong capacity which caused this output during preseed:

root@micro03:~# microcloud preseed < p
Successfully joined the MicroCloud cluster and closing the session.
Commencing cluster join of the remaining services (, , , , LXD, MicroCeph, MicroOVN)

masnax

Other than that 1 nit, this looks great! Thanks for the hard work @roosterfish!

masnax · 2024-10-08T20:35:26Z

test/includes/microcloud.sh

+
+# capture_and_join: extracts the passphrase from stdin and outputs text that is being passed to `TEST_CONSOLE=1 microcloud join`
+# to simulate terminal input to the interactive CLI.
+# Set the first argument to either true or false if you want to skip missing services.


I take it we removed the missing services question from microcloud join, so this description is misleading now.

Ah good catch, fixed with #416.

Follow up on #383 (comment).

I figured the README requires some more clarification after merging #383 as you also have to run `microcloud join` on all the other machines if you don't want to setup MicroCloud on a single machine. In addition this also gives the hint that you can now have a MicroCloud using a single machine.

The `--auto` and `--wipe` flags seem to be a left over of from the `init` command when crafting the skeleton for the new `join` command introduced with #383. This PR removes the flags that don't have any value for the `join` command.

github-advanced-security bot found potential problems Aug 26, 2024

View reviewed changes

cmd/microcloud/session.go Fixed Show resolved Hide resolved

roosterfish force-pushed the explicit_trust branch from e61b196 to 5d17236 Compare August 27, 2024 09:39

roosterfish changed the title ~~Add excplicit trust establishment~~ Add explicit trust establishment Aug 27, 2024

roosterfish force-pushed the explicit_trust branch 2 times, most recently from f59031a to 719e37f Compare August 29, 2024 11:38

roosterfish force-pushed the explicit_trust branch 2 times, most recently from e60811f to d34ebe5 Compare August 29, 2024 14:23

masnax requested changes Aug 29, 2024

View reviewed changes

roosterfish force-pushed the explicit_trust branch 2 times, most recently from 89d9fb8 to 0d67d00 Compare September 2, 2024 14:53

github-advanced-security bot found potential problems Sep 2, 2024

View reviewed changes

cmd/microcloud/session.go Fixed Show fixed Hide fixed

roosterfish force-pushed the explicit_trust branch 2 times, most recently from 9a68cc8 to d492049 Compare September 3, 2024 14:00

masnax mentioned this pull request Sep 3, 2024

Investigate need for lookup_interface preseed key #357

Closed

masnax reviewed Sep 4, 2024

View reviewed changes

api/session_join.go Outdated Show resolved Hide resolved

cmd/microcloud/session.go Outdated Show resolved Hide resolved

masnax requested changes Sep 4, 2024

View reviewed changes

roosterfish force-pushed the explicit_trust branch from d492049 to 61ed5e4 Compare September 4, 2024 14:43

github-advanced-security bot found potential problems Sep 4, 2024

View reviewed changes

cmd/microcloud/session.go Dismissed Show dismissed Hide dismissed

roosterfish force-pushed the explicit_trust branch from 61ed5e4 to cba3052 Compare September 5, 2024 13:46

roosterfish mentioned this pull request Sep 6, 2024

Client: Add new QueryRaw function canonical/microcluster#249

Merged

roosterfish force-pushed the explicit_trust branch 7 times, most recently from 829fe4c to 136a328 Compare September 9, 2024 14:25

roosterfish force-pushed the explicit_trust branch from 136a328 to f481bc9 Compare September 11, 2024 15:49

roosterfish added 4 commits October 4, 2024 17:28

cmd/microcloud: Add ask utilities and replace secret in client calls

0f54bba

Signed-off-by: Julian Pelizäus <[email protected]>

cmd/microcloud: Use session concept for init

a2ef02f

Signed-off-by: Julian Pelizäus <[email protected]>

cmd/microcloud: Use session concept for add

cd814e2

Signed-off-by: Julian Pelizäus <[email protected]>

cmd/microcloud: Add join subcommand

147355f

Signed-off-by: Julian Pelizäus <[email protected]>

roosterfish force-pushed the explicit_trust branch from 83c339f to c544123 Compare October 4, 2024 15:30

roosterfish added 12 commits October 7, 2024 12:05

cmd/microcloud: Move preseed code into its own subcommand

e29abc8

Signed-off-by: Julian Pelizäus <[email protected]>

cmd/microcloud/preseed: Update test

276dd98

Signed-off-by: Julian Pelizäus <[email protected]>

cmd/microcloud: Add initiating and joining session handlers for the CLI

c8dafac

Signed-off-by: Julian Pelizäus <[email protected]>

cmd/microcloudd: Remove secret and mDNS from daemon

61a25f4

Signed-off-by: Julian Pelizäus <[email protected]>

doc: Apply trust establishment changes to the init, add and preseed docs

6e6a1a4

Signed-off-by: Julian Pelizäus <[email protected]>

test/includes: Add capture_and_join

d347a7d

This new func allows extracting the passphrase from the stdout of 'microcloud init' and running multiple 'microcloud join' using this passphrase in interactive mode. Signed-off-by: Julian Pelizäus <[email protected]>

test/suites: Rework preseed to work with explicit trust establishment

459ff56

Signed-off-by: Julian Pelizäus <[email protected]>

test/suites: Rework add to work with explicit trust establishment

91e2452

Signed-off-by: Julian Pelizäus <[email protected]>

test/suites: Rework recover to work with explicit trust establishment

c522ff0

Signed-off-by: Julian Pelizäus <[email protected]>

test/suites: Rework basic to work with explicit trust establishment

f9322a3

Signed-off-by: Julian Pelizäus <[email protected]>

cmd/microcloud: Always join the MicroCloud microcluster first

aa54fb6

Also move the join message out of waitForJoin to not indicate a join if not all of the peers clusters are joined. Signed-off-by: Julian Pelizäus <[email protected]>

test/suites/basic: Use openvswitch bridge driver for IPv6 tests

46fd760

This ensures the mDNS multicast traffic flowing between the nodes isn't interrupted as this behavior was observed on the GitHub runners. Signed-off-by: Julian Pelizäus <[email protected]>

roosterfish force-pushed the explicit_trust branch from c544123 to 46fd760 Compare October 7, 2024 10:05

masnax approved these changes Oct 8, 2024

View reviewed changes

masnax merged commit 780afbb into canonical:main Oct 8, 2024
16 checks passed

roosterfish deleted the explicit_trust branch October 9, 2024 07:38

roosterfish mentioned this pull request Oct 9, 2024

Test: Clarify comments on the interactive join funcs #416

Merged

masnax added a commit that referenced this pull request Oct 10, 2024

Test: Clarify comments on the interactive join funcs (#416)

2d902cb

Follow up on #383 (comment).

roosterfish mentioned this pull request Oct 14, 2024

README: Clarify commands for deployment #425

Merged

This was referenced Oct 17, 2024

Support installation on VMs running in a cloud provider (where mDNS does not work) #134

Closed

Join: Remove --auto and --wipe flag #441

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add explicit trust establishment #383

Add explicit trust establishment #383

roosterfish commented Aug 26, 2024 •

edited

Loading

roosterfish commented Aug 29, 2024

masnax left a comment •

edited

Loading

masnax left a comment

masnax Sep 4, 2024

roosterfish commented Oct 4, 2024 •

edited

Loading

roosterfish commented Oct 7, 2024

masnax left a comment

masnax Oct 8, 2024

roosterfish Oct 9, 2024

Add explicit trust establishment #383

Add explicit trust establishment #383

Conversation

roosterfish commented Aug 26, 2024 • edited Loading

roosterfish commented Aug 29, 2024

masnax left a comment • edited Loading

Choose a reason for hiding this comment

masnax left a comment

Choose a reason for hiding this comment

masnax Sep 4, 2024

Choose a reason for hiding this comment

roosterfish commented Oct 4, 2024 • edited Loading

roosterfish commented Oct 7, 2024

masnax left a comment

Choose a reason for hiding this comment

masnax Oct 8, 2024

Choose a reason for hiding this comment

roosterfish Oct 9, 2024

Choose a reason for hiding this comment

roosterfish commented Aug 26, 2024 •

edited

Loading

masnax left a comment •

edited

Loading

roosterfish commented Oct 4, 2024 •

edited

Loading