From 46069214d429d62987903189c98cd462db1dc170 Mon Sep 17 00:00:00 2001 From: Daniel Potapov Date: Sat, 13 May 2023 04:20:57 +0000 Subject: [PATCH 1/2] gen ID when machine-id file is empty Docker containers may have /etc/machine-id file with no contents. The change addresses this by ignoring empty files and generating a new ID as needed. --- helper.go | 25 ++++++---- id_linux.go | 52 +++++++++++++------ id_linux_test.go | 127 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 181 insertions(+), 23 deletions(-) create mode 100644 id_linux_test.go diff --git a/helper.go b/helper.go index ad7776e..f10db0b 100644 --- a/helper.go +++ b/helper.go @@ -1,9 +1,11 @@ package machineid import ( + "bytes" "crypto/hmac" "crypto/sha256" "encoding/hex" + "errors" "io" "io/ioutil" "os" @@ -35,19 +37,24 @@ func writeFile(filename string, data []byte) error { return ioutil.WriteFile(filename, data, 0644) } -// readFirstFile tries all the pathnames listed and returns the contents of the first readable file. +// readFirstFile tries all the pathnames listed and returns the contents of the first non-empty readable file. +// If all files are unreadable, it returns the error from the attempt to read the last file. +// The function also trims any leading and trailing white space characters from the file contents. func readFirstFile(pathnames []string) ([]byte, error) { - contents := []byte{} - var err error + lastErr := errors.New("no files provided") for _, pathname := range pathnames { - if pathname != "" { - contents, err = readFile(pathname) - if err == nil { - return contents, nil - } + contents, err := readFile(pathname) + if err != nil && lastErr != nil { + lastErr = err + continue + } + lastErr = nil + contents = bytes.TrimSpace(contents) + if len(contents) > 0 { + return contents, nil } } - return contents, err + return nil, lastErr } // writeFirstFile writes to the first file that "works" between all pathnames listed. diff --git a/id_linux.go b/id_linux.go index 914abe0..814a048 100644 --- a/id_linux.go +++ b/id_linux.go @@ -1,3 +1,4 @@ +//go:build linux // +build linux package machineid @@ -36,24 +37,47 @@ const ( // The logic implemented is a variation of the one described in https://github.com/denisbrodbeck/machineid/issues/5#issuecomment-523803164 // See also https://unix.stackexchange.com/questions/144812/generate-consistent-machine-unique-id func machineID() (string, error) { - env_pathname := os.Getenv(ENV_VARNAME) - - home := os.Getenv("HOME") - userMachineId := path.Join(home, ".config", "machine-id") + sp := searchPaths() + return lookupMachineID(sp) +} - id, err := readFirstFile([]string{ - env_pathname, dbusPath, dbusPathEtc, userMachineId, - }) +func lookupMachineID(sp []string) (string, error) { + b, err := readFirstFile(sp) if err != nil { - id, err = readFile(linuxRandomUuid) - if err == nil { - writeFirstFile([]string{ - env_pathname, dbusPathEtc, dbusPath, userMachineId, - }, id) - } + return "", err } + if len(b) == 0 { + return generateID(sp) + } + return string(b), nil +} + +func generateID(paths []string) (string, error) { + b, err := readFile(linuxRandomUuid) if err != nil { return "", err } - return trim(string(id)), nil + if err := writeFirstFile(paths, b); err != nil { + return "", err + } + return trim(string(b)), nil +} + +func searchPaths() []string { + paths := make([]string, 0, 4) + + env_pathname := os.Getenv(ENV_VARNAME) + if env_pathname != "" { + paths = append(paths, env_pathname) + } + + paths = append(paths, dbusPath, dbusPathEtc) + + home := os.Getenv("HOME") + if home != "" { + userMachineId := path.Join(home, ".config", "machine-id") + paths = append(paths, userMachineId) + } + + return paths } diff --git a/id_linux_test.go b/id_linux_test.go new file mode 100644 index 0000000..b85a884 --- /dev/null +++ b/id_linux_test.go @@ -0,0 +1,127 @@ +//go:build linux +// +build linux + +package machineid + +import ( + "os" + "reflect" + "testing" +) + +func TestMachineID(t *testing.T) { + id, err := machineID() + if err != nil { + t.Errorf("machineID() err = %v", err) + } + if len(id) == 0 { + t.Errorf("Got empty ID") + } +} + +func TestLookupMachineID(t *testing.T) { + emptyTempFile := makeTempFile(t, 0600) + defer os.Remove(emptyTempFile) + + // Test 1: when readFirstFile has bad files in the search list + paths := []string{"/nonexistent/directory", emptyTempFile, "/nonexistent/directory"} + + _, err := lookupMachineID(paths) + if err != nil { + t.Errorf("lookupMachineID() err = %v", err) + } + + // Test 2: when readFirstFile doesn't return error even if one of the files error'ed + paths = []string{"/nonexistent/directory", emptyTempFile} + _, err = lookupMachineID(paths) + if err != nil { + t.Errorf("lookupMachineID() err = %v", err) + } +} + +func TestGenerateID(t *testing.T) { + tempFile := makeTempFile(t, 0600) + defer os.Remove(tempFile) + + // Test 1: Test when readFile and writeFirstFile succeed + paths := []string{tempFile} + + id, err := generateID(paths) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if len(id) == 0 { + t.Errorf("Got empty ID") + } + b, err := readFile(tempFile) + if err != nil { + t.Errorf("Unable to read temp file: %v", err) + } + if id != trim(string(b)) { + t.Errorf("Generated ID was not written correctly into a file, want %v, got %v", id, string(b)) + } + + // Test 2: Test when it is unable to write to a file + tempFile = makeTempFile(t, 0400) + paths = []string{tempFile} + _, err = generateID(paths) + if err == nil { + t.Errorf("Expected error, got nil") + } + + // Test 3: Test when operating on non-existing file + paths = []string{"/nonexistent/directory"} + + _, err = generateID(paths) + if err == nil { + t.Errorf("Expected error, got nil") + } +} + +func TestSearchPaths(t *testing.T) { + // Save original environment variables + originalEnvPathname := os.Getenv(ENV_VARNAME) + originalHome := os.Getenv("HOME") + + defer func() { + // Restore original environment variables after test + os.Setenv(ENV_VARNAME, originalEnvPathname) + os.Setenv("HOME", originalHome) + }() + + // Test 1: ENV_VARNAME and HOME are not empty + os.Setenv(ENV_VARNAME, "/test/path") + os.Setenv("HOME", "/home/test") + + expected := []string{"/test/path", dbusPath, dbusPathEtc, "/home/test/.config/machine-id"} + + result := searchPaths() + + if !reflect.DeepEqual(result, expected) { + t.Errorf("Expected %v, got %v", expected, result) + } + + // Test 2: ENV_VARNAME and HOME are empty + os.Setenv(ENV_VARNAME, "") + os.Setenv("HOME", "") + + expected = []string{dbusPath, dbusPathEtc} + + result = searchPaths() + + if !reflect.DeepEqual(result, expected) { + t.Errorf("Expected %v, got %v", expected, result) + } +} + +func makeTempFile(t *testing.T, mode os.FileMode) string { + tempFile, err := os.CreateTemp("", "machineid_test") + if err != nil { + t.Fatalf("Failed to create temp file: %v", err) + } + _ = tempFile.Close() + if err := os.Chmod(tempFile.Name(), mode); err != nil { + t.Fatalf("Unable to set file mode: %v", err) + } + return tempFile.Name() +} From c54256ae3b8671f0e3279178269627ab3db45851 Mon Sep 17 00:00:00 2001 From: Daniel Potapov Date: Sat, 13 May 2023 04:23:40 +0000 Subject: [PATCH 2/2] fix redundant newline and applied gofmt --- cmd/machineid/main.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cmd/machineid/main.go b/cmd/machineid/main.go index 2646ced..d8e51ee 100644 --- a/cmd/machineid/main.go +++ b/cmd/machineid/main.go @@ -3,11 +3,13 @@ // Usage: machineid [options] // // Options: -// --appid Protect machine id by hashing it together with an app id. +// +// --appid Protect machine id by hashing it together with an app id. // // Try: -// machineid -// machineid --appid MyAppID +// +// machineid +// machineid --appid MyAppID package main import ( @@ -26,8 +28,7 @@ Options: Try: machineid - machineid --appid MyAppID -` + machineid --appid MyAppID` func usage() { log.Fatalln(usageStr)