Skip to content

Commit

Permalink
refactor: parameterize ura enforcement for CanRDP processing (#295)
Browse files Browse the repository at this point in the history 
* refactor: parameterize ura enforcement for CanRDP processing

* test: update canrdp integration tests
  • Loading branch information
@ddlees
ddlees authored Jan 8, 2024
1 parent 6f10d53 commit 9130ac6
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 26 deletions.
2 changes: 1 addition & 1 deletion cmd/api/src/analysis/ad/post.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func Post(ctx context.Context, db graph.Database, adcsEnabled bool) (*analysis.A
return &aggregateStats, err
} else if groupExpansions, err := adAnalysis.ExpandAllRDPLocalGroups(ctx, db); err != nil {
return &aggregateStats, err
} else if localGroupStats, err := adAnalysis.PostLocalGroups(ctx, db, groupExpansions); err != nil {
} else if localGroupStats, err := adAnalysis.PostLocalGroups(ctx, db, groupExpansions, false); err != nil {
return &aggregateStats, err
} else if adcsStats, err := adAnalysis.PostADCS(ctx, db, groupExpansions, adcsEnabled); err != nil {
return &aggregateStats, err
Expand Down
50 changes: 42 additions & 8 deletions cmd/api/src/analysis/analysis_integration_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
// Copyright 2023 Specter Ops, Inc.
//
//
// Licensed under the Apache License, Version 2.0
// 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 integration
Expand All @@ -26,9 +26,9 @@ import (
analysis "github.com/specterops/bloodhound/analysis/ad"
"github.com/specterops/bloodhound/graphschema/ad"

"github.com/specterops/bloodhound/dawgs/graph"
"github.com/specterops/bloodhound/src/test/integration"
"github.com/stretchr/testify/require"
"github.com/specterops/bloodhound/dawgs/graph"
)

func TestFetchRDPEnsureNoDescent(t *testing.T) {
Expand All @@ -40,7 +40,7 @@ func TestFetchRDPEnsureNoDescent(t *testing.T) {
require.Nil(t, err)

require.Nil(t, db.ReadTransaction(context.Background(), func(tx graph.Transaction) error {
rdpEnabledEntityIDBitmap, err := analysis.FetchRDPEntityBitmapForComputer(tx, harness.RDPB.Computer.ID, groupExpansions)
rdpEnabledEntityIDBitmap, err := analysis.FetchRDPEntityBitmapForComputer(tx, harness.RDPB.Computer.ID, groupExpansions, false)
require.Nil(t, err)

// We should expect all groups that have the RIL incoming privilege to the computer
Expand All @@ -63,8 +63,9 @@ func TestFetchRDPEntityBitmapForComputer(t *testing.T) {
groupExpansions, err := analysis.ExpandAllRDPLocalGroups(context.Background(), db)
require.Nil(t, err)

// Enforced URA validation
require.Nil(t, db.ReadTransaction(context.Background(), func(tx graph.Transaction) error {
rdpEnabledEntityIDBitmap, err := analysis.FetchRDPEntityBitmapForComputer(tx, harness.RDP.Computer.ID, groupExpansions)
rdpEnabledEntityIDBitmap, err := analysis.FetchRDPEntityBitmapForComputer(tx, harness.RDP.Computer.ID, groupExpansions, true)
require.Nil(t, err)

// We should expect all groups that have the RIL incoming privilege to the computer
Expand All @@ -77,6 +78,39 @@ func TestFetchRDPEntityBitmapForComputer(t *testing.T) {
require.True(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.DomainGroupA.ID.Uint32()))
require.True(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.DomainGroupB.ID.Uint32()))

require.False(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.DomainGroupC.ID.Uint32()))
require.False(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.DomainGroupD.ID.Uint32()))
require.False(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.DomainGroupE.ID.Uint32()))
require.False(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.RDPDomainUsersGroup.ID.Uint32()))
require.False(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.AlyxUser.ID.Uint32()))
require.False(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.AndyUser.ID.Uint32()))
require.False(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.RohanUser.ID.Uint32()))
require.False(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.JohnUser.ID.Uint32()))

return nil
}))

// Unenforced URA validation
require.Nil(t, db.ReadTransaction(context.Background(), func(tx graph.Transaction) error {
rdpEnabledEntityIDBitmap, err := analysis.FetchRDPEntityBitmapForComputer(tx, harness.RDP.Computer.ID, groupExpansions, false)
require.Nil(t, err)

require.True(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.IrshadUser.ID.Uint32()))
require.True(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.UliUser.ID.Uint32()))
require.True(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.EliUser.ID.Uint32()))
require.True(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.DomainGroupA.ID.Uint32()))
require.True(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.DomainGroupB.ID.Uint32()))
require.True(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.DomainGroupC.ID.Uint32()))

require.False(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.DomainGroupD.ID.Uint32()))
require.False(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.DomainGroupE.ID.Uint32()))
require.False(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.RDPDomainUsersGroup.ID.Uint32()))
require.False(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.AlyxUser.ID.Uint32()))
require.False(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.DillonUser.ID.Uint32()))
require.False(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.AndyUser.ID.Uint32()))
require.False(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.RohanUser.ID.Uint32()))
require.False(t, rdpEnabledEntityIDBitmap.Contains(harness.RDP.JohnUser.ID.Uint32()))

