Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rfc: node access #3051

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

rfc: node access #3051

wants to merge 2 commits into from

Conversation

burgerdev
Copy link
Contributor

Context

This RFC proposes a mechanism for accessing Constellation nodes for maintenance.

Checklist

  • Add labels (e.g., for changelog category)
  • Is PR title adequate for changelog?
  • Link to Milestone

@burgerdev burgerdev added the no changelog Change won't be listed in release changelog label Apr 29, 2024
@burgerdev burgerdev added this to the v2.17.0 milestone Apr 29, 2024
Copy link

netlify bot commented Apr 29, 2024

Deploy Preview for constellation-docs ready!

Name Link
🔨 Latest commit 6fbcba4
🔍 Latest deploy log https://app.netlify.com/sites/constellation-docs/deploys/6641daab41fbc8000879c7b3
😎 Deploy Preview https://deploy-preview-3051--constellation-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@daniel-weisse daniel-weisse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this design looks like a good approach to me.

Comment on lines +78 to +81
### SSH with Authorized Keys

We could ask users to add a public key to their `constellation-conf.yaml` and add that to `/root/.ssh/authorized_keys` after joining.
This would require the cluster owner to permanently manage a second secret, and there would be no built-in way to revoke access.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually used to do this, but decided to remove it in v2.2.0.
I like the proposed design a lot better, since it makes distributing the SSH access (i.e. the SSH CA) to other nodes of the cluster a lot easier.


An OpenSSH server is added to the node image software stack.
It's configured with a `TrustedUserCAKeys` file that is empty on startup.
After initialization, the bootstrapper fills that file with the derived CA's public key.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just as a side note: Worker nodes don't have access to the master secret, so we will need to generate and distribute the CA during the join process.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposing to only keep the CA in the join server, ptal


### Client-side Details

A new subcommand `constellation credentials ssh` is added to the CLI.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: We may want to use a different parent command name, e.g. key-gen or key so we can bundle things like data encryption key generation under this command.
credentials also works, but seems a bit less on topic for the other key gen operations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left the exact placement of the subcommand open for now.

The choice of curve allows to directly use the derived secret bytes as key.
This makes the implementation deterministic, and thus the CA key recoverable.

### Server-side Details
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also enable SSH host certificates on the Constellation nodes.
This should ensure a user is actually talking to a Constellation node over SSH, not just any VM that accepted the SSH key.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added host certs to the proposal.

Copy link
Member

@3u13r 3u13r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@thomasten thomasten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks solid to me.

Copy link
Contributor

@msanft msanft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this! Thanks for taking the time to spec all of this out. It will also save us some time in development if we don't have to use CSP Serial Consoles :)

### Server-side Details

An OpenSSH server is added to the node image software stack.
It's configured with a `TrustedUserCAKeys` file that is empty on startup.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also have some CRL-alike thing that'd enable an administrator to revoke access for a specific key? In theory, you'd have to throw away your cluster anyway, as it might've been backdoored in such a case. But for real-world scenarios, revoking keys might be handy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added KRL as an option for the admin - no real complexity added for us, but as mentioned it's probably not useful in general.

@burgerdev
Copy link
Contributor Author

This is good to go as far as I'm concerned. Please let me know until 2024-05-21 if you'd like to see modifications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog Change won't be listed in release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants