Fix fetch-offline logging interactions & vmware kernel lockdown

Fetch-Offline:
--------------

Right now, even if fetch-offline gets ErrNeedNet, it might've still
logged info about configs which it did fetch before hitting the error.
This then results in double-logging of e.g. the base config and at least
the first layer of user configs when fetch re-fetches them.

But it's also misleading, because anything which runs between
fetch-offline and fetch and sees the journal messages will think
that Ignition did successfully fetch and cache the merged user config,
when it did not.

And sadly, we still have code which peek at the cached config for
$reasons (legacy-style RHCOS LUKS is one of them, RHCOS FIPS support
is another), and those bits get thrown off by seeing the logging
messages yet not seeing a cached Ignition config.

Let's tweak things so that we buffer those messages and only actually
write them out once we've successfully acquired the configs.

While we're here, clean up the base config logging hack now that the
fetch stages are canonical.

VMware Kernel Lockdown:
-----------------------

This is a quickfix to avoid performing an `iopl`, which is blocked by
kernel_lockdown under SecureBoot.

Refs:
 * https://bugzilla.redhat.com/show_bug.cgi?id=1877995
 * https://github.com/lucab/vmw_backdoor-rs/issues/6
 * #1092

engine-fix-logging-interactions-with-fetch-offline.patch