return nil
}))

Expand All @@ -91,7 +125,7 @@ func TestFetchRDPEntityBitmapForComputer(t *testing.T) {
require.Nil(t, err)

return db.ReadTransaction(context.Background(), func(tx graph.Transaction) error {
rdpEnabledEntityIDBitmap, err := analysis.FetchRDPEntityBitmapForComputer(tx, harness.RDP.Computer.ID, groupExpansions)
rdpEnabledEntityIDBitmap, err := analysis.FetchRDPEntityBitmapForComputer(tx, harness.RDP.Computer.ID, groupExpansions, true)
require.Nil(t, err)

require.Equal(t, 6, int(rdpEnabledEntityIDBitmap.Cardinality()))
Expand Down
22 changes: 5 additions & 17 deletions packages/go/analysis/ad/post.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ func getLAPSComputersForDomain(tx graph.Transaction, domain *graph.Node) ([]grap
}
}

func PostLocalGroups(ctx context.Context, db graph.Database, localGroupExpansions impact.PathAggregator) (*analysis.AtomicPostProcessingStats, error) {
func PostLocalGroups(ctx context.Context, db graph.Database, localGroupExpansions impact.PathAggregator, enforceURA bool) (*analysis.AtomicPostProcessingStats, error) {
var (
adminGroupSuffix = "-544"
psRemoteGroupSuffix = "-580"
Expand Down Expand Up @@ -281,7 +281,7 @@ func PostLocalGroups(ctx context.Context, db graph.Database, localGroupExpansion
}

if err := operation.Operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- analysis.CreatePostRelationshipJob) error {
if entities, err := FetchRDPEntityBitmapForComputerWithUnenforcedURA(tx, computerID, threadSafeLocalGroupExpansions); err != nil {
if entities, err := FetchRDPEntityBitmapForComputer(tx, computerID, threadSafeLocalGroupExpansions, enforceURA); err != nil {
return err
} else {
for _, rdp := range entities.Slice() {
Expand Down Expand Up @@ -437,26 +437,14 @@ func ExpandAllRDPLocalGroups(ctx context.Context, db graph.Database) (impact.Pat
))
}

func FetchRDPEntityBitmapForComputer(tx graph.Transaction, computer graph.ID, localGroupExpansions impact.PathAggregator) (cardinality.Duplex[uint32], error) {
func FetchRDPEntityBitmapForComputer(tx graph.Transaction, computer graph.ID, localGroupExpansions impact.PathAggregator, enforceURA bool) (cardinality.Duplex[uint32], error) {
if rdpLocalGroup, err := FetchComputerLocalGroupBySIDSuffix(tx, computer, RDPGroupSuffix); err != nil {
if graph.IsErrNotFound(err) {
return cardinality.NewBitmap32(), nil
}

return nil, err
} else {
return ProcessRDPWithUra(tx, rdpLocalGroup, computer, localGroupExpansions)
}
}

func FetchRDPEntityBitmapForComputerWithUnenforcedURA(tx graph.Transaction, computer graph.ID, localGroupExpansions impact.PathAggregator) (cardinality.Duplex[uint32], error) {
if rdpLocalGroup, err := FetchComputerLocalGroupBySIDSuffix(tx, computer, RDPGroupSuffix); err != nil {
if graph.IsErrNotFound(err) {
return cardinality.NewBitmap32(), nil
}

return nil, err
} else if ComputerHasURACollection(tx, computer) {
} else if enforceURA || ComputerHasURACollection(tx, computer) {
return ProcessRDPWithUra(tx, rdpLocalGroup, computer, localGroupExpansions)
} else if bitmap, err := FetchLocalGroupBitmapForComputer(tx, computer, RDPGroupSuffix); err != nil {
return nil, err
Expand All @@ -481,7 +469,7 @@ func ComputerHasURACollection(tx graph.Transaction, computerID graph.ID) bool {

func ProcessRDPWithUra(tx graph.Transaction, rdpLocalGroup *graph.Node, computer graph.ID, localGroupExpansions impact.PathAggregator) (cardinality.Duplex[uint32], error) {
rdpLocalGroupMembers := localGroupExpansions.Cardinality(rdpLocalGroup.ID.Uint32()).(cardinality.Duplex[uint32])
//Shortcut opportunity: see if the RDP group has RIL privilege. If it does, get the first degree members and return those ids, since everything in RDP group has CanRDP privs. No reason to look any further
// Shortcut opportunity: see if the RDP group has RIL privilege. If it does, get the first degree members and return those ids, since everything in RDP group has CanRDP privs. No reason to look any further
if HasRemoteInteractiveLogonPrivilege(tx, rdpLocalGroup.ID, computer) {
firstDegreeMembers := cardinality.NewBitmap32()

Expand Down

0 comments on commit 9130ac6

Please sign in to comment.