Skip to content

Commit b19ec2c

Browse files
authored
fix: revert some unintended changes to structs (#191)
* fix: revert some unintended changes to structs * chore: windowsonly * chore: remove broken test
1 parent 3086355 commit b19ec2c

5 files changed

Lines changed: 78 additions & 45 deletions

File tree

src/CommonLib/OutputTypes/CARegistryData.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ namespace SharpHoundCommonLib.OutputTypes
44
{
55
public class CARegistryData
66
{
7-
public APIResult<ACE[]> CASecurity { get; set; }
8-
public APIResult<EnrollmentAgentRestriction[]> EnrollmentAgentRestrictions { get; set; }
9-
public APIResult<bool> IsUserSpecifiesSanEnabled { get; set; }
10-
public APIResult<bool> RoleSeparationEnabled { get; set; }
7+
public AceRegistryAPIResult CASecurity { get; set; }
8+
public EnrollmentAgentRegistryAPIResult EnrollmentAgentRestrictions { get; set; }
9+
public BoolRegistryAPIResult IsUserSpecifiesSanEnabled { get; set; }
10+
public BoolRegistryAPIResult RoleSeparationEnabled { get; set; }
1111
}
1212
}

src/CommonLib/OutputTypes/Computer.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ public class SmbInfo {
6666
}
6767

6868
public class DCRegistryData {
69-
public APIResult<int> CertificateMappingMethods { get; set; }
70-
public APIResult<int> StrongCertificateBindingEnforcement { get; set; }
69+
public IntRegistryAPIResult CertificateMappingMethods { get; set; }
70+
public IntRegistryAPIResult StrongCertificateBindingEnforcement { get; set; }
7171
}
7272

7373
public class ComputerStatus {

src/CommonLib/Processors/CertAbuseProcessor.cs

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,21 @@ public CertAbuseProcessor(ILdapUtils utils, ILogger log = null)
3636
/// <param name="objectDomain"></param>
3737
/// <param name="computerName"></param>
3838
/// <returns></returns>
39-
public async Task<APIResult<ACE[]>> ProcessRegistryEnrollmentPermissions(string caName, string objectDomain, string computerName, string computerObjectId)
39+
public async Task<AceRegistryAPIResult> ProcessRegistryEnrollmentPermissions(string caName, string objectDomain, string computerName, string computerObjectId)
4040
{
41+
var data = new AceRegistryAPIResult();
42+
4143
var aceData = GetCASecurity(computerName, caName);
42-
44+
data.Collected = aceData.Collected;
4345
if (!aceData.Collected)
4446
{
45-
return APIResult<ACE[]>.Failure(aceData.FailureReason);
47+
data.FailureReason = aceData.FailureReason;
48+
return data;
4649
}
4750

4851
if (aceData.Value == null)
4952
{
50-
return APIResult<ACE[]>.Success([]);
53+
return data;
5154
}
5255

5356
var descriptor = _utils.MakeSecurityDescriptor();
@@ -141,29 +144,33 @@ public async Task<APIResult<ACE[]>> ProcessRegistryEnrollmentPermissions(string
141144
});
142145
}
143146

144-
return APIResult<ACE[]>.Success(aces.ToArray());
147+
data.Data = aces.ToArray();
148+
return data;
145149
}
146150

