[Fixed] Parse GPO ignoring when registry policy name is not Registry.…

…pol (#1102)

Currently, if the registry policy name file is not exactly `Registry.pol` the AD parse GPO does not find it, ignoring it.
However, there are cases where the registry file name is `registry.pol` for a GPO as reported in issue #1098.
In this pull request the implementation lists the files in the policy class directory and looks for the `registry.pol` file ignoring the case.

If the file is found it's parsed, otherwise the policy is ignored.

Implemented by Davi Henrique <[email protected]>. A big thanks
to him!

internal/ad/ad.go

@@ -491,116 +491,134 @@ func (ad *AD) parseGPOs(ctx context.Context, gpos []gpo, objectClass ObjectClass
 			Rules: make(map[string][]entry.Entry),
 		}
 		r = append(r, gpoWithRules)
-		if err := func() error {
-			ad.downloadables[name].mu.RLock()
-			defer ad.downloadables[name].mu.RUnlock()
-			_ = ad.downloadables[name].testConcurrent
-
-			log.Debugf(ctx, "Parsing GPO %q", name)
-
-			// We need to consider the uppercase version of the name as well,
-			// which could occur in some of the default GPOs such as Default
-			// Domain Policy.
-			classes := []string{"User", "USER"}
-			if objectClass == ComputerObject {
-				classes = []string{"Machine", "MACHINE"}
-			}
+		if err = ad.parseGPO(ctx, name, url, keyFilterPrefix, objectClass, gpoWithRules); err != nil {
+			return r, err
+		}
+	}
 
-			var err error
-			var f *os.File
-			for _, class := range classes {
-				var e error
-				f, e = os.Open(filepath.Join(ad.sysvolCacheDir, "Policies", filepath.Base(url), class, "Registry.pol"))
-
-				// We only care about the first error which is caused by opening
-				// the capitalized version of the class, instead of the
-				// uppercase version which is less common and more of an edge case.
-				if e != nil && err == nil {
-					err = e
-				} else if e == nil {
-					err = nil
-					break
-				}
-			}
+	return r, nil
+}
 
