diff --git a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/IRCollection.cs b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/IRCollection.cs index a21f1bf05d4c7..ecfc5b4c3a10c 100644 --- a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/IRCollection.cs +++ b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/IRCollection.cs @@ -18,7 +18,7 @@ public class IdentityReferenceCollection : ICollection // Container enumerated by this collection // - private readonly List _Identities; + private readonly List _identities; #endregion @@ -39,7 +39,7 @@ public IdentityReferenceCollection() public IdentityReferenceCollection(int capacity) { - _Identities = new List(capacity); + _identities = new List(capacity); } #endregion @@ -48,14 +48,14 @@ public IdentityReferenceCollection(int capacity) public void CopyTo(IdentityReference[] array, int offset) { - _Identities.CopyTo(0, array, offset, Count); + _identities.CopyTo(0, array, offset, Count); } public int Count { get { - return _Identities.Count; + return _identities.Count; } } @@ -74,7 +74,7 @@ public void Add(IdentityReference identity) throw new ArgumentNullException(nameof(identity)); } - _Identities.Add(identity); + _identities.Add(identity); } public bool Remove(IdentityReference identity) @@ -86,7 +86,7 @@ public bool Remove(IdentityReference identity) if (Contains(identity)) { - return _Identities.Remove(identity); + return _identities.Remove(identity); } return false; @@ -94,7 +94,7 @@ public bool Remove(IdentityReference identity) public void Clear() { - _Identities.Clear(); + _identities.Clear(); } public bool Contains(IdentityReference identity) @@ -104,7 +104,7 @@ public bool Contains(IdentityReference identity) throw new ArgumentNullException(nameof(identity)); } - return _Identities.Contains(identity); + return _identities.Contains(identity); } #endregion @@ -129,17 +129,16 @@ public IdentityReference this[int index] { get { - return _Identities[index]; + return _identities[index]; } set { - if (value == null) + if (value is null) { throw new ArgumentNullException(nameof(value)); } - - _Identities[index] = value; + _identities[index] = value; } } @@ -147,7 +146,7 @@ internal List Identities { get { - return _Identities; + return _identities; } } @@ -398,11 +397,10 @@ internal class IdentityReferenceEnumerator : IEnumerator, IDi internal IdentityReferenceEnumerator(IdentityReferenceCollection collection) { - if (collection == null) + if (collection is null) { throw new ArgumentNullException(nameof(collection)); } - _collection = collection; _current = -1; } diff --git a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/IdentityReference.cs b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/IdentityReference.cs index 0fbfc120506fd..67621c826762f 100644 --- a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/IdentityReference.cs +++ b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/IdentityReference.cs @@ -17,11 +17,11 @@ internal IdentityReference() public abstract IdentityReference Translate(Type targetType); - public override abstract bool Equals(object o); + public abstract override bool Equals(object o); - public override abstract int GetHashCode(); + public abstract override int GetHashCode(); - public override abstract string ToString(); + public abstract override string ToString(); public static bool operator ==(IdentityReference left, IdentityReference right) { diff --git a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs index 1259f1177ceb6..bc3fb179b9da2 100644 --- a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs +++ b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs @@ -154,16 +154,13 @@ public override string ToString() internal static IdentityReferenceCollection Translate(IdentityReferenceCollection sourceAccounts, Type targetType, bool forceSuccess) { - bool SomeFailed = false; - IdentityReferenceCollection Result; + IdentityReferenceCollection result = Translate(sourceAccounts, targetType, out bool someFailed); - Result = Translate(sourceAccounts, targetType, out SomeFailed); - - if (forceSuccess && SomeFailed) + if (forceSuccess && someFailed) { IdentityReferenceCollection UnmappedIdentities = new IdentityReferenceCollection(); - foreach (IdentityReference id in Result) + foreach (IdentityReference id in result) { if (id.GetType() != targetType) { @@ -174,7 +171,7 @@ internal static IdentityReferenceCollection Translate(IdentityReferenceCollectio throw new IdentityNotMappedException(SR.IdentityReference_IdentityNotMapped, UnmappedIdentities); } - return Result; + return result; } internal static IdentityReferenceCollection Translate(IdentityReferenceCollection sourceAccounts, Type targetType, out bool someFailed) @@ -348,7 +345,7 @@ private static IdentityReferenceCollection TranslateToSids(IdentityReferenceColl case SidNameUse.Alias: case SidNameUse.Computer: case SidNameUse.WellKnownGroup: - Result.Add(new SecurityIdentifier(Lts.Sid, true)); + Result.Add(new SecurityIdentifier(Lts.Sid)); break; default: diff --git a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs index 83686ed130c29..26ee151619582 100644 --- a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs +++ b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs @@ -265,7 +265,7 @@ public sealed class SecurityIdentifier : IdentityReference, IComparable subAuthorities) +#endif { - if (subAuthorities == null) - { - throw new ArgumentNullException(nameof(subAuthorities)); - } - // // Check the number of subauthorities passed in // - if (subAuthorities.Length > MaxSubAuthorities) { throw new ArgumentOutOfRangeException( "subAuthorities.Length", subAuthorities.Length, - SR.Format(SR.IdentityReference_InvalidNumberOfSubauthorities, MaxSubAuthorities)); + SR.Format(SR.IdentityReference_InvalidNumberOfSubauthorities, MaxSubAuthorities) + ); } // // Identifier authority is at most 6 bytes long // - if (identifierAuthority < 0 || - (long)identifierAuthority > MaxIdentifierAuthority) + if (identifierAuthority < 0 || (long)identifierAuthority > MaxIdentifierAuthority) { throw new ArgumentOutOfRangeException( -nameof(identifierAuthority), + nameof(identifierAuthority), identifierAuthority, - SR.IdentityReference_IdentifierAuthorityTooLarge); + SR.IdentityReference_IdentifierAuthorityTooLarge + ); } // @@ -347,8 +346,11 @@ private void CreateFromParts(IdentifierAuthority identifierAuthority, int[] subA // _identifierAuthority = identifierAuthority; - _subAuthorities = new int[subAuthorities.Length]; - subAuthorities.CopyTo(_subAuthorities, 0); +#if NETCOREAPP2_0 + _subAuthorities = (int[])subAuthorities.Clone(); +#else + _subAuthorities = subAuthorities.ToArray(); +#endif // // Compute and store the binary form @@ -361,21 +363,20 @@ private void CreateFromParts(IdentifierAuthority identifierAuthority, int[] subA // } SID, *PISID; // - byte i; - _binaryForm = new byte[1 + 1 + 6 + 4 * this.SubAuthorityCount]; + _binaryForm = new byte[1 + 1 + 6 + 4 * _subAuthorities.Length]; // // First two bytes contain revision and subauthority count // _binaryForm[0] = Revision; - _binaryForm[1] = (byte)this.SubAuthorityCount; + _binaryForm[1] = (byte)_subAuthorities.Length; // // Identifier authority takes up 6 bytes // - for (i = 0; i < 6; i++) + for (int i = 0; i < 6; i++) { _binaryForm[2 + i] = (byte)((((ulong)_identifierAuthority) >> ((5 - i) * 8)) & 0xFF); } @@ -384,10 +385,9 @@ private void CreateFromParts(IdentifierAuthority identifierAuthority, int[] subA // Subauthorities go last, preserving big-endian representation // - for (i = 0; i < this.SubAuthorityCount; i++) + for (int i = 0; i < _subAuthorities.Length; i++) { - byte shift; - for (shift = 0; shift < 4; shift += 1) + for (byte shift = 0; shift < 4; shift += 1) { _binaryForm[8 + 4 * i + shift] = unchecked((byte)(((ulong)_subAuthorities[i]) >> (shift * 8))); } @@ -411,10 +411,7 @@ private void CreateFromBinaryForm(byte[] binaryForm, int offset) if (offset < 0) { - throw new ArgumentOutOfRangeException( -nameof(offset), - offset, - SR.ArgumentOutOfRange_NeedNonNegNum); + throw new ArgumentOutOfRangeException(nameof(offset), offset, SR.ArgumentOutOfRange_NeedNonNegNum); } // @@ -423,14 +420,9 @@ private void CreateFromBinaryForm(byte[] binaryForm, int offset) if (binaryForm.Length - offset < SecurityIdentifier.MinBinaryLength) { - throw new ArgumentOutOfRangeException( -nameof(binaryForm), - SR.ArgumentOutOfRange_ArrayTooSmall); + throw new ArgumentOutOfRangeException(nameof(binaryForm), SR.ArgumentOutOfRange_ArrayTooSmall); } - IdentifierAuthority Authority; - int[] SubAuthorities; - // // Extract the elements of a SID // @@ -441,64 +433,67 @@ private void CreateFromBinaryForm(byte[] binaryForm, int offset) // Revision is incorrect // - throw new ArgumentException( - SR.IdentityReference_InvalidSidRevision, -nameof(binaryForm)); + throw new ArgumentException(SR.IdentityReference_InvalidSidRevision, nameof(binaryForm)); } // // Insist on the correct number of subauthorities // - - if (binaryForm[offset + 1] > MaxSubAuthorities) + int subAuthoritiesLength = binaryForm[offset + 1]; + if (subAuthoritiesLength > MaxSubAuthorities) { - throw new ArgumentException( - SR.Format(SR.IdentityReference_InvalidNumberOfSubauthorities, MaxSubAuthorities), -nameof(binaryForm)); + throw new ArgumentException(SR.Format(SR.IdentityReference_InvalidNumberOfSubauthorities, MaxSubAuthorities), nameof(binaryForm)); } // // Make sure the buffer is big enough // - int Length = 1 + 1 + 6 + 4 * binaryForm[offset + 1]; + int totalLength = 1 + 1 + 6 + 4 * subAuthoritiesLength; - if (binaryForm.Length - offset < Length) + if (binaryForm.Length - offset < totalLength) { - throw new ArgumentException( - SR.ArgumentOutOfRange_ArrayTooSmall, -nameof(binaryForm)); + throw new ArgumentException(SR.ArgumentOutOfRange_ArrayTooSmall, nameof(binaryForm)); } - Authority = - (IdentifierAuthority)( +#if NETCOREAPP2_0 + int[] subAuthorities = new int[subAuthoritiesLength]; +#else + Span subAuthorities = stackalloc int[MaxSubAuthorities]; +#endif + + IdentifierAuthority authority = (IdentifierAuthority)( (((long)binaryForm[offset + 2]) << 40) + (((long)binaryForm[offset + 3]) << 32) + (((long)binaryForm[offset + 4]) << 24) + (((long)binaryForm[offset + 5]) << 16) + (((long)binaryForm[offset + 6]) << 8) + - (((long)binaryForm[offset + 7]))); - - SubAuthorities = new int[binaryForm[offset + 1]]; + (((long)binaryForm[offset + 7])) + ); // // Subauthorities are represented in big-endian format // - for (byte i = 0; i < binaryForm[offset + 1]; i++) + for (int i = 0; i < subAuthoritiesLength; i++) { - unchecked - { - SubAuthorities[i] = - (int)( - (((uint)binaryForm[offset + 8 + 4 * i + 0]) << 0) + - (((uint)binaryForm[offset + 8 + 4 * i + 1]) << 8) + - (((uint)binaryForm[offset + 8 + 4 * i + 2]) << 16) + - (((uint)binaryForm[offset + 8 + 4 * i + 3]) << 24)); - } + subAuthorities[i] = + (int)( + (((uint)binaryForm[offset + 8 + 4 * i + 0]) << 0) + + (((uint)binaryForm[offset + 8 + 4 * i + 1]) << 8) + + (((uint)binaryForm[offset + 8 + 4 * i + 2]) << 16) + + (((uint)binaryForm[offset + 8 + 4 * i + 3]) << 24) + ); } - CreateFromParts(Authority, SubAuthorities); + CreateFromParts( + authority, +#if NETCOREAPP2_0 + subAuthorities +#else + subAuthorities.Slice(0, subAuthoritiesLength) +#endif + ); return; } @@ -513,8 +508,6 @@ private void CreateFromBinaryForm(byte[] binaryForm, int offset) public SecurityIdentifier(string sddlForm) { - byte[] resultSid; - // // Give us something to work with // @@ -528,20 +521,20 @@ public SecurityIdentifier(string sddlForm) // Call into the underlying O/S conversion routine // - int Error = Win32.CreateSidFromString(sddlForm, out resultSid); + int error = Win32.CreateSidFromString(sddlForm, out byte[] resultSid); - if (Error == Interop.Errors.ERROR_INVALID_SID) + if (error == Interop.Errors.ERROR_INVALID_SID) { throw new ArgumentException(SR.Argument_InvalidValue, nameof(sddlForm)); } - else if (Error == Interop.Errors.ERROR_NOT_ENOUGH_MEMORY) + else if (error == Interop.Errors.ERROR_NOT_ENOUGH_MEMORY) { throw new OutOfMemoryException(); } - else if (Error != Interop.Errors.ERROR_SUCCESS) + else if (error != Interop.Errors.ERROR_SUCCESS) { - Debug.Fail($"Win32.CreateSidFromString returned unrecognized error {Error}"); - throw new Win32Exception(Error); + Debug.Fail($"Win32.CreateSidFromString returned unrecognized error {error}"); + throw new Win32Exception(error); } CreateFromBinaryForm(resultSid, 0); @@ -553,6 +546,10 @@ public SecurityIdentifier(string sddlForm) public SecurityIdentifier(byte[] binaryForm, int offset) { + if (binaryForm is null) + { + throw new ArgumentNullException(nameof(binaryForm)); + } CreateFromBinaryForm(binaryForm, offset); } @@ -561,12 +558,6 @@ public SecurityIdentifier(byte[] binaryForm, int offset) // public SecurityIdentifier(IntPtr binaryForm) - : this(binaryForm, true) - { - } - - - internal SecurityIdentifier(IntPtr binaryForm, bool noDemand) : this(Win32.ConvertIntPtrSidToByteArraySid(binaryForm), 0) { } @@ -591,9 +582,6 @@ public SecurityIdentifier(WellKnownSidType sidType, SecurityIdentifier domainSid throw new ArgumentException(SR.IdentityReference_CannotCreateLogonIdsSid, nameof(sidType)); } - byte[] resultSid; - int Error; - // // sidType should not exceed the max defined value // @@ -606,7 +594,7 @@ public SecurityIdentifier(WellKnownSidType sidType, SecurityIdentifier domainSid // // for sidType between 38 to 50, the domainSid parameter must be specified // - + int error; if ((sidType >= WellKnownSidType.AccountAdministratorSid) && (sidType <= WellKnownSidType.AccountRasAndIasServersSid)) { if (domainSid == null) @@ -618,25 +606,21 @@ public SecurityIdentifier(WellKnownSidType sidType, SecurityIdentifier domainSid // verify that the domain sid is a valid windows domain sid // to do that we call GetAccountDomainSid and the return value should be the same as the domainSid // + error = Win32.GetWindowsAccountDomainSid(domainSid, out SecurityIdentifier resultDomainSid); - SecurityIdentifier resultDomainSid; - int ErrorCode; - - ErrorCode = Win32.GetWindowsAccountDomainSid(domainSid, out resultDomainSid); - - if (ErrorCode == Interop.Errors.ERROR_INSUFFICIENT_BUFFER) + if (error == Interop.Errors.ERROR_INSUFFICIENT_BUFFER) { throw new OutOfMemoryException(); } - else if (ErrorCode == Interop.Errors.ERROR_NON_ACCOUNT_SID) + else if (error == Interop.Errors.ERROR_NON_ACCOUNT_SID) { // this means that the domain sid is not valid throw new ArgumentException(SR.IdentityReference_NotAWindowsDomain, nameof(domainSid)); } - else if (ErrorCode != Interop.Errors.ERROR_SUCCESS) + else if (error != Interop.Errors.ERROR_SUCCESS) { - Debug.Fail($"Win32.GetWindowsAccountDomainSid returned unrecognized error {ErrorCode}"); - throw new Win32Exception(ErrorCode); + Debug.Fail($"Win32.GetWindowsAccountDomainSid returned unrecognized error {error}"); + throw new Win32Exception(error); } // @@ -650,45 +634,43 @@ public SecurityIdentifier(WellKnownSidType sidType, SecurityIdentifier domainSid } - Error = Win32.CreateWellKnownSid(sidType, domainSid, out resultSid); + error = Win32.CreateWellKnownSid(sidType, domainSid, out byte[] resultSid); - if (Error == Interop.Errors.ERROR_INVALID_PARAMETER) + if (error == Interop.Errors.ERROR_INVALID_PARAMETER) { - throw new ArgumentException(new Win32Exception(Error).Message, "sidType/domainSid"); + throw new ArgumentException(new Win32Exception(error).Message, "sidType/domainSid"); } - else if (Error != Interop.Errors.ERROR_SUCCESS) + else if (error != Interop.Errors.ERROR_SUCCESS) { - Debug.Fail($"Win32.CreateWellKnownSid returned unrecognized error {Error}"); - throw new Win32Exception(Error); + Debug.Fail($"Win32.CreateWellKnownSid returned unrecognized error {error}"); + throw new Win32Exception(error); } CreateFromBinaryForm(resultSid, 0); } +#if NETCOREAPP2_0 internal SecurityIdentifier(IdentifierAuthority identifierAuthority, int[] subAuthorities) +#else + internal SecurityIdentifier(IdentifierAuthority identifierAuthority, ReadOnlySpan subAuthorities) +#endif { CreateFromParts(identifierAuthority, subAuthorities); } - #endregion +#endregion - #region Static Properties +#region Static Properties // // Revision is always '1' // - internal static byte Revision - { - get - { - return 1; - } - } + internal static byte Revision => 1; - #endregion +#endregion - #region Non-static Properties +#region Non-static Properties // // This is for internal consumption only, hence it is marked 'internal' @@ -696,37 +678,13 @@ internal static byte Revision // prevent the caller from messing with the internal representation. // - internal byte[] BinaryForm - { - get - { - return _binaryForm; - } - } + internal byte[] BinaryForm => _binaryForm; - internal IdentifierAuthority IdentifierAuthority - { - get - { - return _identifierAuthority; - } - } + internal IdentifierAuthority IdentifierAuthority => _identifierAuthority; - internal int SubAuthorityCount - { - get - { - return _subAuthorities.Length; - } - } + internal int SubAuthorityCount => _subAuthorities.Length; - public int BinaryLength - { - get - { - return _binaryForm.Length; - } - } + public int BinaryLength => _binaryForm.Length; // // Returns the domain portion of a SID or null if the specified @@ -749,9 +707,9 @@ public SecurityIdentifier AccountDomainSid } } - #endregion +#endregion - #region Inherited properties and methods +#region Inherited properties and methods public override bool Equals(object o) { @@ -765,10 +723,10 @@ public bool Equals(SecurityIdentifier sid) public override int GetHashCode() { - int hashCode = ((long)this.IdentifierAuthority).GetHashCode(); + int hashCode = ((long)IdentifierAuthority).GetHashCode(); for (int i = 0; i < SubAuthorityCount; i++) { - hashCode ^= this.GetSubAuthority(i); + hashCode ^= GetSubAuthority(i); } return hashCode; } @@ -801,9 +759,8 @@ public override string ToString() result[1] = '-'; result[2] = '1'; result[3] = '-'; - int written; int length = 4; - ((ulong)_identifierAuthority).TryFormat(result.Slice(length), out written); + ((ulong)_identifierAuthority).TryFormat(result.Slice(length), out int written); length += written; int[] values = _subAuthorities; for (int index = 0; index < values.Length; index++) @@ -852,25 +809,22 @@ public override bool IsValidTargetType(Type targetType) internal SecurityIdentifier GetAccountDomainSid() { - SecurityIdentifier ResultSid; - int Error; - - Error = Win32.GetWindowsAccountDomainSid(this, out ResultSid); + int error = Win32.GetWindowsAccountDomainSid(this, out SecurityIdentifier resultSid); - if (Error == Interop.Errors.ERROR_INSUFFICIENT_BUFFER) + if (error == Interop.Errors.ERROR_INSUFFICIENT_BUFFER) { throw new OutOfMemoryException(); } - else if (Error == Interop.Errors.ERROR_NON_ACCOUNT_SID) + else if (error == Interop.Errors.ERROR_NON_ACCOUNT_SID) { - ResultSid = null; + resultSid = null; } - else if (Error != Interop.Errors.ERROR_SUCCESS) + else if (error != Interop.Errors.ERROR_SUCCESS) { - Debug.Fail($"Win32.GetWindowsAccountDomainSid returned unrecognized error {Error}"); - throw new Win32Exception(Error); + Debug.Fail($"Win32.GetWindowsAccountDomainSid returned unrecognized error {error}"); + throw new Win32Exception(error); } - return ResultSid; + return resultSid; } @@ -1189,17 +1143,13 @@ private static IdentityReferenceCollection TranslateToNTAccounts(IdentityReferen internal static IdentityReferenceCollection Translate(IdentityReferenceCollection sourceSids, Type targetType, bool forceSuccess) { - bool SomeFailed = false; - IdentityReferenceCollection Result; - - - Result = Translate(sourceSids, targetType, out SomeFailed); + IdentityReferenceCollection result = Translate(sourceSids, targetType, out bool someFailed); - if (forceSuccess && SomeFailed) + if (forceSuccess && someFailed) { IdentityReferenceCollection UnmappedIdentities = new IdentityReferenceCollection(); - foreach (IdentityReference id in Result) + foreach (IdentityReference id in result) { if (id.GetType() != targetType) { @@ -1210,7 +1160,7 @@ internal static IdentityReferenceCollection Translate(IdentityReferenceCollectio throw new IdentityNotMappedException(SR.IdentityReference_IdentityNotMapped, UnmappedIdentities); } - return Result; + return result; } diff --git a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/Win32.cs b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/Win32.cs index 49154ee7c35ff..2a2f51095ad4b 100644 --- a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/Win32.cs +++ b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/Win32.cs @@ -40,10 +40,9 @@ internal static SafeLsaPolicyHandle LsaOpenPolicy( string systemName, PolicyRights rights) { - SafeLsaPolicyHandle policyHandle; Interop.OBJECT_ATTRIBUTES attributes = default; - uint error = Interop.Advapi32.LsaOpenPolicy(systemName, ref attributes, (int)rights, out policyHandle); + uint error = Interop.Advapi32.LsaOpenPolicy(systemName, ref attributes, (int)rights, out SafeLsaPolicyHandle policyHandle); if (error == 0) { return policyHandle; @@ -172,7 +171,7 @@ out byte[] resultSid uint length = (uint)SecurityIdentifier.MaxBinaryLength; resultSid = new byte[length]; - if (FALSE != Interop.Advapi32.CreateWellKnownSid((int)sidType, domainSid == null ? null : domainSid.BinaryForm, resultSid, ref length)) + if (FALSE != Interop.Advapi32.CreateWellKnownSid((int)sidType, domainSid?.BinaryForm, resultSid, ref length)) { return Interop.Errors.ERROR_SUCCESS; } @@ -197,15 +196,13 @@ internal static bool IsEqualDomainSid(SecurityIdentifier sid1, SecurityIdentifie } else { - bool result; - byte[] BinaryForm1 = new byte[sid1.BinaryLength]; sid1.GetBinaryForm(BinaryForm1, 0); byte[] BinaryForm2 = new byte[sid2.BinaryLength]; sid2.GetBinaryForm(BinaryForm2, 0); - return (Interop.Advapi32.IsEqualDomainSid(BinaryForm1, BinaryForm2, out result) == FALSE ? false : result); + return (Interop.Advapi32.IsEqualDomainSid(BinaryForm1, BinaryForm2, out bool result) == FALSE ? false : result); } } diff --git a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs index a0dc34884f271..a0ea67355b000 100644 --- a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs +++ b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs @@ -35,6 +35,11 @@ public enum WindowsAccountType public class WindowsIdentity : ClaimsIdentity, IDisposable, ISerializable, IDeserializationCallback { + private static SecurityIdentifier s_authenticatedUserRid; + private static SecurityIdentifier s_domainRid; + private static SecurityIdentifier s_localSystemRid; + private static SecurityIdentifier s_anonymousRid; + private string _name = null; private SecurityIdentifier _owner = null; private SecurityIdentifier _user = null; @@ -48,8 +53,8 @@ public class WindowsIdentity : ClaimsIdentity, IDisposable, ISerializable, IDese public new const string DefaultIssuer = @"AD AUTHORITY"; private readonly string _issuerName = DefaultIssuer; - private readonly object _claimsIntiailizedLock = new object(); - private volatile bool _claimsInitialized; + private object _claimsIntiailizedLock; + private bool _claimsInitialized; private List _deviceClaims; private List _userClaims; @@ -177,12 +182,6 @@ public WindowsIdentity(string sUserPrincipalName) Marshal.Copy(sourceName, 0, sourceNameBuffer.DangerousGetHandle(), sourceName.Length); LSA_STRING lsaOriginName = new LSA_STRING(sourceNameBuffer.DangerousGetHandle(), sourceNameLength); - SafeLsaReturnBufferHandle profileBuffer; - int profileBufferLength; - LUID logonId; - SafeAccessTokenHandle accessTokenHandle; - QUOTA_LIMITS quota; - int subStatus; int ntStatus = Interop.SspiCli.LsaLogonUser( lsaHandle, ref lsaOriginName, @@ -192,12 +191,12 @@ public WindowsIdentity(string sUserPrincipalName) authenticationInfoLength, IntPtr.Zero, ref sourceContext, - out profileBuffer, - out profileBufferLength, - out logonId, - out accessTokenHandle, - out quota, - out subStatus); + out SafeLsaReturnBufferHandle profileBuffer, + out int profileBufferLength, + out LUID logonId, + out SafeAccessTokenHandle accessTokenHandle, + out QUOTA_LIMITS quota, + out int subStatus); if (ntStatus == unchecked((int)Interop.StatusOptions.STATUS_ACCOUNT_RESTRICTION) && subStatus < 0) ntStatus = subStatus; @@ -218,8 +217,7 @@ public WindowsIdentity(string sUserPrincipalName) private static SafeLsaHandle ConnectToLsa() { - SafeLsaHandle lsaHandle; - int ntStatus = Interop.SspiCli.LsaConnectUntrusted(out lsaHandle); + int ntStatus = Interop.SspiCli.LsaConnectUntrusted(out SafeLsaHandle lsaHandle); if (ntStatus < 0) // non-negative numbers indicate success throw GetExceptionFromNtStatus(ntStatus); return lsaHandle; @@ -358,7 +356,7 @@ public static WindowsIdentity GetAnonymous() // Properties. // // this is defined 'override sealed' for back compat. Il generated is 'virtual final' and this needs to be the same. - public override sealed string AuthenticationType + public sealed override string AuthenticationType { get { @@ -437,14 +435,21 @@ public override bool IsAuthenticated { if (_isAuthenticated == -1) { + if (s_authenticatedUserRid is null) + { + s_authenticatedUserRid = new SecurityIdentifier( + IdentifierAuthority.NTAuthority, + new int[] { Interop.SecurityIdentifier.SECURITY_AUTHENTICATED_USER_RID } + ); + } // This approach will not work correctly for domain guests (will return false // instead of true). This is a corner-case that is not very interesting. - _isAuthenticated = CheckNtTokenForSid(new SecurityIdentifier(IdentifierAuthority.NTAuthority, - new int[] { Interop.SecurityIdentifier.SECURITY_AUTHENTICATED_USER_RID })) ? 1 : 0; + _isAuthenticated = CheckNtTokenForSid(s_authenticatedUserRid) ? 1 : 0; } return _isAuthenticated == 1; } } + private bool CheckNtTokenForSid(SecurityIdentifier sid) { // special case the anonymous identity. @@ -503,8 +508,15 @@ public virtual bool IsGuest if (_safeTokenHandle.IsInvalid) return false; - return CheckNtTokenForSid(new SecurityIdentifier(IdentifierAuthority.NTAuthority, - new int[] { Interop.SecurityIdentifier.SECURITY_BUILTIN_DOMAIN_RID, (int)WindowsBuiltInRole.Guest })); + if (s_domainRid is null) + { + s_domainRid = new SecurityIdentifier( + IdentifierAuthority.NTAuthority, + new int[] { Interop.SecurityIdentifier.SECURITY_BUILTIN_DOMAIN_RID, (int)WindowsBuiltInRole.Guest } + ); + } + + return CheckNtTokenForSid(s_domainRid); } } @@ -515,9 +527,16 @@ public virtual bool IsSystem // special case the anonymous identity. if (_safeTokenHandle.IsInvalid) return false; - SecurityIdentifier sid = new SecurityIdentifier(IdentifierAuthority.NTAuthority, - new int[] { Interop.SecurityIdentifier.SECURITY_LOCAL_SYSTEM_RID }); - return (this.User == sid); + + if (s_localSystemRid is null) + { + s_localSystemRid = new SecurityIdentifier( + IdentifierAuthority.NTAuthority, + new int[] { Interop.SecurityIdentifier.SECURITY_LOCAL_SYSTEM_RID } + ); + } + + return User == s_localSystemRid; } } @@ -528,9 +547,16 @@ public virtual bool IsAnonymous // special case the anonymous identity. if (_safeTokenHandle.IsInvalid) return true; - SecurityIdentifier sid = new SecurityIdentifier(IdentifierAuthority.NTAuthority, - new int[] { Interop.SecurityIdentifier.SECURITY_ANONYMOUS_LOGON_RID }); - return (this.User == sid); + + if (s_anonymousRid is null) + { + s_anonymousRid = new SecurityIdentifier( + IdentifierAuthority.NTAuthority, + new int[] { Interop.SecurityIdentifier.SECURITY_ANONYMOUS_LOGON_RID } + ); + } + + return User == s_anonymousRid; } } @@ -553,7 +579,7 @@ internal string GetName() // revert thread impersonation for the duration of the call to get the name. RunImpersonated(SafeAccessTokenHandle.InvalidHandle, delegate { - NTAccount ntAccount = this.User.Translate(typeof(NTAccount)) as NTAccount; + NTAccount ntAccount = User.Translate(typeof(NTAccount)) as NTAccount; _name = ntAccount.ToString(); }); } @@ -573,7 +599,7 @@ public SecurityIdentifier Owner { using (SafeLocalAllocHandle tokenOwner = GetTokenInformation(_safeTokenHandle, TokenInformationClass.TokenOwner)) { - _owner = new SecurityIdentifier(tokenOwner.Read(0), true); + _owner = new SecurityIdentifier(tokenOwner.Read(0)); } } @@ -593,7 +619,7 @@ public SecurityIdentifier User { using (SafeLocalAllocHandle tokenUser = GetTokenInformation(_safeTokenHandle, TokenInformationClass.TokenUser)) { - _user = new SecurityIdentifier(tokenUser.Read(0), true); + _user = new SecurityIdentifier(tokenUser.Read(0)); } } @@ -627,7 +653,7 @@ public IdentityReferenceCollection Groups uint mask = Interop.SecurityGroups.SE_GROUP_ENABLED | Interop.SecurityGroups.SE_GROUP_LOGON_ID | Interop.SecurityGroups.SE_GROUP_USE_FOR_DENY_ONLY; if ((group.Attributes & mask) == Interop.SecurityGroups.SE_GROUP_ENABLED) { - groups.Add(new SecurityIdentifier(group.Sid, true)); + groups.Add(new SecurityIdentifier(group.Sid)); } } } @@ -672,7 +698,7 @@ public static T RunImpersonated(SafeAccessTokenHandle safeAccessTokenHandle, if (func == null) throw new ArgumentNullException(nameof(func)); - T result = default(T); + T result = default; RunImpersonatedInternal(safeAccessTokenHandle, () => result = func()); return result; } @@ -705,9 +731,7 @@ private static void RunImpersonatedInternal(SafeAccessTokenHandle token, Action { token = DuplicateAccessToken(token); - bool isImpersonating; - int hr; - SafeAccessTokenHandle previousToken = GetCurrentToken(TokenAccessLevels.MaximumAllowed, false, out isImpersonating, out hr); + SafeAccessTokenHandle previousToken = GetCurrentToken(TokenAccessLevels.MaximumAllowed, false, out bool isImpersonating, out int hr); if (previousToken == null || previousToken.IsInvalid) throw new SecurityException(new Win32Exception(hr).Message); @@ -753,9 +777,7 @@ private static void CurrentImpersonatedTokenChanged(AsyncLocalValueChangedArgs Claims /// private void InitializeClaims() { - if (!_claimsInitialized) - { - lock (_claimsIntiailizedLock) + bool discard = false; + + LazyInitializer.EnsureInitialized( + ref discard, + ref _claimsInitialized, + ref _claimsIntiailizedLock, + () => { - if (!_claimsInitialized) + _userClaims = new List(); + _deviceClaims = new List(); + + if (!string.IsNullOrEmpty(Name)) { - _userClaims = new List(); - _deviceClaims = new List(); + // + // Add the name claim only if the WindowsIdentity.Name is populated + // WindowsIdentity.Name will be null when it is the fake anonymous user + // with a token value of IntPtr.Zero + // + _userClaims.Add(new Claim(NameClaimType, Name, ClaimValueTypes.String, _issuerName, _issuerName, this)); + } - if (!string.IsNullOrEmpty(Name)) - { - // - // Add the name claim only if the WindowsIdentity.Name is populated - // WindowsIdentity.Name will be null when it is the fake anonymous user - // with a token value of IntPtr.Zero - // - _userClaims.Add(new Claim(NameClaimType, Name, ClaimValueTypes.String, _issuerName, _issuerName, this)); - } + // primary sid + AddPrimarySidClaim(_userClaims); - // primary sid - AddPrimarySidClaim(_userClaims); + // group sids + AddGroupSidClaims(_userClaims); - // group sids - AddGroupSidClaims(_userClaims); + if (!s_ignoreWindows8Properties) + { + // Device group sids (may cause s_ignoreWindows8Properties to be set to true, so must be first in this block) + AddDeviceGroupSidClaims(_deviceClaims, TokenInformationClass.TokenDeviceGroups); if (!s_ignoreWindows8Properties) { - // Device group sids (may cause s_ignoreWindows8Properties to be set to true, so must be first in this block) - AddDeviceGroupSidClaims(_deviceClaims, TokenInformationClass.TokenDeviceGroups); + // User token claims + AddTokenClaims(_userClaims, TokenInformationClass.TokenUserClaimAttributes, ClaimTypes.WindowsUserClaim); - if (!s_ignoreWindows8Properties) - { - // User token claims - AddTokenClaims(_userClaims, TokenInformationClass.TokenUserClaimAttributes, ClaimTypes.WindowsUserClaim); - - // Device token claims - AddTokenClaims(_deviceClaims, TokenInformationClass.TokenDeviceClaimAttributes, ClaimTypes.WindowsDeviceClaim); - } + // Device token claims + AddTokenClaims(_deviceClaims, TokenInformationClass.TokenDeviceClaimAttributes, ClaimTypes.WindowsDeviceClaim); } - - _claimsInitialized = true; } + return true; } - } + ); } /// @@ -1029,7 +1049,7 @@ private void AddGroupSidClaims(List instanceClaims) // Retrieve the primary group sid safeAllocHandlePrimaryGroup = GetTokenInformation(_safeTokenHandle, TokenInformationClass.TokenPrimaryGroup); Interop.TOKEN_PRIMARY_GROUP primaryGroup = (Interop.TOKEN_PRIMARY_GROUP)Marshal.PtrToStructure(safeAllocHandlePrimaryGroup.DangerousGetHandle()); - SecurityIdentifier primaryGroupSid = new SecurityIdentifier(primaryGroup.PrimaryGroup, true); + SecurityIdentifier primaryGroupSid = new SecurityIdentifier(primaryGroup.PrimaryGroup); // only add one primary group sid bool foundPrimaryGroupSid = false; @@ -1043,7 +1063,7 @@ private void AddGroupSidClaims(List instanceClaims) { Interop.SID_AND_ATTRIBUTES group = (Interop.SID_AND_ATTRIBUTES)Marshal.PtrToStructure(pSidAndAttributes); uint mask = Interop.SecurityGroups.SE_GROUP_ENABLED | Interop.SecurityGroups.SE_GROUP_LOGON_ID | Interop.SecurityGroups.SE_GROUP_USE_FOR_DENY_ONLY; - SecurityIdentifier groupSid = new SecurityIdentifier(group.Sid, true); + SecurityIdentifier groupSid = new SecurityIdentifier(group.Sid); if ((group.Attributes & mask) == Interop.SecurityGroups.SE_GROUP_ENABLED) { @@ -1099,7 +1119,7 @@ private void AddPrimarySidClaim(List instanceClaims) Interop.SID_AND_ATTRIBUTES user = (Interop.SID_AND_ATTRIBUTES)Marshal.PtrToStructure(safeAllocHandle.DangerousGetHandle()); uint mask = Interop.SecurityGroups.SE_GROUP_USE_FOR_DENY_ONLY; - SecurityIdentifier sid = new SecurityIdentifier(user.Sid, true); + SecurityIdentifier sid = new SecurityIdentifier(user.Sid); Claim claim; if (user.Attributes == 0) { @@ -1141,13 +1161,12 @@ private void AddDeviceGroupSidClaims(List instanceClaims, TokenInformatio int count = Marshal.ReadInt32(safeAllocHandle.DangerousGetHandle()); IntPtr pSidAndAttributes = new IntPtr((long)safeAllocHandle.DangerousGetHandle() + (long)Marshal.OffsetOf(typeof(Interop.TOKEN_GROUPS), "Groups")); - string claimType = null; - for (int i = 0; i < count; ++i) { Interop.SID_AND_ATTRIBUTES group = (Interop.SID_AND_ATTRIBUTES)Marshal.PtrToStructure(pSidAndAttributes, typeof(Interop.SID_AND_ATTRIBUTES)); uint mask = Interop.SecurityGroups.SE_GROUP_ENABLED | Interop.SecurityGroups.SE_GROUP_LOGON_ID | Interop.SecurityGroups.SE_GROUP_USE_FOR_DENY_ONLY; - SecurityIdentifier groupSid = new SecurityIdentifier(group.Sid, true); + SecurityIdentifier groupSid = new SecurityIdentifier(group.Sid); + string claimType; if ((group.Attributes & mask) == Interop.SecurityGroups.SE_GROUP_ENABLED) { claimType = ClaimTypes.WindowsDeviceGroup; diff --git a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsPrincipal.cs b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsPrincipal.cs index 6c02e523ffe1c..6741b6f9f9232 100644 --- a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsPrincipal.cs +++ b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsPrincipal.cs @@ -35,28 +35,18 @@ private WindowsPrincipal() { } public WindowsPrincipal(WindowsIdentity ntIdentity) : base(ntIdentity) { - if (ntIdentity == null) - throw new ArgumentNullException(nameof(ntIdentity)); - - _identity = ntIdentity; + _identity = ntIdentity ?? throw new ArgumentNullException(nameof(ntIdentity)); } // // Properties. // - public override IIdentity Identity - { - get - { - return _identity; - } - } + public override IIdentity Identity => _identity; // // Public methods. // - public override bool IsInRole(string role) { if (role == null || role.Length == 0) @@ -91,8 +81,7 @@ public virtual IEnumerable UserClaims { foreach (ClaimsIdentity identity in Identities) { - WindowsIdentity wi = identity as WindowsIdentity; - if (wi != null) + if (identity is WindowsIdentity wi) { foreach (Claim claim in wi.UserClaims) { @@ -113,8 +102,7 @@ public virtual IEnumerable DeviceClaims { foreach (ClaimsIdentity identity in Identities) { - WindowsIdentity wi = identity as WindowsIdentity; - if (wi != null) + if (identity is WindowsIdentity wi) { foreach (Claim claim in wi.DeviceClaims) { @@ -135,10 +123,17 @@ public virtual bool IsInRole(WindowsBuiltInRole role) public virtual bool IsInRole(int rid) { - SecurityIdentifier sid = new SecurityIdentifier(IdentifierAuthority.NTAuthority, - new int[] { Interop.SecurityIdentifier.SECURITY_BUILTIN_DOMAIN_RID, rid }); - - return IsInRole(sid); + return IsInRole( + new SecurityIdentifier( + IdentifierAuthority.NTAuthority, +#if NETCOREAPP2_0 + new +#else + stackalloc +#endif + int[] { Interop.SecurityIdentifier.SECURITY_BUILTIN_DOMAIN_RID, rid } + ) + ); } // This method (with a SID parameter) is more general than the 2 overloads that accept a WindowsBuiltInRole or diff --git a/src/libraries/System.Security.Principal.Windows/tests/SecurityIdentifierTests.cs b/src/libraries/System.Security.Principal.Windows/tests/SecurityIdentifierTests.cs index 8fcf5111e0fb8..ea66ce39c4d95 100644 --- a/src/libraries/System.Security.Principal.Windows/tests/SecurityIdentifierTests.cs +++ b/src/libraries/System.Security.Principal.Windows/tests/SecurityIdentifierTests.cs @@ -9,6 +9,43 @@ public class SecurityIdentifierTests { + + [Fact] + public void SecurityIdentifierBinaryCtorThrowsOnNull() + { + AssertExtensions.Throws("binaryForm", () => new SecurityIdentifier(null, 0)); + } + + [Fact] + public void SecurityIdentifierBinaryCtorThrowsOnEmpty() + { + AssertExtensions.Throws("binaryForm", () => new SecurityIdentifier(Array.Empty(), 0)); + } + + [Fact] + public void SecurityIdentifierBinaryCtorThrowsOnNegativeOffset() + { + AssertExtensions.Throws("offset", () => new SecurityIdentifier(new byte[] { 1, 1, 0, 0, 0, 0, 0, 0 }, -1)); + } + + [Fact] + public void SecurityIdentifierBinaryCtorThrowsOnExcessiveOffset() + { + AssertExtensions.Throws("binaryForm", () => new SecurityIdentifier(new byte[] { 1, 1, 0, 0, 0, 0, 0, 0 }, 9)); + } + + [Fact] + public void SecurityIdentifierBinaryCtorThrowsOnInvalidRevision() + { + AssertExtensions.Throws("binaryForm", () => new SecurityIdentifier(new byte[] { 2, 1, 0, 0, 0, 0, 0, 0 }, 0)); + } + + [Fact] + public void SecurityIdentifierBinaryCtorThrowsOnTooManySubAuthorities() + { + AssertExtensions.Throws("binaryForm", () => new SecurityIdentifier(new byte[] { 1, 100, 0, 0, 0, 0, 0, 0 }, 0)); + } + [Fact] public void ValidateGetCurrentUser() {