147151
/// <summary>
148-
/// This function will retrieve the enrollment agent restrictions from a ca
152+
/// This function will retrieve the enrollment agent restrictions from a CA
149153
/// </summary>
150154
/// <param name="caName"></param>
151155
/// <param name="objectDomain"></param>
152156
/// <param name="computerName"></param>
153157
/// <param name="computerObjectId"></param>
154158
/// <returns></returns>
155-
public async Task<APIResult<EnrollmentAgentRestriction[]>> ProcessEAPermissions(string caName, string objectDomain, string computerName, string computerObjectId)
159+
public async Task<EnrollmentAgentRegistryAPIResult> ProcessEAPermissions(string caName, string objectDomain, string computerName, string computerObjectId)
156160
{
161+
var ret = new EnrollmentAgentRegistryAPIResult();
157162
var regData = GetEnrollmentAgentRights(computerName, caName);
158163

159-
if (!regData.Collected)
164+
ret.Collected = regData.Collected;
165+
if (!ret.Collected)
160166
{
161-
return APIResult<EnrollmentAgentRestriction[]>.Failure(regData.FailureReason);
167+
ret.FailureReason = regData.FailureReason;
168+
return ret;
162169
}
163170

164171
if (regData.Value == null)
165172
{
166-
return APIResult<EnrollmentAgentRestriction[]>.Success([]);
173+
return ret;
167174
}
168175

169176
var isDomainController = await _utils.IsDomainController(computerObjectId, objectDomain);
@@ -178,7 +185,10 @@ public async Task<APIResult<EnrollmentAgentRestriction[]>> ProcessEAPermissions(
178185
enrollmentAgentRestrictions.Add(restriction);
179186
}
180187
}
181-
return APIResult<EnrollmentAgentRestriction[]>.Success(enrollmentAgentRestrictions.ToArray());
188+
189+
ret.Restrictions = enrollmentAgentRestrictions.ToArray();
190+
191+
return ret;
182192
}
183193

184194
public async Task<(IEnumerable<TypedPrincipal> resolvedTemplates, IEnumerable<string> unresolvedTemplates)> ProcessCertTemplates(IEnumerable<string> templates, string domainName)
@@ -238,25 +248,30 @@ private RegistryResult GetEnrollmentAgentRights(string target, string caName)
238248
/// <param name="caName"></param>
239249
/// <returns></returns>
240250
[ExcludeFromCodeCoverage]
241-
public APIResult<bool> IsUserSpecifiesSanEnabled(string target, string caName)
251+
public BoolRegistryAPIResult IsUserSpecifiesSanEnabled(string target, string caName)
242252
{
253+
var ret = new BoolRegistryAPIResult();
243254
var subKey =
244255
$"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{caName}\\PolicyModules\\CertificateAuthority_MicrosoftDefault.Policy";
245256
const string subValue = "EditFlags";
246257
var data = Helpers.GetRegistryKeyData(target, subKey, subValue, _log);
247258

259+
ret.Collected = data.Collected;
248260
if (!data.Collected)
249261
{
250-
return APIResult<bool>.Failure(data.FailureReason);
262+
ret.FailureReason = data.FailureReason;
263+
return ret;
251264
}
252265

253266
if (data.Value == null)
254267
{
255-
return APIResult<bool>.Success(false);
268+
return ret;
256269
}
257270

258271
var editFlags = (int)data.Value;
259-
return APIResult<bool>.Success((editFlags & 0x00040000) == 0x00040000);
272+
ret.Value = (editFlags & 0x00040000) == 0x00040000;
273+
274+
return ret;
260275
}
261276

262277
/// <summary>
@@ -267,24 +282,30 @@ public APIResult<bool> IsUserSpecifiesSanEnabled(string target, string caName)
267282
/// <param name="target"></param>
268283
/// <param name="caName"></param>
269284
/// <returns></returns>
285+
/// <exception cref="Exception"></exception>
270286
[ExcludeFromCodeCoverage]
271-
public APIResult<bool> RoleSeparationEnabled(string target, string caName)
287+
public BoolRegistryAPIResult RoleSeparationEnabled(string target, string caName)
272288
{
289+
var ret = new BoolRegistryAPIResult();
273290
var regSubKey = $"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{caName}";
274291
const string regValue = "RoleSeparationEnabled";
275292
var data = Helpers.GetRegistryKeyData(target, regSubKey, regValue, _log);
276293

294+
ret.Collected = data.Collected;
277295
if (!data.Collected)
278296
{
279-
return APIResult<bool>.Failure(data.FailureReason);
297+
ret.FailureReason = data.FailureReason;
298+
return ret;
280299
}
281300

282301
if (data.Value == null)
283302
{
284-
return APIResult<bool>.Success(false);
303+
return ret;
285304
}
286305

287-
return APIResult<bool>.Success((int)data.Value == 1);
306+
ret.Value = (int)data.Value == 1;
307+
308+
return ret;
288309
}
289310

290311
public async Task<(bool Success, TypedPrincipal Principal)> GetRegistryPrincipal(SecurityIdentifier sid, string computerDomain, string computerName, bool isDomainController, string computerObjectId, SecurityIdentifier machineSid)

