templates: Disable SSH keys lookup from authorized_keys.d on FCOS by LorbusChris · Pull Request #2087 · openshift/machine-config-operator

LorbusChris · 2020-09-15T14:02:17Z

- What I did
On FCOS, this sshd config dropin ensures that only SSH keys from the
/home/core/.ssh/authorized_keys file are picked up, and not ones
present in the /home/core/.ssh/authorized_keys.d/ directory,
which might be written by Ignition and/or Afterburn.

On RHCOS this is a no-op, as it already looks up SSH keys from the
authorized_keys file only.

- How to verify it
CI e2e testing

- Description for the changelog
templates: Disable SSH keys lookup from authorized_keys.d on FCOS

/hold
for testing

LorbusChris · 2020-09-15T14:03:03Z

/cc @bgilbert

ashcrow

The code makes sense 👍 . My only question is should this be done whenever update is called or should it be done once (bootstrap/firstboot)? I don't think it matters right now from the functionality perspective but still worth thinking about.

bgilbert

My only question is should this be done whenever update is called or should it be done once (bootstrap/firstboot)? I don't think it matters right now from the functionality perspective but still worth thinking about.

On some platforms, Afterburn may run on subsequent boots and recreate its fragment file.

Note that this PR makes a tradeoff: to avoid having to programmatically restart sshd, it disables authorized_keys.d at provisioning time. As a result, after an FCOS node first boots but before the MCO first syncs SSH keys, it will be impossible to SSH into the machine.

If we're comfortable with that tradeoff, there are a couple consequences:

Since we're disabling authorized_keys.d from a template, we don't actually need to remove the fragment files at all, except perhaps as documentation that they're not being used. On FCOS they'll never be read because of the sshd_config.d fragment. On RHCOS, we can simply not re-enable reading of .ssh/authorized_keys.d/ignition in sshd_config.
Furthermore, we could consider having Ignition on RHCOS continue to write the authorized_keys file directly. That would enable SSHing into an RHCOS node before the MCO SSH key sync runs, at the cost of divergent behavior between the OSes.

ashcrow · 2020-09-16T14:19:54Z

On some platforms, Afterburn may run on subsequent boots and recreate its fragment file.

Ah, great point 👍

LorbusChris · 2020-09-24T16:36:10Z

@bgilbert updated, PTAL.

bgilbert

@LorbusChris Any thoughts on #2087 (review)? I see this version is still deleting the fragment files.

LorbusChris · 2020-09-24T20:53:17Z

@bgilbert if we're ok with Ignition continuing to write directly to /home/core/.ssh/authorized_keys on RHCOS we could indeed leave the files in the .d dir. I'll update.

On FCOS, this sshd config dropin ensures that only SSH keys from the `/home/core/.ssh/authorized_keys` file are picked up, and not ones present in the `/home/core/.ssh/authorized_keys.d/` directory, which might be written by Ignition and/or Afterburn. On RHCOS this is a no-op, as it already looks up SSH keys from the authorized_keys file only.

bgilbert

LGTM, provided we're okay with the tradeoffs in #2087 (review).

cgwalters · 2020-09-24T22:44:33Z

Random aside, a lot of our thinking now is "rhcos == rhel8" but that will change - it's likely rhcos9 would potentially inherit any default changes we make to OpenSSH for example, so down the line ideally we'd do more "feature detection" rather than hardcode expectations that "rhcos" means a whole set of things.

ashcrow

LGTM too ... cc @yuqi-zhang @runcom

bgilbert · 2020-09-28T15:32:28Z

Random aside, a lot of our thinking now is "rhcos == rhel8" but that will change - it's likely rhcos9 would potentially inherit any default changes we make to OpenSSH for example

In this case there shouldn't be an issue. If future RHEL versions start reading sshd_config.d, this config fragment will be a no-op without the fragment it's overriding, and the latter is added by the ssh-key-dir package in fedora-coreos-base.yaml.

LorbusChris · 2020-10-06T11:49:23Z

/hold cancel
/retest

openshift-ci-robot · 2020-10-06T13:22:58Z

@LorbusChris: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-ovn-step-registry	`5ed6fa3`	link	`/test e2e-ovn-step-registry`
ci/prow/okd-e2e-aws	`5ed6fa3`	link	`/test okd-e2e-aws`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

ashcrow · 2020-10-23T14:16:24Z

/retest

LorbusChris · 2020-11-03T12:56:27Z

/retest

LorbusChris · 2020-11-03T16:50:31Z

/cc @yuqi-zhang @runcom

openshift-bot · 2020-11-12T12:11:24Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-11-12T12:34:00Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-11-12T13:26:08Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-11-12T15:10:47Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-11-12T15:23:10Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-11-12T16:28:01Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-11-12T17:19:58Z

/retest

Please review the full test history for this PR and help us cut down flakes.

LorbusChris · 2020-11-12T19:38:08Z

/cherry-pick release-4.6

openshift-cherrypick-robot · 2020-11-12T19:38:09Z

@LorbusChris: once the present PR merges, I will cherry-pick it on top of release-4.6 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-4.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-bot · 2020-11-12T19:55:27Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-11-13T02:12:30Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-11-18T11:20:20Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-11-19T00:46:04Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-11-19T02:30:04Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-11-19T02:56:08Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-11-19T03:22:12Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-11-19T10:18:16Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-11-19T10:32:13Z

/retest

Please review the full test history for this PR and help us cut down flakes.

