Validate OU paths in DomainJoin action.

certtostore_windows.go

@@ -104,6 +104,7 @@ const (
 	findIssuerStr           = compareNameStrW<<compareShift | infoIssuerFlag  // CERT_FIND_ISSUER_STR_W
 	findSubjectCert         = compareSubjectCert << compareShift              // CERT_FIND_SUBJECT_CERT
 	signatureKeyUsage       = 0x80                                            // CERT_DIGITAL_SIGNATURE_KEY_USAGE
+	maxTolerance            = 5 * time.Minute
 
 	// Legacy CryptoAPI flags
 	bCryptPadPKCS1 uintptr = 0x2
@@ -1166,19 +1167,26 @@ func (k *Key) SetACL(access string, sid string, perm string) error {
 // setACL sets permissions for the private key by wrapping the Microsoft
 // icacls utility. icacls is used for simplicity working with NTFS ACLs.
 func setACL(file, access, sid, perm string) error {
-	deck.Infof("running: %s %s /%s %s:%s", icaclsPath, file, access, sid, perm)
+	fi, err := os.Lstat(file)
+	if err != nil {
+		return fmt.Errorf("setACL unable to stat %s: %w", file, err)
+	}
+	if !fi.Mode().IsRegular() {
+		return fmt.Errorf("setACL target %s is not a regular file", file)
+	}
+	deck.Infof("running: %s %s /L /%s %s:%s", icaclsPath, file, access, sid, perm)
 	// Parameter validation isn't required, icacls handles this on its own.
-	err := exec.Command(icaclsPath, file, "/"+access, sid+":"+perm).Run()
+	err = exec.Command(icaclsPath, file, "/L", "/"+access, sid+":"+perm).Run()
 	// Error 1798 can safely be ignored, because it occurs when trying to set an acl
 	// for a non-existend sid, which only happens for certain permissions needed on later
 	// versions of Windows.
 	if err1, ok := err.(*exec.ExitError); ok && !strings.Contains(err1.Error(), "1798") {
 		deck.Infof("ignoring error while %sing '%s' access to %s for sid: %v", access, perm, file, sid)
 		return nil
 	} else if err1 != nil {
-		return fmt.Errorf("certstorage.SetFileACL is unable to %s %s access on %s to sid %s, %v", access, perm, file, sid, err1)
+		return fmt.Errorf("setACL is unable to %s %s access on %s to sid %s, %v", access, perm, file, sid, err1)
 	} else if !ok && err != nil {
-		return fmt.Errorf("certstorage.SetFileACL failed to pull exit error while %s %s access on %s to sid %s, %v", access, perm, file, sid, err)
+		return fmt.Errorf("setACL failed to pull exit error while %s %s access on %s to sid %s, %v", access, perm, file, sid, err)
 	}
 	return nil
 }
@@ -1788,10 +1796,13 @@ func softwareKeyContainers(uniqueID string, storeDomain uint32) (string, string,
 // TODO: Find a more deterministic way to link CNG and CAPI keys than
 // comparing modification times.
 func keyMatch(keyPath, dir string) (string, error) {
-	key, err := os.Stat(keyPath)
+	key, err := os.Lstat(keyPath)
 	if err != nil {
 		return "", fmt.Errorf("unable to determine key creation date: %v", err)
 	}
+	if !key.Mode().IsRegular() {
+		return "", fmt.Errorf("key path %q is not a regular file", keyPath)
+	}
 	files, err := ioutil.ReadDir(dir)
 	if err != nil {
 		return "", fmt.Errorf("unable to locate search directory: %v", err)
@@ -1801,13 +1812,23 @@ func keyMatch(keyPath, dir string) (string, error) {
 	// necessary to select the right key. Typically, there are several machine
 	// keys present, only one of which was created at the same time as the
 	// known key.
+	var bestMatch string
+	minDiff := time.Duration(1<<63 - 1)
+
 	for _, f := range files {
-		age := int(key.ModTime().Sub(f.ModTime()) / time.Second)
-		if age >= -300 && age < 300 {
-			return dir + f.Name(), nil
+		if !f.Mode().IsRegular() {
+			continue
+		}
+		diff := key.ModTime().Sub(f.ModTime())
+		if diff < 0 {
+			diff = -diff
+		}
+		if diff <= maxTolerance && diff < minDiff {
+			minDiff = diff
+			bestMatch = filepath.Join(dir, f.Name())
 		}
 	}
-	return "", nil
+	return bestMatch, nil
 }
 
 // Verify interface conformance.

certtostore_windows_test.go

@@ -21,7 +21,10 @@ import (
 	"crypto/x509/pkix"
 	"errors"
 	"fmt"
+	"io/ioutil"
 	"math/big"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 	"time"
@@ -676,3 +679,180 @@ func TestCertByCommonName(t *testing.T) {
 		t.Errorf("chains[0][0] is not the leaf; got %v, want leaf %v", chains[0][0].Subject, found.Subject)
 	}
 }
+
+func TestKeyMatch_ClosestTimestamp(t *testing.T) {
+	tmpDir := t.TempDir()
+	now := time.Now()
+
+	knownKeyPath := filepath.Join(tmpDir, "known_key")
+	if err := ioutil.WriteFile(knownKeyPath, []byte("known key content"), 0600); err != nil {
+		t.Fatalf("failed to write known key file: %v", err)
+	}
+	if err := os.Chtimes(knownKeyPath, now, now); err != nil {
+		t.Fatalf("failed to set times on known key file: %v", err)
+	}
+
+	searchDir := filepath.Join(tmpDir, "search_dir")
+	if err := os.Mkdir(searchDir, 0755); err != nil {
+		t.Fatalf("failed to create search dir: %v", err)
+	}
+
+	// Candidate with alphabetical priority (starts with 'a'), 2 minutes difference.
+	candA := filepath.Join(searchDir, "a_candidate")
+	if err := ioutil.WriteFile(candA, []byte("cand A"), 0600); err != nil {
+		t.Fatalf("failed to write candA: %v", err)
+	}
+	tA := now.Add(2 * time.Minute)
+	if err := os.Chtimes(candA, tA, tA); err != nil {
+		t.Fatalf("failed to set times on candA: %v", err)
+	}
+
+	// Candidate with closest timestamp (10 seconds difference), starts with 'z'.
+	candZ := filepath.Join(searchDir, "z_candidate")
+	if err := ioutil.WriteFile(candZ, []byte("cand Z"), 0600); err != nil {
+		t.Fatalf("failed to write candZ: %v", err)
+	}
+	tZ := now.Add(10 * time.Second)
+	if err := os.Chtimes(candZ, tZ, tZ); err != nil {
+		t.Fatalf("failed to set times on candZ: %v", err)
+	}
+
+	// Should select candZ because it has the closest timestamp, not candA which is first alphabetically.
+	got, err := keyMatch(knownKeyPath, searchDir)
+	if err != nil {
+		t.Fatalf("keyMatch returned unexpected error: %v", err)
+	}
+	if got != candZ {
+		t.Errorf("keyMatch got %q, want closest candidate %q", got, candZ)
+	}
+}
+
+func TestKeyMatch_IgnoresNonRegularFiles(t *testing.T) {
+	tmpDir := t.TempDir()
+	now := time.Now()
+
+	knownKeyPath := filepath.Join(tmpDir, "known_key")
+	if err := ioutil.WriteFile(knownKeyPath, []byte("known key content"), 0600); err != nil {
+		t.Fatalf("failed to write known key file: %v", err)
+	}
+	if err := os.Chtimes(knownKeyPath, now, now); err != nil {
+		t.Fatalf("failed to set times on known key file: %v", err)
+	}
+
+	searchDir := filepath.Join(tmpDir, "search_dir")
+	if err := os.Mkdir(searchDir, 0755); err != nil {
+		t.Fatalf("failed to create search dir: %v", err)
+	}
+
+	// Subdirectory with exact timestamp match. Must be ignored because it is not a regular file.
+	subDir := filepath.Join(searchDir, "0_subdir_candidate")
+	if err := os.Mkdir(subDir, 0755); err != nil {
+		t.Fatalf("failed to create subdir: %v", err)
+	}
+	if err := os.Chtimes(subDir, now, now); err != nil {
+		t.Fatalf("failed to set times on subdir: %v", err)
+	}
+
+	// Regular candidate with 1 minute difference.
+	cand := filepath.Join(searchDir, "valid_candidate")
+	if err := ioutil.WriteFile(cand, []byte("valid cand"), 0600); err != nil {
+		t.Fatalf("failed to write cand: %v", err)
+	}
+	tCand := now.Add(1 * time.Minute)
+	if err := os.Chtimes(cand, tCand, tCand); err != nil {
+		t.Fatalf("failed to set times on cand: %v", err)
+	}
+
+	got, err := keyMatch(knownKeyPath, searchDir)
+	if err != nil {
+		t.Fatalf("keyMatch returned unexpected error: %v", err)
+	}
+	if got != cand {
+		t.Errorf("keyMatch got %q, want regular file candidate %q", got, cand)
+	}
+}
+
+func TestKeyMatch_NonRegularKeyPath(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	// A directory passed as keyPath should fail validation.
+	dirKeyPath := filepath.Join(tmpDir, "dir_key")
+	if err := os.Mkdir(dirKeyPath, 0755); err != nil {
+		t.Fatalf("failed to create dir key: %v", err)
+	}
+
+	searchDir := filepath.Join(tmpDir, "search_dir")
+	if err := os.Mkdir(searchDir, 0755); err != nil {
+		t.Fatalf("failed to create search dir: %v", err)
+	}
+
+	_, err := keyMatch(dirKeyPath, searchDir)
+	if err == nil {
+		t.Errorf("keyMatch expected error when keyPath is not a regular file, got nil")
+	}
+}
+
+func TestKeyMatch_OutsideTolerance(t *testing.T) {
+	tmpDir := t.TempDir()
+	now := time.Now()
+
+	knownKeyPath := filepath.Join(tmpDir, "known_key")
+	if err := ioutil.WriteFile(knownKeyPath, []byte("known key content"), 0600); err != nil {
+		t.Fatalf("failed to write known key file: %v", err)
+	}
+	if err := os.Chtimes(knownKeyPath, now, now); err != nil {
+		t.Fatalf("failed to set times on known key file: %v", err)
+	}
+
+	searchDir := filepath.Join(tmpDir, "search_dir")
+	if err := os.Mkdir(searchDir, 0755); err != nil {
+		t.Fatalf("failed to create search dir: %v", err)
+	}
+
+	// Candidate file outside the 5-minute tolerance (6 minutes difference).
+	candOld := filepath.Join(searchDir, "old_candidate")
+	if err := ioutil.WriteFile(candOld, []byte("old cand"), 0600); err != nil {
+		t.Fatalf("failed to write candOld: %v", err)
+	}
+	tOld := now.Add(6 * time.Minute)
+	if err := os.Chtimes(candOld, tOld, tOld); err != nil {
+		t.Fatalf("failed to set times on candOld: %v", err)
+	}
+
+	got, err := keyMatch(knownKeyPath, searchDir)
+	if err != nil {
+		t.Fatalf("keyMatch returned unexpected error: %v", err)
+	}
+	if got != "" {
+		t.Errorf("keyMatch expected empty string for candidates outside tolerance, got %q", got)
+	}
+}
+
+func TestSetACL_NonExistentFile(t *testing.T) {
+	tmpDir := t.TempDir()
+	nonExistent := filepath.Join(tmpDir, "does_not_exist")
+
+	err := setACL(nonExistent, "grant", "*S-1-1-0", "R")
+	if err == nil {
+		t.Fatal("setACL expected error for non-existent file, got nil")
+	}
+	if !strings.Contains(err.Error(), "unable to stat") {
+		t.Errorf("setACL unexpected error string: got %q, want containing 'unable to stat'", err)
+	}
+}
+
+func TestSetACL_NonRegularFile(t *testing.T) {
+	tmpDir := t.TempDir()
+	subDir := filepath.Join(tmpDir, "subdir")
+	if err := os.Mkdir(subDir, 0755); err != nil {
+		t.Fatalf("failed to create subdir: %v", err)
+	}
+
+	err := setACL(subDir, "grant", "*S-1-1-0", "R")
+	if err == nil {
+		t.Fatal("setACL expected error for non-regular file (directory), got nil")
+	}
+	if !strings.Contains(err.Error(), "is not a regular file") {
+		t.Errorf("setACL unexpected error string: got %q, want containing 'is not a regular file'", err)
+	}
+}