From 19049280fae75d3cb76aad869915be25b0ba058e Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Wed, 8 May 2024 12:01:10 -0400 Subject: [PATCH 01/11] make: split lint and test targets We are going to have other linters soon and will also want to be able to run go tests without linting too. --- Makefile | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 657228cf8..48c1867f4 100644 --- a/Makefile +++ b/Makefile @@ -1,13 +1,19 @@ .DEFAULT_GOAL := help ## lint -lint: +lint: golangci-lint + +## golangci-lint +golangci-lint: go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.57 run --config .golangci.yml -## Go test -test: lint +## go test +go-test: CGO_ENABLED=0 go test -v -covermode=atomic ./... +## test +test: go-test lint + # https://gist.github.com/prwhite/8168133 # COLORS GREEN := $(shell tput -Txterm setaf 2) From 82baee957e06b9e09a5aff63747888c9e9754041 Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Wed, 8 May 2024 12:12:03 -0400 Subject: [PATCH 02/11] make: Add more descriptive help strings to targets --- Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 48c1867f4..caed6cd62 100644 --- a/Makefile +++ b/Makefile @@ -1,17 +1,17 @@ .DEFAULT_GOAL := help -## lint +## Run all linters lint: golangci-lint -## golangci-lint +## Run golangci-lint golangci-lint: go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.57 run --config .golangci.yml -## go test +## Run go test go-test: CGO_ENABLED=0 go test -v -covermode=atomic ./... -## test +## Run all tests and linters test: go-test lint # https://gist.github.com/prwhite/8168133 From a4de88d87ddf9f26eb9b3d297c78b0ddfcadf43b Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Fri, 10 May 2024 16:10:01 -0400 Subject: [PATCH 03/11] make: Install golangci-lint to $CWD/bin This way we start a pattern of installing them locally. Not a big deal for golangci-lint but will be useful later in this PR. --- .gitignore | 1 + Makefile | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 3bdf0aa57..58c6002c0 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ ### Go template +/bin/ # Test binary, build with `go test -c` *.test diff --git a/Makefile b/Makefile index caed6cd62..3bae06508 100644 --- a/Makefile +++ b/Makefile @@ -1,11 +1,15 @@ .DEFAULT_GOAL := help +export GOBIN=$(CURDIR)/bin +export PATH:=$(PATH):$(GOBIN) + ## Run all linters lint: golangci-lint ## Run golangci-lint golangci-lint: - go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.57 run --config .golangci.yml + go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.57 + golangci-lint run --config .golangci.yml ## Run go test go-test: From 4d73ac5a2c35c1356d4907112de0fbb85df51f67 Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Wed, 8 May 2024 12:07:23 -0400 Subject: [PATCH 04/11] ci: Use make targets in GHA We have make already, so lets make (no pun intended) more use of it. --- .github/workflows/push-pr-lint.yaml | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/.github/workflows/push-pr-lint.yaml b/.github/workflows/push-pr-lint.yaml index b4aea3206..13bd65dae 100644 --- a/.github/workflows/push-pr-lint.yaml +++ b/.github/workflows/push-pr-lint.yaml @@ -13,16 +13,13 @@ jobs: with: go-version-file: go.mod - - name: golangci-lint - uses: golangci/golangci-lint-action@v6 - with: - args: --config .golangci.yml - version: v1.57 + - name: Run golangci-lint + run: make golangci-lint - - name: Test - run: go test ./... + - name: Run go tests + run: make go-test - - name: Set up Docker Buildx + - name: Set up docker buildx uses: docker/setup-buildx-action@v3 - name: Build image - no push From b830e39caaf5444a002abb851e3a79cb3862a6e4 Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Thu, 9 May 2024 16:23:51 -0400 Subject: [PATCH 05/11] utils/watermark: Use stdlib/errors over github.com/pkg/errors The stdlib/errors is good enough, so lets use it. --- utils/watermark_disk.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/utils/watermark_disk.go b/utils/watermark_disk.go index 330ec2046..3cbfa9302 100644 --- a/utils/watermark_disk.go +++ b/utils/watermark_disk.go @@ -2,13 +2,12 @@ package utils import ( "crypto/rand" + "errors" "fmt" "io" "math/big" "os" "slices" - - "github.com/pkg/errors" ) var ErrIneffectiveWipe = errors.New("found left over data after wiping disk") From e3255012a2e95d0331c6617cd926f3b5373b2778 Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Thu, 9 May 2024 16:24:14 -0400 Subject: [PATCH 06/11] utils/watermark: Return better errors Lets return wrapped errors so we can figure out where things went wrong. --- utils/watermark_disk.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/utils/watermark_disk.go b/utils/watermark_disk.go index 3cbfa9302..7abe9ba92 100644 --- a/utils/watermark_disk.go +++ b/utils/watermark_disk.go @@ -41,20 +41,22 @@ func ApplyWatermarks(logicalName string) (func() error, error) { } defer file.Close() - for _, watermark := range watermarks { + for i, watermark := range watermarks { _, err = file.Seek(watermark.position, io.SeekStart) if err != nil { - return err + return fmt.Errorf("watermark verification, %s@%d(mark=%d), seek: %w", logicalName, watermark.position, i, err) } + // Read the watermark written to the position currentValue := make([]byte, watermarkSize) _, err = io.ReadFull(file, currentValue) if err != nil { - return err + return fmt.Errorf("read watermark %s@%d(mark=%d): %w", logicalName, watermark.position, i, err) } + // Check if the watermark is still in the disk if slices.Equal(currentValue, watermark.data) { - return fmt.Errorf("verify wipe %s@%d: %w", logicalName, watermark.position, ErrIneffectiveWipe) + return fmt.Errorf("verify wipe %s@%d(mark=%d): %w", logicalName, watermark.position, i, ErrIneffectiveWipe) } } return nil From f221fb7dc68c1a285497647eebbe0942c6eaac35 Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Thu, 9 May 2024 16:55:49 -0400 Subject: [PATCH 07/11] utils/watermark: Fix for loop terminating condition in writeWatermarks The terminating condition was previously wrong if fileSize was not a multiple of watermarksCount. In this case we'd have some left over bytes in an 11th chunk and the chunkStart < fileSize is true for the next iteration but seeking would fail. So lets iterate over the watermarksCount count instead. For example, here's a run where I came across this on a real nvme device (coming from the future ;D ): ``` chunkStart: 230454462870 fileSize: 256060514304 seek-to: 234037338277 end: 234037338789 chunkStart: 256060514300 fileSize: 256060514304 seek-to: 280780001714 end: 280780002226 ``` The final seek is past the end of the device and returns an error. --- utils/watermark_disk.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/watermark_disk.go b/utils/watermark_disk.go index 7abe9ba92..92b0b7343 100644 --- a/utils/watermark_disk.go +++ b/utils/watermark_disk.go @@ -84,7 +84,7 @@ func writeWatermarks(file *os.File, watermarksCount, watermarksSize int64) ([]wa chunkSize := fileSize / watermarksCount watermarks := make([]watermark, watermarksCount) - for chunkStart, i := int64(0), 0; chunkStart < fileSize; chunkStart, i = chunkStart+chunkSize, i+1 { + for chunkStart, i := int64(0), int64(0); i < watermarksCount; chunkStart, i = chunkStart+chunkSize, i+1 { data := make([]byte, watermarksSize) _, err := rand.Read(data) if err != nil { From 82bb156b7abf887e1f69055201e92d55e6ca25ad Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Wed, 24 Apr 2024 12:25:02 -0400 Subject: [PATCH 08/11] utils/nvme: Add support for sanitize based WipeDisk Uses the appropriate santize action according to detected capabilities, :toot:! --- utils/fake_executor.go | 36 +++++++++++ utils/nvme.go | 111 ++++++++++++++++++++++++++++++++- utils/nvme_test.go | 78 +++++++++++++++++++++++ utils/sanitizeaction_string.go | 27 ++++++++ 4 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 utils/sanitizeaction_string.go diff --git a/utils/fake_executor.go b/utils/fake_executor.go index 40dce0d36..f1e48f9a1 100644 --- a/utils/fake_executor.go +++ b/utils/fake_executor.go @@ -3,8 +3,10 @@ package utils import ( "bytes" "context" + "fmt" "io" "os" + "path" "strings" ) @@ -65,6 +67,40 @@ func (e *FakeExecute) Exec(_ context.Context) (*Result, error) { } e.Stdout = b + case "sanitize": + dev := e.Args[len(e.Args)-1] + f, err := os.OpenFile(dev, os.O_WRONLY, 0) + if err != nil { + return nil, err + } + size, err := f.Seek(0, io.SeekEnd) + if err != nil { + return nil, err + } + err = f.Truncate(0) + if err != nil { + return nil, err + } + err = f.Sync() + if err != nil { + return nil, err + } + err = f.Truncate(size) + if err != nil { + return nil, err + } + err = f.Sync() + if err != nil { + return nil, err + } + err = f.Close() + if err != nil { + return nil, err + } + case "sanitize-log": + dev := e.Args[len(e.Args)-1] + dev = path.Base(dev) + e.Stdout = []byte(fmt.Sprintf(`{%q:{"sprog":65535}}`, dev)) } case "hdparm": if e.Args[0] == "-I" { diff --git a/utils/nvme.go b/utils/nvme.go index 11be25632..401e3fd12 100644 --- a/utils/nvme.go +++ b/utils/nvme.go @@ -4,17 +4,25 @@ import ( "context" "encoding/json" "errors" + "fmt" + "io" "os" + "path" "strconv" "strings" + "time" "github.com/bmc-toolbox/common" "github.com/metal-toolbox/ironlib/model" + "github.com/sirupsen/logrus" ) const EnvNvmeUtility = "IRONLIB_UTIL_NVME" -var errSanicapNODMMASReserved = errors.New("sanicap nodmmas reserved bits set, not sure what to do with them") +var ( + errSanicapNODMMASReserved = errors.New("sanicap nodmmas reserved bits set, not sure what to do with them") + errSanitizeInvalidAction = errors.New("invalid sanitize action") +) type Nvme struct { Executor Executor @@ -259,6 +267,107 @@ func parseSanicap(sanicap uint) ([]*common.Capability, error) { return caps, nil } +//go:generate stringer -type SanitizeAction +type SanitizeAction uint8 + +const ( + Invalid SanitizeAction = iota + ExitFailureMode + BlockErase + Overwrite + CryptoErase +) + +// WipeDisk implements DiskWiper by running nvme sanitize +func (n *Nvme) WipeDisk(ctx context.Context, logger *logrus.Logger, device string) error { + caps, err := n.DriveCapabilities(ctx, device) + if err != nil { + return fmt.Errorf("WipeDisk: %w", err) + } + return n.wipe(ctx, logger, device, caps) +} + +func (n *Nvme) wipe(ctx context.Context, logger *logrus.Logger, device string, caps []*common.Capability) error { + var ber bool + var cer bool + for _, cap := range caps { + switch cap.Name { + case "ber": + ber = cap.Enabled + case "cer": + cer = cap.Enabled + } + } + + if cer { + l := logger.WithField("method", "sanitize").WithField("action", CryptoErase) + l.Info("trying wipe") + err := n.Sanitize(ctx, device, CryptoErase) + if err == nil { + return nil + } + l.WithError(err).Info("failed") + } + if ber { + l := logger.WithField("method", "sanitize").WithField("action", BlockErase) + l.Info("trying wipe") + err := n.Sanitize(ctx, device, BlockErase) + if err == nil { + return nil + } + l.WithError(err).Info("failed") + } + return ErrIneffectiveWipe +} + +func (n *Nvme) Sanitize(ctx context.Context, device string, sanact SanitizeAction) error { + switch sanact { // nolint:exhaustive + case BlockErase, CryptoErase: + default: + return fmt.Errorf("%w: %v", errSanitizeInvalidAction, sanact) + } + + verify, err := ApplyWatermarks(device) + if err != nil { + return err + } + + n.Executor.SetArgs("sanitize", "--sanact="+strconv.Itoa(int(sanact)), device) + _, err = n.Executor.Exec(ctx) + if err != nil { + return err + } + + // now we loop until sanitize-log reports that sanitization is complete + dev := path.Base(device) + var log map[string]struct { + Progress uint16 `json:"sprog"` + } + for { + n.Executor.SetArgs("sanitize-log", "--output-format=json", device) + result, err := n.Executor.Exec(ctx) + if err != nil { + return err + } + err = json.Unmarshal(result.Stdout, &log) + if err != nil { + return err + } + + l, ok := log[dev] + if !ok { + return fmt.Errorf("%s: device not present in sanitize-log: %w: %s", dev, io.ErrUnexpectedEOF, result.Stdout) + } + + if l.Progress == 65535 { + break + } + time.Sleep(100 * time.Millisecond) + } + + return verify() +} + // NewFakeNvme returns a mock nvme collector that returns mock data for use in tests. func NewFakeNvme() *Nvme { return &Nvme{ diff --git a/utils/nvme_test.go b/utils/nvme_test.go index 0382de281..9051a3425 100644 --- a/utils/nvme_test.go +++ b/utils/nvme_test.go @@ -2,10 +2,13 @@ package utils import ( "context" + "fmt" + "os" "strconv" "testing" "github.com/bmc-toolbox/common" + tlogrus "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -157,3 +160,78 @@ func Test_NvmeParseSanicap(t *testing.T) { require.Nil(t, caps) }) } + +func fakeNVMEDevice(t *testing.T) string { + dir := t.TempDir() + f, err := os.Create(dir + "/nvme0n1") + require.NoError(t, err) + require.NoError(t, f.Truncate(20*1024)) + require.NoError(t, f.Close()) + return f.Name() +} + +func Test_NvmeSanitize(t *testing.T) { + for action := range CryptoErase { + t.Run(action.String(), func(t *testing.T) { + n := NewFakeNvme() + dev := fakeNVMEDevice(t) + err := n.Sanitize(context.Background(), dev, action) + + switch action { // nolint:exhaustive + case BlockErase, CryptoErase: + require.NoError(t, err) + // FakeExecute is a bad mocker since it doesn't record all calls and sanitize-log calls aren't that interesting + // TODO: Setup better mocks + // + // e, ok := n.Executor.(*FakeExecute) + // require.True(t, ok) + // require.Equal(t, []string{"sanitize", "--sanact=2", dev}, e.Args) + default: + require.Error(t, err) + require.ErrorIs(t, err, errSanitizeInvalidAction) + } + }) + } +} + +func Test_NvmeWipe(t *testing.T) { + tests := []struct { + caps map[string]bool + args []string + }{ + {caps: map[string]bool{"ber": false, "cer": false}}, + {caps: map[string]bool{"ber": false, "cer": true}, args: []string{"sanitize", "--sanact=4"}}, + {caps: map[string]bool{"ber": true, "cer": false}, args: []string{"sanitize", "--sanact=2"}}, + {caps: map[string]bool{"ber": true, "cer": true}, args: []string{"sanitize", "--sanact=4"}}, + } + for _, test := range tests { + name := fmt.Sprintf("ber=%v,cer=%v", test.caps["ber"], test.caps["cer"]) + t.Run(name, func(t *testing.T) { + caps := []*common.Capability{ + {Name: "ber", Enabled: test.caps["ber"]}, + {Name: "cer", Enabled: test.caps["cer"]}, + } + n := NewFakeNvme() + dev := fakeNVMEDevice(t) + logger, hook := tlogrus.NewNullLogger() + defer hook.Reset() + + err := n.wipe(context.Background(), logger, dev, caps) + + if test.args == nil { + require.Error(t, err) + require.ErrorIs(t, err, ErrIneffectiveWipe) + return + } + + require.NoError(t, err) + // FakeExecute is a bad mocker since it doesn't record all calls and sanitize-log calls aren't that interesting + // TODO: Setup better mocks + // + // e, ok := n.Executor.(*FakeExecute) + // require.True(t, ok) + // test.args = append(test.args, dev) + // require.Equal(t, test.args, e.Args) + }) + } +} diff --git a/utils/sanitizeaction_string.go b/utils/sanitizeaction_string.go new file mode 100644 index 000000000..ca9c0232b --- /dev/null +++ b/utils/sanitizeaction_string.go @@ -0,0 +1,27 @@ +// Code generated by "stringer -type SanitizeAction"; DO NOT EDIT. + +package utils + +import "strconv" + +func _() { + // An "invalid array index" compiler error signifies that the constant values have changed. + // Re-run the stringer command to generate them again. + var x [1]struct{} + _ = x[Invalid-0] + _ = x[ExitFailureMode-1] + _ = x[BlockErase-2] + _ = x[Overwrite-3] + _ = x[CryptoErase-4] +} + +const _SanitizeAction_name = "InvalidExitFailureModeBlockEraseOverwriteCryptoErase" + +var _SanitizeAction_index = [...]uint8{0, 7, 22, 32, 41, 52} + +func (i SanitizeAction) String() string { + if i >= SanitizeAction(len(_SanitizeAction_index)-1) { + return "SanitizeAction(" + strconv.FormatInt(int64(i), 10) + ")" + } + return _SanitizeAction_name[_SanitizeAction_index[i]:_SanitizeAction_index[i+1]] +} From 943fae5b1f78d6812f0ff338950af0ab7f55c26f Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Wed, 24 Apr 2024 12:27:29 -0400 Subject: [PATCH 09/11] examples: Add nvme-wipe --- examples/nvme-wipe/.gitignore | 1 + examples/nvme-wipe/main.go | 40 +++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 examples/nvme-wipe/.gitignore create mode 100644 examples/nvme-wipe/main.go diff --git a/examples/nvme-wipe/.gitignore b/examples/nvme-wipe/.gitignore new file mode 100644 index 000000000..8ee415d0e --- /dev/null +++ b/examples/nvme-wipe/.gitignore @@ -0,0 +1 @@ +nvme-wipe diff --git a/examples/nvme-wipe/main.go b/examples/nvme-wipe/main.go new file mode 100644 index 000000000..96e26d745 --- /dev/null +++ b/examples/nvme-wipe/main.go @@ -0,0 +1,40 @@ +package main + +import ( + "context" + "flag" + "time" + + "github.com/metal-toolbox/ironlib/utils" + "github.com/sirupsen/logrus" +) + +var ( + device = flag.String("device", "/dev/nvmeX", "nvme disk to wipe") + verbose = flag.Bool("verbose", false, "show command runs and output") + dur = flag.String("timeout", (1 * time.Minute).String(), "time to wait for command to complete") +) + +func main() { + flag.Parse() + + logger := logrus.New() + logger.Formatter = new(logrus.TextFormatter) + logger.SetLevel(logrus.TraceLevel) + + timeout, err := time.ParseDuration(*dur) + if err != nil { + logger.WithError(err).Fatal("failed to parse timeout duration") + } + + nvme := utils.NewNvmeCmd(*verbose) + + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + + logger.Info("wiping") + err = nvme.WipeDisk(ctx, logger, *device) + if err != nil { + logger.WithError(err).Fatal("exiting") + } +} From 902d26dfdf5736faac5f53e20c7bcdcf46fddd44 Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Wed, 8 May 2024 12:11:23 -0400 Subject: [PATCH 10/11] make: Add target to verify generated files are up to date --- Makefile | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 3bae06508..b658411d1 100644 --- a/Makefile +++ b/Makefile @@ -4,13 +4,22 @@ export GOBIN=$(CURDIR)/bin export PATH:=$(PATH):$(GOBIN) ## Run all linters -lint: golangci-lint +lint: golangci-lint check-go-generated ## Run golangci-lint golangci-lint: go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.57 golangci-lint run --config .golangci.yml +## Run go generate +generate: + go install golang.org/x/tools/cmd/stringer@v0.21.0 + go generate ./... + +## Check generated files are up to date +check-go-generated: generate + git diff | (! grep .) + ## Run go test go-test: CGO_ENABLED=0 go test -v -covermode=atomic ./... From c9f2811f862b2c4d00fdb36e76972bff366377af Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Wed, 8 May 2024 12:11:50 -0400 Subject: [PATCH 11/11] ci: Verify go generated files are up to date --- .github/workflows/push-pr-lint.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/push-pr-lint.yaml b/.github/workflows/push-pr-lint.yaml index 13bd65dae..0a2b64b1d 100644 --- a/.github/workflows/push-pr-lint.yaml +++ b/.github/workflows/push-pr-lint.yaml @@ -16,6 +16,9 @@ jobs: - name: Run golangci-lint run: make golangci-lint + - name: Check go generated files + run: make check-go-generated + - name: Run go tests run: make go-test