From a82628186d84a4f2f49c51b1cb219cf482a3653e Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Wed, 17 Nov 2021 22:28:49 +0300 Subject: [PATCH] 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: * https://github.com/talos-systems/goipmi/pull/2 * https://github.com/talos-systems/goipmi/pull/3 Signed-off-by: Andrey Smirnov --- app/sidero-controller-manager/cmd/agent/main.go | 4 ++++ .../controllers/server_controller.go | 2 ++ .../internal/power/api/api.go | 5 +++++ .../internal/power/ipmi/ipmi.go | 11 +++++++++++ .../internal/power/metal/fake.go | 4 ++++ .../internal/power/metal/metal.go | 1 + go.mod | 2 +- go.sum | 4 ++-- 8 files changed, 30 insertions(+), 3 deletions(-) diff --git a/app/sidero-controller-manager/cmd/agent/main.go b/app/sidero-controller-manager/cmd/agent/main.go index 171f6932a..55018da47 100644 --- 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 index eb3585551..67ed57222 100644 --- 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 index 937e52b15..9d01029c2 100644 --- 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 index 6c7002b2f..e8f0200b1 100644 --- 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 index 400c6a012..de12e58d6 100644 --- 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 index 4aa8c58be..34f4e7734 100644 --- 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 index b59f6abf3..5f56c6251 100644 --- 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 index bb67e7b16..d41fc45c9 100644 --- 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=