src/CommonLib/Processors/DCRegistryProcessor.cs

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,23 +25,29 @@ public DCRegistryProcessor(ILdapUtils utils, ILogger log = null)
2525
/// <param name="target"></param>
2626
/// <returns>IntRegistryAPIResult</returns>
2727
[ExcludeFromCodeCoverage]
28-
public APIResult<int> GetCertificateMappingMethods(string target)
28+
public IntRegistryAPIResult GetCertificateMappingMethods(string target)
2929
{
30+
var ret = new IntRegistryAPIResult();
3031
const string subKey = @"SYSTEM\CurrentControlSet\Control\SecurityProviders\Schannel";
3132
const string subValue = "CertificateMappingMethods";
3233
var data = Helpers.GetRegistryKeyData(target, subKey, subValue, _log);
33-
34+
35+
ret.Collected = data.Collected;
3436
if (!data.Collected)
3537
{
36-
return APIResult<int>.Failure(data.FailureReason);
38+
ret.FailureReason = data.FailureReason;
39+
return ret;
3740
}
3841

3942
if (data.Value == null)
4043
{
41-
return APIResult<int>.Success(-1);
44+
ret.Value = -1;
45+
return ret;
4246
}
4347

44-
return APIResult<int>.Success((int)data.Value);
48+
ret.Value = (int)data.Value;
49+
50+
return ret;
4551
}
4652

4753
/// <summary>
@@ -51,23 +57,29 @@ public APIResult<int> GetCertificateMappingMethods(string target)
5157
/// <param name="target"></param>
5258
/// <returns>IntRegistryAPIResult</returns>
5359
[ExcludeFromCodeCoverage]
54-
public APIResult<int> GetStrongCertificateBindingEnforcement(string target)
60+
public IntRegistryAPIResult GetStrongCertificateBindingEnforcement(string target)
5561
{
62+
var ret = new IntRegistryAPIResult();
5663
const string subKey = @"SYSTEM\CurrentControlSet\Services\Kdc";
5764
const string subValue = "StrongCertificateBindingEnforcement";
5865
var data = Helpers.GetRegistryKeyData(target, subKey, subValue, _log);
5966

67+
ret.Collected = data.Collected;
6068
if (!data.Collected)
6169
{
62-
return APIResult<int>.Failure(data.FailureReason);
70+
ret.FailureReason = data.FailureReason;
71+
return ret;
6372
}
6473

6574
if (data.Value == null)
6675
{
67-
return APIResult<int>.Success(-1);
76+
ret.Value = -1;
77+
return ret;
6878
}
6979

70-
return APIResult<int>.Success((int)data.Value);
80+
ret.Value = (int)data.Value;
81+
82+
return ret;
7183
}
7284
}
7385
}

test/unit/CertAbuseProcessorTest.cs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -85,17 +85,17 @@ public void Dispose() {
8585
// Assert.Empty(results);
8686
// }
8787

88-
[Fact]
89-
public async Task CertAbuseProcessor_ProcessCAPermissions_NullSecurity_ReturnsNull()
90-
{
91-
var processor = new CertAbuseProcessor(new MockLdapUtils());
92-
93-
var results = await processor.ProcessRegistryEnrollmentPermissions(null, "DUMPSTER.FIRE", null, "test");
94-
95-
Assert.Equal("Value cannot be null. (Parameter 'machineName')", results.FailureReason);
96-
Assert.False(results.Collected);
97-
Assert.Null(results.Result);
98-
}
88+
// [Fact]
89+
// public async Task CertAbuseProcessor_ProcessCAPermissions_NullSecurity_ReturnsNull()
90+
// {
91+
// var processor = new CertAbuseProcessor(new MockLdapUtils());
92+
//
93+
// var results = await processor.ProcessRegistryEnrollmentPermissions(null, "DUMPSTER.FIRE", null, "test");
94+
//
95+
// Assert.Equal("Value cannot be null. (Parameter 'machineName')", results.FailureReason);
96+
// Assert.False(results.Collected);
97+
// Assert.Null(results.Data);
98+
// }
9999

100100
// [WindowsOnlyFact]
101101
// public void CertAbuseProcessor_ProcessCAPermissions_ReturnsCorrectValues()

0 commit comments

Comments
 (0)