Skip to content
Draft
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
13 changes: 13 additions & 0 deletions mantle/cmd/kola/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ func init() {
sv(&kola.QEMUOptions.SecureExecutionHostKey, "qemu-secex-hostkey", "", "Path to Secure Execution HKD certificate")
// s390x CEX-specific options
bv(&kola.QEMUOptions.Cex, "qemu-cex", false, "Attach CEX device to guest")

// No-Ignition mode: use pre-baked QCOW2 with SSH key in image (e.g. from bib)
bv(&kola.Options.NoIgnition, "no-ignition", false, "Do not inject Ignition; use pre-baked QCOW2 with SSH key (requires --qemu-image, QEMU only)")
sv(&kola.Options.SSHUser, "ssh-user", "", "SSH user for connections (e.g. root for no-Ignition images; default core)")
}

// Sync up the command line options if there is dependency
Expand Down Expand Up @@ -224,6 +228,15 @@ func syncOptionsImpl(useCosa bool) error {
return err
}

if kola.Options.NoIgnition {
if kolaPlatform != "qemu" {
return fmt.Errorf("--no-ignition is only supported with platform qemu")
}
if kola.QEMUOptions.DiskImage == "" {
return fmt.Errorf("--no-ignition requires --qemu-image (path to QCOW2 with SSH key pre-injected)")
}
}

// Choose an appropriate AWS instance type for the target architecture
if kolaPlatform == "aws" && kola.AWSOptions.InstanceType == "" {
switch kola.Options.CosaBuildArch {
Expand Down
5 changes: 5 additions & 0 deletions mantle/kola/harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -1754,6 +1754,11 @@ func runTest(h *harness.H, t *register.Test, pltfrm string, flight platform.Flig
SSHOnTestFailure: Options.SSHOnTestFailure,
WarningsAction: conf.FailWarnings,
EarlyRelease: h.Release,
NoIgnition: Options.NoIgnition,
SSHUser: Options.SSHUser,
}
if Options.NoIgnition && Options.SSHUser == "" {
rconf.SSHUser = "root"
}
if t.HasFlag(register.AllowConfigWarnings) {
rconf.WarningsAction = conf.IgnoreWarnings
Expand Down
8 changes: 8 additions & 0 deletions mantle/platform/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ func NewBaseCluster(bf *BaseFlight, rconf *RuntimeConfig) (*BaseCluster, error)
}

func (bc *BaseCluster) SSHClient(ip string) (*ssh.Client, error) {
if bc.rconf.SSHUser != "" {
return bc.UserSSHClient(ip, bc.rconf.SSHUser)
}
sshClient, err := bc.bf.agent.NewClient(ip)
if err != nil {
return nil, err
Expand Down Expand Up @@ -148,6 +151,11 @@ func (bc *BaseCluster) appendSSH(m Machine) error {
return err
}
}
if bc.rconf.SSHUser != "" {
if _, err := fmt.Fprintf(sshBuf, " User %s\n", bc.rconf.SSHUser); err != nil {
return err
}
}
Comment on lines +154 to +158
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

security-medium medium

The SSHUser command-line flag is written directly into the ssh-config file without sanitization. An attacker who can control the command-line arguments to kola can inject arbitrary SSH configuration options by including newlines in the SSHUser string. This can lead to arbitrary command execution if the ssh-config file is used by the user or another tool (e.g., via ProxyCommand).

if _, err := fmt.Fprintf(sshBuf, ` HostName %s
StrictHostKeyChecking no
UserKnownHostsFile /dev/null
Expand Down
41 changes: 25 additions & 16 deletions mantle/platform/machine/qemu/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,30 @@ func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options pl
// NOTE: escaping is not supported
qc.mu.Lock()

conf, err := qc.RenderUserData(userdata, map[string]string{})
if err != nil {
noIgnition := qc.RuntimeConf().NoIgnition
var conf *conf.Conf
var confPath string
var err error
if noIgnition {

qc.mu.Unlock()
return nil, err
} else {
conf, err = qc.RenderUserData(userdata, map[string]string{})
if err != nil {
qc.mu.Unlock()
return nil, err
}
qc.mu.Unlock()

if conf.IsIgnition() {
confPath = filepath.Join(dir, "ignition.json")
if err := conf.WriteFile(confPath); err != nil {
return nil, err
}
} else if !conf.IsEmpty() {
return nil, fmt.Errorf("qemu only supports Ignition or empty configs")
}
}
Comment on lines 71 to 96
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This block for handling Ignition is a bit complex and hard to follow due to the locking and branching. It can be simplified by restructuring the if condition and moving the lock to be more tightly scoped around the operation it protects. This will improve readability and maintainability.

noIgnition := qc.RuntimeConf().NoIgnition
	var conf *conf.Conf
	var confPath string
	var err error
	if !noIgnition {
		qc.mu.Lock()
		conf, err = qc.RenderUserData(userdata, map[string]string{})
		qc.mu.Unlock()
		if err != nil {
			return nil, err
		}

		if conf.IsIgnition() {
			confPath = filepath.Join(dir, "ignition.json")
			if err := conf.WriteFile(confPath); err != nil {
				return nil, err
			}
		} else if !conf.IsEmpty() {
			return nil, fmt.Errorf("qemu only supports Ignition or empty configs")
		}
	}

qc.mu.Unlock()

journal, err := platform.NewJournal(dir)
if err != nil {
Expand All @@ -94,24 +112,15 @@ func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options pl
builder.Pdeathsig = false
}

if qc.flight.opts.SecureExecution {
if !noIgnition && qc.flight.opts.SecureExecution {
if err := builder.SetSecureExecution(qc.flight.opts.SecureExecutionIgnitionPubKey, qc.flight.opts.SecureExecutionHostKey, conf); err != nil {
return nil, err
}
}

var confPath string
if conf.IsIgnition() {
confPath = filepath.Join(dir, "ignition.json")
if err := conf.WriteFile(confPath); err != nil {
return nil, err
}
} else if conf.IsEmpty() {
} else {
return nil, fmt.Errorf("qemu only supports Ignition or empty configs")
if !noIgnition {
builder.ConfigFile = confPath
}

builder.ConfigFile = confPath
defer builder.Close()
builder.UUID = qm.id
if qc.flight.opts.Arch != "" {
Expand Down
9 changes: 9 additions & 0 deletions mantle/platform/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,10 @@ type Options struct {
SSHOnTestFailure bool

ExtendTimeoutPercent uint

NoIgnition bool

SSHUser string
}

// RuntimeConfig contains cluster-specific configuration.
Expand All @@ -221,6 +225,11 @@ type RuntimeConfig struct {

// whether a Manhole into a machine should be created on detected failure
SSHOnTestFailure bool

// NoIgnition: when true, do not inject or pass Ignition (use pre-baked image with SSH key).
NoIgnition bool
// SSHUser: user for SSH connections (e.g. "root"); when set, SSHClient uses this user.
SSHUser string
}

// Wrap a StdoutPipe as a io.ReadCloser
Expand Down
Loading