Improved getting the "right" SID object at the cost of some performance

lkarlslund · May 17, 2022 · 6e32c9c · 6e32c9c
1 parent 9046c61
commit 6e32c9c
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 23 deletions.
diff --git a/modules/engine/attributes.go b/modules/engine/attributes.go
@@ -82,7 +82,7 @@ var (
 	DownLevelLogonName = NewAttribute("downLevelLogonName").Merge()
 	UserPrincipalName  = NewAttribute("userPrincipalName").Merge()
 	NetbiosDomain      = NewAttribute("netbiosDomain") // Used to merge users with - if we only have a DOMAIN\USER type of info
-	DomainPart         = NewAttribute("domainPart")
+	DomainPart         = NewAttribute("domainPart").Single()
 
 	MetaProtectedUser           = NewAttribute("_protecteduser")
 	MetaUnconstrainedDelegation = NewAttribute("_unconstraineddelegation")

diff --git a/modules/engine/objects.go b/modules/engine/objects.go
@@ -502,6 +502,9 @@ func (os *Objects) FindTwoMultiOrAdd(attribute Attribute, value AttributeValue,
 	if !found {
 		no := addifnotfound()
 		if no != nil {
+			if len(os.DefaultValues) > 0 {
+				no.SetFlex(os.DefaultValues...)
+			}
 			os.add(no)
 			os.objectmutex.Unlock()
 			return []*Object{no}, false
@@ -604,15 +607,17 @@ func (os *Objects) FindOrAddAdjacentSID(s windowssecurity.SID, r *Object) *Objec
 		dp := r.OneAttr(DomainPart)
 
 		domain, _ := os.FindTwoMulti(
+			DistinguishedName, dp,
 			ObjectClass, AttributeValueString("domain"),
-			DistinguishedName, dp)
+		)
 
 		if len(domain) == 1 {
 			ds := domain[0].SID()
 
 			if s.StripRID() != ds {
 				// Foreign security principal
-				no.SetFlex(DistinguishedName, s.String()+",CN=ForeignSecurityPrincipals,"+dp.String())
+				no.SetFlex(DistinguishedName, s.String()+",CN=ForeignSecurityPrincipals,"+dp.String(),
+					ObjectCategorySimple, "Foreign-Security-Principal")
 			}
 		}
 	}
@@ -628,24 +633,52 @@ func (os *Objects) FindAdjacentSID(s windowssecurity.SID, r *Object) *Object {
 	// These are the "local" groups shared between DCs
 	// We need to find the right one, and we'll use the DomainPart for this
 
-	// From inside same source, that is easy
-	if r.HasAttr(UniqueSource) {
-		if os, found := os.FindTwoMulti(ObjectSid, AttributeValueSID(s), UniqueSource, r.OneAttr(UniqueSource)); found {
-			return findMostLocal(os)
+	if r.HasAttr(DomainPart) {
+		// From outside, we need to find the domain part
+		if o, found := os.FindTwo(ObjectSid, AttributeValueSID(s), DomainPart, r.OneAttr(DomainPart)); found {
+			return o
 		}
 	}
 
-	if r.HasAttr(DomainPart) {
-		// From outside, we need to find the domain part
-		if os, found := os.FindTwoMulti(ObjectSid, AttributeValueSID(s), DomainPart, r.OneAttr(DomainPart)); found {
-			return findMostLocal(os)
+	// From inside same source, that is easy
+	if r.HasAttr(UniqueSource) {
+		if o, found := os.FindTwo(ObjectSid, AttributeValueSID(s), UniqueSource, r.OneAttr(UniqueSource)); found {
+			return o
 		}
 	}
 
-	if os, found := os.FindMulti(ObjectSid, AttributeValueSID(s)); found {
-		return findMostLocal(os)
+	// Are we looking for a SID in our own domain?
+	if r.HasAttr(ObjectSid) {
+		if s.StripRID() == r.SID().StripRID() {
+			// Same domain
+			if os, found := os.FindMulti(ObjectSid, AttributeValueSID(s)); found {
+				for _, o := range os {
+					if o.Type() != ObjectTypeForeignSecurityPrincipal {
+						return o
+					}
+				}
+			}
+		} else {
+			// Other domain ... hmm
+			if os, found := os.FindMulti(ObjectSid, AttributeValueSID(s)); found {
+				if len(os) == 1 {
+					return os[0]
+				}
+				for _, o := range os {
+					if o.Type() != ObjectTypeForeignSecurityPrincipal {
+						return o
+					}
+					if strings.Contains(o.DN(), ",CN=WellKnown Security Principals,") {
+						return o
+					}
+				}
+				log.Warn().Msgf("Found multiple SIDs for %s, returning first found", s.String())
+				return os[0]
+			}
+		}
 	}
 
+	// Not found
 	return nil
 }
 

diff --git a/modules/integrations/activedirectory/analyze/analyze-ad.go b/modules/integrations/activedirectory/analyze/analyze-ad.go
@@ -280,7 +280,7 @@ func init() {
 					}
 				}
 				if !sd.Owner.IsNull() && !aclhasdeny {
-					ao.FindOrAddSID(sd.Owner).Pwns(o, activedirectory.PwnOwns)
+					ao.FindOrAddAdjacentSID(sd.Owner, o).Pwns(o, activedirectory.PwnOwns)
 				}
 			},
 		},
@@ -703,7 +703,7 @@ func init() {
 				sids := o.Attr(activedirectory.SIDHistory).Slice()
 				for _, sidval := range sids {
 					if sid, ok := sidval.Raw().(windowssecurity.SID); ok {
-						target := ao.FindOrAddSID(sid)
+						target := ao.FindOrAddAdjacentSID(sid, o)
 						o.Pwns(target, activedirectory.PwnSIDHistoryEquality)
 					}
 				}
@@ -892,6 +892,10 @@ func init() {
 				continue
 			}
 
+			if o.HasAttr(engine.DomainPart) {
+				continue
+			}
+
 			parts := strings.Split(o.DN(), ",")
 			lastpart := -1
 
@@ -1092,7 +1096,7 @@ func init() {
 				if len(sid) > 8 {
 					sidbytes := []byte(sid)
 					binary.LittleEndian.PutUint32(sidbytes[len(sid)-4:], uint32(rid))
-					primarygroup := ao.FindOrAddSID(windowssecurity.SID(sidbytes))
+					primarygroup := ao.FindOrAddAdjacentSID(windowssecurity.SID(sidbytes), object)
 					primarygroup.AddMember(object)
 				}
 			}
@@ -1135,7 +1139,7 @@ func init() {
 					object.SetValues(engine.MetaServer, engine.AttributeValueInt(1))
 
 					// All DCs are members of Enterprise Domain Controllers
-					object.Pwns(ao.FindOrAddSID(EnterpriseDomainControllers), activedirectory.PwnMemberOfGroup)
+					object.Pwns(ao.FindOrAddAdjacentSID(EnterpriseDomainControllers, object), activedirectory.PwnMemberOfGroup)
 				}
 				if uac&engine.UAC_ACCOUNTDISABLE != 0 {
 					object.SetValues(engine.MetaAccountDisabled, engine.AttributeValueInt(1))
@@ -1240,7 +1244,7 @@ func init() {
 		for _, domaindns := range ao.Filter(func(o *engine.Object) bool {
 			return o.Type() == engine.ObjectTypeDomainDNS && o.HasAttr(engine.ObjectSid)
 		}).Slice() {
-			// ourDomainDN := domaindns.DN()
+			ourDomainDN := domaindns.DN()
 			ourDomainSid := domaindns.SID()
 
 			for _, o := range ao.Slice() {
@@ -1250,11 +1254,11 @@ func init() {
 						log.Warn().Msgf("Found a 'lost' local SID object %v, but not taking action (don't know where to place it). This will affect your analysis results, try dumping as Domain Admin!", o.SID())
 					} else {
 						log.Debug().Msgf("Found a 'lost' foreign SID object %v, adding it as a synthetic Foreign-Security-Principal", o.SID())
-						// o.SetFlex(
-						// 	engine.DistinguishedName, engine.AttributeValueString(o.SID().String()+",CN=ForeignSecurityPrincipals,"+ourDomainDN),
-						// 	engine.ObjectCategorySimple, "Foreign-Security-Principal",
-						// 	engine.MetaDataSource, "Autogenerated",
-						// )
+						o.SetFlex(
+							engine.DistinguishedName, engine.AttributeValueString(o.SID().String()+",CN=ForeignSecurityPrincipals,"+ourDomainDN),
+							engine.ObjectCategorySimple, "Foreign-Security-Principal",
+							engine.MetaDataSource, "Autogenerated",
+						)
 					}
 				}
 			}
@@ -1384,6 +1388,8 @@ func init() {
 						}
 					}
 				}
+			} else {
+				log.Warn().Msgf("Found a foreign security principal %v with an non type 21 SID %v", foreign.DN(), sid.String())
 			}
 		}
 	}, "Link foreign security principals to their native objects",