-			if errors.Is(err, fs.ErrNotExist) {
-				log.Debugf(ctx, "Policy %q doesn't have any policy for class %q %s", name, objectClass, err)
-				return nil
-			} else if err != nil {
-				return err
+func (ad *AD) parseGPO(ctx context.Context, name, url, keyFilterPrefix string, objectClass ObjectClass, gpoWithRules policies.GPO) error {
+	ad.downloadables[name].mu.RLock()
+	defer ad.downloadables[name].mu.RUnlock()
+	_ = ad.downloadables[name].testConcurrent
+
+	log.Debugf(ctx, "Parsing GPO %q of class %q", name, objectClass)
+
+	// We need to consider the uppercase version of the name as well,
+	// which could occur in some of the default GPOs such as Default
+	// Domain Policy.
+	classes := []string{"User", "USER"}
+	if objectClass == ComputerObject {
+		classes = []string{"Machine", "MACHINE"}
+	}
+
+	var f *os.File
+classLoop:
+	for _, class := range classes {
+		var err error
+		var files []os.DirEntry
+
+		policyDir := filepath.Join(ad.sysvolCacheDir, "Policies", filepath.Base(url), class)
+		files, err = os.ReadDir(policyDir)
+		if errors.Is(err, fs.ErrNotExist) {
+			log.Debugf(ctx, "Policy directory %q not found", policyDir)
+			continue
+		} else if err != nil {
+			return err
+		}
+
+		// Registry.pol can have different cases, ensure we can find it whatever its case is
+		for _, file := range files {
+			if !strings.EqualFold(file.Name(), "Registry.pol") {
+				continue
 			}
-			defer decorate.LogFuncOnErrorContext(ctx, f.Close)
 
-			// Decode and apply policies in gpo order. First win
-			pols, err := registry.DecodePolicy(f)
+			policyPath := filepath.Join(policyDir, file.Name())
+			log.Debugf(ctx, "Found registry policy file %q", policyPath)
+
+			f, err = os.Open(policyPath)
 			if err != nil {
-				return errors.New(gotext.Get("%s: %v", f.Name(), err))
+				return err
 			}
 
-			// filter keys to be overridden
-			var currentKey string
-			var overrideEnabled bool
-			for _, pol := range pols {
-				// Rewrite the certificate autoenrollment key so we can easily
-				// use it in the policy manager
-				if pol.Key == certAutoEnrollKey {
-					pol.Key = fmt.Sprintf("%scertificate/autoenroll/all", keyFilterPrefix)
-				}
-
-				if strings.HasPrefix(pol.Key, policyServersPrefix) {
-					pol.Key = fmt.Sprintf("%scertificate/%s/all", keyFilterPrefix, pol.Key)
-				}
-
-				// Only consider supported policies for this distro
-				if !strings.HasPrefix(pol.Key, keyFilterPrefix) {
-					continue
-				}
-				if pol.Err != nil {
-					return errors.New(gotext.Get("%s: %v", f.Name(), pol.Err))
-				}
-				pol.Key = strings.TrimPrefix(pol.Key, keyFilterPrefix)
-
-				// Some keys can be overridden
-				releaseID := filepath.Base(pol.Key)
-				keyType := strings.Split(pol.Key, "/")[0]
-				pol.Key = filepath.Dir(strings.TrimPrefix(pol.Key, keyType+"/"))
-
-				if releaseID == "all" {
-					currentKey = pol.Key
-					overrideEnabled = false
-					gpoWithRules.Rules[keyType] = append(gpoWithRules.Rules[keyType], pol)
-					continue
-				}
-
-				// This is not an "all" key and the key name don’t match
-				// This shouldn’t happen with our admx, but just to stay safe…
-				if currentKey != pol.Key {
-					continue
-				}
-
-				if strings.HasPrefix(releaseID, "Override"+ad.versionID) && pol.Value == "true" {
-					overrideEnabled = true
-					continue
-				}
-				// Check we have a matching override
-				if !overrideEnabled || releaseID != ad.versionID {
-					continue
-				}
-
-				// Matching enabled override
-				// Replace value with the override content
-				iLast := len(gpoWithRules.Rules[keyType]) - 1
-				p := gpoWithRules.Rules[keyType][iLast]
-				p.Value = pol.Value
-				gpoWithRules.Rules[keyType][iLast] = p
-			}
-			return nil
-		}(); err != nil {
-			return r, err
+			break classLoop
 		}
 	}
 
-	return r, nil
+	if f == nil {
+		log.Debugf(ctx, "Policy %q doesn't have any policy for class %q", name, objectClass)
+		return nil
+	}
+
+	defer decorate.LogFuncOnErrorContext(ctx, f.Close)
+
+	// Decode and apply policies in gpo order. First win
+	pols, err := registry.DecodePolicy(f)
+	if err != nil {
+		return errors.New(gotext.Get("%s: %v", f.Name(), err))
+	}
+
+	// filter keys to be overridden
+	var currentKey string
+	var overrideEnabled bool
+	for _, pol := range pols {
+		// Rewrite the certificate autoenrollment key so we can easily
+		// use it in the policy manager
+		if pol.Key == certAutoEnrollKey {
+			pol.Key = fmt.Sprintf("%scertificate/autoenroll/all", keyFilterPrefix)
+		}
+
+		if strings.HasPrefix(pol.Key, policyServersPrefix) {
+			pol.Key = fmt.Sprintf("%scertificate/%s/all", keyFilterPrefix, pol.Key)
+		}
+
+		// Only consider supported policies for this distro
+		if !strings.HasPrefix(pol.Key, keyFilterPrefix) {
+			continue
+		}
+		if pol.Err != nil {
+			return errors.New(gotext.Get("%s: %v", f.Name(), pol.Err))
+		}
+		pol.Key = strings.TrimPrefix(pol.Key, keyFilterPrefix)
+
+		// Some keys can be overridden
+		releaseID := filepath.Base(pol.Key)
+		keyType := strings.Split(pol.Key, "/")[0]
+		pol.Key = filepath.Dir(strings.TrimPrefix(pol.Key, keyType+"/"))
+
+		if releaseID == "all" {
+			currentKey = pol.Key
+			overrideEnabled = false
+			gpoWithRules.Rules[keyType] = append(gpoWithRules.Rules[keyType], pol)
+			continue
+		}
+
+		// This is not an "all" key and the key name don’t match
+		// This shouldn’t happen with our admx, but just to stay safe…
+		if currentKey != pol.Key {
+			continue
+		}
+
+		if strings.HasPrefix(releaseID, "Override"+ad.versionID) && pol.Value == "true" {
+			overrideEnabled = true
+			continue
+		}
+		// Check we have a matching override
+		if !overrideEnabled || releaseID != ad.versionID {
+			continue
+		}
+
+		// Matching enabled override
+		// Replace value with the override content
+		iLast := len(gpoWithRules.Rules[keyType]) - 1
+		p := gpoWithRules.Rules[keyType][iLast]
+		p.Value = pol.Value
+		gpoWithRules.Rules[keyType][iLast] = p
+	}
+
+	return nil
 }
 
 // GetInfo returns all information from the selected backend: static and dynamic part.

internal/ad/ad_test.go

@@ -424,7 +424,7 @@ func TestGetPolicies(t *testing.T) {
 			}},
 		},
 
