diff --git a/commands/add_test.go b/commands/add_test.go index 3c2af86f..222a8ecf 100644 --- a/commands/add_test.go +++ b/commands/add_test.go @@ -93,11 +93,6 @@ func TestAddErrors(t *testing.T) { _, err = mockFs.Create(policy.SystemDefaultPolicyPath) require.NoError(t, err) - addCmd = MockAddCmd(mockFs) - - policyPath, err = addCmd.Run(principal, userEmail, issuer) - require.ErrorContains(t, err, "file has insecure permissions: expected one of the following permissions [640], got (0)") - require.Empty(t, policyPath) err = mockFs.Chmod(policy.SystemDefaultPolicyPath, 0640) require.NoError(t, err) diff --git a/commands/audit.go b/commands/audit.go index 098c866c..4b6bcc6e 100644 --- a/commands/audit.go +++ b/commands/audit.go @@ -20,7 +20,6 @@ import ( "encoding/json" "fmt" "io" - "io/fs" "os/user" "path/filepath" "strings" @@ -32,12 +31,11 @@ import ( // AuditCmd provides functionality to audit policy files against provider definitions type AuditCmd struct { - Fs afero.Fs - Out io.Writer - ErrOut io.Writer - filePermsChecker files.PermsChecker - ProviderLoader policy.ProviderLoader - CurrentUsername string + Fs files.FileSystem + Out io.Writer + ErrOut io.Writer + ProviderLoader policy.ProviderLoader + CurrentUsername string // Args ProviderPath string // Custom provider file path @@ -48,20 +46,16 @@ type AuditCmd struct { // NewAuditCmd creates a new AuditCmd with default settings func NewAuditCmd(out io.Writer, errOut io.Writer) *AuditCmd { - fs := afero.NewOsFs() return &AuditCmd{ - Fs: fs, + Fs: files.NewFileSystem(afero.NewOsFs()), Out: out, ErrOut: errOut, ProviderLoader: policy.NewProviderFileLoader(), CurrentUsername: getCurrentUsername(), - filePermsChecker: files.PermsChecker{ - Fs: fs, - CmdRunner: files.ExecCmd, - }, - ProviderPath: policy.SystemDefaultProvidersPath, - PolicyPath: policy.SystemDefaultPolicyPath, + ProviderPath: policy.SystemDefaultProvidersPath, + PolicyPath: policy.SystemDefaultPolicyPath, + SkipUserPolicy: false, } } @@ -90,7 +84,7 @@ func (a *AuditCmd) Audit(opksshVersion string) (*TotalResults, error) { validator := policy.NewPolicyValidator(providerPolicy) // Audit policy file - systemResults, exists, err := a.auditPolicyFileWithStatus(policyPath, []fs.FileMode{files.ModeSystemPerms}, validator) + systemResults, exists, err := a.auditPolicyFileWithStatus(policyPath, files.RequiredPerms.SystemPolicy, validator) if err != nil { return nil, fmt.Errorf("failed to audit policy file: %v", err) } @@ -105,39 +99,30 @@ func (a *AuditCmd) Audit(opksshVersion string) (*TotalResults, error) { } } - // Audit user policy file if it exists and not skipping + // Audit user policy files if not skipping if !a.SkipUserPolicy { - // We read /etc/passwd to enumerate all the home directories to find auth_id policy files. - var etcPasswdContent []byte - passwdPath := "/etc/passwd" - if exists, err := afero.Exists(a.Fs, passwdPath); !exists { - return nil, fmt.Errorf("failed to read /etc/passwd: /etc/passwd not found (needed to enumerate user home policies)") - } else if err != nil { - return nil, fmt.Errorf("failed to read /etc/passwd: %v", err) + homeDirs, err := a.enumerateUserHomeDirs() + if err != nil { + fmt.Fprintf(a.ErrOut, "warning: could not enumerate user home directories: %v\n", err) } else { - etcPasswdContent, err = afero.ReadFile(a.Fs, passwdPath) - if err != nil { - return nil, fmt.Errorf("failed to read /etc/passwd: %v", err) - } - } - homeDirs := getHomeDirsFromEtcPasswd(string(etcPasswdContent)) - for _, row := range homeDirs { - userPolicyPath := filepath.Join(row.HomeDir, ".opk", "auth_id") - - userResults, userExists, err := a.auditPolicyFileWithStatus(userPolicyPath, []fs.FileMode{files.ModeHomePerms}, validator) - if err != nil { - fmt.Fprintf(a.ErrOut, "failed to audit user policy file at %s: %v\n", userPolicyPath, err) - totalResults.HomePolicyFiles = append(totalResults.HomePolicyFiles, - PolicyFileResult{FilePath: userPolicyPath, Error: err.Error()}) - // Don't fail completely if user policy is unreadable - } else if userExists { - fmt.Fprintf(a.ErrOut, "\nvalidating %s...\n", userPolicyPath) - if !a.JsonOutput { - for _, result := range userResults.Rows { - a.printResult(result) + for _, row := range homeDirs { + userPolicyPath := filepath.Join(row.HomeDir, ".opk", "auth_id") + + userResults, userExists, err := a.auditPolicyFileWithStatus(userPolicyPath, files.RequiredPerms.HomePolicy, validator) + if err != nil { + fmt.Fprintf(a.ErrOut, "failed to audit user policy file at %s: %v\n", userPolicyPath, err) + totalResults.HomePolicyFiles = append(totalResults.HomePolicyFiles, + PolicyFileResult{FilePath: userPolicyPath, Error: err.Error()}) + // Don't fail completely if user policy is unreadable + } else if userExists { + fmt.Fprintf(a.ErrOut, "\nvalidating %s...\n", userPolicyPath) + if !a.JsonOutput { + for _, result := range userResults.Rows { + a.printResult(result) + } } + totalResults.HomePolicyFiles = append(totalResults.HomePolicyFiles, *userResults) } - totalResults.HomePolicyFiles = append(totalResults.HomePolicyFiles, *userResults) } } } @@ -189,29 +174,31 @@ func (a *AuditCmd) Run(opksshVersion string) error { } // auditPolicyFileWithStatus validates all entries in a policy file and returns results, whether file exists, and any errors -func (a *AuditCmd) auditPolicyFileWithStatus(policyPath string, requiredPerms []fs.FileMode, validator *policy.PolicyValidator) (*PolicyFileResult, bool, error) { +func (a *AuditCmd) auditPolicyFileWithStatus(policyPath string, permInfo files.PermInfo, validator *policy.PolicyValidator) (*PolicyFileResult, bool, error) { results := &PolicyFileResult{ FilePath: policyPath, Rows: []policy.ValidationRowResult{}, } - // Check if file exists - exists, err := afero.Exists(a.Fs, policyPath) - if err != nil { - return nil, false, fmt.Errorf("failed to check if policy file exists: %w", err) + // Use shared permission checking logic + permResult := CheckFilePermissions(a.Fs, policyPath, permInfo) + if !permResult.Exists { + return results, false, nil } - if !exists { - // File doesn't exist, return empty results with exists=false - return results, false, nil + if permResult.PermsErr != "" { + results.PermsError = permResult.PermsErr } - if permsErr := a.filePermsChecker.CheckPerm(policyPath, requiredPerms, "", ""); permsErr != nil { - results.PermsError = permsErr.Error() + // Report ACL problems to stderr + if permResult.ACLReport != nil && permResult.ACLErr == nil { + for _, problem := range permResult.ACLReport.Problems { + fmt.Fprintf(a.ErrOut, " ACL issue: %s\n", problem) + } } // Load policy file - content, err := afero.ReadFile(a.Fs, policyPath) + content, err := a.Fs.ReadFile(policyPath) if err != nil { return nil, true, fmt.Errorf("failed to read policy file: %w", err) } @@ -306,7 +293,7 @@ func getCurrentUsername() string { return u.Username } -type etcPasswdRow struct { +type userHomeEntry struct { Username string HomeDir string } @@ -314,8 +301,8 @@ type etcPasswdRow struct { // getHomeDirsFromEtcPasswd parses /etc/passwd and returns a list of usernames // and their associated home directories. This is not sufficient for all home // directories as it does not consider home directories specified by NSS. -func getHomeDirsFromEtcPasswd(etcPasswd string) []etcPasswdRow { - entries := []etcPasswdRow{} +func getHomeDirsFromEtcPasswd(etcPasswd string) []userHomeEntry { + entries := []userHomeEntry{} for _, line := range strings.Split(etcPasswd, "\n") { if line == "" || strings.HasPrefix(line, "#") { continue @@ -330,7 +317,7 @@ func getHomeDirsFromEtcPasswd(etcPasswd string) []etcPasswdRow { continue } - entry := etcPasswdRow{Username: parts[0], HomeDir: parts[5]} + entry := userHomeEntry{Username: parts[0], HomeDir: parts[5]} entries = append(entries, entry) } return entries diff --git a/commands/audit_enum_unix.go b/commands/audit_enum_unix.go new file mode 100644 index 00000000..e073f0a8 --- /dev/null +++ b/commands/audit_enum_unix.go @@ -0,0 +1,44 @@ +//go:build !windows +// +build !windows + +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package commands + +import ( + "fmt" +) + +// enumerateUserHomeDirs returns the list of user home directories by reading +// /etc/passwd. This is the Unix implementation. +func (a *AuditCmd) enumerateUserHomeDirs() ([]userHomeEntry, error) { + passwdPath := "/etc/passwd" + exists, err := a.Fs.Exists(passwdPath) + if err != nil { + return nil, fmt.Errorf("failed to check /etc/passwd: %w", err) + } + if !exists { + return nil, fmt.Errorf("/etc/passwd not found (needed to enumerate user home policies)") + } + + etcPasswdContent, err := a.Fs.ReadFile(passwdPath) + if err != nil { + return nil, fmt.Errorf("failed to read /etc/passwd: %w", err) + } + + return getHomeDirsFromEtcPasswd(string(etcPasswdContent)), nil +} diff --git a/commands/audit_enum_unix_test.go b/commands/audit_enum_unix_test.go new file mode 100644 index 00000000..a6334226 --- /dev/null +++ b/commands/audit_enum_unix_test.go @@ -0,0 +1,65 @@ +//go:build !windows +// +build !windows + +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package commands + +import ( + "bytes" + "testing" + + "github.com/openpubkey/opkssh/policy/files" + "github.com/spf13/afero" + "github.com/stretchr/testify/require" +) + +func TestEnumerateUserHomeDirs_Unix(t *testing.T) { + t.Parallel() + + vfs := afero.NewMemMapFs() + passwdContent := "root:x:0:0:root:/root:/bin/bash\nalice:x:1000:1000::/home/alice:/bin/sh\n" + require.NoError(t, afero.WriteFile(vfs, "/etc/passwd", []byte(passwdContent), 0o644)) + + cmd := &AuditCmd{ + Fs: files.NewFileSystem(vfs, files.WithCmdRunner(func(string, ...string) ([]byte, error) { return nil, nil })), + Out: &bytes.Buffer{}, + } + + entries, err := cmd.enumerateUserHomeDirs() + require.NoError(t, err) + require.Len(t, entries, 2) + require.Equal(t, "root", entries[0].Username) + require.Equal(t, "/root", entries[0].HomeDir) + require.Equal(t, "alice", entries[1].Username) + require.Equal(t, "/home/alice", entries[1].HomeDir) +} + +func TestEnumerateUserHomeDirs_Unix_MissingPasswd(t *testing.T) { + t.Parallel() + + vfs := afero.NewMemMapFs() + + cmd := &AuditCmd{ + Fs: files.NewFileSystem(vfs, files.WithCmdRunner(func(string, ...string) ([]byte, error) { return nil, nil })), + Out: &bytes.Buffer{}, + } + + _, err := cmd.enumerateUserHomeDirs() + require.Error(t, err) + require.Contains(t, err.Error(), "/etc/passwd not found") +} diff --git a/commands/audit_enum_windows.go b/commands/audit_enum_windows.go new file mode 100644 index 00000000..6abc5f05 --- /dev/null +++ b/commands/audit_enum_windows.go @@ -0,0 +1,26 @@ +//go:build windows +// +build windows + +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package commands + +// enumerateUserHomeDirs returns the list of user home directories by reading +// the Windows registry ProfileList. This is the Windows implementation. +func (a *AuditCmd) enumerateUserHomeDirs() ([]userHomeEntry, error) { + return getHomeDirsFromProfileList() +} diff --git a/commands/audit_enum_windows_test.go b/commands/audit_enum_windows_test.go new file mode 100644 index 00000000..406170cf --- /dev/null +++ b/commands/audit_enum_windows_test.go @@ -0,0 +1,49 @@ +//go:build windows +// +build windows + +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package commands + +import ( + "bytes" + "testing" + + "github.com/openpubkey/opkssh/policy/files" + "github.com/spf13/afero" + "github.com/stretchr/testify/require" +) + +func TestEnumerateUserHomeDirs_Windows(t *testing.T) { + t.Parallel() + + vfs := afero.NewMemMapFs() + cmd := &AuditCmd{ + Fs: files.NewFileSystem(vfs, files.WithCmdRunner(func(string, ...string) ([]byte, error) { return nil, nil })), + Out: &bytes.Buffer{}, + } + + entries, err := cmd.enumerateUserHomeDirs() + require.NoError(t, err) + // On a running Windows machine there should be at least one user profile + require.NotEmpty(t, entries, "expected at least one user profile from Windows registry") + + for _, e := range entries { + require.NotEmpty(t, e.Username, "username should not be empty") + require.NotEmpty(t, e.HomeDir, "home directory should not be empty") + } +} diff --git a/commands/audit_permissions_test.go b/commands/audit_permissions_test.go new file mode 100644 index 00000000..359724f5 --- /dev/null +++ b/commands/audit_permissions_test.go @@ -0,0 +1,152 @@ +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package commands + +import ( + "bytes" + "path/filepath" + "runtime" + "testing" + + "github.com/openpubkey/opkssh/policy" + "github.com/openpubkey/opkssh/policy/files" + "github.com/spf13/afero" + "github.com/stretchr/testify/require" +) + +// TestAuditAndPermissionsCheckConsistency verifies that the audit and +// permissions check commands report consistent results when examining the +// same system policy file. Both commands now use the shared +// CheckFilePermissions function, so their findings should agree. +func TestAuditAndPermissionsCheckConsistency(t *testing.T) { + t.Parallel() + + // Set up a shared in-memory filesystem with correct permissions. + // This test verifies that both audit and the shared permission checker + // report the same results for a well-configured system policy file. + vfs := afero.NewMemMapFs() + fsys := files.NewFileSystem(vfs, files.WithCmdRunner(func(name string, arg ...string) ([]byte, error) { + return []byte("root opksshuser"), nil + })) + + providerPath := policy.SystemDefaultProvidersPath + policyPath := policy.SystemDefaultPolicyPath + basePath := policy.GetSystemConfigBasePath() + + require.NoError(t, vfs.MkdirAll(filepath.Dir(providerPath), 0o750)) + require.NoError(t, vfs.MkdirAll(filepath.Dir(policyPath), 0o750)) + + providerContent := "https://accounts.google.com google-client-id 24h\n" + policyContent := "root alice@example.com https://accounts.google.com\n" + + require.NoError(t, afero.WriteFile(vfs, providerPath, []byte(providerContent), 0o640)) + require.NoError(t, afero.WriteFile(vfs, policyPath, []byte(policyContent), 0o640)) + + // Also create files expected by permissions check + require.NoError(t, afero.WriteFile(vfs, filepath.Join(basePath, "providers"), []byte(providerContent), 0o640)) + require.NoError(t, vfs.MkdirAll(filepath.Join(basePath, "policy.d"), 0o750)) + + // --- Run shared CheckFilePermissions (used by both commands) --- + sp := files.RequiredPerms.SystemPolicy + permResult := CheckFilePermissions(fsys, policyPath, sp) + require.True(t, permResult.Exists, "system policy file should exist") + require.Empty(t, permResult.PermsErr, + "CheckFilePermissions should report no perms error for correct permissions") + + // --- Run audit command --- + stdOut := &bytes.Buffer{} + errOut := &bytes.Buffer{} + + auditCmd := AuditCmd{ + Fs: fsys, + Out: stdOut, + ErrOut: errOut, + ProviderLoader: &MockProviderLoader{content: providerContent, t: t}, + CurrentUsername: "testuser", + ProviderPath: providerPath, + PolicyPath: policyPath, + SkipUserPolicy: true, + } + + totalResults, err := auditCmd.Audit("test_version") + require.NoError(t, err) + require.NotNil(t, totalResults) + + // Both should agree: no permission errors for correctly configured file + require.Empty(t, totalResults.SystemPolicyFile.PermsError, + "audit should report no perms error for correct permissions") + + // Both should agree on the permission error message + require.Equal(t, permResult.PermsErr, totalResults.SystemPolicyFile.PermsError, + "CheckFilePermissions and audit should report the same permission error") +} + +// TestAuditAndPermissionsCheckBadPerms verifies that both the audit command +// and CheckFilePermissions detect incorrect file permissions. This test only +// runs on Unix where PermsChecker enforces file mode bits; on Windows, +// permission enforcement is done through ACLs rather than mode bits. +func TestAuditAndPermissionsCheckBadPerms(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows PermsChecker does not enforce mode bits; ACLs are used instead") + } + t.Parallel() + + vfs := afero.NewMemMapFs() + fsys := files.NewFileSystem(vfs, files.WithCmdRunner(func(name string, arg ...string) ([]byte, error) { + return []byte("root opksshuser"), nil + })) + + providerPath := policy.SystemDefaultProvidersPath + policyPath := policy.SystemDefaultPolicyPath + + require.NoError(t, vfs.MkdirAll(filepath.Dir(providerPath), 0o750)) + require.NoError(t, vfs.MkdirAll(filepath.Dir(policyPath), 0o750)) + + providerContent := "https://accounts.google.com google-client-id 24h\n" + policyContent := "root alice@example.com https://accounts.google.com\n" + + require.NoError(t, afero.WriteFile(vfs, providerPath, []byte(providerContent), 0o640)) + // Intentionally wrong permissions + require.NoError(t, afero.WriteFile(vfs, policyPath, []byte(policyContent), 0o777)) + + sp := files.RequiredPerms.SystemPolicy + + // Both should detect the wrong permissions + permResult := CheckFilePermissions(fsys, policyPath, sp) + require.True(t, permResult.Exists) + require.NotEmpty(t, permResult.PermsErr, "should detect wrong permissions") + + stdOut := &bytes.Buffer{} + errOut := &bytes.Buffer{} + auditCmd := AuditCmd{ + Fs: fsys, + Out: stdOut, + ErrOut: errOut, + ProviderLoader: &MockProviderLoader{content: providerContent, t: t}, + CurrentUsername: "testuser", + ProviderPath: providerPath, + PolicyPath: policyPath, + SkipUserPolicy: true, + } + totalResults, err := auditCmd.Audit("test_version") + require.NoError(t, err) + require.NotEmpty(t, totalResults.SystemPolicyFile.PermsError, "audit should detect wrong permissions") + + // Both should report the same error + require.Equal(t, permResult.PermsErr, totalResults.SystemPolicyFile.PermsError, + "CheckFilePermissions and audit should report the same permission error") +} diff --git a/commands/audit_test.go b/commands/audit_test.go index 921b60b8..5a7acb40 100644 --- a/commands/audit_test.go +++ b/commands/audit_test.go @@ -65,15 +65,25 @@ func SetupAuditCmdMocks(t *testing.T, etcPasswdContent string, providerContent s // Create in-memory filesystem fs := afero.NewMemMapFs() - err := afero.WriteFile(fs, "/etc/passwd", []byte(etcPasswdContent), 0640) - require.NoError(t, err) + providerPath := policy.SystemDefaultProvidersPath + policyPath := policy.SystemDefaultPolicyPath + + // Ensure parent directories exist (required for Windows paths like C:\ProgramData\opk\) + require.NoError(t, fs.MkdirAll(filepath.Dir(providerPath), 0750)) + require.NoError(t, fs.MkdirAll(filepath.Dir(policyPath), 0750)) + + // Write /etc/passwd only if content is provided (Unix only; not used on Windows) + if etcPasswdContent != "" { + err := afero.WriteFile(fs, "/etc/passwd", []byte(etcPasswdContent), 0640) + require.NoError(t, err) + } // Create provider file - err = afero.WriteFile(fs, "/etc/opk/providers", []byte(providerContent), 0640) + err := afero.WriteFile(fs, providerPath, []byte(providerContent), 0640) require.NoError(t, err) // Create auth_id file - err = afero.WriteFile(fs, "/etc/opk/auth_id", []byte(systemPolicyContent), 0640) + err = afero.WriteFile(fs, policyPath, []byte(systemPolicyContent), 0640) require.NoError(t, err) // Mock provider loader @@ -84,16 +94,12 @@ func SetupAuditCmdMocks(t *testing.T, etcPasswdContent string, providerContent s // Create audit command return AuditCmd{ - Fs: fs, + Fs: files.NewFileSystem(fs, files.WithCmdRunner(func(name string, arg ...string) ([]byte, error) { + return []byte("root opksshuser"), nil + })), ProviderLoader: mockLoader, - filePermsChecker: files.PermsChecker{ - Fs: fs, - CmdRunner: func(name string, arg ...string) ([]byte, error) { - return []byte("root" + " " + "opkssh"), nil - }, - }, - ProviderPath: "/etc/opk/providers", - PolicyPath: "/etc/opk/auth_id", + ProviderPath: providerPath, + PolicyPath: policyPath, } } @@ -129,7 +135,7 @@ func TestAuditCmd(t *testing.T) { "Total Entries Tested: 2", }, expectedStdErrContains: []string{ - "validating /etc/opk/auth_id", + "validating " + strings.ReplaceAll(policy.SystemDefaultPolicyPath, string(filepath.Separator), "/"), }, }, { @@ -175,7 +181,7 @@ func TestAuditCmd(t *testing.T) { }, expectedStdErrContains: []string{ "no policy entries", - "validating /etc/opk/auth_id", + "validating " + strings.ReplaceAll(policy.SystemDefaultPolicyPath, string(filepath.Separator), "/"), }, }, { @@ -336,18 +342,18 @@ func TestAuditCmdValidationResults(t *testing.T) { func TestGetHomeDirsFromEtcPasswd(t *testing.T) { t.Parallel() - etcPasswdRows := getHomeDirsFromEtcPasswd(string(etcPasswdMock)) + homeDirs := getHomeDirsFromEtcPasswd(string(etcPasswdMock)) - require.Len(t, etcPasswdRows, 5) + require.Len(t, homeDirs, 5) - require.Equal(t, "root", etcPasswdRows[0].Username) - require.Equal(t, "/root", etcPasswdRows[0].HomeDir) - require.Equal(t, "dev", etcPasswdRows[1].Username) - require.Equal(t, "/home/dev", etcPasswdRows[1].HomeDir) - require.Equal(t, "alice", etcPasswdRows[2].Username) - require.Equal(t, "/home/alice", etcPasswdRows[2].HomeDir) - require.Equal(t, "bob", etcPasswdRows[3].Username) - require.Equal(t, "/home/bob", etcPasswdRows[3].HomeDir) - require.Equal(t, "carol", etcPasswdRows[4].Username) - require.Equal(t, "/home/carol", etcPasswdRows[4].HomeDir) + require.Equal(t, "root", homeDirs[0].Username) + require.Equal(t, "/root", homeDirs[0].HomeDir) + require.Equal(t, "dev", homeDirs[1].Username) + require.Equal(t, "/home/dev", homeDirs[1].HomeDir) + require.Equal(t, "alice", homeDirs[2].Username) + require.Equal(t, "/home/alice", homeDirs[2].HomeDir) + require.Equal(t, "bob", homeDirs[3].Username) + require.Equal(t, "/home/bob", homeDirs[3].HomeDir) + require.Equal(t, "carol", homeDirs[4].Username) + require.Equal(t, "/home/carol", homeDirs[4].HomeDir) } diff --git a/commands/audit_windows_profiles.go b/commands/audit_windows_profiles.go new file mode 100644 index 00000000..319b337c --- /dev/null +++ b/commands/audit_windows_profiles.go @@ -0,0 +1,93 @@ +//go:build windows +// +build windows + +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package commands + +import ( + "fmt" + "os/user" + "strings" + + "golang.org/x/sys/windows/registry" +) + +// profileListKey is the registry path under HKLM where Windows stores a +// mapping of every SID that has ever logged in to its profile directory. +const profileListKey = `SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList` + +// getHomeDirsFromProfileList enumerates Windows user profiles from the +// registry ProfileList. This is the Windows equivalent of +// getHomeDirsFromEtcPasswd and returns one entry per real user (SID prefix +// S-1-5-21-) that has a profile directory on this machine. +func getHomeDirsFromProfileList() ([]userHomeEntry, error) { + key, err := registry.OpenKey(registry.LOCAL_MACHINE, profileListKey, + registry.ENUMERATE_SUB_KEYS) + if err != nil { + return nil, fmt.Errorf("failed to open ProfileList registry key: %w", err) + } + defer key.Close() + + sids, err := key.ReadSubKeyNames(-1) + if err != nil { + return nil, fmt.Errorf("failed to enumerate ProfileList subkeys: %w", err) + } + + var entries []userHomeEntry + for _, sid := range sids { + // Real user SIDs start with S-1-5-21-; skip well-known SIDs like + // SYSTEM (S-1-5-18), LOCAL SERVICE (S-1-5-19), NETWORK SERVICE + // (S-1-5-20), etc. + if !strings.HasPrefix(sid, "S-1-5-21-") { + continue + } + + subkey, err := registry.OpenKey(registry.LOCAL_MACHINE, + profileListKey+`\`+sid, registry.QUERY_VALUE) + if err != nil { + // Stale registry entries can exist for removed SIDs; skip silently. + continue + } + // GetStringValue already expands REG_EXPAND_SZ values. + profilePath, _, err := subkey.GetStringValue("ProfileImagePath") + subkey.Close() + if err != nil || profilePath == "" { + // Profile entries without a path are incomplete/corrupt; skip. + continue + } + + // Resolve SID to username; skip if account has been deleted. + u, err := user.LookupId(sid) + if err != nil { + // Orphaned SIDs (deleted accounts) are common on Windows; skip. + continue + } + + username := u.Username + // Strip DOMAIN\ prefix if present (e.g. "COMPUTERNAME\alice" → "alice") + if parts := strings.SplitN(username, `\`, 2); len(parts) == 2 { + username = parts[1] + } + + entries = append(entries, userHomeEntry{ + Username: username, + HomeDir: profilePath, + }) + } + return entries, nil +} diff --git a/commands/audit_windows_test.go b/commands/audit_windows_test.go new file mode 100644 index 00000000..d8762d82 --- /dev/null +++ b/commands/audit_windows_test.go @@ -0,0 +1,71 @@ +//go:build windows +// +build windows + +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package commands + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestAuditCmd_WindowsEnumeratesUserProfiles(t *testing.T) { + providerContent := "https://accounts.google.com google-client-id 24h\n" + policyContent := "root alice@example.com https://accounts.google.com\n" + + stdOut := &bytes.Buffer{} + errOut := &bytes.Buffer{} + + auditCmd := SetupAuditCmdMocks(t, "", providerContent, policyContent) + auditCmd.Out = stdOut + auditCmd.ErrOut = errOut + auditCmd.CurrentUsername = "testuser" + auditCmd.SkipUserPolicy = false + + totalResults, err := auditCmd.Audit("test_version") + require.NoError(t, err) + require.NotNil(t, totalResults) + + // On Windows, user policy enumeration should now use the registry + // ProfileList instead of skipping. The audit should complete without + // the old "skipping user policy audit on Windows" message. + require.NotContains(t, errOut.String(), "skipping user policy audit on Windows") +} + +func TestAuditCmd_WindowsSkipUserPolicyFlag(t *testing.T) { + providerContent := "https://accounts.google.com google-client-id 24h\n" + policyContent := "root alice@example.com https://accounts.google.com\n" + + stdOut := &bytes.Buffer{} + errOut := &bytes.Buffer{} + + auditCmd := SetupAuditCmdMocks(t, "", providerContent, policyContent) + auditCmd.Out = stdOut + auditCmd.ErrOut = errOut + auditCmd.CurrentUsername = "testuser" + auditCmd.SkipUserPolicy = true + + totalResults, err := auditCmd.Audit("test_version") + require.NoError(t, err) + require.NotNil(t, totalResults) + + // When SkipUserPolicy is true, no home policy files should be audited + require.Empty(t, totalResults.HomePolicyFiles) +} diff --git a/commands/elevate_unix.go b/commands/elevate_unix.go new file mode 100644 index 00000000..b20f4c2e --- /dev/null +++ b/commands/elevate_unix.go @@ -0,0 +1,27 @@ +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +//go:build !windows +// +build !windows + +package commands + +import "os" + +// IsElevated returns true if the current process is running as root on Unix-like systems. +func IsElevated() (bool, error) { + return os.Geteuid() == 0, nil +} diff --git a/commands/elevate_windows.go b/commands/elevate_windows.go new file mode 100644 index 00000000..aa12f0e5 --- /dev/null +++ b/commands/elevate_windows.go @@ -0,0 +1,44 @@ +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +//go:build windows +// +build windows + +package commands + +import ( + "unsafe" + + "golang.org/x/sys/windows" +) + +// IsElevated returns true if the current process token indicates elevation on Windows. +func IsElevated() (bool, error) { + var token windows.Token + err := windows.OpenProcessToken(windows.CurrentProcess(), windows.TOKEN_QUERY, &token) + if err != nil { + return false, err + } + defer token.Close() + + var elevation uint32 + var retLen uint32 + err = windows.GetTokenInformation(token, windows.TokenElevation, (*byte)(unsafe.Pointer(&elevation)), uint32(unsafe.Sizeof(elevation)), &retLen) + if err != nil { + return false, err + } + return elevation != 0, nil +} diff --git a/commands/permcheck.go b/commands/permcheck.go new file mode 100644 index 00000000..ce6c0c6e --- /dev/null +++ b/commands/permcheck.go @@ -0,0 +1,76 @@ +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package commands + +import ( + "io/fs" + + "github.com/openpubkey/opkssh/policy/files" +) + +// PermCheckResult contains the results of checking permissions on a single file +// or directory. It is returned by CheckFilePermissions and consumed by both the +// audit and permissions commands. +type PermCheckResult struct { + // Path is the filesystem path that was checked. + Path string + // Exists is true if the file/directory was found on disk. + Exists bool + // PermsErr is non-empty when the mode or ownership check failed. + PermsErr string + // ACLReport contains detailed ACL information (owner, ACEs, problems). + // It is nil when no ACLVerifier was provided or the path does not exist. + ACLReport *files.ACLReport + // ACLErr is non-nil when VerifyACL itself returned an error. + ACLErr error +} + +// CheckFilePermissions checks the existence, permission mode/ownership, and ACLs +// of the file at path. It centralises the permission-checking logic shared by +// the audit and permissions commands so that both report consistent results. +func CheckFilePermissions( + fsys files.FileSystem, + path string, + permInfo files.PermInfo, +) PermCheckResult { + result := PermCheckResult{Path: path} + + // Check existence + exists, err := fsys.Exists(path) + if err != nil { + result.Exists = false + result.PermsErr = err.Error() + return result + } + if !exists { + result.Exists = false + return result + } + result.Exists = true + + // Check file mode and ownership + if err := fsys.CheckPerm(path, []fs.FileMode{permInfo.Mode}, permInfo.Owner, permInfo.Group); err != nil { + result.PermsErr = err.Error() + } + + // Check ACLs + report, err := fsys.VerifyACL(path, files.ExpectedACLFromPerm(permInfo)) + result.ACLReport = &report + result.ACLErr = err + + return result +} diff --git a/commands/permissions.go b/commands/permissions.go new file mode 100644 index 00000000..e883bbec --- /dev/null +++ b/commands/permissions.go @@ -0,0 +1,485 @@ +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package commands + +import ( + "bufio" + "encoding/json" + "fmt" + "io" + "os" + "path/filepath" + "runtime" + "strings" + + "github.com/openpubkey/opkssh/policy" + "github.com/openpubkey/opkssh/policy/files" + "github.com/openpubkey/opkssh/policy/plugins" + "github.com/spf13/afero" + "github.com/spf13/cobra" +) + +// defaultConfirmPrompt reads a yes/no answer from stdin. +func defaultConfirmPrompt(prompt string, in io.Reader) (bool, error) { + fmt.Print(prompt) + r := bufio.NewReader(in) + s, err := r.ReadString('\n') + if err != nil { + return false, err + } + s = strings.TrimSpace(strings.ToLower(s)) + return s == "y" || s == "yes", nil +} + +// PermissionsCmd provides functionality to check and fix file permissions +type PermissionsCmd struct { + FileSystem files.FileSystem + Out io.Writer + ErrOut io.Writer + In io.Reader + IsElevatedFn func() (bool, error) + ConfirmPrompt func(string, io.Reader) (bool, error) + + // Flags + DryRun bool + Yes bool + Verbose bool + JsonOutput bool +} + +// NewPermissionsCmd creates a new PermissionsCmd with default settings +func NewPermissionsCmd(out io.Writer, errOut io.Writer) *PermissionsCmd { + return &PermissionsCmd{ + FileSystem: files.NewFileSystem(afero.NewOsFs()), + Out: out, + ErrOut: errOut, + In: os.Stdin, + IsElevatedFn: IsElevated, + ConfirmPrompt: defaultConfirmPrompt, + } +} + +// CobraCommand returns the cobra command tree for the permissions command. +func (p *PermissionsCmd) CobraCommand() *cobra.Command { + permissionsCmd := &cobra.Command{ + Use: "permissions", + Short: "Check and fix filesystem permissions required by opkssh", + Args: cobra.NoArgs, + } + + checkCmd := &cobra.Command{ + Use: "check", + Short: "Verify permissions and ownership for opkssh files", + RunE: func(cmd *cobra.Command, args []string) error { + return p.Check() + }, + } + checkCmd.Flags().BoolVarP(&p.JsonOutput, "json", "j", false, "Output results in JSON") + + fixCmd := &cobra.Command{ + Use: "fix", + Short: "Fix permissions and ownership for opkssh files (requires admin)", + RunE: func(cmd *cobra.Command, args []string) error { + return p.Fix() + }, + } + fixCmd.Flags().BoolVar(&p.DryRun, "dry-run", false, "Don't modify anything; show planned changes") + fixCmd.Flags().BoolVarP(&p.Yes, "yes", "y", false, "Apply changes without confirmation") + fixCmd.Flags().BoolVarP(&p.Verbose, "verbose", "v", false, "Verbose output") + fixCmd.Flags().BoolVarP(&p.JsonOutput, "json", "j", false, "Output results in JSON") + + installCmd := &cobra.Command{ + Use: "install", + Short: "Idempotent installer-friendly permissions fix (non-interactive)", + RunE: func(cmd *cobra.Command, args []string) error { + // installers expect non-interactive behavior; force yes=true + p.Yes = true + return p.Fix() + }, + } + installCmd.Flags().BoolVar(&p.DryRun, "dry-run", false, "Don't modify anything; show planned changes") + installCmd.Flags().BoolVarP(&p.Verbose, "verbose", "v", false, "Verbose output") + + permissionsCmd.AddCommand(checkCmd) + permissionsCmd.AddCommand(fixCmd) + permissionsCmd.AddCommand(installCmd) + return permissionsCmd +} + +// checkResult is the JSON-serializable result of a permissions check. +type checkResult struct { + Path string `json:"path"` + Exists bool `json:"exists"` + PermsErr string `json:"permsErr,omitempty"` + ACLErr string `json:"aclErr,omitempty"` +} + +// Check verifies permissions and ownership for opkssh files. +func (p *PermissionsCmd) Check() error { + var problems []string + var results []checkResult + + // checkACLResult is a helper that processes ACL findings from + // CheckFilePermissions and appends them to problems/checkResult. + checkACLResult := func(path string, result PermCheckResult, cr *checkResult) { + if result.ACLErr != nil { + problems = append(problems, fmt.Sprintf("%s: acl verify error: %v", path, result.ACLErr)) + cr.ACLErr = result.ACLErr.Error() + } else if result.ACLReport != nil { + report := result.ACLReport + if report.OwnerSIDStr != "" { + fmt.Fprintf(p.Out, "%s: owner=%s ownerSID=%s mode=%o\n", path, report.Owner, report.OwnerSIDStr, report.Mode) + } else { + fmt.Fprintf(p.Out, "%s: owner=%s mode=%o\n", path, report.Owner, report.Mode) + } + if len(report.ACEs) > 0 { + fmt.Fprintln(p.Out, " ACEs:") + for _, a := range report.ACEs { + if a.PrincipalSIDStr != "" { + fmt.Fprintf(p.Out, " - %s [%s]: %s (%s) inherited=%v\n", a.Principal, a.PrincipalSIDStr, a.Type, a.Rights, a.Inherited) + } else { + fmt.Fprintf(p.Out, " - %s: %s (%s) inherited=%v\n", a.Principal, a.Type, a.Rights, a.Inherited) + } + } + } + for _, prob := range report.Problems { + fmt.Fprintln(p.Out, " ACL problem:", prob) + } + } + } + + // System policy file — use shared permission check + sp := files.RequiredPerms.SystemPolicy + systemPolicy := policy.SystemDefaultPolicyPath + sysResult := CheckFilePermissions(p.FileSystem, systemPolicy, sp) + if !sysResult.Exists { + problems = append(problems, fmt.Sprintf("%s: file does not exist", systemPolicy)) + results = append(results, checkResult{Path: systemPolicy, Exists: false}) + } else { + cr := checkResult{Path: systemPolicy, Exists: true, PermsErr: sysResult.PermsErr} + if sysResult.PermsErr != "" { + problems = append(problems, fmt.Sprintf("%s: %s", systemPolicy, sysResult.PermsErr)) + } + checkACLResult(systemPolicy, sysResult, &cr) + results = append(results, cr) + } + + // Providers file + providersFile := policy.SystemDefaultProvidersPath + pp := files.RequiredPerms.Providers + provResult := CheckFilePermissions(p.FileSystem, providersFile, pp) + if !provResult.Exists { + // not fatal, but report + problems = append(problems, fmt.Sprintf("%s: file does not exist", providersFile)) + results = append(results, checkResult{Path: providersFile, Exists: false}) + } else { + cr := checkResult{Path: providersFile, Exists: true, PermsErr: provResult.PermsErr} + if provResult.PermsErr != "" { + problems = append(problems, fmt.Sprintf("%s: %s", providersFile, provResult.PermsErr)) + } + checkACLResult(providersFile, provResult, &cr) + results = append(results, cr) + } + + // Config file + configFile := filepath.Join(policy.GetSystemConfigBasePath(), "config.yml") + cp := files.RequiredPerms.Config + cfgResult := CheckFilePermissions(p.FileSystem, configFile, cp) + if cfgResult.Exists { + cr := checkResult{Path: configFile, Exists: true, PermsErr: cfgResult.PermsErr} + if cfgResult.PermsErr != "" { + problems = append(problems, fmt.Sprintf("%s: %s", configFile, cfgResult.PermsErr)) + } + checkACLResult(configFile, cfgResult, &cr) + results = append(results, cr) + } + + // Policy plugins dir + pluginsDir := filepath.Join(policy.GetSystemConfigBasePath(), "policy.d") + if _, err := p.FileSystem.Stat(pluginsDir); err != nil { + problems = append(problems, fmt.Sprintf("%s: %v", pluginsDir, err)) + results = append(results, checkResult{Path: pluginsDir, Exists: false, PermsErr: err.Error()}) + } else { + cr := checkResult{Path: pluginsDir, Exists: true} + // Check directory perms using plugin package expectations + if err := p.FileSystem.CheckPerm(pluginsDir, plugins.RequiredPolicyDirPerms(), files.RequiredPerms.PluginsDir.Owner, files.RequiredPerms.PluginsDir.Group); err != nil { + problems = append(problems, fmt.Sprintf("%s: %v", pluginsDir, err)) + cr.PermsErr = err.Error() + } + results = append(results, cr) + } + + if p.JsonOutput { + enc := json.NewEncoder(p.Out) + enc.SetIndent("", " ") + return enc.Encode(results) + } + + if len(problems) > 0 { + for _, prob := range problems { + fmt.Fprintln(p.Out, "Problem:", prob) + } + return fmt.Errorf("permissions check failed: %d problems found", len(problems)) + } + // Success: print nothing and return nil + return nil +} + +// fixResult is the JSON-serializable result of a permissions fix. +type fixResult struct { + Planned []string `json:"planned"` + Errors []string `json:"errors,omitempty"` + DryRun bool `json:"dryRun"` +} + +// Fix attempts to repair permissions/ownership for key paths. +func (p *PermissionsCmd) Fix() error { + // Planning phase: determine actions without performing them + var planned []string + + sp := files.RequiredPerms.SystemPolicy + pv := files.RequiredPerms.Providers + pld := files.RequiredPerms.PluginsDir + pf := files.RequiredPerms.PluginFile + + systemPolicy := policy.SystemDefaultPolicyPath + if _, err := p.FileSystem.Stat(systemPolicy); err != nil { + planned = append(planned, "create file: "+systemPolicy) + } + planned = append(planned, "chmod "+systemPolicy+" to "+sp.Mode.String()) + plannedOwner := sp.Owner + if sp.Group != "" { + plannedOwner += ":" + sp.Group + } + planned = append(planned, "chown "+systemPolicy+" to "+plannedOwner) + + providersFile := policy.SystemDefaultProvidersPath + if _, err := p.FileSystem.Stat(providersFile); err == nil { + planned = append(planned, "chmod "+providersFile+" to "+pv.Mode.String()) + pvOwner := pv.Owner + if pv.Group != "" { + pvOwner += ":" + pv.Group + } + planned = append(planned, "chown "+providersFile+" to "+pvOwner) + } + + configFile := filepath.Join(policy.GetSystemConfigBasePath(), "config.yml") + cp := files.RequiredPerms.Config + if _, err := p.FileSystem.Stat(configFile); err == nil { + planned = append(planned, "chmod "+configFile+" to "+cp.Mode.String()) + cpOwner := cp.Owner + if cp.Group != "" { + cpOwner += ":" + cp.Group + } + planned = append(planned, "chown "+configFile+" to "+cpOwner) + } + + pluginsDir := filepath.Join(policy.GetSystemConfigBasePath(), "policy.d") + if _, err := p.FileSystem.Stat(pluginsDir); err != nil { + planned = append(planned, "mkdir "+pluginsDir) + } + // include plugin files if present + if fi, err := p.FileSystem.Open(pluginsDir); err == nil { + entries, _ := fi.Readdir(-1) + for _, e := range entries { + if !e.IsDir() && strings.HasSuffix(e.Name(), ".yml") { + planned = append(planned, fmt.Sprintf("chmod %s to %04o", filepath.Join(pluginsDir, e.Name()), pf.Mode)) + planned = append(planned, "chown "+filepath.Join(pluginsDir, e.Name())+" to "+pf.Owner) + } + } + fi.Close() + } + + // If dry-run, just print planned actions + if p.DryRun { + if p.JsonOutput { + enc := json.NewEncoder(p.Out) + enc.SetIndent("", " ") + return enc.Encode(fixResult{Planned: planned, DryRun: true}) + } + for _, a := range planned { + fmt.Fprintln(p.Out, "Action:", a) + } + fmt.Fprintln(p.Out, "dry-run complete") + return nil + } + + // Require elevated privileges to perform fixes + elevated, err := p.IsElevatedFn() + if err != nil { + return fmt.Errorf("failed to determine elevation: %w", err) + } + if !elevated { + return fmt.Errorf("fix requires elevated privileges (run as root or Administrator)") + } + + // Confirm with user unless --yes + if !p.Yes { + // show planned actions and ask + fmt.Fprintln(p.Out, "Planned actions:") + for _, a := range planned { + fmt.Fprintln(p.Out, " -", a) + } + ok, err := p.ConfirmPrompt("Apply these changes? [y/N]: ", p.In) + if err != nil { + return err + } + if !ok { + return fmt.Errorf("aborted by user") + } + } + + // Execution phase: perform actions + var errorsFound []string + + // Create system policy file if missing + if _, err := p.FileSystem.Stat(systemPolicy); err != nil { + if f, err := p.FileSystem.CreateFile(systemPolicy); err != nil { + errorsFound = append(errorsFound, "create "+systemPolicy+": "+err.Error()) + } else { + f.Close() + } + } + if err := p.FileSystem.Chmod(systemPolicy, sp.Mode); err != nil { + errorsFound = append(errorsFound, "chmod "+systemPolicy+": "+err.Error()) + } + if err := p.FileSystem.Chown(systemPolicy, sp.Owner, sp.Group); err != nil { + errorsFound = append(errorsFound, "chown "+systemPolicy+": "+err.Error()) + } + + // Verify ACLs after changes and apply ACE fixes on Windows if needed + if runtime.GOOS == "windows" { + // Pre-resolve commonly used SIDs to avoid repeated lookups and use SID-based trustees + adminSID, _, _ := files.ResolveAccountToSID("Administrators") + systemSID, _, _ := files.ResolveAccountToSID("SYSTEM") + + report, err := p.FileSystem.VerifyACL(systemPolicy, files.ExpectedACLFromPerm(sp)) + if err != nil { + errorsFound = append(errorsFound, "acl verify: "+err.Error()) + } else { + // Ensure Administrators and SYSTEM have full control; if missing, apply via ApplyACE + needAdmin := true + needSystem := true + for _, a := range report.ACEs { + if a.Principal == "Administrators" && strings.Contains(a.Rights, "GENERIC_ALL") { + needAdmin = false + } + if a.Principal == "SYSTEM" && strings.Contains(a.Rights, "GENERIC_ALL") { + needSystem = false + } + } + if needAdmin { + ace := files.ACE{Principal: "Administrators", Rights: "GENERIC_ALL", Type: "allow"} + if len(adminSID) > 0 { + ace.PrincipalSID = adminSID + } + if err := p.FileSystem.ApplyACE(systemPolicy, ace); err != nil { + errorsFound = append(errorsFound, "apply ACE Administrators:F: "+err.Error()) + } + } + if needSystem { + ace := files.ACE{Principal: "SYSTEM", Rights: "GENERIC_ALL", Type: "allow"} + if len(systemSID) > 0 { + ace.PrincipalSID = systemSID + } + if err := p.FileSystem.ApplyACE(systemPolicy, ace); err != nil { + errorsFound = append(errorsFound, "apply ACE SYSTEM:F: "+err.Error()) + } + } + } + } + + // Providers file + if _, err := p.FileSystem.Stat(providersFile); err == nil { + if err := p.FileSystem.Chmod(providersFile, pv.Mode); err != nil { + errorsFound = append(errorsFound, "chmod "+providersFile+": "+err.Error()) + } + if err := p.FileSystem.Chown(providersFile, pv.Owner, pv.Group); err != nil { + errorsFound = append(errorsFound, "chown "+providersFile+": "+err.Error()) + } + } + + // Config file + if _, err := p.FileSystem.Stat(configFile); err == nil { + if err := p.FileSystem.Chmod(configFile, cp.Mode); err != nil { + errorsFound = append(errorsFound, "chmod "+configFile+": "+err.Error()) + } + if err := p.FileSystem.Chown(configFile, cp.Owner, cp.Group); err != nil { + errorsFound = append(errorsFound, "chown "+configFile+": "+err.Error()) + } + } + + // Plugins dir + if _, err := p.FileSystem.Stat(pluginsDir); err != nil { + if err := p.FileSystem.MkdirAll(pluginsDir, pld.Mode); err != nil { + errorsFound = append(errorsFound, "mkdir "+pluginsDir+": "+err.Error()) + } + } + if fi, err := p.FileSystem.Open(pluginsDir); err == nil { + entries, _ := fi.Readdir(-1) + for _, e := range entries { + if !e.IsDir() && strings.HasSuffix(e.Name(), ".yml") { + path := filepath.Join(pluginsDir, e.Name()) + if err := p.FileSystem.Chmod(path, pf.Mode); err != nil { + errorsFound = append(errorsFound, "chmod "+path+": "+err.Error()) + } + if err := p.FileSystem.Chown(path, pf.Owner, pf.Group); err != nil { + errorsFound = append(errorsFound, "chown "+path+": "+err.Error()) + } + // On Windows, ensure ACLs for plugin files as well + if runtime.GOOS == "windows" { + if report, err := p.FileSystem.VerifyACL(path, files.ExpectedACLFromPerm(pf)); err == nil { + needAdmin := true + for _, a := range report.ACEs { + if a.Principal == "Administrators" && strings.Contains(a.Rights, "GENERIC_ALL") { + needAdmin = false + } + } + if needAdmin { + ace := files.ACE{Principal: "Administrators", Rights: "GENERIC_ALL", Type: "allow"} + if adminSID, _, _ := files.ResolveAccountToSID("Administrators"); len(adminSID) > 0 { + ace.PrincipalSID = adminSID + } + if err := p.FileSystem.ApplyACE(path, ace); err != nil { + errorsFound = append(errorsFound, "apply ACE Administrators:F for "+path+": "+err.Error()) + } + } + } else { + errorsFound = append(errorsFound, "acl verify for "+path+": "+err.Error()) + } + } + } + } + fi.Close() + } + + if p.JsonOutput { + enc := json.NewEncoder(p.Out) + enc.SetIndent("", " ") + return enc.Encode(fixResult{Planned: planned, Errors: errorsFound}) + } + + if len(errorsFound) > 0 { + for _, e := range errorsFound { + fmt.Fprintln(p.Out, "Error:", e) + } + return fmt.Errorf("fix completed with %d errors", len(errorsFound)) + } + + fmt.Fprintln(p.Out, "fix completed successfully") + return nil +} diff --git a/commands/permissions_fix_nonwindows_test.go b/commands/permissions_fix_nonwindows_test.go new file mode 100644 index 00000000..a1b9259d --- /dev/null +++ b/commands/permissions_fix_nonwindows_test.go @@ -0,0 +1,40 @@ +//go:build !windows +// +build !windows + +package commands + +import ( + "bytes" + "io" + "testing" + + "github.com/spf13/afero" +) + +func TestRunPermissionsFix_NonWindows_CreatesAndSetsPerms(t *testing.T) { + mem := afero.NewMemMapFs() + mfs := &mockFileSystem{fs: mem} + + p := &PermissionsCmd{ + FileSystem: mfs, + Out: &bytes.Buffer{}, + ErrOut: &bytes.Buffer{}, + IsElevatedFn: func() (bool, error) { return true, nil }, + ConfirmPrompt: func(prompt string, in io.Reader) (bool, error) { return true, nil }, + Yes: true, + } + + err := p.Fix() + if err != nil { + t.Fatalf("Fix failed: %v", err) + } + if !mfs.Created { + t.Fatalf("expected CreateFile to be called") + } + if !mfs.ChmodCalled { + t.Fatalf("expected Chmod to be called") + } + if !mfs.ChownCalled { + t.Fatalf("expected Chown to be called") + } +} diff --git a/commands/permissions_fix_windows_test.go b/commands/permissions_fix_windows_test.go new file mode 100644 index 00000000..8a899b21 --- /dev/null +++ b/commands/permissions_fix_windows_test.go @@ -0,0 +1,62 @@ +//go:build windows +// +build windows + +package commands + +import ( + "bytes" + "io" + "testing" + + "github.com/openpubkey/opkssh/policy" + "github.com/openpubkey/opkssh/policy/files" + "github.com/spf13/afero" +) + +func TestRunPermissionsFix_AppliesAdminACE_Windows(t *testing.T) { + // Setup in-memory fs with system policy file + mem := afero.NewMemMapFs() + systemPolicy := policy.SystemDefaultPolicyPath + afero.WriteFile(mem, systemPolicy, []byte("x"), 0o644) + // ensure plugins dir exists but no ACEs present + pluginsDir := policy.GetSystemConfigBasePath() + "/policy.d" + mem.MkdirAll(pluginsDir, 0o750) + afero.WriteFile(mem, pluginsDir+"/plugin.yml", []byte("a"), 0o644) + + mfs := &mockFileSystem{ + fs: mem, + aclReport: files.ACLReport{Path: systemPolicy, Exists: true, ACEs: []files.ACE{}}, + } + + p := &PermissionsCmd{ + FileSystem: mfs, + Out: &bytes.Buffer{}, + ErrOut: &bytes.Buffer{}, + IsElevatedFn: func() (bool, error) { return true, nil }, + ConfirmPrompt: func(prompt string, in io.Reader) (bool, error) { return true, nil }, + Yes: true, + } + + err := p.Fix() + if err != nil { + t.Fatalf("Fix failed: %v", err) + } + + if len(mfs.Applied) < 2 { + t.Fatalf("expected at least 2 ApplyACE calls for Admin and SYSTEM, got %d", len(mfs.Applied)) + } + + // check principals present + var foundAdmin, foundSystem bool + for _, a := range mfs.Applied { + if a.Principal == "Administrators" { + foundAdmin = true + } + if a.Principal == "SYSTEM" { + foundSystem = true + } + } + if !foundAdmin || !foundSystem { + t.Fatalf("expected ApplyACE for Administrators and SYSTEM, got: %+v", mfs.Applied) + } +} diff --git a/commands/permissions_install_test.go b/commands/permissions_install_test.go new file mode 100644 index 00000000..5f5378be --- /dev/null +++ b/commands/permissions_install_test.go @@ -0,0 +1,33 @@ +package commands + +import ( + "bytes" + "io" + "testing" + + "github.com/spf13/afero" +) + +func TestInstallCmd_ForceYes(t *testing.T) { + mem := afero.NewMemMapFs() + mfs := &mockFileSystem{fs: mem} + + p := &PermissionsCmd{ + FileSystem: mfs, + Out: &bytes.Buffer{}, + ErrOut: &bytes.Buffer{}, + IsElevatedFn: func() (bool, error) { return true, nil }, + ConfirmPrompt: func(prompt string, in io.Reader) (bool, error) { return true, nil }, + } + + // Execute the cobra command with 'install' + cmd := p.CobraCommand() + cmd.SetArgs([]string{"install"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("install command failed: %v", err) + } + // install sets Yes=true internally; verify fix ran + if !mfs.ChmodCalled { + t.Fatalf("expected Chmod to be called") + } +} diff --git a/commands/permissions_mocks_test.go b/commands/permissions_mocks_test.go new file mode 100644 index 00000000..b6c35c13 --- /dev/null +++ b/commands/permissions_mocks_test.go @@ -0,0 +1,91 @@ +package commands + +import ( + "io" + "io/fs" + + "github.com/openpubkey/opkssh/policy/files" + "github.com/spf13/afero" +) + +// newTestPermissionsCmd creates a PermissionsCmd wired to an in-memory +// filesystem and the given writer. It uses mock-friendly defaults so that +// tests don't need real OS privileges. +func newTestPermissionsCmd(vfs afero.Fs, out io.Writer) *PermissionsCmd { + return &PermissionsCmd{ + FileSystem: files.NewFileSystem(vfs, files.WithCmdRunner(func(name string, arg ...string) ([]byte, error) { + return []byte("root opksshuser"), nil + })), + Out: out, + ErrOut: out, + IsElevatedFn: func() (bool, error) { return true, nil }, + ConfirmPrompt: func(prompt string, in io.Reader) (bool, error) { return true, nil }, + } +} + +// mockFileSystem is a configurable mock implementing files.FileSystem. +// It wraps an in-memory afero.Fs for real file I/O while tracking +// permission-mutation calls for assertions. +type mockFileSystem struct { + fs afero.Fs + Created bool + ChmodCalled bool + ChownCalled bool + Applied []files.ACE + aclReport files.ACLReport +} + +func (m *mockFileSystem) Stat(path string) (fs.FileInfo, error) { + return m.fs.Stat(path) +} + +func (m *mockFileSystem) Exists(path string) (bool, error) { + return afero.Exists(m.fs, path) +} + +func (m *mockFileSystem) Open(path string) (afero.File, error) { + return m.fs.Open(path) +} + +func (m *mockFileSystem) ReadFile(path string) ([]byte, error) { + return afero.ReadFile(m.fs, path) +} + +func (m *mockFileSystem) MkdirAll(path string, perm fs.FileMode) error { + return m.fs.MkdirAll(path, perm) +} + +func (m *mockFileSystem) CreateFile(path string) (afero.File, error) { + m.Created = true + return m.fs.Create(path) +} + +func (m *mockFileSystem) WriteFile(path string, data []byte, perm fs.FileMode) error { + return afero.WriteFile(m.fs, path, data, perm) +} + +func (m *mockFileSystem) Chmod(path string, perm fs.FileMode) error { + m.ChmodCalled = true + return m.fs.Chmod(path, perm) +} + +func (m *mockFileSystem) Chown(path string, owner string, group string) error { + m.ChownCalled = true + return nil +} + +func (m *mockFileSystem) ApplyACE(path string, ace files.ACE) error { + m.Applied = append(m.Applied, ace) + return nil +} + +func (m *mockFileSystem) CheckPerm(path string, requirePerm []fs.FileMode, requiredOwner string, requiredGroup string) error { + return nil +} + +func (m *mockFileSystem) VerifyACL(path string, expected files.ExpectedACL) (files.ACLReport, error) { + if m.aclReport.Path == "" { + return files.ACLReport{Path: path, Exists: true}, nil + } + return m.aclReport, nil +} diff --git a/commands/permissions_test.go b/commands/permissions_test.go new file mode 100644 index 00000000..ca972728 --- /dev/null +++ b/commands/permissions_test.go @@ -0,0 +1,58 @@ +package commands + +import ( + "bytes" + "path/filepath" + "testing" + + "github.com/openpubkey/opkssh/policy" + "github.com/spf13/afero" + "github.com/stretchr/testify/require" +) + +func TestPermissionsCheck_MissingSystemPolicyReportsProblem(t *testing.T) { + vfs := afero.NewMemMapFs() + out := &bytes.Buffer{} + p := newTestPermissionsCmd(vfs, out) + + // No system policy file created -> check should report problems + err := p.Check() + require.Error(t, err) +} + +func TestPermissionsCheck_WithSystemPolicyAndPlugins_Succeeds(t *testing.T) { + vfs := afero.NewMemMapFs() + out := &bytes.Buffer{} + p := newTestPermissionsCmd(vfs, out) + + // Create system policy file and parents under the system config base + path := policy.SystemDefaultPolicyPath + base := policy.GetSystemConfigBasePath() + _ = vfs.MkdirAll(base, 0o750) + err := afero.WriteFile(vfs, path, []byte("user1 alice@example.com google\n"), 0o640) + require.NoError(t, err) + + // Create providers file and plugins dir + providersFile := filepath.Join(base, "providers") + err = afero.WriteFile(vfs, providersFile, []byte("https://accounts.google.com google-client-id 24h\n"), 0o640) + require.NoError(t, err) + pluginsDir := filepath.Join(base, "policy.d") + _ = vfs.MkdirAll(pluginsDir, 0o750) + err = afero.WriteFile(vfs, filepath.Join(pluginsDir, "example.yml"), []byte("name: test\ncommand: /bin/true\n"), 0o640) + require.NoError(t, err) + + err = p.Check() + require.NoError(t, err) +} + +func TestPermissionsFix_DryRun_NoPanic(t *testing.T) { + vfs := afero.NewMemMapFs() + out := &bytes.Buffer{} + p := newTestPermissionsCmd(vfs, out) + p.DryRun = true + p.Verbose = true + + // Dry-run should not attempt to change real FS and should return nil + err := p.Fix() + require.NoError(t, err) +} diff --git a/commands/verify_test.go b/commands/verify_test.go index 568362eb..bcc42e5c 100644 --- a/commands/verify_test.go +++ b/commands/verify_test.go @@ -205,22 +205,6 @@ env_vars: group: "opksshuser", errorString: "", }, - { - name: "Wrong Permissions", - configFile: map[string]string{"server_config.yml": configContent}, - permission: 0677, - owner: "root", - group: "opksshuser", - errorString: "expected one of the following permissions [640], got (677)", - }, - { - name: "Wrong ownership", - configFile: map[string]string{"server_config.yml": configContent}, - permission: 0640, - owner: "opksshuser", - group: "opksshuser", - errorString: "expected owner (root), got (opksshuser)", - }, { name: "Missing config", configFile: map[string]string{"wrong-filename.yml": configContent}, diff --git a/commands/verify_unix_test.go b/commands/verify_unix_test.go new file mode 100644 index 00000000..ed38bd7a --- /dev/null +++ b/commands/verify_unix_test.go @@ -0,0 +1,102 @@ +//go:build !windows +// +build !windows + +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package commands + +import ( + "io/fs" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/openpubkey/opkssh/policy/files" + "github.com/spf13/afero" + "github.com/stretchr/testify/require" +) + +// TestEnvFromConfig_UnixPermissions tests Unix-specific permission checking +func TestEnvFromConfig_UnixPermissions(t *testing.T) { + configContent := `--- +env_vars: + OPKSSH_TEST_EXAMPLE_VAR1: ABC + OPKSSH_TEST_EXAMPLE_VAR2: DEF +` + + tests := []struct { + name string + configFile map[string]string + permission fs.FileMode + Content string + owner string + group string + errorString string + }{ + { + name: "Wrong Permissions", + configFile: map[string]string{"server_config.yml": configContent}, + permission: 0o677, + owner: "root", + group: "opksshuser", + errorString: "expected one of the following permissions [640], got (677)", + }, + { + name: "Wrong ownership", + configFile: map[string]string{"server_config.yml": configContent}, + permission: 0o640, + owner: "opksshuser", + group: "opksshuser", + errorString: "expected owner (root), got (opksshuser)", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Unset the environment variables after the test is done to avoid side effects + defer func() { + for _, v := range os.Environ() { + if strings.HasPrefix(v, "OPKSSH_TEST_EXAMPLE_VAR") { + parts := strings.SplitN(v, "=", 2) + os.Unsetenv(parts[0]) + } + } + }() + + mockFs := afero.NewMemMapFs() + tempDir, _ := afero.TempDir(mockFs, "opk", "config") + for name, content := range tt.configFile { + err := afero.WriteFile(mockFs, filepath.Join(tempDir, name), []byte(content), tt.permission) + require.NoError(t, err) + } + + ver := VerifyCmd{ + Fs: mockFs, + ConfigPathArg: filepath.Join(tempDir, "server_config.yml"), + filePermChecker: files.PermsChecker{ + Fs: mockFs, + CmdRunner: func(name string, arg ...string) ([]byte, error) { + return []byte(tt.owner + " " + tt.group), nil + }, + }, + } + err := ver.ReadFromServerConfig() + + require.ErrorContains(t, err, tt.errorString) + }) + } +} diff --git a/docs/audit.md b/docs/audit.md index 0b9aded3..df757bcf 100644 --- a/docs/audit.md +++ b/docs/audit.md @@ -40,6 +40,25 @@ Exit Code: 0 (no issues detected) * The audit command currently only checks server side configurations. It does not report on client-side configurations. * The audit command does not currently support checking [policy plugins](policyplugins.m) or openssh server config (`sshd_config`). +## audit vs permissions + +The `opkssh audit` and `opkssh permissions` commands are related but serve different purposes: + +| | `opkssh audit` | `opkssh permissions check` | +|---|---|---| +| **Purpose** | Validate policy file *contents* against provider definitions | Check and *fix* filesystem permissions/ACLs | +| **Focus** | Are issuers valid? Do entries match providers? | Are file modes, ownership and ACLs correct? | +| **User home dirs** | Enumerates and audits all user `~/.opk/auth_id` files | Does **not** check user policy files | +| **Modifies files** | Never (read-only) | `permissions fix` can repair permissions | +| **Output** | Human-readable table or JSON (`--json`) | Problem list or planned actions | + +**When to use which:** + +* **`opkssh audit`** — Run after editing policy or provider files to verify the configuration is correct. Also useful for generating a JSON report for bug reports. +* **`opkssh permissions check`** — Run after installation or upgrades to verify that file permissions and ACLs are correct. If problems are found, run `opkssh permissions fix` to repair them. + +Both commands check the permissions on the system policy file (`/etc/opk/auth_id` on Linux, `%ProgramData%\opk\auth_id` on Windows) using the same shared logic, so their permission-related findings will be consistent. + ## JSON output To get the full audit report use the `--json` flag: diff --git a/main.go b/main.go index abdb4f72..c980223e 100644 --- a/main.go +++ b/main.go @@ -27,6 +27,7 @@ import ( "os" "os/signal" "regexp" + "runtime" "strings" "syscall" "text/tabwriter" @@ -335,7 +336,7 @@ Arguments: SilenceUsage: true, Use: "audit", Short: "Validate policy file entries against provider definitions", - Long: `Audit validates all entries in /etc/opk/auth_id and ~/.opk/auth_id against the provider definitions in /etc/opk/providers. For complete audit details use the --json flag. Returns a non-zero exit code if any warnings or errors are found. + Long: `Audit validates all entries in the system policy file and user policy files against the provider definitions in the providers file. For complete audit details use the --json flag. Returns a non-zero exit code if any warnings or errors are found. The audit command checks that: - Each issuer in policy files is defined in the providers file @@ -372,9 +373,9 @@ Exit code: 0 if all entries are valid, 1 if any warnings or errors are found.`, }, } - auditCmd.Flags().String("providers-file", "/etc/opk/providers", "Path to providers file") - auditCmd.Flags().String("policy-file", "/etc/opk/auth_id", "Path to policy file") - auditCmd.Flags().Bool("skip-user-policy", false, "Skip auditing user policy file (~/.opk/auth_id)") + auditCmd.Flags().String("providers-file", policy.SystemDefaultProvidersPath, "Path to providers file") + auditCmd.Flags().String("policy-file", policy.SystemDefaultPolicyPath, "Path to policy file") + auditCmd.Flags().Bool("skip-user-policy", runtime.GOOS == "windows", "Skip auditing user policy file (~/.opk/auth_id)") auditCmd.Flags().BoolP("json", "j", false, "Output complete audit results in JSON") rootCmd.AddCommand(auditCmd) @@ -442,6 +443,10 @@ Exit code: 0 if all entries are valid, 1 if any warnings or errors are found.`, rootCmd.AddCommand(clientCmd) + // permissions command for checking and fixing file permissions/ACLs + permsCmd := commands.NewPermissionsCmd(os.Stdout, os.Stderr) + rootCmd.AddCommand(permsCmd.CobraCommand()) + // genDocsCmd is a hidden command used as a helper for generating our // command line reference documentation. genDocsCmd := &cobra.Command{ diff --git a/policy/files/acl.go b/policy/files/acl.go new file mode 100644 index 00000000..60121f1f --- /dev/null +++ b/policy/files/acl.go @@ -0,0 +1,46 @@ +package files + +import ( + "io/fs" +) + +// ACE represents an access control entry (platform-agnostic minimal view) +type ACE struct { + Principal string + // PrincipalSID if present contains the raw SID bytes to use when applying + // the ACE on Windows. When non-nil, implementations should prefer the SID + // form of TRUSTEE to avoid name-resolution ambiguity. + PrincipalSID []byte + Rights string + // PrincipalSIDStr contains the textual SID (S-1-5-...) when available. + PrincipalSIDStr string + Type string // Allow or Deny + Inherited bool +} + +// ExpectedACL contains the expectations for a path's ownership/ACL +type ExpectedACL struct { + Owner string + Mode fs.FileMode // expected mode bits; 0 means ignore +} + +// ACLReport is the structured result from verifying ACLs/ownership for a path +type ACLReport struct { + Path string + Exists bool + Owner string + // OwnerSID contains the raw owner SID bytes on Windows when available. + // On non-Windows platforms this will be nil. + OwnerSID []byte + // OwnerSIDStr is the textual SID value (S-1-5-...) when available. + OwnerSIDStr string + Mode fs.FileMode + ACEs []ACE + Problems []string +} + +// ACLVerifier verifies ACLs and ownership for a given path against expectations. +// Implementations are platform-specific (Unix uses syscalls; Windows uses Win32 APIs). +type ACLVerifier interface { + VerifyACL(path string, expected ExpectedACL) (ACLReport, error) +} diff --git a/policy/files/acl_unix.go b/policy/files/acl_unix.go new file mode 100644 index 00000000..4de5a403 --- /dev/null +++ b/policy/files/acl_unix.go @@ -0,0 +1,73 @@ +//go:build !windows +// +build !windows + +package files + +import ( + "fmt" + "github.com/spf13/afero" + "os/user" + "strconv" + "syscall" +) + +// UnixACLVerifier implements ACLVerifier for Unix-like systems. +type UnixACLVerifier struct { + Fs afero.Fs +} + +func NewDefaultACLVerifier(fs afero.Fs) ACLVerifier { + return &UnixACLVerifier{Fs: fs} +} + +func (u *UnixACLVerifier) VerifyACL(path string, expected ExpectedACL) (ACLReport, error) { + r := ACLReport{Path: path} + if u.Fs == nil { + u.Fs = afero.NewOsFs() + } + fi, err := u.Fs.Stat(path) + if err != nil { + // file doesn't exist or other stat error + r.Exists = false + r.Problems = append(r.Problems, fmt.Sprintf("open %s: %v", path, err)) + return r, nil + } + r.Exists = true + // Mode bits + r.Mode = fi.Mode().Perm() + if expected.Mode != 0 { + if r.Mode != expected.Mode { + r.Problems = append(r.Problems, fmt.Sprintf("expected mode %o, got %o", expected.Mode, r.Mode)) + } + } + + // Owner lookup if available via Sys() + if statT, ok := fi.Sys().(*syscall.Stat_t); ok { + uid := strconv.FormatUint(uint64(statT.Uid), 10) + gid := strconv.FormatUint(uint64(statT.Gid), 10) + ownerName := "" + groupName := "" + if uobj, err := user.LookupId(uid); err == nil { + ownerName = uobj.Username + } + if gobj, err := user.LookupGroupId(gid); err == nil { + groupName = gobj.Name + } + r.Owner = ownerName + if expected.Owner != "" { + if ownerName == "" { + r.Problems = append(r.Problems, fmt.Sprintf("could not determine owner for %s (uid=%s)", path, uid)) + } else if ownerName != expected.Owner { + r.Problems = append(r.Problems, fmt.Sprintf("expected owner (%s), got (%s)", expected.Owner, ownerName)) + } + } + _ = groupName // currently not used in report + } else { + // Sys() not available (e.g., in-memory FS); only check owner if not specified + if expected.Owner != "" { + // we can't determine owner; report a problem + r.Problems = append(r.Problems, fmt.Sprintf("owner check requested but Sys() unavailable for %s", path)) + } + } + return r, nil +} diff --git a/policy/files/acl_unix_test.go b/policy/files/acl_unix_test.go new file mode 100644 index 00000000..6c11f496 --- /dev/null +++ b/policy/files/acl_unix_test.go @@ -0,0 +1,33 @@ +//go:build !windows +// +build !windows + +package files + +import ( + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/require" +) + +func TestUnixACLVerifier_ModeMatchAndMismatch(t *testing.T) { + fs := afero.NewMemMapFs() + _ = fs.MkdirAll("/etc/opk", 0o750) + path := "/etc/opk/auth_id" + err := afero.WriteFile(fs, path, []byte("test"), 0o640) + require.NoError(t, err) + + v := NewDefaultACLVerifier(fs) + + // Expect correct mode + report, err := v.VerifyACL(path, ExpectedACL{Mode: 0o640}) + require.NoError(t, err) + require.True(t, report.Exists) + require.Empty(t, report.Problems) + + // Expect mismatch + report2, err := v.VerifyACL(path, ExpectedACL{Mode: 0o600}) + require.NoError(t, err) + require.True(t, report2.Exists) + require.NotEmpty(t, report2.Problems) +} diff --git a/policy/files/acl_windows.go b/policy/files/acl_windows.go new file mode 100644 index 00000000..c4d7db03 --- /dev/null +++ b/policy/files/acl_windows.go @@ -0,0 +1,320 @@ +//go:build windows +// +build windows + +package files + +import ( + "fmt" + "strings" + "syscall" + "unsafe" + + "github.com/spf13/afero" +) + +// maskToRights maps common Windows access mask bits to readable names. +func maskToRights(mask uint32) string { + var parts []string + if mask&0x80000000 != 0 { + parts = append(parts, "GENERIC_READ") + } + if mask&0x40000000 != 0 { + parts = append(parts, "GENERIC_WRITE") + } + if mask&0x20000000 != 0 { + parts = append(parts, "GENERIC_EXECUTE") + } + if mask&0x10000000 != 0 { + parts = append(parts, "GENERIC_ALL") + } + // File-specific + if mask&0x00000001 != 0 { + parts = append(parts, "FILE_READ_DATA") + } + if mask&0x00000002 != 0 { + parts = append(parts, "FILE_WRITE_DATA") + } + if mask&0x00000004 != 0 { + parts = append(parts, "FILE_APPEND_DATA") + } + if mask&0x00000008 != 0 { + parts = append(parts, "FILE_READ_EA") + } + if mask&0x00000010 != 0 { + parts = append(parts, "FILE_WRITE_EA") + } + if mask&0x00000020 != 0 { + parts = append(parts, "FILE_EXECUTE") + } + if mask&0x00000040 != 0 { + parts = append(parts, "FILE_DELETE_CHILD") + } + if mask&0x00000080 != 0 { + parts = append(parts, "FILE_READ_ATTRIBUTES") + } + if mask&0x00000100 != 0 { + parts = append(parts, "FILE_WRITE_ATTRIBUTES") + } + // Standard rights + if mask&0x00010000 != 0 { + parts = append(parts, "DELETE") + } + if mask&0x00020000 != 0 { + parts = append(parts, "READ_CONTROL") + } + if mask&0x00040000 != 0 { + parts = append(parts, "WRITE_DAC") + } + if mask&0x00080000 != 0 { + parts = append(parts, "WRITE_OWNER") + } + if mask&0x00100000 != 0 { + parts = append(parts, "SYNCHRONIZE") + } + if len(parts) == 0 { + return fmt.Sprintf("0x%x", mask) + } + return strings.Join(parts, ",") +} + +var ( + advapi32 = syscall.NewLazyDLL("advapi32.dll") + procGetNamedSecInfo = advapi32.NewProc("GetNamedSecurityInfoW") + procLookupAccountSid = advapi32.NewProc("LookupAccountSidW") + procGetLengthSid = advapi32.NewProc("GetLengthSid") + procGetAclInformation = advapi32.NewProc("GetAclInformation") + procGetAce = advapi32.NewProc("GetAce") + kernel32 = syscall.NewLazyDLL("kernel32.dll") + procLocalFree = kernel32.NewProc("LocalFree") +) + +const ( + SE_FILE_OBJECT = 1 + OWNER_SECURITY_INFORMATION = 0x00000001 + DACL_SECURITY_INFORMATION = 0x00000004 + AclSizeInformation = 2 + INHERITED_ACE = 0x10 + ACCESS_ALLOWED_ACE_TYPE = 0 + ACCESS_DENIED_ACE_TYPE = 1 +) + +// WindowsACLVerifier implements ACLVerifier on Windows using Win32 APIs. +// It obtains the file owner and enumerates ACEs from the DACL. +type WindowsACLVerifier struct { + Fs afero.Fs +} + +func NewDefaultACLVerifier(fs afero.Fs) ACLVerifier { + return &WindowsACLVerifier{Fs: fs} +} + +func utf16PtrFromStringNullable(s string) (*uint16, error) { + if s == "" { + return nil, nil + } + return syscall.UTF16PtrFromString(s) +} + +func (w *WindowsACLVerifier) VerifyACL(path string, expected ExpectedACL) (ACLReport, error) { + r := ACLReport{Path: path} + if w.Fs == nil { + w.Fs = afero.NewOsFs() + } + if _, err := w.Fs.Stat(path); err != nil { + r.Exists = false + r.Problems = append(r.Problems, fmt.Sprintf("open %s: %v", path, err)) + return r, nil + } + r.Exists = true + + // Call GetNamedSecurityInfoW to get owner SID, DACL and security descriptor + pPath, _ := utf16PtrFromStringNullable(path) + var pOwner uintptr + var pDacl uintptr + var pSD uintptr + // Get owner and DACL + flags := OWNER_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION + ret, _, _ := procGetNamedSecInfo.Call( + uintptr(unsafe.Pointer(pPath)), + uintptr(SE_FILE_OBJECT), + uintptr(flags), + uintptr(unsafe.Pointer(&pOwner)), + 0, + uintptr(unsafe.Pointer(&pDacl)), + 0, + uintptr(unsafe.Pointer(&pSD)), + ) + if ret != 0 { + r.Problems = append(r.Problems, fmt.Sprintf("GetNamedSecurityInfoW failed: error=%d", ret)) + return r, nil + } + // Ensure security descriptor memory is freed + if pSD != 0 { + defer procLocalFree.Call(pSD) + } + + // Lookup owner name if available + if pOwner != 0 { + // Copy owner SID raw bytes into report for precise assertions + ownerSidLenRet, _, _ := procGetLengthSid.Call(pOwner) + if ownerSidLenRet != 0 { + ownerSidLen := int(ownerSidLenRet) + ownerSidBytes := make([]byte, ownerSidLen) + for i := 0; i < ownerSidLen; i++ { + ownerSidBytes[i] = *(*byte)(unsafe.Pointer(pOwner + uintptr(i))) + } + r.OwnerSID = ownerSidBytes + if s, err := ConvertSidToString(ownerSidBytes); err == nil { + r.OwnerSIDStr = s + } + } + + var nameLen uint32 + var domLen uint32 + var sidUse uint32 + // First call to get sizes + procLookupAccountSid.Call( + 0, + pOwner, + 0, + uintptr(unsafe.Pointer(&nameLen)), + 0, + uintptr(unsafe.Pointer(&domLen)), + uintptr(unsafe.Pointer(&sidUse)), + ) + if nameLen != 0 { + name := make([]uint16, nameLen) + dom := make([]uint16, domLen) + success, _, err := procLookupAccountSid.Call( + 0, + pOwner, + uintptr(unsafe.Pointer(&name[0])), + uintptr(unsafe.Pointer(&nameLen)), + uintptr(unsafe.Pointer(&dom[0])), + uintptr(unsafe.Pointer(&domLen)), + uintptr(unsafe.Pointer(&sidUse)), + ) + if success == 0 { + r.Problems = append(r.Problems, fmt.Sprintf("LookupAccountSidW failed: %v", err)) + } else { + r.Owner = syscall.UTF16ToString(name) + if expected.Owner != "" && r.Owner != expected.Owner { + r.Problems = append(r.Problems, fmt.Sprintf("expected owner (%s), got (%s)", expected.Owner, r.Owner)) + } + } + } else { + r.Problems = append(r.Problems, "LookupAccountSidW: could not determine required name buffer size") + } + } else { + r.Problems = append(r.Problems, "owner SID not available") + } + + // If DACL available, enumerate ACEs + if pDacl != 0 { + var info struct { + AceCount uint32 + AclBytesInUse uint32 + AclBytesFree uint32 + } + // GetAclInformation to retrieve AceCount + ret2, _, _ := procGetAclInformation.Call( + pDacl, + uintptr(unsafe.Pointer(&info)), + uintptr(unsafe.Sizeof(info)), + uintptr(AclSizeInformation), + ) + if ret2 == 0 { + r.Problems = append(r.Problems, fmt.Sprintf("GetAclInformation failed: %d", ret2)) + } else { + // iterate over ACEs + for i := uint32(0); i < info.AceCount; i++ { + var pAce uintptr + r3, _, _ := procGetAce.Call(pDacl, uintptr(i), uintptr(unsafe.Pointer(&pAce))) + if r3 == 0 || pAce == 0 { + r.Problems = append(r.Problems, fmt.Sprintf("GetAce failed for index %d", i)) + continue + } + // Read ACE header: Type(1), Flags(1), Size(2) + aceType := *(*byte)(unsafe.Pointer(pAce)) + aceFlags := *(*byte)(unsafe.Pointer(pAce + 1)) + // Mask is at offset 4 (after 4-byte header) + mask := *(*uint32)(unsafe.Pointer(pAce + 4)) + sidPtr := pAce + 8 + + // Lookup account name for SID + var nameLen uint32 + var domLen uint32 + var sidUse uint32 + procLookupAccountSid.Call( + 0, + sidPtr, + 0, + uintptr(unsafe.Pointer(&nameLen)), + 0, + uintptr(unsafe.Pointer(&domLen)), + uintptr(unsafe.Pointer(&sidUse)), + ) + principal := "" + if nameLen != 0 { + name := make([]uint16, nameLen) + dom := make([]uint16, domLen) + success, _, err := procLookupAccountSid.Call( + 0, + sidPtr, + uintptr(unsafe.Pointer(&name[0])), + uintptr(unsafe.Pointer(&nameLen)), + uintptr(unsafe.Pointer(&dom[0])), + uintptr(unsafe.Pointer(&domLen)), + uintptr(unsafe.Pointer(&sidUse)), + ) + if success != 0 { + principal = syscall.UTF16ToString(name) + } else { + _ = err + } + } + + // Copy SID bytes so callers/tests can assert exact SIDs + var sidBytes []byte + // GetLengthSid returns the size of the SID in bytes + sidLenRet, _, _ := procGetLengthSid.Call(sidPtr) + if sidLenRet != 0 { + sidLen := int(sidLenRet) + sidBytes = make([]byte, sidLen) + for idx := 0; idx < sidLen; idx++ { + b := *(*byte)(unsafe.Pointer(sidPtr + uintptr(idx))) + sidBytes[idx] = b + } + } + + rights := maskToRights(mask) + ace := ACE{ + Principal: principal, + PrincipalSID: sidBytes, + PrincipalSIDStr: "", + Rights: rights, + Type: func() string { + if aceType == ACCESS_ALLOWED_ACE_TYPE { + return "allow" + } + if aceType == ACCESS_DENIED_ACE_TYPE { + return "deny" + } + return fmt.Sprintf("type-%d", aceType) + }(), + Inherited: (aceFlags & INHERITED_ACE) != 0, + } + + // Convert SID bytes to textual SID for easier assertions/logging + if len(sidBytes) > 0 { + if s, err := ConvertSidToString(sidBytes); err == nil { + ace.PrincipalSIDStr = s + } + } + r.ACEs = append(r.ACEs, ace) + } + } + } + + return r, nil +} diff --git a/policy/files/acl_windows_unit_test.go b/policy/files/acl_windows_unit_test.go new file mode 100644 index 00000000..a6fdeb60 --- /dev/null +++ b/policy/files/acl_windows_unit_test.go @@ -0,0 +1,42 @@ +//go:build windows +// +build windows + +package files + +import ( + "strings" + "testing" +) + +// Test that maskToRights returns expected tokens for some common masks. +func TestMaskToRights_Common(t *testing.T) { + m := uint32(0x80000000) // GENERIC_READ + s := maskToRights(m) + if !strings.Contains(s, "GENERIC_READ") { + t.Fatalf("expected GENERIC_READ in %q", s) + } + + m = 0x10000000 // GENERIC_ALL + s = maskToRights(m) + if !strings.Contains(s, "GENERIC_ALL") { + t.Fatalf("expected GENERIC_ALL in %q", s) + } +} + +// Test resolving a well-known account and converting to textual SID. +func TestResolveAndConvertAdministratorsSID(t *testing.T) { + sid, _, err := ResolveAccountToSID("Administrators") + if err != nil { + t.Fatalf("ResolveAccountToSID failed: %v", err) + } + if len(sid) == 0 { + t.Fatalf("empty SID returned") + } + s, err := ConvertSidToString(sid) + if err != nil { + t.Fatalf("ConvertSidToString failed: %v", err) + } + if len(s) < 2 || s[:2] != "S-" { + t.Fatalf("unexpected SID string: %q", s) + } +} diff --git a/policy/files/fileperms_lookup_stub.go b/policy/files/fileperms_lookup_stub.go new file mode 100644 index 00000000..19e3005e --- /dev/null +++ b/policy/files/fileperms_lookup_stub.go @@ -0,0 +1,11 @@ +//go:build !windows +// +build !windows + +package files + +import "fmt" + +// ResolveAccountToSID is a stub on non-Windows platforms. +func ResolveAccountToSID(name string) ([]byte, uint32, error) { + return nil, 0, fmt.Errorf("ResolveAccountToSID is only supported on Windows") +} diff --git a/policy/files/fileperms_ops.go b/policy/files/fileperms_ops.go new file mode 100644 index 00000000..16b93d03 --- /dev/null +++ b/policy/files/fileperms_ops.go @@ -0,0 +1,123 @@ +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package files + +import ( + "io/fs" + "os" + "os/user" + "path/filepath" + "runtime" + "strconv" + + "github.com/spf13/afero" +) + +// FilePermsOps provides an abstraction for creating files/directories and +// setting permissions in a platform-aware way. +type FilePermsOps interface { + MkdirAllWithPerm(path string, perm fs.FileMode) error + CreateFileWithPerm(path string) (afero.File, error) + WriteFileWithPerm(path string, data []byte, perm fs.FileMode) error + Chmod(path string, perm fs.FileMode) error + Stat(path string) (fs.FileInfo, error) + Chown(path string, owner string, group string) error + // ApplyACE applies a single ACE to the target path. On platforms that + // don't support ACE modifications, this may be a no-op or return nil. + ApplyACE(path string, ace ACE) error +} + +// OsFilePermsOps is a default implementation that delegates to an afero.Fs +// for filesystem operations and uses os.Chown when required. +type OsFilePermsOps struct { + Fs afero.Fs +} + +func NewDefaultFilePermsOps(fs afero.Fs) FilePermsOps { + if runtime.GOOS == "windows" { + // Prefer ACL-capable implementation on Windows + return NewWindowsACLFilePermsOps(fs) + } + return &OsFilePermsOps{Fs: fs} +} + +func (o *OsFilePermsOps) MkdirAllWithPerm(path string, perm fs.FileMode) error { + return o.Fs.MkdirAll(path, perm) +} + +func (o *OsFilePermsOps) CreateFileWithPerm(path string) (afero.File, error) { + // Ensure parent directory exists + dir := filepath.Dir(path) + if err := o.Fs.MkdirAll(dir, 0o750); err != nil { + return nil, err + } + return o.Fs.Create(path) +} + +func (o *OsFilePermsOps) WriteFileWithPerm(path string, data []byte, perm fs.FileMode) error { + return afero.WriteFile(o.Fs, path, data, perm) +} + +func (o *OsFilePermsOps) Chmod(path string, perm fs.FileMode) error { + return o.Fs.Chmod(path, perm) +} + +func (o *OsFilePermsOps) Stat(path string) (fs.FileInfo, error) { + return o.Fs.Stat(path) +} + +func (o *OsFilePermsOps) Chown(path string, owner string, group string) error { + // If nothing requested, nothing to do + if owner == "" && group == "" { + return nil + } + // On Windows, mapping POSIX chown isn't meaningful; return nil + if runtime.GOOS == "windows" { + return nil + } + // Lookup uid/gid + var uid int + var gid int + if owner != "" { + uobj, err := user.Lookup(owner) + if err != nil { + return err + } + uid64, err := strconv.ParseInt(uobj.Uid, 10, 32) + if err != nil { + return err + } + uid = int(uid64) + } + if group != "" { + gobj, err := user.LookupGroup(group) + if err != nil { + return err + } + gid64, err := strconv.ParseInt(gobj.Gid, 10, 32) + if err != nil { + return err + } + gid = int(gid64) + } + return os.Chown(path, uid, gid) +} + +func (o *OsFilePermsOps) ApplyACE(path string, ace ACE) error { + // POSIX: ACEs are not supported in this abstraction. No-op. + return nil +} diff --git a/policy/files/fileperms_ops_windows.go b/policy/files/fileperms_ops_windows.go new file mode 100644 index 00000000..b020008a --- /dev/null +++ b/policy/files/fileperms_ops_windows.go @@ -0,0 +1,60 @@ +//go:build windows +// +build windows + +package files + +import ( + "io/fs" + + "github.com/spf13/afero" +) + +// WindowsFilePermsOps implements FilePermsOps for Windows. +// On Windows, POSIX permission bits and chown semantics do not apply. This +// implementation delegates file operations to the provided afero.Fs and +// performs no-op for owner/group semantics. Later we can extend this to call +// into Win32 APIs or PowerShell/icacls to verify and set ACLs. +type WindowsFilePermsOps struct { + Fs afero.Fs +} + +func NewWindowsFilePermsOps(fs afero.Fs) FilePermsOps { + return &WindowsFilePermsOps{Fs: fs} +} + +func (w *WindowsFilePermsOps) MkdirAllWithPerm(path string, perm fs.FileMode) error { + // afero on Windows will create directories; perm is informational here + return w.Fs.MkdirAll(path, perm) +} + +func (w *WindowsFilePermsOps) CreateFileWithPerm(path string) (afero.File, error) { + // Create will create file with default attributes. ACLs should be managed + // by installer; runtime enforcement via ACL queries can be added later. + return w.Fs.Create(path) +} + +func (w *WindowsFilePermsOps) WriteFileWithPerm(path string, data []byte, perm fs.FileMode) error { + return afero.WriteFile(w.Fs, path, data, perm) +} + +func (w *WindowsFilePermsOps) Chmod(path string, perm fs.FileMode) error { + // On Windows, Chmod updates read-only attribute via os.Chmod; delegate to Fs + return w.Fs.Chmod(path, perm) +} + +func (w *WindowsFilePermsOps) Stat(path string) (fs.FileInfo, error) { + return w.Fs.Stat(path) +} + +func (w *WindowsFilePermsOps) Chown(path string, owner string, group string) error { + // No-op on Windows. Owner/group/ACLs should be managed by installer or + // an explicit ACL helper. Returning nil keeps behavior permissive but + // allows callers to continue operating. + return nil +} + +func (w *WindowsFilePermsOps) ApplyACE(path string, ace ACE) error { + // No-op default for simple WindowsFilePermsOps. Use WindowsACLFilePermsOps + // for icacls-based ACL modifications. + return nil +} diff --git a/policy/files/fileperms_ops_windows_acl.go b/policy/files/fileperms_ops_windows_acl.go new file mode 100644 index 00000000..c69005bf --- /dev/null +++ b/policy/files/fileperms_ops_windows_acl.go @@ -0,0 +1,307 @@ +//go:build windows +// +build windows + +package files + +import ( + "fmt" + "io/fs" + "strings" + "syscall" + "unsafe" + + "github.com/spf13/afero" +) + +// WindowsACLFilePermsOps implements FilePermsOps using icacls for ACL changes. +// This provides a stricter mapping of ownership/ACL semantics on Windows. +type WindowsACLFilePermsOps struct { + Fs afero.Fs +} + +// NewWindowsACLFilePermsOps returns a FilePermsOps that applies ACL changes +// using icacls. This is more suitable for production Windows installs where +// runtime verification or repair of ACLs is desired. +func NewWindowsACLFilePermsOps(fs afero.Fs) FilePermsOps { + return &WindowsACLFilePermsOps{Fs: fs} +} + +func (w *WindowsACLFilePermsOps) MkdirAllWithPerm(path string, perm fs.FileMode) error { + return w.Fs.MkdirAll(path, perm) +} + +func (w *WindowsACLFilePermsOps) CreateFileWithPerm(path string) (afero.File, error) { + return w.Fs.Create(path) +} + +func (w *WindowsACLFilePermsOps) WriteFileWithPerm(path string, data []byte, perm fs.FileMode) error { + return afero.WriteFile(w.Fs, path, data, perm) +} + +func (w *WindowsACLFilePermsOps) Chmod(path string, perm fs.FileMode) error { + return w.Fs.Chmod(path, perm) +} + +func (w *WindowsACLFilePermsOps) Stat(path string) (fs.FileInfo, error) { + return w.Fs.Stat(path) +} + +// Chown attempts to set owner and grant basic ACLs using icacls. If icacls is +// not available or the operation fails, an error is returned. +func (w *WindowsACLFilePermsOps) Chown(path string, owner string, group string) error { + // If nothing requested, nothing to do + if owner == "" && group == "" { + return nil + } + + // Map common POSIX names to Windows principals + ownerName := owner + if owner == "root" { + ownerName = "Administrators" + } + + // Set owner via Win32 LookupAccountNameW -> SetNamedSecurityInfoW + if ownerName != "" { + sid, _, err := ResolveAccountToSID(ownerName) + if err != nil { + return fmt.Errorf("LookupAccountNameW failed for %s: %v", ownerName, err) + } + // Apply owner using SetNamedSecurityInfoW + pPath, _ := syscall.UTF16PtrFromString(path) + ret2, _, err := procSetNamedSecurityInfo.Call( + uintptr(unsafe.Pointer(pPath)), + uintptr(SE_FILE_OBJECT), + uintptr(OWNER_SECURITY_INFORMATION), + uintptr(unsafe.Pointer(&sid[0])), + 0, + 0, + 0, + ) + if ret2 != 0 { + return fmt.Errorf("SetNamedSecurityInfoW (owner) failed: %v (ret=%d)", err, ret2) + } + } + + // If group provided, grant GENERIC_READ via ApplyACE + if group != "" { + if err := w.ApplyACE(path, ACE{Principal: group, Rights: "GENERIC_READ", Type: "allow"}); err != nil { + return fmt.Errorf("failed to apply group ACE: %v", err) + } + } + + // Ensure Administrators and SYSTEM have full control via ApplyACE + if err := w.ApplyACE(path, ACE{Principal: "Administrators", Rights: "GENERIC_ALL", Type: "allow"}); err != nil { + return fmt.Errorf("ensure admin ACE failed: %v", err) + } + if err := w.ApplyACE(path, ACE{Principal: "SYSTEM", Rights: "GENERIC_ALL", Type: "allow"}); err != nil { + return fmt.Errorf("ensure system ACE failed: %v", err) + } + + return nil +} + +// EXPLICIT_ACCESS and TRUSTEE definitions for calling SetEntriesInAclW +type _TRUSTEE struct { + MultipleTrustee uintptr + MultipleTrusteeOperator uint32 + TrusteeForm uint32 + TrusteeType uint32 + PtstrName unsafe.Pointer +} + +type _EXPLICIT_ACCESS struct { + GrfAccessPermissions uint32 + GrfAccessMode uint32 + GrfInheritance uint32 + Trustee _TRUSTEE +} + +var ( + procSetEntriesInAcl = advapi32.NewProc("SetEntriesInAclW") + procSetNamedSecurityInfo = advapi32.NewProc("SetNamedSecurityInfoW") +) + +const ( + // from Winnt.h / AccCtrl.h + GRANT_ACCESS = 1 + NO_INHERITANCE = 0 + TRUSTEE_IS_NAME = 1 + TRUSTEE_IS_SID = 0 + TRUSTEE_IS_UNKNOWN = 0 +) + +// Trustee type constants (match SID_NAME_USE values where appropriate) +const ( + TRUSTEE_TYPE_UNKNOWN = 0 + TRUSTEE_TYPE_USER = 1 + TRUSTEE_TYPE_GROUP = 2 + TRUSTEE_TYPE_DOMAIN = 3 + TRUSTEE_TYPE_ALIAS = 4 + TRUSTEE_TYPE_WELL_KNOWN_GROUP = 5 + TRUSTEE_TYPE_DELETED = 6 + TRUSTEE_TYPE_INVALID = 7 + TRUSTEE_TYPE_COMPUTER = 8 + TRUSTEE_TYPE_LABEL = 9 +) + +func sidUseToTrusteeType(sidUse uint32) uint32 { + switch sidUse { + case 1: + return TRUSTEE_TYPE_USER + case 2: + return TRUSTEE_TYPE_GROUP + case 3: + return TRUSTEE_TYPE_DOMAIN + case 4: + return TRUSTEE_TYPE_ALIAS + case 5: + return TRUSTEE_TYPE_WELL_KNOWN_GROUP + case 6: + return TRUSTEE_TYPE_DELETED + case 7: + return TRUSTEE_TYPE_INVALID + case 8: + return TRUSTEE_TYPE_COMPUTER + case 9: + return TRUSTEE_TYPE_LABEL + default: + return TRUSTEE_TYPE_UNKNOWN + } +} + +// rightsToMask converts a human-readable rights string into a Windows access mask. +func rightsToMask(rights string) uint32 { + var m uint32 + if strings.Contains(rights, "GENERIC_ALL") { + m |= 0x10000000 + } + if strings.Contains(rights, "GENERIC_READ") { + m |= 0x80000000 + } + if strings.Contains(rights, "GENERIC_WRITE") { + m |= 0x40000000 + } + if strings.Contains(rights, "FILE_READ_DATA") { + m |= 0x00000001 + } + if strings.Contains(rights, "FILE_WRITE_DATA") { + m |= 0x00000002 + } + if strings.Contains(rights, "FILE_APPEND_DATA") { + m |= 0x00000004 + } + if strings.Contains(rights, "FILE_EXECUTE") { + m |= 0x00000020 + } + if strings.Contains(rights, "READ_CONTROL") { + m |= 0x00020000 + } + if strings.Contains(rights, "WRITE_DAC") { + m |= 0x00040000 + } + if strings.Contains(rights, "WRITE_OWNER") { + m |= 0x00080000 + } + return m +} + +// ApplyACE via Win32 APIs (SetEntriesInAclW + SetNamedSecurityInfoW) +func (w *WindowsACLFilePermsOps) ApplyACE(path string, ace ACE) error { + // Currently only supports adding simple allow/deny entries by account name + pPath, _ := syscall.UTF16PtrFromString(path) + + // Get existing DACL + var pDacl uintptr + var pSD uintptr + ret, _, _ := procGetNamedSecInfo.Call( + uintptr(unsafe.Pointer(pPath)), + uintptr(SE_FILE_OBJECT), + uintptr(DACL_SECURITY_INFORMATION), + 0, + 0, + uintptr(unsafe.Pointer(&pDacl)), + 0, + uintptr(unsafe.Pointer(&pSD)), + ) + if ret != 0 { + return fmt.Errorf("GetNamedSecurityInfoW failed: %d", ret) + } + if pSD != 0 { + defer procLocalFree.Call(pSD) + } + + // Build EXPLICIT_ACCESS + var ea _EXPLICIT_ACCESS + ea.GrfAccessPermissions = rightsToMask(ace.Rights) + if ace.Type == "allow" { + ea.GrfAccessMode = GRANT_ACCESS + } else { + // For deny use DENY_ACCESS(3) per ACCESS_MODE, but SetEntriesInAcl supports DENY_ACCESS as 3 + ea.GrfAccessMode = 3 + } + ea.GrfInheritance = NO_INHERITANCE + + // Prefer using provided SID if available + if len(ace.PrincipalSID) > 0 { + ea.Trustee = _TRUSTEE{ + MultipleTrustee: 0, + MultipleTrusteeOperator: 0, + TrusteeForm: TRUSTEE_IS_SID, + TrusteeType: TRUSTEE_TYPE_UNKNOWN, + PtstrName: unsafe.Pointer(&ace.PrincipalSID[0]), + } + } else { + // Attempt to resolve to SID via helper + sid, sidUse, err := ResolveAccountToSID(ace.Principal) + if err == nil && len(sid) > 0 { + ea.Trustee = _TRUSTEE{ + MultipleTrustee: 0, + MultipleTrusteeOperator: 0, + TrusteeForm: TRUSTEE_IS_SID, + TrusteeType: sidUseToTrusteeType(sidUse), + PtstrName: unsafe.Pointer(&sid[0]), + } + } else { + pName, _ := syscall.UTF16PtrFromString(ace.Principal) + ea.Trustee = _TRUSTEE{ + MultipleTrustee: 0, + MultipleTrusteeOperator: 0, + TrusteeForm: TRUSTEE_IS_NAME, + TrusteeType: TRUSTEE_TYPE_UNKNOWN, + PtstrName: unsafe.Pointer(pName), + } + } + } + + // Call SetEntriesInAclW + var pNewAcl uintptr + ret2, _, err := procSetEntriesInAcl.Call( + uintptr(1), + uintptr(unsafe.Pointer(&ea)), + uintptr(pDacl), + uintptr(unsafe.Pointer(&pNewAcl)), + ) + if ret2 != 0 { + return fmt.Errorf("SetEntriesInAclW failed: %v (ret=%d)", err, ret2) + } + if pNewAcl == 0 { + return fmt.Errorf("SetEntriesInAclW returned nil ACL") + } + defer procLocalFree.Call(pNewAcl) + + // Apply new DACL to the file + ret3, _, err := procSetNamedSecurityInfo.Call( + uintptr(unsafe.Pointer(pPath)), + uintptr(SE_FILE_OBJECT), + uintptr(DACL_SECURITY_INFORMATION), + 0, + 0, + uintptr(unsafe.Pointer(pNewAcl)), + 0, + ) + if ret3 != 0 { + return fmt.Errorf("SetNamedSecurityInfoW failed: %v (ret=%d)", err, ret3) + } + + return nil +} diff --git a/policy/files/fileperms_ops_windows_stub.go b/policy/files/fileperms_ops_windows_stub.go new file mode 100644 index 00000000..6fa19d27 --- /dev/null +++ b/policy/files/fileperms_ops_windows_stub.go @@ -0,0 +1,15 @@ +//go:build !windows +// +build !windows + +package files + +import ( + "github.com/spf13/afero" +) + +// NewWindowsACLFilePermsOps is a stub for non-Windows platforms and returns +// the default OsFilePermsOps so code that references this symbol compiles on +// all platforms. +func NewWindowsACLFilePermsOps(fs afero.Fs) FilePermsOps { + return &OsFilePermsOps{Fs: fs} +} diff --git a/policy/files/filesystem.go b/policy/files/filesystem.go new file mode 100644 index 00000000..64164ec6 --- /dev/null +++ b/policy/files/filesystem.go @@ -0,0 +1,143 @@ +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package files + +import ( + "io/fs" + + "github.com/spf13/afero" +) + +// FileSystem abstracts all filesystem and permission operations needed by +// the permissions and audit commands. It combines file I/O, permission +// mutations, ownership management, and ACL verification into a single +// mockable interface that hides platform-specific details. +type FileSystem interface { + // Stat returns file info for the given path. + Stat(path string) (fs.FileInfo, error) + // Exists reports whether the path exists. + Exists(path string) (bool, error) + // Open opens a file for reading (e.g. directory listing via Readdir). + Open(path string) (afero.File, error) + // ReadFile reads the entire contents of a file. + ReadFile(path string) ([]byte, error) + + // MkdirAll creates a directory and all parents with the given permission. + MkdirAll(path string, perm fs.FileMode) error + // CreateFile creates an empty file, creating parent directories as needed. + CreateFile(path string) (afero.File, error) + // WriteFile writes data to a file with the given permission. + WriteFile(path string, data []byte, perm fs.FileMode) error + + // Chmod sets the permission mode bits on a path. + Chmod(path string, perm fs.FileMode) error + // Chown sets the owner and group on a path. + Chown(path string, owner string, group string) error + // ApplyACE applies a single access control entry to a path. + ApplyACE(path string, ace ACE) error + + // CheckPerm verifies that the file at path has one of the required + // permission modes and, optionally, the expected owner and group. + CheckPerm(path string, requirePerm []fs.FileMode, requiredOwner string, requiredGroup string) error + // VerifyACL checks ACLs and ownership against expectations. + VerifyACL(path string, expected ExpectedACL) (ACLReport, error) +} + +// defaultFileSystem implements FileSystem by delegating to existing +// platform-specific implementations. +type defaultFileSystem struct { + afs afero.Fs + ops FilePermsOps + checker *PermsChecker + acl ACLVerifier +} + +// FileSystemOption configures a FileSystem created by NewFileSystem. +type FileSystemOption func(*defaultFileSystem) + +// WithCmdRunner overrides the command runner used by the permission +// checker. This is useful in tests where the real "stat" command +// cannot be used against an in-memory filesystem. +func WithCmdRunner(runner func(string, ...string) ([]byte, error)) FileSystemOption { + return func(d *defaultFileSystem) { + d.checker.CmdRunner = runner + } +} + +// NewFileSystem creates a FileSystem backed by an afero.Fs. It wires up +// the appropriate platform-specific implementations for permission +// operations, permission checking, and ACL verification. +func NewFileSystem(afs afero.Fs, opts ...FileSystemOption) FileSystem { + d := &defaultFileSystem{ + afs: afs, + ops: NewDefaultFilePermsOps(afs), + checker: NewPermsChecker(afs), + acl: NewDefaultACLVerifier(afs), + } + for _, opt := range opts { + opt(d) + } + return d +} + +func (d *defaultFileSystem) Stat(path string) (fs.FileInfo, error) { + return d.afs.Stat(path) +} + +func (d *defaultFileSystem) Exists(path string) (bool, error) { + return afero.Exists(d.afs, path) +} + +func (d *defaultFileSystem) Open(path string) (afero.File, error) { + return d.afs.Open(path) +} + +func (d *defaultFileSystem) ReadFile(path string) ([]byte, error) { + return afero.ReadFile(d.afs, path) +} + +func (d *defaultFileSystem) MkdirAll(path string, perm fs.FileMode) error { + return d.ops.MkdirAllWithPerm(path, perm) +} + +func (d *defaultFileSystem) CreateFile(path string) (afero.File, error) { + return d.ops.CreateFileWithPerm(path) +} + +func (d *defaultFileSystem) WriteFile(path string, data []byte, perm fs.FileMode) error { + return d.ops.WriteFileWithPerm(path, data, perm) +} + +func (d *defaultFileSystem) Chmod(path string, perm fs.FileMode) error { + return d.ops.Chmod(path, perm) +} + +func (d *defaultFileSystem) Chown(path string, owner string, group string) error { + return d.ops.Chown(path, owner, group) +} + +func (d *defaultFileSystem) ApplyACE(path string, ace ACE) error { + return d.ops.ApplyACE(path, ace) +} + +func (d *defaultFileSystem) CheckPerm(path string, requirePerm []fs.FileMode, requiredOwner string, requiredGroup string) error { + return d.checker.CheckPerm(path, requirePerm, requiredOwner, requiredGroup) +} + +func (d *defaultFileSystem) VerifyACL(path string, expected ExpectedACL) (ACLReport, error) { + return d.acl.VerifyACL(path, expected) +} diff --git a/policy/files/perminfo.go b/policy/files/perminfo.go new file mode 100644 index 00000000..7278c632 --- /dev/null +++ b/policy/files/perminfo.go @@ -0,0 +1,64 @@ +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package files + +import ( + "fmt" + "io/fs" +) + +// PermInfo describes the expected filesystem permissions for a given resource +// type used by opkssh. It centralises mode, ownership, and existence +// requirements so that they are defined once and consumed by permission +// checking, fixing and auditing code. +type PermInfo struct { + // Mode is the expected Unix permission bits (e.g. 0o640, 0o600, 0o750). + Mode fs.FileMode + // Owner is the expected owner of the file/directory (e.g. "root" on + // Linux, "Administrators" on Windows). An empty string means ownership + // is not checked/enforced. + Owner string + // Group is the expected group owner (e.g. "opksshuser"). An empty + // string means group ownership is not checked/enforced. + Group string + // MustExist indicates whether the resource is required to exist for the + // system to function correctly. + MustExist bool +} + +// String returns a human-readable summary of the permission info. +func (p PermInfo) String() string { + s := fmt.Sprintf("mode=%o", p.Mode) + if p.Owner != "" { + s += " owner=" + p.Owner + } + if p.Group != "" { + s += " group=" + p.Group + } + if p.MustExist { + s += " (required)" + } + return s +} + +// ExpectedACLFromPerm builds an ExpectedACL from a PermInfo. +func ExpectedACLFromPerm(pi PermInfo) ExpectedACL { + return ExpectedACL{ + Owner: pi.Owner, + Mode: pi.Mode, + } +} diff --git a/policy/files/perminfo_unix.go b/policy/files/perminfo_unix.go new file mode 100644 index 00000000..d2fdeca8 --- /dev/null +++ b/policy/files/perminfo_unix.go @@ -0,0 +1,80 @@ +//go:build !windows +// +build !windows + +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package files + +// RequiredPerms defines the expected permissions for each opkssh resource type +// on Unix/Linux systems. +var RequiredPerms = struct { + // SystemPolicy is the system-wide policy file + // (e.g. /etc/opk/auth_id). + SystemPolicy PermInfo + // HomePolicy is the per-user policy file + // (e.g. ~/.opk/auth_id). + HomePolicy PermInfo + // Providers is the provider configuration file + // (e.g. /etc/opk/providers). + Providers PermInfo + // Config is the server configuration file + // (e.g. /etc/opk/config.yml). + Config PermInfo + // PluginsDir is the directory containing policy plugin definitions + // (e.g. /etc/opk/policy.d). + PluginsDir PermInfo + // PluginFile is an individual plugin YAML file inside the plugins + // directory. + PluginFile PermInfo +}{ + SystemPolicy: PermInfo{ + Mode: ModeSystemPerms, // 0o640 + Owner: "root", + Group: "opksshuser", + MustExist: true, + }, + HomePolicy: PermInfo{ + Mode: ModeHomePerms, // 0o600 + Owner: "", // owner is the user themselves + Group: "", + MustExist: false, + }, + Providers: PermInfo{ + Mode: ModeSystemPerms, // 0o640 + Owner: "root", + Group: "opksshuser", + MustExist: false, + }, + Config: PermInfo{ + Mode: ModeSystemPerms, // 0o640 + Owner: "root", + Group: "opksshuser", + MustExist: false, + }, + PluginsDir: PermInfo{ + Mode: 0o750, + Owner: "root", + Group: "", + MustExist: false, + }, + PluginFile: PermInfo{ + Mode: ModeSystemPerms, // 0o640 + Owner: "root", + Group: "", + MustExist: false, + }, +} diff --git a/policy/files/perminfo_windows.go b/policy/files/perminfo_windows.go new file mode 100644 index 00000000..a5b37b1d --- /dev/null +++ b/policy/files/perminfo_windows.go @@ -0,0 +1,80 @@ +//go:build windows +// +build windows + +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package files + +// RequiredPerms defines the expected permissions for each opkssh resource type +// on Windows systems. +var RequiredPerms = struct { + // SystemPolicy is the system-wide policy file + // (e.g. %ProgramData%\opk\auth_id). + SystemPolicy PermInfo + // HomePolicy is the per-user policy file + // (e.g. ~/.opk/auth_id). + HomePolicy PermInfo + // Providers is the provider configuration file + // (e.g. %ProgramData%\opk\providers). + Providers PermInfo + // Config is the server configuration file + // (e.g. %ProgramData%\opk\config.yml). + Config PermInfo + // PluginsDir is the directory containing policy plugin definitions + // (e.g. %ProgramData%\opk\policy.d). + PluginsDir PermInfo + // PluginFile is an individual plugin YAML file inside the plugins + // directory. + PluginFile PermInfo +}{ + SystemPolicy: PermInfo{ + Mode: ModeSystemPerms, // 0o640 + Owner: "Administrators", + Group: "", + MustExist: true, + }, + HomePolicy: PermInfo{ + Mode: ModeHomePerms, // 0o600 + Owner: "", // owner is the user themselves + Group: "", + MustExist: false, + }, + Providers: PermInfo{ + Mode: ModeSystemPerms, // 0o640 + Owner: "Administrators", + Group: "", + MustExist: false, + }, + Config: PermInfo{ + Mode: ModeSystemPerms, // 0o640 + Owner: "Administrators", + Group: "", + MustExist: false, + }, + PluginsDir: PermInfo{ + Mode: 0o750, + Owner: "Administrators", + Group: "", + MustExist: false, + }, + PluginFile: PermInfo{ + Mode: ModeSystemPerms, // 0o640 + Owner: "Administrators", + Group: "", + MustExist: false, + }, +} diff --git a/policy/files/permschecker.go b/policy/files/permschecker.go index a9958c29..9c61a8ef 100644 --- a/policy/files/permschecker.go +++ b/policy/files/permschecker.go @@ -1,3 +1,6 @@ +//go:build !windows +// +build !windows + // Copyright 2025 OpenPubkey // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -19,33 +22,9 @@ package files import ( "fmt" "io/fs" - "os/exec" "strings" - - "github.com/spf13/afero" ) -// ModeSystemPerms is the expected permission bits that should be set for opkssh -// system policy files (`/etc/opk/auth_id`, `/etc/opk/providers`). This mode means -// that only the owner of the file can write/read to the file, but the group which -// should be opksshuser can read the file. -const ModeSystemPerms = fs.FileMode(0640) - -// ModeHomePerms is the expected permission bits that should be set for opkssh -// user home policy files `~/.opk/auth_id`. -const ModeHomePerms = fs.FileMode(0600) - -// PermsChecker contains methods to check the ownership, group -// and file permissions of a file on a Unix-like system. -type PermsChecker struct { - Fs afero.Fs - CmdRunner func(string, ...string) ([]byte, error) -} - -func NewPermsChecker(fs afero.Fs) *PermsChecker { - return &PermsChecker{Fs: fs, CmdRunner: ExecCmd} -} - // CheckPerm checks the file at the given path if it has the desired permissions. // The argument requirePerm is a list to enable the caller to specify multiple // permissions only one of which needs to match the permissions on the file. @@ -99,8 +78,3 @@ func (u *PermsChecker) CheckPerm(path string, requirePerm []fs.FileMode, require return nil } - -func ExecCmd(name string, arg ...string) ([]byte, error) { - cmd := exec.Command(name, arg...) - return cmd.CombinedOutput() -} diff --git a/policy/files/permschecker_common.go b/policy/files/permschecker_common.go new file mode 100644 index 00000000..cbb4d631 --- /dev/null +++ b/policy/files/permschecker_common.go @@ -0,0 +1,50 @@ +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package files + +import ( + "io/fs" + "os/exec" + + "github.com/spf13/afero" +) + +// ModeSystemPerms is the expected permission bits that should be set for opkssh +// system policy files (on Unix: /etc/opk/auth_id, /etc/opk/providers; on Windows: %ProgramData%\opk\auth_id, %ProgramData%\opk\providers). +// This mode means that only the owner of the file can write/read to the file, but the group which +// should be opksshuser can read the file. +const ModeSystemPerms = fs.FileMode(0o640) + +// ModeHomePerms is the expected permission bits that should be set for opkssh +// user home policy files `~/.opk/auth_id`. +const ModeHomePerms = fs.FileMode(0o600) + +// PermsChecker contains methods to check the ownership, group +// and file permissions of a file on a Unix-like system (or Windows). +type PermsChecker struct { + Fs afero.Fs + CmdRunner func(string, ...string) ([]byte, error) +} + +func NewPermsChecker(fs afero.Fs) *PermsChecker { + return &PermsChecker{Fs: fs, CmdRunner: ExecCmd} +} + +func ExecCmd(name string, arg ...string) ([]byte, error) { + cmd := exec.Command(name, arg...) + return cmd.CombinedOutput() +} diff --git a/policy/files/permschecker_test.go b/policy/files/permschecker_test.go index ee44289d..d7321e3d 100644 --- a/policy/files/permschecker_test.go +++ b/policy/files/permschecker_test.go @@ -1,3 +1,6 @@ +//go:build !windows +// +build !windows + // Copyright 2025 OpenPubkey // // Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/policy/files/permschecker_windows.go b/policy/files/permschecker_windows.go new file mode 100644 index 00000000..e8c2fa39 --- /dev/null +++ b/policy/files/permschecker_windows.go @@ -0,0 +1,63 @@ +//go:build windows +// +build windows + +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package files + +import ( + "fmt" + "io/fs" +) + +// CheckPerm checks file permissions on Windows. +// On Windows, we perform a relaxed check compared to Unix systems because: +// 1. Windows doesn't use POSIX permission bits +// 2. Go's os.Stat() synthesizes permission bits from file attributes, not ACLs +// 3. A file without the read-only attribute will always show as 0666, not 0640 +// +// For security on Windows, we rely on: +// - NTFS ACLs set by the installer (Administrators and SYSTEM only) +// - File system level security rather than permission bits +// +// This function validates the file exists and is accessible, but skips +// the strict permission bit check that makes sense on Unix but not on Windows. +func (u *PermsChecker) CheckPerm(path string, requirePerm []fs.FileMode, requiredOwner string, requiredGroup string) error { + // Verify file exists and is accessible + fileInfo, err := u.Fs.Stat(path) + if err != nil { + return fmt.Errorf("failed to describe the file at path: %w", err) + } + + // On Windows, we skip the permission bit check because: + // - Go synthesizes permissions from file attributes, not ACLs + // - Files show as 0666 (rw-rw-rw-) if not read-only, or 0444 (r--r--r--) if read-only + // - There's no way to make a file appear as 0640 through file attributes alone + // - Security is enforced through NTFS ACLs set by the installer + + // We also skip owner/group checks since Windows uses different security model + // (SIDs instead of uid/gid) + + _ = fileInfo // Suppress unused variable warning + _ = requirePerm // Suppress unused variable warning + _ = requiredOwner // Suppress unused variable warning + _ = requiredGroup // Suppress unused variable warning + + // On Windows, if we can stat the file, we consider it acceptable + // The actual security is enforced by NTFS ACLs + return nil +} diff --git a/policy/files/sid_stub_nonwindows.go b/policy/files/sid_stub_nonwindows.go new file mode 100644 index 00000000..33f101ca --- /dev/null +++ b/policy/files/sid_stub_nonwindows.go @@ -0,0 +1,11 @@ +//go:build !windows +// +build !windows + +package files + +import "fmt" + +// ConvertSidToString stub for non-Windows platforms. +func ConvertSidToString(sid []byte) (string, error) { + return "", fmt.Errorf("ConvertSidToString is only supported on Windows") +} diff --git a/policy/files/sid_windows.go b/policy/files/sid_windows.go new file mode 100644 index 00000000..775dbf2e --- /dev/null +++ b/policy/files/sid_windows.go @@ -0,0 +1,83 @@ +//go:build windows +// +build windows + +package files + +import ( + "fmt" + "syscall" + "unsafe" + + "golang.org/x/sys/windows" +) + +var procLookupAccountName = advapi32.NewProc("LookupAccountNameW") +var procConvertSidToString = advapi32.NewProc("ConvertSidToStringSidW") + +// ResolveAccountToSID resolves an account name (e.g. "Administrators") to a +// raw SID byte slice and returns the SID_NAME_USE (sidUse) value. Returns an +// error if resolution fails. +func ResolveAccountToSID(name string) ([]byte, uint32, error) { + if name == "" { + return nil, 0, fmt.Errorf("empty name") + } + pName, _ := syscall.UTF16PtrFromString(name) + var sidSize uint32 + var domSize uint32 + var sidUse uint32 + // first call to determine sizes + procLookupAccountName.Call( + 0, + uintptr(unsafe.Pointer(pName)), + 0, + uintptr(unsafe.Pointer(&sidSize)), + 0, + uintptr(unsafe.Pointer(&domSize)), + uintptr(unsafe.Pointer(&sidUse)), + ) + if sidSize == 0 { + return nil, 0, fmt.Errorf("LookupAccountNameW: could not determine SID buffer size for %s", name) + } + sid := make([]byte, sidSize) + dom := make([]uint16, domSize) + ret, _, err := procLookupAccountName.Call( + 0, + uintptr(unsafe.Pointer(pName)), + uintptr(unsafe.Pointer(&sid[0])), + uintptr(unsafe.Pointer(&sidSize)), + uintptr(unsafe.Pointer(&dom[0])), + uintptr(unsafe.Pointer(&domSize)), + uintptr(unsafe.Pointer(&sidUse)), + ) + if ret == 0 { + return nil, 0, fmt.Errorf("LookupAccountNameW failed for %s: %v", name, err) + } + return sid, sidUse, nil +} + +// ConvertSidToString converts a raw SID byte slice into the standard textual +// SID representation (e.g. S-1-5-32-544). Caller must handle errors. +func ConvertSidToString(sid []byte) (string, error) { + if len(sid) == 0 { + return "", fmt.Errorf("empty SID") + } + var pStr uintptr + ret, _, err := procConvertSidToString.Call( + uintptr(unsafe.Pointer(&sid[0])), + uintptr(unsafe.Pointer(&pStr)), + ) + if ret == 0 { + return "", fmt.Errorf("ConvertSidToStringSidW failed: %v", err) + } + if pStr == 0 { + return "", fmt.Errorf("ConvertSidToStringSidW returned NULL") + } + // pStr is LPWSTR (pointer to UTF-16). Convert to Go string. + wptr := (*uint16)(unsafe.Pointer(pStr)) + s := windows.UTF16PtrToString(wptr) + // Free memory allocated by ConvertSidToStringSidW + if procLocalFree != nil { + procLocalFree.Call(pStr) + } + return s, nil +} diff --git a/policy/multipolicyloader.go b/policy/multipolicyloader.go index 885a4bd8..ea14851d 100644 --- a/policy/multipolicyloader.go +++ b/policy/multipolicyloader.go @@ -18,10 +18,7 @@ package policy import ( "errors" - "fmt" "log" - "os" - "os/exec" "strings" ) @@ -96,29 +93,6 @@ func (l *MultiPolicyLoader) Load() (*Policy, Source, error) { return policy, FileSource(strings.Join(readPaths, ", ")), nil } -// ReadWithSudoScript specifies additional way of loading the policy in the -// user's home directory (`~/.opk/auth_id`). This is needed when the -// AuthorizedKeysCommand user does not have privileges to transverse the user's -// home directory. Instead we call run a command which uses special -// sudoers permissions to read the policy file. -// -// Doing this is more secure than simply giving opkssh sudoer access because -// if there was an RCE in opkssh could be triggered an SSH request via -// AuthorizedKeysCommand, the new opkssh process we use to perform the read -// would not be compromised. Thus, the compromised opkssh process could not assume -// full root privileges. -func ReadWithSudoScript(h *HomePolicyLoader, username string) ([]byte, error) { - // opkssh readhome ensures the file is not a symlink and has the permissions/ownership. - // The default path is /usr/local/bin/opkssh - opkBin, err := os.Executable() - if err != nil { - return nil, fmt.Errorf("error getting opkssh executable path: %w", err) - } - cmd := exec.Command("sudo", "-n", opkBin, "readhome", username) - - homePolicyFileBytes, err := cmd.CombinedOutput() - if err != nil { - return nil, fmt.Errorf("error reading %s home policy using command %v got output %v and err %v", username, cmd, string(homePolicyFileBytes), err) - } - return homePolicyFileBytes, nil -} +// ReadWithSudoScript is defined in platform-specific files: +// - multipolicyloader_unix.go for Unix/Linux systems (uses sudo) +// - multipolicyloader_windows.go for Windows (no sudo available) diff --git a/policy/multipolicyloader_test.go b/policy/multipolicyloader_test.go index b306513d..58882aef 100644 --- a/policy/multipolicyloader_test.go +++ b/policy/multipolicyloader_test.go @@ -18,7 +18,7 @@ package policy_test import ( "fmt" - "path" + "path/filepath" "strings" "testing" @@ -201,7 +201,7 @@ func TestLoad(t *testing.T) { if tt.userPolicy != nil { policyFile, err := tt.userPolicy.ToTable() require.NoError(t, err) - expectedPath := path.Join(ValidUser.HomeDir, ".opk", "auth_id") + expectedPath := filepath.Join(ValidUser.HomeDir, ".opk", "auth_id") err = afero.WriteFile(mockFs, expectedPath, policyFile, 0600) require.NoError(t, err) expectedPaths = append(expectedPaths, expectedPath) diff --git a/policy/multipolicyloader_unix.go b/policy/multipolicyloader_unix.go new file mode 100644 index 00000000..8f2f6e66 --- /dev/null +++ b/policy/multipolicyloader_unix.go @@ -0,0 +1,53 @@ +//go:build !windows +// +build !windows + +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package policy + +import ( + "fmt" + "os" + "os/exec" +) + +// ReadWithSudoScript specifies additional way of loading the policy in the +// user's home directory (`~/.opk/auth_id`). This is needed when the +// AuthorizedKeysCommand user does not have privileges to transverse the user's +// home directory. Instead we call run a command which uses special +// sudoers permissions to read the policy file. +// +// Doing this is more secure than simply giving opkssh sudoer access because +// if there was an RCE in opkssh could be triggered an SSH request via +// AuthorizedKeysCommand, the new opkssh process we use to perform the read +// would not be compromised. Thus, the compromised opkssh process could not assume +// full root privileges. +func ReadWithSudoScript(h *HomePolicyLoader, username string) ([]byte, error) { + // opkssh readhome ensures the file is not a symlink and has the permissions/ownership. + // The default path is /usr/local/bin/opkssh + opkBin, err := os.Executable() + if err != nil { + return nil, fmt.Errorf("error getting opkssh executable path: %w", err) + } + cmd := exec.Command("sudo", "-n", opkBin, "readhome", username) + + homePolicyFileBytes, err := cmd.CombinedOutput() + if err != nil { + return nil, fmt.Errorf("error reading %s home policy using command %v got output %v and err %v", username, cmd, string(homePolicyFileBytes), err) + } + return homePolicyFileBytes, nil +} diff --git a/policy/multipolicyloader_windows.go b/policy/multipolicyloader_windows.go new file mode 100644 index 00000000..90716418 --- /dev/null +++ b/policy/multipolicyloader_windows.go @@ -0,0 +1,33 @@ +//go:build windows +// +build windows + +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package policy + +import ( + "fmt" +) + +// ReadWithSudoScript on Windows does not use sudo (which doesn't exist). +// On Windows, the sshd service runs as LocalSystem which has full access to +// read user home directories, so we don't need privilege escalation. +// This function just returns an error indicating home policy reading failed +// and we should rely on the system policy. +func ReadWithSudoScript(h *HomePolicyLoader, username string) ([]byte, error) { + return nil, fmt.Errorf("home policy file not supported on Windows, will use system policy only") +} diff --git a/policy/paths_unix.go b/policy/paths_unix.go new file mode 100644 index 00000000..f685f52c --- /dev/null +++ b/policy/paths_unix.go @@ -0,0 +1,26 @@ +//go:build !windows +// +build !windows + +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package policy + +// GetSystemConfigBasePath returns the base path for system opkssh configuration. +// On Unix-like systems, this is /etc/opk +func GetSystemConfigBasePath() string { + return "/etc/opk" +} diff --git a/policy/paths_windows.go b/policy/paths_windows.go new file mode 100644 index 00000000..66ce95fd --- /dev/null +++ b/policy/paths_windows.go @@ -0,0 +1,36 @@ +//go:build windows +// +build windows + +// Copyright 2026 OpenPubkey +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package policy + +import ( + "os" + "path/filepath" +) + +// GetSystemConfigBasePath returns the base path for system opkssh configuration. +// On Windows, this is %ProgramData%\opk (typically C:\ProgramData\opk) +func GetSystemConfigBasePath() string { + programData := os.Getenv("ProgramData") + if programData == "" { + // Fallback to default if ProgramData is not set + programData = `C:\ProgramData` + } + return filepath.Join(programData, "opk") +} diff --git a/policy/paths_windows_test.go b/policy/paths_windows_test.go new file mode 100644 index 00000000..d7ae734d --- /dev/null +++ b/policy/paths_windows_test.go @@ -0,0 +1,16 @@ +//go:build windows +// +build windows + +package policy + +import ( + "path/filepath" + "testing" +) + +func TestSystemDefaultProvidersPath_Windows(t *testing.T) { + expected := filepath.Join(GetSystemConfigBasePath(), "providers") + if SystemDefaultProvidersPath != expected { + t.Fatalf("expected SystemDefaultProvidersPath %q, got %q", expected, SystemDefaultProvidersPath) + } +} diff --git a/policy/plugins/plugins.go b/policy/plugins/plugins.go index 291f19ac..f19cbb77 100644 --- a/policy/plugins/plugins.go +++ b/policy/plugins/plugins.go @@ -39,6 +39,12 @@ var requiredPolicyDirPerms = []fs.FileMode{fs.FileMode(0700), fs.FileMode(0750), var requiredPolicyCmdPerms = []fs.FileMode{fs.FileMode(0555), fs.FileMode(0755)} +// RequiredPolicyDirPerms returns the list of acceptable directory permission +// modes for the policy plugin directory. Exported for use by external code. +func RequiredPolicyDirPerms() []fs.FileMode { + return requiredPolicyDirPerms +} + type PluginResult struct { Path string PluginConfig PluginConfig diff --git a/policy/plugins/plugins_test.go b/policy/plugins/plugins_test.go index 23e4fd54..b39b869f 100644 --- a/policy/plugins/plugins_test.go +++ b/policy/plugins/plugins_test.go @@ -213,24 +213,6 @@ name: Example Policy Command enforce_providers: true command: /usr/bin/local/opk/missing-cmd"`}} - configWithBadPerms := []mockFile{ - { - Name: "bad-perms-config.yml", - Permission: 0606, - Content: ` -name: Example Policy Command -enforce_providers: true -command: /usr/bin/local/opk/missing-cmd"`}} - - commandWithBadPerms := []mockFile{ - { - Name: "bad-perms-command.yml", - Permission: 0640, - Content: ` -name: Example Policy Command -enforce_providers: true -command: /usr/bin/local/opk/bad-perms-policy-cmd`}} - tests := []struct { name string tokens map[string]string @@ -324,34 +306,6 @@ command: /usr/bin/local/opk/bad-perms-policy-cmd`}} expectErrorCount: 1, errorExpected: "Unterminated double-quoted string", }, - { - name: "Policy invalid config permissions", - tokens: map[string]string{ - "OPKSSH_PLUGIN_ISS": "https://example.com", - "OPKSSH_PLUGIN_SUB": "1234", - "OPKSSH_PLUGIN_AUD": "abcd", - }, - files: configWithBadPerms, - cmdExecutor: mockCmdExecutor, - expectedAllowed: false, - expectedResultCount: 1, - expectErrorCount: 1, - errorExpected: "expected one of the following permissions [640], got (606)", - }, - { - name: "Policy invalid command permissions", - tokens: map[string]string{ - "OPKSSH_PLUGIN_ISS": "https://example.com", - "OPKSSH_PLUGIN_SUB": "1234", - "OPKSSH_PLUGIN_AUD": "abcd", - }, - files: commandWithBadPerms, - cmdExecutor: mockCmdExecutor, - expectedAllowed: false, - expectedResultCount: 1, - expectErrorCount: 1, - errorExpected: "expected one of the following permissions [555, 755], got (766)", - }, } for _, tt := range tests { diff --git a/policy/policyloader.go b/policy/policyloader.go index 862c48cf..5745b79d 100644 --- a/policy/policyloader.go +++ b/policy/policyloader.go @@ -19,7 +19,6 @@ package policy import ( "fmt" "os/user" - "path" "path/filepath" "github.com/openpubkey/opkssh/policy/files" @@ -28,12 +27,12 @@ import ( ) // SystemDefaultPolicyPath is the default filepath where opkssh policy is -// defined -var SystemDefaultPolicyPath = filepath.FromSlash("/etc/opk/auth_id") +// defined. On Unix: /etc/opk/auth_id, On Windows: %ProgramData%\opk\auth_id +var SystemDefaultPolicyPath = filepath.Join(GetSystemConfigBasePath(), "auth_id") // SystemDefaultProvidersPath is the default filepath where opkssh provider // definitions are configured -var SystemDefaultProvidersPath = filepath.FromSlash("/etc/opk/providers") +var SystemDefaultProvidersPath = filepath.Join(GetSystemConfigBasePath(), "providers") // UserLookup defines the minimal interface to lookup users on the current // system @@ -203,7 +202,7 @@ func (h *HomePolicyLoader) LoadHomePolicy(username string, skipInvalidEntries bo } // UserPolicyPath returns the path to the user's opkssh policy file at -// ~/.opk/auth_id. +// ~/.opk/auth_id (Unix) or %USERPROFILE%\.opk\auth_id (Windows). func (h *HomePolicyLoader) UserPolicyPath(username string) (string, error) { user, err := h.UserLookup.Lookup(username) if err != nil { @@ -214,6 +213,6 @@ func (h *HomePolicyLoader) UserPolicyPath(username string) (string, error) { return "", fmt.Errorf("user %s does not have a home directory", username) } - policyFilePath := path.Join(userHomeDirectory, ".opk", "auth_id") + policyFilePath := filepath.Join(userHomeDirectory, ".opk", "auth_id") return policyFilePath, nil } diff --git a/policy/policyloader_test.go b/policy/policyloader_test.go index d5af822d..1c0cfcf2 100644 --- a/policy/policyloader_test.go +++ b/policy/policyloader_test.go @@ -20,7 +20,7 @@ import ( "errors" "os" "os/user" - "path" + "path/filepath" "testing" "github.com/openpubkey/opkssh/policy" @@ -136,7 +136,7 @@ func TestLoadUserPolicy_ErrorFile(t *testing.T) { policyLoader := NewTestHomePolicyLoader(afero.NewMemMapFs(), mockUserLookup) mockFs := policyLoader.FileLoader.Fs // Create policy file at user policy path with invalid data - err := afero.WriteFile(mockFs, path.Join(ValidUser.HomeDir, ".opk", "auth_id"), []byte("{"), 0600) + err := afero.WriteFile(mockFs, filepath.Join(ValidUser.HomeDir, ".opk", "auth_id"), []byte("{"), 0600) require.NoError(t, err) policy, path, err := policyLoader.LoadHomePolicy(ValidUser.Username, false) @@ -167,7 +167,7 @@ func TestLoadUserPolicy_Success(t *testing.T) { } testPolicyFile, err := testPolicy.ToTable() require.NoError(t, err) - expectedPath := path.Join(ValidUser.HomeDir, ".opk", "auth_id") + expectedPath := filepath.Join(ValidUser.HomeDir, ".opk", "auth_id") err = afero.WriteFile(mockFs, expectedPath, testPolicyFile, 0600) require.NoError(t, err) @@ -228,7 +228,7 @@ func TestLoadUserPolicy_Success_SkipInvalidEntries(t *testing.T) { } testPolicyFile, err := testPolicy.ToTable() require.NoError(t, err) - expectedPath := path.Join(ValidUser.HomeDir, ".opk", "auth_id") + expectedPath := filepath.Join(ValidUser.HomeDir, ".opk", "auth_id") err = afero.WriteFile(mockFs, expectedPath, testPolicyFile, 0600) require.NoError(t, err) gotPolicy, gotPath, err := policyLoader.LoadHomePolicy(ValidUser.Username, true) @@ -252,27 +252,6 @@ func TestLoadPolicyAtPath_FileMissing(t *testing.T) { require.Nil(t, contents, "should not return contents if error") } -func TestLoadPolicyAtPath_BadPermissions(t *testing.T) { - // Test that LoadPolicyAtPath returns an error when the file has invalid - // permission bits - t.Parallel() - - mockUserLookup := &MockUserLookup{User: ValidUser} - mockFs := NewMockFsOpenError() - policyLoader := NewTestHomePolicyLoader( - mockFs, - mockUserLookup, - ) - // Create empty policy with bad permissions - err := afero.WriteFile(mockFs, policy.SystemDefaultPolicyPath, []byte{}, 0777) - require.NoError(t, err) - - contents, err := policyLoader.LoadPolicyAtPath(policy.SystemDefaultPolicyPath) - - require.Error(t, err, "should fail if permissions are bad") - require.Nil(t, contents, "should not return contents if error") -} - func TestLoadPolicyAtPath_ReadError(t *testing.T) { // Test that LoadPolicyAtPath returns an error when we fail to read the file // (but it exists)