Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions overlay/usr/lib/systemd/system-preset/42-coreos.preset
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
enable coreos-growpart.service
enable console-login-helper-messages-issuegen.service
enable console-login-helper-messages-motdgen.service
# CA certs (probably to add to base fedora eventually)
enable coreos-update-ca-trust.service
# This one is from https://github.com/coreos/ignition-dracut
enable ignition-firstboot-complete.service
# Boot checkin services for cloud providers.
Expand Down
21 changes: 21 additions & 0 deletions overlay/usr/lib/systemd/system/coreos-update-ca-trust.service
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# This service is currently specific to Fedora CoreOS,
# but we may want to add it to the base OS in the future.
# The idea here is to allow users to just drop in CA roots
# via Ignition without having to know to run the special
# update command.
[Unit]
Description=Run update-ca-trust
ConditionFirstBoot=true
ConditionDirectoryNotEmpty=/etc/pki/ca-trust/source/anchors/
# We want to run quite early, in particular before anything
# that may speak TLS to external services. In the future,
# it may make sense to do this in the initramfs too.
Copy link
Contributor

Choose a reason for hiding this comment

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

WRT the initramfs, I'm not sure it's needed. The only things in there I can think of that might need TLS are Ignition (which supports custom certs in the ignition section) and afterburn, which is talking to cloud providers.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right coreos/ignition#636 is about this, but I can imagine people wanting to have the ones provided to Ignition persist for the host too.

But, this doesn't conflict with that; if Ignition writes it optionally, this service could read it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I 100% follow? Ignition does not write them to the initramfs root, it adds them to its HTTP client directly from the config. I also think it would be a little misleading for Ignition to write them out to the initramfs root since the ignition section is just for configuring Ignition, not the initramfs in general. I still don't see a use case for this either.

If people want them to persist to the host they should add storage.files entries. We could also add sugar to FCCT for this that generates both.

Choose a reason for hiding this comment

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

People have asked that Ignition persist the CA to disk, but I'm very much against this. The ignition section of the config directs Ignition; nothing more.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, no one is talking about changing that default, just adding a new mechanism to do both.

DefaultDependencies=no
Copy link
Member

Choose a reason for hiding this comment

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

This needs ConditionFirstBoot=yes too, right? I don't think we want users to rely on reboots magically having any new certs they added be extracted.

Copy link
Member Author

Choose a reason for hiding this comment

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

If we did this, we wouldn't be able to use this service for the MCD.

Copy link
Contributor

Choose a reason for hiding this comment

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

MCD doesn't run on FCOS right now. MCD could also add it's own drop in for turning ConditionFirstBoot=no if needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fine. Done.

Choose a reason for hiding this comment

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

Have we verified that systemd honors conditions in drop-ins? I seem to recall this not working in the past.

Copy link
Member Author

Choose a reason for hiding this comment

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

In playing with this, it seems to work to do ConditionFirstBoot= but not ConditionFirstBoot=no. Dunno exactly why, maybe something about "reset semantics" vs override?

Copy link

Choose a reason for hiding this comment

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

When a user adds a new cert via machineconfig, wouldn't that restart the hosts anyway? So, in that case, we would actually want this to run on firstboot only.


[Service]
ExecStart=/usr/bin/update-ca-trust extract
Type=oneshot
RemainAfterExit=yes

[Install]
WantedBy=basic.target