-		// Policy class directory spelling cases
+		// Policy class directory and Registry.pol spelling cases
 		"Policy user directory is uppercase": {
 			gpoListArgs: []string{"gpoonly.com", "bob:uppercase-class"},
 			want:        policies.Policies{GPOs: []policies.GPO{standardUserGPO("uppercase-class")}},
@@ -450,6 +450,26 @@ func TestGetPolicies(t *testing.T) {
 				{ID: "lowercase-class", Name: "lowercase-class-name", Rules: map[string][]entry.Entry{}}},
 			},
 		},
+		"User policy Registry.pol is lower case": {
+			gpoListArgs: []string{"gpoonly.com", "bob:lowercase-registry"},
+			want:        policies.Policies{GPOs: []policies.GPO{standardUserGPO("lowercase-registry")}},
+		},
+		"Computer policy Registry.pol is lower case": {
+			objectName:  hostname,
+			objectClass: ad.ComputerObject,
+			gpoListArgs: []string{"gpoonly.com", hostname + ":lowercase-registry"},
+			want:        policies.Policies{GPOs: []policies.GPO{standardComputerGPO("lowercase-registry")}},
+		},
+		"User policy Registry.pol is mixed case": {
+			gpoListArgs: []string{"gpoonly.com", "bob:mixedcase-registry"},
+			want:        policies.Policies{GPOs: []policies.GPO{standardUserGPO("mixedcase-registry")}},
+		},
+		"Computer policy Registry.pol is mixed case": {
+			objectName:  hostname,
+			objectClass: ad.ComputerObject,
+			gpoListArgs: []string{"gpoonly.com", hostname + ":mixedcase-registry"},
+			want:        policies.Policies{GPOs: []policies.GPO{standardComputerGPO("mixedcase-registry")}},
+		},
 
 		// Error cases
 		"Machine doesn’t match": {

internal/ad/testdata/AD/SYSVOL/gpoonly.com/Policies/lowercase-registry/GPT.INI

@@ -0,0 +1,3 @@
+[General]
+Version=1000
+displayName=New Group Policy Object

internal/ad/testdata/AD/SYSVOL/gpoonly.com/Policies/mixedcase-registry/GPT.INI

@@ -0,0 +1,3 @@
+[General]
+Version=1000
+displayName=New Group Policy Object