Ignition Logs Configuration to journalctl

[captn3m0]

Bug

Ignition logs all parsed and fetched configuration to journalctl. This is a security risk for organizations which send all journalctl output to a central log storage. At the very least, using ignition_file for secure configurations (keys/secrets) must be warned against in the documentation.

Operating System Version

CoreOS-stable-1967.5.0-hvm

Ignition Version

0.28.0

Environment

AWS/ap-south-1/c5.large ec2 instance

Expected Behavior

Ignition should not log complete configuration to journalctl.

Actual Behavior

Ignition logs complete configuration to journalctl.

The simple journalctl --identifier=ignition --all command mentioned in the documentation gives the following 2 traces:

ignition/internal/providers/util/config.go

Line 25 in 3c7dbd3

logger.Debug("parsing config: %s", string(rawConfig))

ignition/internal/exec/engine.go

Line 264 in aad24ad

e.Logger.Debug("fetched referenced config: %s", string(rawCfg))

They show up as the following:

Feb 13 09:08:55 localhost ignition[422]: parsing config: {
Feb 13 09:08:55 localhost ignition[422]: parsing config: {"ignition":{"config":{"replace":{"source":"s3://eco-example-config/config.json","verification":{"hash":"sha512-9ff7f8f0bc00d37f32e013c792c3411b18db3dc9333881003ecc0f307150301a188b8fc9b6bc1016e9498db2be57f679eaaab86080ce814a8ac336981dc2a76c"}}},"timeouts":{},"version":"2.1.0"},"networkd":{},"passwd":{},"storage":{},"systemd":{}}
Feb 13 09:09:49 localhost ignition[472]: parsing config: {
Feb 13 05:09:49 localhost ignition[417]: fetched referenced config: {"ignition":{"config":{"append":[{"source":"data:text/plain;charset=utf-8;base64,eyJpZ25pd>
Feb 13 05:09:49 localhost ignition[417]: fetched referenced config: {"ignition":{"config":{},"timeouts":{},"version":"2.1.0"},"networkd":{},"passwd":{},"stora>
Feb 13 05:09:49 localhost ignition[417]: disks: op(1): [started]  waiting for udev to settle

While both are marked as Debug, the default configuration on latest CoreOS (CoreOS-stable-1967.5.0-hvm (ami-09642e32f99945765)) seems to be logging this.

[ajeddeloh]

Thanks for the report! There's a couple of ways to handle this:

1. Log only a hash of the config. This still enables users to verify Ignition fetched the correct config without actually revealing the contents
2. Log a sanitized config. Replace all the leaf nodes with placeholder values and log that instead. Again this allows verification that the config was fetched but doesn't leak any sensitive data

I'm leaning towards #1 to keep things simple.

Regardless of whether we choose 1 or 2, we should still log the full, unedited config in the case of failure. In those cases the OS will never reach the real root and thus will not be able to flush the runtime journal cache to disk. There's still the possibility of leaking secrets from the console, but on most clouds if you have access to the console you have access to the userdata and other sensitive bits anyway.

Finally if we wanted to add the ability to log the full console for debugging we could add a field to the ignition section of the config, but I think this is sufficiently low priority to not implement unless there is significant demand for it.

Thoughts on 1 vs 2 and logging in case of failure only?

[captn3m0]

There's still the possibility of leaking secrets from the console, but on most clouds if you have access to the console you have access to the userdata and other sensitive bits anyway.

We'd discussed this in Quentin-M/etcd-cloud-operator#31. The reason for the PR was that DescribeInstance on AWS is very widely given and has userdata read access. But the only possible workaround to avoid this (write ignition config to S3, and fetch config from S3 at runtime) does not work either (since fetched configs are logged anyway).

We have scenarios where people have userdata and not S3 access.

Logging on failure sounds good to me. I like (1) as an approach since it is easier to implement as well.

Aside: It would also be nice to log the fetched configuration URL (as long as it is not data-uri), since we are losing the easiest way to debug issues with ignition now.

[ajeddeloh]

Hmm. Thinking about this more we may need to sanitize Ignition's warning/info/deprecation messages as well. but that's significantly trickier. When Ignition encounters an error it logs the line and column as well as the offending config snippet (with some context). This snippet is from the raw config data, from before unmarshalling. If there is a non-fatal error at or around a secret then Ignition will log the snippet to the journal possibly containing the secret.

This is much trickier to fix since we still want that logic to print the literal string that was typed for tools like ignition-validate and because to sanitize we'd need to determine where in the original raw config data the possible secret is (which isn't easy). I suppose we could drop the context strings from Ignition and keep them in ignition-validate (i.e. just log line+column). With that we'd also need to ensure none of the validation code injects possible secrets into the error messages themselves.

Edit: now addressed in the linked PR. I'm just clearing them before logging

[bgilbert]

Apologies for the late comment. I think I'd prefer option 2; hashes of configs aren't great UX, and may not be super-helpful if the config is dynamically generated. It also avoids the logging distinction between success and failure.

(I suppose we could do both, for cases where the user wants to verify that the correct file contents are being written.)

[ajeddeloh]

I don't see a big problem with the logging distinction between success/failure. It's pretty common to have problems dump extra info when they fail.

I agree it's not ideal with automatically generated configs, but with the url being logged as well I think that's mostly mitigated.

Doing both is probably be the right path, but I'm not sure if logging the structure is worth it right now.

[cgwalters]

One random idea I had is the idea of a description element in Ignition that could be populated however the tooling wants. For https://github.com/openshift/machine-config-operator/ this would clearly be e.g. master-<hash>. People storing Ignition configs in git might put the output of git describe there.

[ajeddeloh]

That would double the size of the config if you wanted to log the config, yes? It'd also only be useful for spec 2.4.0-exp configs since 2.3.0 and below would need to populate the field during translation.

[ajeddeloh]

closed via #729 and #726