fix: hide password from ipmitool args

Fixes #468 This removes IPMI password from command line arguments. Sidero was fixed to use `Open`/`Close` with goipmi client. See: * siderolabs/goipmi#2 * siderolabs/goipmi#3 Signed-off-by: Andrey Smirnov <andrey.smirnov@talos-systems.com>
siderolabs · Nov 17, 2021 · a826281 · a826281
1 parent ef270df
commit a826281
Show file tree

Hide file tree

Showing 8 changed files with 30 additions and 3 deletions.
diff --git a/app/sidero-controller-manager/cmd/agent/main.go b/app/sidero-controller-manager/cmd/agent/main.go
@@ -409,6 +409,8 @@ func attemptBMCIP(ctx context.Context, client api.AgentClient, s *smbios.SMBIOS)
 		return err
 	}
 
+	defer ipmiClient.Close() //nolint:errcheck
+
 	// Fetch BMC IP (param 3 in LAN config)
 	ipResp, err := ipmiClient.GetLANConfig(0x03)
 	if err != nil {
@@ -469,6 +471,8 @@ func attemptBMCUserSetup(ctx context.Context, client api.AgentClient, s *smbios.
 		return err
 	}
 
+	defer ipmiClient.Close() //nolint:errcheck
+
 	// Get user summary to see how many user slots
 	summResp, err := ipmiClient.GetUserSummary()
 	if err != nil {

diff --git a/app/sidero-controller-manager/controllers/server_controller.go b/app/sidero-controller-manager/controllers/server_controller.go
@@ -87,6 +87,8 @@ func (r *ServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 		return ctrl.Result{RequeueAfter: constants.DefaultRequeueAfter}, err
 	}
 
+	defer mgmtClient.Close() //nolint:errcheck
+
 	s.Status.Power = "off"
 
 	poweredOn, powerErr := mgmtClient.IsPoweredOn()

diff --git a/app/sidero-controller-manager/internal/power/api/api.go b/app/sidero-controller-manager/internal/power/api/api.go
@@ -29,6 +29,11 @@ func NewClient(spec metalv1alpha1.ManagementAPI) (*Client, error) {
 	}, nil
 }
 
+// Close the client.
+func (c *Client) Close() error {
+	return nil
+}
+
 func (c *Client) postRequest(path string) error {
 	failureMode := DefaultDice.Roll()
 

diff --git a/app/sidero-controller-manager/internal/power/ipmi/ipmi.go b/app/sidero-controller-manager/internal/power/ipmi/ipmi.go
@@ -5,6 +5,8 @@
 package ipmi
 
 import (
+	"fmt"
+
 	goipmi "github.com/pensando/goipmi"
 
 	metalv1alpha1 "github.com/talos-systems/sidero/app/sidero-controller-manager/api/v1alpha1"
@@ -34,9 +36,18 @@ func NewClient(bmcInfo metalv1alpha1.BMC) (*Client, error) {
 		return nil, err
 	}
 
+	if err = ipmiClient.Open(); err != nil {
+		return nil, fmt.Errorf("error opening client: %w", err)
+	}
+
 	return &Client{IPMIClient: ipmiClient}, nil
 }
 
+// Close the client.
+func (c *Client) Close() error {
+	return c.IPMIClient.Close()
+}
+
 // PowerOn will power on a given machine.
 func (c *Client) PowerOn() error {
 	return c.IPMIClient.Control(goipmi.ControlPowerUp)

diff --git a/app/sidero-controller-manager/internal/power/metal/fake.go b/app/sidero-controller-manager/internal/power/metal/fake.go
@@ -29,3 +29,7 @@ func (fakeClient) IsPoweredOn() (bool, error) {
 func (fakeClient) IsFake() bool {
 	return true
 }
+
+func (fakeClient) Close() error {
+	return nil
+}
diff --git a/app/sidero-controller-manager/internal/power/metal/metal.go b/app/sidero-controller-manager/internal/power/metal/metal.go
@@ -24,6 +24,7 @@ type ManagementClient interface {
 	IsPoweredOn() (bool, error)
 	SetPXE() error
 	IsFake() bool
+	Close() error
 }
 
 // NewManagementClient builds ManagementClient from the server spec.

diff --git a/go.mod b/go.mod
@@ -2,7 +2,7 @@ module github.com/talos-systems/sidero
 
 go 1.17
 
-replace github.com/pensando/goipmi v0.0.0-20200303170213-e858ec1cf0b5 => github.com/talos-systems/goipmi v0.0.0-20210504182258-b54796c8d678
+replace github.com/pensando/goipmi v0.0.0-20200303170213-e858ec1cf0b5 => github.com/talos-systems/goipmi v0.0.0-20211117190708-9fec3531c1bc
 
 require (
 	github.com/evanphx/json-patch v4.11.0+incompatible

diff --git a/go.sum b/go.sum
@@ -683,8 +683,8 @@ github.com/talos-systems/go-retry v0.3.1 h1:GjjyHB8i1CJpb1O5qYPMljq74cRQ5uiDoyMa
 github.com/talos-systems/go-retry v0.3.1/go.mod h1:HiXQqyVStZ35uSY/MTLWVvQVmC3lIW2MS5VdDaMtoKM=
 github.com/talos-systems/go-smbios v0.0.0-20210422124317-d3a32bea731a h1:uUAH6oFZwHdWRlHyBIsM8SEYU4fLM6KGu6bmPZOUKd8=
 github.com/talos-systems/go-smbios v0.0.0-20210422124317-d3a32bea731a/go.mod h1:HxhrzAoTZ7ed5Z5VvtCvnCIrOxyXDS7V2B5hCetAMW8=
-github.com/talos-systems/goipmi v0.0.0-20210504182258-b54796c8d678 h1:udj668k0XBRUBVATjkgjUfW25OS4/aH9sZnMQkmCOXE=
-github.com/talos-systems/goipmi v0.0.0-20210504182258-b54796c8d678/go.mod h1:Vr1Oadtcem03hG2RUT/dpSQS5md9d6rJ9nA0lUBC91Q=
+github.com/talos-systems/goipmi v0.0.0-20211117190708-9fec3531c1bc h1:LerxMYi6JGe5ilm+lohwXOaaeNwyefS/zGaiyrZcMDc=
+github.com/talos-systems/goipmi v0.0.0-20211117190708-9fec3531c1bc/go.mod h1:Vr1Oadtcem03hG2RUT/dpSQS5md9d6rJ9nA0lUBC91Q=
 github.com/talos-systems/net v0.3.0 h1:TG6PoiNdg9NmSeSjyecSgguUXzoJ8wp5a8RYlIdkq3Y=
 github.com/talos-systems/net v0.3.0/go.mod h1:VreSAyRmxMtqussAHSKMKkJQa1YwBTSVfkmE4Jydam4=
 github.com/talos-systems/talos/pkg/machinery v0.12.3/go.mod h1:qX77JMZawrDTQaJucqecdlFsHy+dbnZ9YL8Kw4qL7d4=