From 42da1e5a52b9fdc9ab3e6ec1a57fb8d2ceff9e17 Mon Sep 17 00:00:00 2001 From: Sven van Ginkel Date: Tue, 27 Jan 2026 23:39:17 +0100 Subject: [PATCH] Bug: Apply SELinux context after binary replacement (#1678) - Move SELinux context handling to internal/ghupdate for reuse - Make chcon a true fallback (only runs if semanage/restorecon unavailable) - Handle existing semanage rules with -m (modify) after -a (add) fails - Apply SELinux handling to both agent and hub updates - Add tests with proper skip behavior for SELinux systems --------- Co-authored-by: henrygd --- agent/update.go | 46 ++------------------- internal/ghupdate/selinux.go | 66 +++++++++++++++++++++++++++++++ internal/ghupdate/selinux_test.go | 53 +++++++++++++++++++++++++ internal/hub/update.go | 5 +++ 4 files changed, 127 insertions(+), 43 deletions(-) create mode 100644 internal/ghupdate/selinux.go create mode 100644 internal/ghupdate/selinux_test.go diff --git a/agent/update.go b/agent/update.go index d5467044..b7cdb716 100644 --- a/agent/update.go +++ b/agent/update.go @@ -1,12 +1,10 @@ package agent import ( - "fmt" "log" "os" "os/exec" "runtime" - "strings" "github.com/henrygd/beszel/internal/ghupdate" ) @@ -108,12 +106,12 @@ func Update(useMirror bool) error { } } - // 6) Fix SELinux context if necessary - if err := handleSELinuxContext(exePath); err != nil { + // Fix SELinux context if necessary + if err := ghupdate.HandleSELinuxContext(exePath); err != nil { ghupdate.ColorPrintf(ghupdate.ColorYellow, "Warning: SELinux context handling: %v", err) } - // 7) Restart service if running under a recognised init system + // Restart service if running under a recognised init system if r := detectRestarter(); r != nil { if err := r.Restart(); err != nil { ghupdate.ColorPrintf(ghupdate.ColorYellow, "Warning: failed to restart service: %v", err) @@ -128,41 +126,3 @@ func Update(useMirror bool) error { return nil } -// handleSELinuxContext restores or applies the correct SELinux label to the binary. -func handleSELinuxContext(path string) error { - out, err := exec.Command("getenforce").Output() - if err != nil { - // SELinux not enabled or getenforce not available - return nil - } - state := strings.TrimSpace(string(out)) - if state == "Disabled" { - return nil - } - - ghupdate.ColorPrint(ghupdate.ColorYellow, "SELinux is enabled; applying context…") - var errs []string - - // Try persistent context via semanage+restorecon - if semanagePath, err := exec.LookPath("semanage"); err == nil { - if err := exec.Command(semanagePath, "fcontext", "-a", "-t", "bin_t", path).Run(); err != nil { - errs = append(errs, "semanage fcontext failed: "+err.Error()) - } else if restoreconPath, err := exec.LookPath("restorecon"); err == nil { - if err := exec.Command(restoreconPath, "-v", path).Run(); err != nil { - errs = append(errs, "restorecon failed: "+err.Error()) - } - } - } - - // Fallback to temporary context via chcon - if chconPath, err := exec.LookPath("chcon"); err == nil { - if err := exec.Command(chconPath, "-t", "bin_t", path).Run(); err != nil { - errs = append(errs, "chcon failed: "+err.Error()) - } - } - - if len(errs) > 0 { - return fmt.Errorf("SELinux context errors: %s", strings.Join(errs, "; ")) - } - return nil -} diff --git a/internal/ghupdate/selinux.go b/internal/ghupdate/selinux.go new file mode 100644 index 00000000..b6d98f23 --- /dev/null +++ b/internal/ghupdate/selinux.go @@ -0,0 +1,66 @@ +package ghupdate + +import ( + "fmt" + "os/exec" + "strings" +) + +// HandleSELinuxContext restores or applies the correct SELinux label to the binary. +func HandleSELinuxContext(path string) error { + out, err := exec.Command("getenforce").Output() + if err != nil { + // SELinux not enabled or getenforce not available + return nil + } + state := strings.TrimSpace(string(out)) + if state == "Disabled" { + return nil + } + + ColorPrint(ColorYellow, "SELinux is enabled; applying context…") + + // Try persistent context via semanage+restorecon + if success := trySemanageRestorecon(path); success { + return nil + } + + // Fallback to temporary context via chcon + if chconPath, err := exec.LookPath("chcon"); err == nil { + if err := exec.Command(chconPath, "-t", "bin_t", path).Run(); err != nil { + return fmt.Errorf("chcon failed: %w", err) + } + return nil + } + + return fmt.Errorf("no SELinux tools available (semanage/restorecon or chcon)") +} + +// trySemanageRestorecon attempts to set persistent SELinux context using semanage and restorecon. +// Returns true if successful, false otherwise. +func trySemanageRestorecon(path string) bool { + semanagePath, err := exec.LookPath("semanage") + if err != nil { + return false + } + + restoreconPath, err := exec.LookPath("restorecon") + if err != nil { + return false + } + + // Try to add the fcontext rule; if it already exists, try to modify it + if err := exec.Command(semanagePath, "fcontext", "-a", "-t", "bin_t", path).Run(); err != nil { + // Rule may already exist, try modify instead + if err := exec.Command(semanagePath, "fcontext", "-m", "-t", "bin_t", path).Run(); err != nil { + return false + } + } + + // Apply the context with restorecon + if err := exec.Command(restoreconPath, "-v", path).Run(); err != nil { + return false + } + + return true +} diff --git a/internal/ghupdate/selinux_test.go b/internal/ghupdate/selinux_test.go new file mode 100644 index 00000000..fd6f1ffe --- /dev/null +++ b/internal/ghupdate/selinux_test.go @@ -0,0 +1,53 @@ +package ghupdate + +import ( + "os" + "os/exec" + "path/filepath" + "testing" +) + +func TestHandleSELinuxContext_NoSELinux(t *testing.T) { + // Skip on SELinux systems - this test is for non-SELinux behavior + if _, err := exec.LookPath("getenforce"); err == nil { + t.Skip("skipping on SELinux-enabled system") + } + + // On systems without SELinux, getenforce will fail and the function + // should return nil without error + tempFile := filepath.Join(t.TempDir(), "test-binary") + if err := os.WriteFile(tempFile, []byte("test"), 0755); err != nil { + t.Fatalf("failed to create temp file: %v", err) + } + + err := HandleSELinuxContext(tempFile) + if err != nil { + t.Errorf("HandleSELinuxContext() on non-SELinux system returned error: %v", err) + } +} + +func TestHandleSELinuxContext_InvalidPath(t *testing.T) { + // Skip on SELinux systems - this test is for non-SELinux behavior + if _, err := exec.LookPath("getenforce"); err == nil { + t.Skip("skipping on SELinux-enabled system") + } + + // On non-SELinux systems, getenforce fails early so even invalid paths succeed + err := HandleSELinuxContext("/nonexistent/path/binary") + if err != nil { + t.Errorf("HandleSELinuxContext() with invalid path on non-SELinux system returned error: %v", err) + } +} + +func TestTrySemanageRestorecon_NoTools(t *testing.T) { + // Skip if semanage is available (we don't want to modify system SELinux policy) + if _, err := exec.LookPath("semanage"); err == nil { + t.Skip("skipping on system with semanage available") + } + + // Should return false when semanage is not available + result := trySemanageRestorecon("/some/path") + if result { + t.Error("trySemanageRestorecon() returned true when semanage is not available") + } +} diff --git a/internal/hub/update.go b/internal/hub/update.go index 3495e974..4ae990ef 100644 --- a/internal/hub/update.go +++ b/internal/hub/update.go @@ -45,6 +45,11 @@ func Update(cmd *cobra.Command, _ []string) { fmt.Printf("Warning: failed to set executable permissions: %v\n", err) } + // Fix SELinux context if necessary + if err := ghupdate.HandleSELinuxContext(exePath); err != nil { + ghupdate.ColorPrintf(ghupdate.ColorYellow, "Warning: SELinux context handling: %v", err) + } + // Try to restart the service if it's running restartService() }