@@ -0,0 +1,154 @@
+From ec4991892a5a4ed12ac1095322863c9fa8d73b65 Mon Sep 17 00:00:00 2001
+From: Jonathan Lebon <[email protected]>
+Date: Fri, 21 Aug 2020 16:32:11 -0400
+Subject: [PATCH] engine: fix logging interactions with fetch-offline
+
+Right now, even if `fetch-offline` gets `ErrNeedNet`, it might've still
+logged info about configs which it did fetch before hitting the error.
+This then results in double-logging of e.g. the base config and at least
+the first layer of user configs when `fetch` re-fetches them.
+
+But it's also misleading, because anything which runs between
+`fetch-offline` and `fetch` and sees the journal messages will think
+that Ignition did successfully fetch and cache the merged user config,
+when it did not.
+
+And sadly, we still have code which peek at the cached config for
+`$reasons` (legacy-style RHCOS LUKS is one of them, RHCOS FIPS support
+is another), and those bits get thrown off by seeing the logging
+messages yet not seeing a cached Ignition config.
+
+Let's tweak things so that we buffer those messages and only actually
+write them out once we've successfully acquired the configs.
+
+While we're here, clean up the base config logging hack now that the
+`fetch` stages are canonical.
+---
+ internal/exec/engine.go | 70 +++++++++++++++++++++++------------------
+ 1 file changed, 39 insertions(+), 31 deletions(-)
+
+diff --git a/internal/exec/engine.go b/internal/exec/engine.go
+index 7ac8052b..2c99801f 100644
+--- a/internal/exec/engine.go
++++ b/internal/exec/engine.go
+@@ -70,6 +70,13 @@ type Engine struct {
+ 	Root           string
+ 	PlatformConfig platform.Config
+ 	Fetcher        *resource.Fetcher
++	fetchedConfigs []fetchedConfig
++}
++
++type fetchedConfig struct {
++	kind       string
++	source     string
++	referenced bool
+ }
+
+ // Run executes the stage of the given name. It returns true if the stage
+@@ -86,25 +93,12 @@ func (e Engine) Run(stageName string) error {
+ 	if err != nil && err != providers.ErrNoProvider {
+ 		e.Logger.Crit("failed to acquire system base config: %v", err)
+ 		return err
+-	}
+-
+-	baseConfigFound := true
+-	if err == providers.ErrNoProvider {
+-		baseConfigFound = false
+-	}
+-
+-	configExists, err := executil.PathExists(e.ConfigCache)
+-	if err != nil {
+-		e.Logger.Info("failed to check for %s : %v", e.ConfigCache, err)
+-	}
+-	// This is a hack to emit the journald message
+-	// only once for the base config. Change this when
+-	// (https://github.com/coreos/ignition/issues/950)
+-	// is implemented.
+-	if !configExists && baseConfigFound {
+-		if err := logStructuredJournalEntry("base", "system", false); err != nil {
+-			e.Logger.Info("failed to log systemd journal entry: %v", err)
+-		}
++	} else if err == nil {
++		e.fetchedConfigs = append(e.fetchedConfigs, fetchedConfig{
++			kind:       "base",
++			source:     "system",
++			referenced: false,
++		})
+ 	}
+
+ 	// We special-case the fetch-offline stage a bit here: we want to be able
+@@ -149,18 +143,18 @@ func (e Engine) Run(stageName string) error {
+
+ // logStructuredJournalEntry logs information related to
+ // a user/base config into the systemd journal log.
+-func logStructuredJournalEntry(config string, src string, isReferenced bool) error {
++func logStructuredJournalEntry(cfgInfo fetchedConfig) error {
+ 	ignitionInfo := map[string]string{
+-		"IGNITION_CONFIG_TYPE":       config,
+-		"IGNITION_CONFIG_SRC":        src,
+-		"IGNITION_CONFIG_REFERENCED": strconv.FormatBool(isReferenced),
++		"IGNITION_CONFIG_TYPE":       cfgInfo.kind,
++		"IGNITION_CONFIG_SRC":        cfgInfo.source,
++		"IGNITION_CONFIG_REFERENCED": strconv.FormatBool(cfgInfo.referenced),
+ 		"MESSAGE_ID":                 ignitionFetchedConfigMsgId,
+ 	}
+ 	referenced := ""
+-	if isReferenced {
++	if cfgInfo.referenced {
+ 		referenced = "referenced "
+ 	}
+-	msg := fmt.Sprintf("fetched %s%s config from %q", referenced, config, src)
++	msg := fmt.Sprintf("fetched %s%s config from %q", referenced, cfgInfo.kind, cfgInfo.source)
+ 	if err := journal.Send(msg, journal.PriInfo, ignitionInfo); err != nil {
+ 		return err
+ 	}
+@@ -175,6 +169,15 @@ func (e *Engine) acquireConfig(stageName string) (cfg types.Config, err error) {
+ 	switch {
+ 	case strings.HasPrefix(stageName, "fetch"):
+ 		cfg, err = e.acquireProviderConfig()
++
++		// if we've successfully fetched and cached the configs, log about them
++		if err == nil {
++			for _, cfgInfo := range e.fetchedConfigs {
++				if logerr := logStructuredJournalEntry(cfgInfo); logerr != nil {
++					e.Logger.Info("failed to log systemd journal entry: %v", logerr)
++				}
++			}
++		}
+ 	default:
+ 		cfg, err = e.acquireCachedConfig()
+ 	}
+@@ -300,9 +303,12 @@ func (e *Engine) fetchProviderConfig() (types.Config, error) {
+ 		return types.Config{}, err
+ 	}
+
+-	if err := logStructuredJournalEntry("user", providerKey, false); err != nil {
+-		e.Logger.Info("failed to log systemd journal entry: %v", err)
+-	}
++	e.fetchedConfigs = append(e.fetchedConfigs, fetchedConfig{
++		kind:       "user",
++		source:     providerKey,
++		referenced: false,
++	})
++
+ 	// Replace the HTTP client in the fetcher to be configured with the
+ 	// timeouts of the config
+ 	err = e.Fetcher.UpdateHttpTimeoutsAndCAs(cfg.Ignition.Timeouts, cfg.Ignition.Security.TLS.CertificateAuthorities, cfg.Ignition.Proxy)
+@@ -402,9 +408,11 @@ func (e *Engine) fetchReferencedConfig(cfgRef types.Resource) (types.Config, err
+ 		e.Logger.Debug("fetched referenced config from data url with SHA512: %s", hex.EncodeToString(hash[:]))
+ 	}
+
+-	if err := logStructuredJournalEntry("user", u.Path, true); err != nil {
+-		e.Logger.Info("failed to log systemd journal entry: %v", err)
+-	}
++	e.fetchedConfigs = append(e.fetchedConfigs, fetchedConfig{
++		kind:       "user",
++		source:     u.Path,
++		referenced: true,
++	})
+
+ 	if err := util.AssertValid(cfgRef.Verification, rawCfg); err != nil {
+ 		return types.Config{}, err
+-- 
+2.21.1
+

fetch-don-t-run-if-fetch-offline-fetched-a-config.patch

@@ -0,0 +1,32 @@
+From ad76955a431baca2a8b6cc875b2633db9f27b12e Mon Sep 17 00:00:00 2001
+From: Jonathan Lebon <[email protected]>
+Date: Fri, 28 Aug 2020 17:27:24 -0400
+Subject: [PATCH] fetch: don't run if fetch-offline fetched a config
+
+It's possible that `fetch-offline` does successfully fetch and cache a
+config but still touch `/run/ignition/neednet`. That stamp file is an
+API between Ignition and the OS that Ignition will need networking to
+fully process it. But the config itself might've successfully been
+fetched (and the only thing that requires networking is e.g. a remote
+file). In those cases, there's no point in running `fetch` and have it
+query the provider again.
+---
+ dracut/30ignition/ignition-fetch.service | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/dracut/30ignition/ignition-fetch.service b/dracut/30ignition/ignition-fetch.service
+index 815208b4..c4b3e363 100644
+--- a/dracut/30ignition/ignition-fetch.service
++++ b/dracut/30ignition/ignition-fetch.service
+@@ -6,6 +6,8 @@ DefaultDependencies=false
+ Before=ignition-complete.target
+ After=basic.target
+ ConditionPathExists=/run/ignition/neednet
++# Don't run if the `fetch-offline` stage successfully fetched a config
++ConditionPathExists=!/run/ignition.json
+
+ # Stage order: setup -> fetch-offline [-> fetch] -> disks -> mount -> files.
+ # We run after the setup stage has run because it may copy in new/different
+-- 
+2.21.1
+

ignition.spec

@@ -64,14 +64,21 @@
 
 Name:           ignition
 Version:        2.6.0
-Release:        3.rhaos4.6.git%{shortcommit}%{?dist}
+Release:        4.rhaos4.6.git%{shortcommit}%{?dist}
 Summary:        First boot installer and configuration tool
 License:        ASL 2.0
 URL:            https://%{provider_prefix}
 Source0:        https://%{provider_prefix}/archive/%{commit}/%{repo}-%{shortcommit}.tar.gz
 # Fix sector size detection on s390x
 # https://github.com/coreos/ignition/pull/1070
 Patch0:         blkid-fix-invalid-pointer-cast-in-DumpDisk.patch
+# engine: fix logging interactions with fetch-offline
+# https://github.com/coreos/ignition/pull/1075
+Patch1:         fetch-don-t-run-if-fetch-offline-fetched-a-config.patch
+Patch2:         engine-fix-logging-interactions-with-fetch-offline.patch
+# vmware: kernel_lockdown breaks guestinfo fetching
+# https://github.com/coreos/ignition/issues/1092
+Patch3:         vendor-vmw-guestinfo-quickfix-to-skip-performing-iop.patch
 
 %define gopath %{_datadir}/gocode
 ExclusiveArch: x86_64 ppc64le aarch64 s390x
@@ -422,6 +429,9 @@ This package contains a tool for validating Ignition configurations.
 # unpack source0 and apply patches
 %setup -T -b 0 -q -n %{repo}-%{commit}
 %patch0 -p1
+%patch1 -p1
+%patch2 -p1
+%patch3 -p1
 
 %build
 # Set up PWD as a proper import path for go
@@ -555,6 +565,10 @@ export GOPATH=%{buildroot}/%{gopath}:$(pwd)/vendor:%{gopath}
 %endif
 
 %changelog
+* Fri Sep 18 2020 Stephen Lowrie <[email protected]> - 2.6.0-4.rhaos.46.git947598e
+- Fix logging interactions with fetch-offline
+- Avoid kernel lockdown on VMware when running with secure boot
+
 * Thu Aug 20 2020 Benjamin Gilbert <[email protected]> - 2.6.0-3.rhaos4.6.git947598e
 - Write SSH keys directly to authorized_keys, not to fragment file
 

vendor-vmw-guestinfo-quickfix-to-skip-performing-iop.patch

@@ -0,0 +1,41 @@
+From 069ab246129be6860aed3389c526543afa87e712 Mon Sep 17 00:00:00 2001
+From: Luca BRUNO <[email protected]>
+Date: Thu, 17 Sep 2020 16:07:59 +0000
+Subject: [PATCH] vendor/vmw-guestinfo: quickfix to skip performing iopl
+
+This is a quickfix to avoid performing an `iopl`, which is blocked by
+kernel_lockdown under SecureBoot.
+
+Refs:
+ * https://bugzilla.redhat.com/show_bug.cgi?id=1877995
+ * https://github.com/lucab/vmw_backdoor-rs/issues/6
+ * https://github.com/coreos/ignition/issues/1092
+---
+ .../vmware/vmw-guestinfo/vmcheck/vmcheck.go           | 11 +++++++----
+ 1 file changed, 7 insertions(+), 4 deletions(-)
+
+diff --git a/vendor/github.com/vmware/vmw-guestinfo/vmcheck/vmcheck.go b/vendor/github.com/vmware/vmw-guestinfo/vmcheck/vmcheck.go
+index c46cc5e4..ffd866c0 100644
+--- a/vendor/github.com/vmware/vmw-guestinfo/vmcheck/vmcheck.go
++++ b/vendor/github.com/vmware/vmw-guestinfo/vmcheck/vmcheck.go
+@@ -41,10 +41,13 @@ func IsVirtualWorld() (bool, error) {
+
+ // hypervisorPortCheck tests the availability of the HV port.
+ func hypervisorPortCheck() (bool, error) {
+-	// Privilege level 3 to access all ports above 0x3ff
+-	if err := openPortsAccess(); err != nil {
+-		return false, err
+-	}
++       // XXX(lucab): quickfix for https://github.com/coreos/ignition/issues/1092.
++       /*
++               // Privilege level 3 to access all ports above 0x3ff
++               if err := openPortsAccess(); err != nil {
++                       return false, err
++               }
++       */
+
+ 	p := &bdoor.BackdoorProto{}
+
+-- 
+2.21.1
+