LorbusChris · 2020-11-19T15:16:55Z

/refresh

openshift-merge-robot · 2020-11-19T17:23:58Z

@LorbusChris: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws	`5ed6fa3`	link	`/test e2e-aws`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

LorbusChris · 2020-11-19T21:13:16Z

/retest

openshift-cherrypick-robot · 2020-11-19T21:15:42Z

@LorbusChris: new pull request created: #2245

Details

In response to this:

/cherry-pick release-4.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 15, 2020

openshift-ci-robot requested review from ashcrow and ericavonb September 15, 2020 14:02

openshift-ci-robot requested a review from bgilbert September 15, 2020 14:03

ashcrow reviewed Sep 15, 2020

View reviewed changes

bgilbert reviewed Sep 16, 2020

View reviewed changes

Comment thread templates/common/_base/files/sshd-disable-authorized-keys-command.yaml Outdated

Comment thread templates/common/_base/files/sshd-disable-authorized-keys-command.yaml Outdated

LorbusChris force-pushed the rm-ssh-fragments branch from f70c681 to 910a66e Compare September 24, 2020 16:35

LorbusChris force-pushed the rm-ssh-fragments branch from 910a66e to 79ca51f Compare September 24, 2020 16:37

bgilbert reviewed Sep 24, 2020

View reviewed changes

Comment thread templates/common/_base/files/sshd-disable-ssh-key-dir.yaml Outdated

LorbusChris force-pushed the rm-ssh-fragments branch from 79ca51f to 05e5d9a Compare September 24, 2020 21:00

LorbusChris force-pushed the rm-ssh-fragments branch from 05e5d9a to 5ed6fa3 Compare September 24, 2020 21:01

LorbusChris changed the title ~~daemon: Disable and remove unmanaged SSH keys in authorized_keys.d~~ templates: Disable SSH keys lookup from authorized_keys.d on FCOS Sep 24, 2020

bgilbert approved these changes Sep 24, 2020

View reviewed changes

ashcrow approved these changes Sep 25, 2020

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 25, 2020

ashcrow requested a review from runcom September 28, 2020 16:07

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 6, 2020

openshift-ci-robot removed the request for review from ericavonb November 3, 2020 16:50

openshift-merge-robot merged commit 337c571 into openshift:master Nov 19, 2020

openshift-cherrypick-robot mentioned this pull request Nov 19, 2020

[release-4.6] Bug 1901187: templates: Disable SSH keys lookup from authorized_keys.d on FCOS #2245

Closed

vrutkovs mentioned this pull request Jan 21, 2021

Bug 1918677: templates: use valid binary in AuthorizedKeysCommand #2355

Closed

This was referenced Feb 2, 2021

/var/home/core/.ssh/authorized_keys not created by MCO during install (OKD 4.6/4.7) #2385

Closed

Revert disabling authorized_keys.d Pull 2087 #2387

Closed

revert Pull #2087 until issue #2385 is fixed #2388

Closed

fortinj66 mentioned this pull request Feb 9, 2021

OKD: Add authorized keys if missing after ignition #2393

Closed

fortinj66 mentioned this pull request Sep 28, 2021

Support ssh-key-dir on OKD/FCOS #2688

Merged

Conversation

LorbusChris commented Sep 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LorbusChris commented Sep 15, 2020

Uh oh!

ashcrow left a comment

Choose a reason for hiding this comment

Uh oh!

bgilbert left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ashcrow commented Sep 16, 2020

Uh oh!

LorbusChris commented Sep 24, 2020

Uh oh!

bgilbert left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LorbusChris commented Sep 24, 2020

Uh oh!

bgilbert left a comment

Choose a reason for hiding this comment

Uh oh!

cgwalters commented Sep 24, 2020

Uh oh!

ashcrow left a comment

Choose a reason for hiding this comment

Uh oh!

bgilbert commented Sep 28, 2020

Uh oh!

LorbusChris commented Oct 6, 2020

Uh oh!

openshift-ci-robot commented Oct 6, 2020

Uh oh!

ashcrow commented Oct 23, 2020

Uh oh!

LorbusChris commented Nov 3, 2020

Uh oh!

LorbusChris commented Nov 3, 2020

Uh oh!

openshift-bot commented Nov 12, 2020

Uh oh!

openshift-bot commented Nov 12, 2020

Uh oh!

openshift-bot commented Nov 12, 2020

Uh oh!

openshift-bot commented Nov 12, 2020

Uh oh!

openshift-bot commented Nov 12, 2020

Uh oh!

openshift-bot commented Nov 12, 2020

Uh oh!

openshift-bot commented Nov 12, 2020

Uh oh!

LorbusChris commented Nov 12, 2020

Uh oh!

openshift-cherrypick-robot commented Nov 12, 2020

Uh oh!

openshift-bot commented Nov 12, 2020

Uh oh!

openshift-bot commented Nov 13, 2020

Uh oh!

openshift-bot commented Nov 18, 2020

Uh oh!

openshift-bot commented Nov 19, 2020

Uh oh!

openshift-bot commented Nov 19, 2020

Uh oh!

openshift-bot commented Nov 19, 2020

Uh oh!

openshift-bot commented Nov 19, 2020

Uh oh!

openshift-bot commented Nov 19, 2020

Uh oh!

openshift-bot commented Nov 19, 2020

Uh oh!

LorbusChris commented Sep 15, 2020 •

edited

Loading