-
Notifications
You must be signed in to change notification settings - Fork 38
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
Add authorized_keys to TrustedUserCAKeys for certificate-based authen… #18
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
… keypair & certificates
Always cool to see SSH-CA's being put to use @backjo 😺 That said, I'm not sure that this belongs in the base container image that we have today and that is used by default on Bottlerocket. The Without a mechanism to optionally enable this feature, I'm hesitant to merge and enable this by default and for all instances of the admin host container. |
Yeah - we're already using a custom image to achieve this, so it doesn't need to be merged and enabled to solve for us and our use case (though it would be nice to be on upstream :) ). For the user experience, I don't think adding this would add too much confusion. The error messages (in my experience testing, at least) don't change just because of the presence of this setting if users are attempting to login with standard public/private keypairs. The only time that the user would get a new message was if they attempted to sign in with a certificate that was invalid (either expired, an invalid signer, or invalid principal, primarily) - in which case, I think it'd be acceptable to assume that the user is aware of SSH certificates and that the error would more useful than confusing. I'd advocate that with the heavy focus on Security features in BottleRocket today (which is why we're leaning so heavily into it), having this feature would outweigh user confusion - at this point, mostly because it's unclear that any additional confusion would be caused for basic SSH users. It also sounds like creating a mechanism to pass flags into the admin container (maybe passing TOML settings under a certain prefix as env variables?) might be something to open an issue for. |
Hey @backjo! I wanted to follow up here because your use case is important to us, and I didn't want you to think we were just letting it sit. We're currently working on some improvements to the admin container, and the interface between the container and Bottlerocket, to make things like this more sustainable. Specifically, we're moving some AWS-specific logic into Bottlerocket, so it doesn't have to be repeated by alternate host containers and to make supporting other platforms easier as well. The admin container PR in particular handles adding TrustedUserCAKeys to sshd_config if the user specified a value in user data. (Similar to the flags you mentioned.) It's still undergoing some changes, but I'm hoping it will address your request here, and I'd be interested to get your thoughts on the approach. |
Hey @tjkirch - appreciate the update! The PR looks good - UserData makes a lot of sense! Only thing I would be ever so slightly worried about is the impact of ssh keys there to the overall size of the user data. But directionally, it totally seems to be something we'd use and get value out of :) |
@backjo Thanks! Yeah, user data is a limited resource, so we don't intend to store much there, but configuration that's important early in the startup process can be a good fit. (I'm working on bottlerocket-os/bottlerocket#1320 to allow compression and extend it a bit.) |
…tication
Issue number:
#17
Description of changes:
Allows the authorized_keys file to also be used for certificate-based authentication. For larger organizations, certificate based authentication is desirable because it allows for short-lived credentials and allows for the CA to be the decider of who has access to which machines.
Testing done:
Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.