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

manifest: move sshd config fragments to overlay sshd_config.d on F32 #349

Merged
merged 2 commits into from
Apr 29, 2020
Merged

manifest: move sshd config fragments to overlay sshd_config.d on F32 #349

merged 2 commits into from
Apr 29, 2020

Conversation

bgilbert
Copy link
Contributor

@bgilbert bgilbert commented Apr 15, 2020

Fedora 32 supports sshd_config.d. Use it. This allows users to easily re-enable password authentication if desired.

We still need to disable the default AuthorizedKeysFile directive, since the Include directive appears after it in sshd_config.

On Fedora 31, the sshd_config.d fragments will be ignored, so continue to edit sshd_config there.

@jamescassell
Copy link

Don't suppose it would work on RHEL 8... But glad to see sshd finally has that support.

Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

LGTM

@dustymabe
Copy link
Member

@jlebon are we safe to merge things to next-devel without fear of them getting stomped on by config-bot ?

@jlebon
Copy link
Member

jlebon commented Apr 15, 2020

Right, this will indeed get stomped on by config-bot. See #180 (comment). As mentioned in #180 (comment), I think folding the overlay stuff into the manifest would be a cleaner way to solve this.

For now though... one hack is to make the postprocess scripts you're modifying just conditionalize on VERSION_ID and for the 05core bits, just emit those directly from manifest.yaml?

@bgilbert bgilbert changed the base branch from next-devel to testing-devel April 16, 2020 03:35
@bgilbert bgilbert changed the title [next-devel] manifest: move sshd config fragments to overlay sshd_config.d manifest: move sshd config fragments to overlay sshd_config.d Apr 16, 2020
@bgilbert bgilbert changed the title manifest: move sshd config fragments to overlay sshd_config.d manifest: move sshd config fragments to overlay sshd_config.d on F32 Apr 16, 2020
@bgilbert
Copy link
Contributor Author

Updated, rebased on testing-devel, and tested on top of both testing-devel and next-devel.

Copy link

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

manifests/fedora-coreos.yaml Outdated Show resolved Hide resolved
manifests/fedora-coreos-base.yaml Show resolved Hide resolved
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Some comments, but LGTM as is too!

manifests/fedora-coreos-base.yaml Outdated Show resolved Hide resolved
@@ -0,0 +1,5 @@
# This file is ignored on Fedora 31.
Copy link
Member

Choose a reason for hiding this comment

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

Ahh right, makes sense. I think we can drop these headers. We're moving off of f31 soon-ish anyway. :)

This is fine too, but we'll probably want to drop it at some point after we move into f32.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed that we'll want to drop eventually. I'd like to leave the headers for now, to avoid confusing users.

F31 and higher already default to PermitRootLogin prohibit-password.
Fedora 32 supports sshd_config.d.  Use it.  This allows users to easily
re-enable password authentication if desired.

We still need to disable the default AuthorizedKeysFile directive, since
the Include directive appears after it in sshd_config.

On Fedora 31, the sshd_config.d fragments will be ignored, so continue
to edit sshd_config there.
@bgilbert
Copy link
Contributor Author

Updated!

@bgilbert bgilbert merged commit 37ab510 into coreos:testing-devel Apr 29, 2020
@bgilbert bgilbert deleted the sshd_config branch April 29, 2020 21:19
Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants