From a55b7122faf8ea5f9fb9eabd9a4aa7834b02aa7b Mon Sep 17 00:00:00 2001 From: Matt Connew Date: Wed, 21 Sep 2016 15:41:19 -0700 Subject: [PATCH] Original commit messages in master were: Fix UpnEndpointIdentity on UWP Re-enable tests disabled by issue 10, except for NET Native. PR #1502 fixed issue #10 by implementing UpnEndpointIdentity. But 2 tests were still disabled by issue 10, so this PR just re-enables those tests. Both require manual setup to run, so we will not see them pass or fail in CI or VSO. These tests still fail in UWP and so remain disabled there. --- .../src/System/IdentityModel/Claims/Claim.cs | 2 - .../IdentityModel/Claims/ClaimComparer.cs | 39 ++++-- .../Diagnostics/SecurityTraceRecordHelper.cs | 4 + .../ServiceModel/UpnEndpointIdentity.cs | 126 ++++++++++++++++-- .../NegotiateStream_Http_Tests.4.1.0.cs | 4 +- .../ServiceModel/UpnEndpointIdentityTest.cs | 1 - 6 files changed, 154 insertions(+), 22 deletions(-) diff --git a/src/System.Private.ServiceModel/src/System/IdentityModel/Claims/Claim.cs b/src/System.Private.ServiceModel/src/System/IdentityModel/Claims/Claim.cs index 29080ae10582..aaa029c40392 100755 --- a/src/System.Private.ServiceModel/src/System/IdentityModel/Claims/Claim.cs +++ b/src/System.Private.ServiceModel/src/System/IdentityModel/Claims/Claim.cs @@ -146,7 +146,6 @@ public static Claim CreateThumbprintClaim(byte[] thumbprint) return new Claim(ClaimTypes.Thumbprint, SecurityUtils.CloneBuffer(thumbprint), Rights.PossessProperty, ClaimComparer.Thumbprint); } -#if SUPPORTS_WINDOWSIDENTITY public static Claim CreateUpnClaim(string upn) { if (upn == null) @@ -154,7 +153,6 @@ public static Claim CreateUpnClaim(string upn) return new Claim(ClaimTypes.Upn, upn, Rights.PossessProperty, ClaimComparer.Upn); } -#endif // SUPPORTS_WINDOWSIDENTITY public static Claim CreateUriClaim(Uri uri) { diff --git a/src/System.Private.ServiceModel/src/System/IdentityModel/Claims/ClaimComparer.cs b/src/System.Private.ServiceModel/src/System/IdentityModel/Claims/ClaimComparer.cs index cce24683a379..d1ed44e7e573 100755 --- a/src/System.Private.ServiceModel/src/System/IdentityModel/Claims/ClaimComparer.cs +++ b/src/System.Private.ServiceModel/src/System/IdentityModel/Claims/ClaimComparer.cs @@ -21,9 +21,7 @@ internal class ClaimComparer : IEqualityComparer private static IEqualityComparer s_dnsComparer; private static IEqualityComparer s_rsaComparer; private static IEqualityComparer s_thumbprintComparer; -#if SUPPORTS_WINDOWSIDENTITY private static IEqualityComparer s_upnComparer; -#endif // SUPPORTS_WINDOWSIDENTITY private static IEqualityComparer s_x500DistinguishedNameComparer; private IEqualityComparer _resourceComparer; @@ -113,7 +111,6 @@ public static IEqualityComparer Thumbprint } } -#if SUPPORTS_WINDOWSIDENTITY public static IEqualityComparer Upn { get @@ -125,7 +122,6 @@ public static IEqualityComparer Upn return s_upnComparer; } } -#endif // SUPPORTS_WINDOWSIDENTITY public static IEqualityComparer X500DistinguishedName { @@ -160,7 +156,7 @@ public int GetHashCode(Claim claim) throw DiagnosticUtility.ExceptionUtility.ThrowHelperArgumentNull("claim"); return claim.ClaimType.GetHashCode() ^ claim.Right.GetHashCode() - ^ ((claim.Resource == null) ? 0 : _resourceComparer.GetHashCode(claim.Resource)); + ^ ((claim.Resource == null) ? 0 : _resourceComparer.GetHashCode(claim.Resource)); } private class ObjectComparer : IEqualityComparer @@ -240,6 +236,7 @@ int IEqualityComparer.GetHashCode(object obj) private class X500DistinguishedNameObjectComparer : IEqualityComparer { private IEqualityComparer _binaryComparer; + public X500DistinguishedNameObjectComparer() { _binaryComparer = new BinaryObjectComparer(); @@ -258,12 +255,35 @@ int IEqualityComparer.GetHashCode(object obj) } } -#if SUPPORTS_WINDOWSIDENTITY private class UpnObjectComparer : IEqualityComparer { bool IEqualityComparer.Equals(object obj1, object obj2) { - throw ExceptionHelper.PlatformNotSupported(); + if (StringComparer.OrdinalIgnoreCase.Equals(obj1 as string, obj2 as string)) + return true; + + string upn1 = obj1 as string; + string upn2 = obj2 as string; + if (upn1 == null || upn2 == null) + return false; + +#if SUPPORTS_WINDOWSIDENTITY + SecurityIdentifier sid1; + if (!TryLookupSidFromName(upn1, out sid1)) + return false; + + // Normalize to sid + SecurityIdentifier sid2; + if (!TryLookupSidFromName(upn2, out sid2)) + return false; + + return sid1 == sid2; +#else + // If WindowsIdentity isn't supported, then we can't + // retrieve the SecurityIdentifier's to compare so + // must return false + return false; +#endif // SUPPORTS_WINDOWSIDENTITY } int IEqualityComparer.GetHashCode(object obj) @@ -272,14 +292,17 @@ int IEqualityComparer.GetHashCode(object obj) if (upn == null) return 0; +#if SUPPORTS_WINDOWSIDENTITY // Normalize to sid SecurityIdentifier sid; if (TryLookupSidFromName(upn, out sid)) return sid.GetHashCode(); +#endif // SUPPORTS_WINDOWSIDENTITY return StringComparer.OrdinalIgnoreCase.GetHashCode(upn); } +#if SUPPORTS_WINDOWSIDENTITY private bool TryLookupSidFromName(string upn, out SecurityIdentifier sid) { sid = null; @@ -293,7 +316,7 @@ private bool TryLookupSidFromName(string upn, out SecurityIdentifier sid) } return sid != null; } +#endif // SUPPORTS_WINDOWSIDENTITY } -#endif // SUPPORTS_WINDOWSIDENTITY } } diff --git a/src/System.Private.ServiceModel/src/System/ServiceModel/Diagnostics/SecurityTraceRecordHelper.cs b/src/System.Private.ServiceModel/src/System/ServiceModel/Diagnostics/SecurityTraceRecordHelper.cs index ba4ec82f7b30..407d0ab51c31 100644 --- a/src/System.Private.ServiceModel/src/System/ServiceModel/Diagnostics/SecurityTraceRecordHelper.cs +++ b/src/System.Private.ServiceModel/src/System/ServiceModel/Diagnostics/SecurityTraceRecordHelper.cs @@ -40,5 +40,9 @@ internal static void TraceIdentityDeterminationSuccess(EndpointAddress epr, Endp internal static void TraceIdentityDeterminationFailure(EndpointAddress epr, Type identityVerifier) { } + + internal static void TraceSpnToSidMappingFailure(string spn, Exception e) + { + } } } diff --git a/src/System.Private.ServiceModel/src/System/ServiceModel/UpnEndpointIdentity.cs b/src/System.Private.ServiceModel/src/System/ServiceModel/UpnEndpointIdentity.cs index 7cee6dbf2fca..f756ac015f56 100755 --- a/src/System.Private.ServiceModel/src/System/ServiceModel/UpnEndpointIdentity.cs +++ b/src/System.Private.ServiceModel/src/System/ServiceModel/UpnEndpointIdentity.cs @@ -3,8 +3,14 @@ // See the LICENSE file in the project root for more information. +using System.ComponentModel; using System.Security.Principal; using System.IdentityModel.Claims; +using System.Runtime; +using System.Runtime.InteropServices; +using System.ServiceModel.Diagnostics; +using System.Text; +using System.Xml; namespace System.ServiceModel { @@ -23,24 +29,22 @@ public class UpnEndpointIdentity : EndpointIdentity public UpnEndpointIdentity(string upnName) { if (upnName == null) - throw DiagnosticUtility.ExceptionUtility.ThrowHelperArgumentNull("upnName"); -#if SUPPORTS_WINDOWSIDENTITY - base.Initialize(Claim.CreateUpnClaim(upnName)); -#else - throw ExceptionHelper.PlatformNotSupported("UpnEndpointIdentity is not supported on this platform"); -#endif // SUPPORTS_WINDOWSIDENTITY + { + throw DiagnosticUtility.ExceptionUtility.ThrowHelperArgumentNull(nameof(upnName)); + } + + Initialize(Claim.CreateUpnClaim(upnName)); } public UpnEndpointIdentity(Claim identity) { if (identity == null) - throw DiagnosticUtility.ExceptionUtility.ThrowHelperArgumentNull("identity"); + throw DiagnosticUtility.ExceptionUtility.ThrowHelperArgumentNull(nameof(identity)); - // PreSharp Bug: Parameter 'identity.ResourceType' to this public method must be validated: A null-dereference can occur here. if (!identity.ClaimType.Equals(ClaimTypes.Upn)) throw DiagnosticUtility.ExceptionUtility.ThrowHelperArgument(SR.Format(SR.UnrecognizedClaimTypeForIdentity, identity.ClaimType, ClaimTypes.Upn)); - base.Initialize(identity); + Initialize(identity); } #if SUPPORTS_WINDOWSIDENTITY @@ -53,6 +57,110 @@ internal UpnEndpointIdentity(WindowsIdentity windowsIdentity) _upnSid = windowsIdentity.User; _hasUpnSidBeenComputed = true; } + + internal override void EnsureIdentityClaim() + { + if (_windowsIdentity != null) + { + lock (_thisLock) + { + if (_windowsIdentity != null) + { + Initialize(Claim.CreateUpnClaim(GetUpnFromWindowsIdentity(_windowsIdentity))); + _windowsIdentity.Dispose(); + _windowsIdentity = null; + } + } + } + } + + string GetUpnFromWindowsIdentity(WindowsIdentity windowsIdentity) + { + string downlevelName = null; + string upnName = null; + + try + { + downlevelName = windowsIdentity.Name; + + if (IsMachineJoinedToDomain()) + { + upnName = GetUpnFromDownlevelName(downlevelName); + } + } + catch (Exception e) + { + if (Fx.IsFatal(e)) + { + throw; + } + } + + // if the AD cannot be queried for the fully qualified domain name, + // fall back to the downlevel UPN name + return upnName ?? downlevelName; + } + + bool IsMachineJoinedToDomain() + { + throw ExceptionHelper.PlatformNotSupported(); + } + + string GetUpnFromDownlevelName(string downlevelName) + { + throw ExceptionHelper.PlatformNotSupported(); + } #endif // SUPPORTS_WINDOWSIDENTITY + + internal override void WriteContentsTo(XmlDictionaryWriter writer) + { + if (writer == null) + throw DiagnosticUtility.ExceptionUtility.ThrowHelperArgumentNull(nameof(writer)); + + writer.WriteElementString(XD.AddressingDictionary.Upn, XD.AddressingDictionary.IdentityExtensionNamespace, (string)this.IdentityClaim.Resource); + } + +#if SUPPORTS_WINDOWSIDENTITY + internal SecurityIdentifier GetUpnSid() + { + Fx.Assert(ClaimTypes.Upn.Equals(this.IdentityClaim.ClaimType), ""); + if (!_hasUpnSidBeenComputed) + { + lock (_thisLock) + { + string upn = (string)this.IdentityClaim.Resource; + if (!_hasUpnSidBeenComputed) + { + try + { + NTAccount userAccount = new NTAccount(upn); + _upnSid = userAccount.Translate(typeof(SecurityIdentifier)) as SecurityIdentifier; + } +#pragma warning suppress 56500 // covered by FxCOP + catch (Exception e) + { + // Always immediately rethrow fatal exceptions. + if (Fx.IsFatal(e)) + { + throw; + } + + if (e is NullReferenceException) + { + throw; + } + + SecurityTraceRecordHelper.TraceSpnToSidMappingFailure(upn, e); + } + finally + { + _hasUpnSidBeenComputed = true; + } + } + } + } + return _upnSid; + } +#endif } } diff --git a/src/System.Private.ServiceModel/tests/Scenarios/Security/TransportSecurity/Negotiate/NegotiateStream_Http_Tests.4.1.0.cs b/src/System.Private.ServiceModel/tests/Scenarios/Security/TransportSecurity/Negotiate/NegotiateStream_Http_Tests.4.1.0.cs index 5def03b2eef5..718fddfb7298 100644 --- a/src/System.Private.ServiceModel/tests/Scenarios/Security/TransportSecurity/Negotiate/NegotiateStream_Http_Tests.4.1.0.cs +++ b/src/System.Private.ServiceModel/tests/Scenarios/Security/TransportSecurity/Negotiate/NegotiateStream_Http_Tests.4.1.0.cs @@ -260,7 +260,7 @@ public static void NegotiateStream_Http_With_ExplicitSpn() [Condition(nameof(Windows_Authentication_Available), nameof(Root_Certificate_Installed), nameof(UPN_Available))] - [Issue(10)] + [Issue(10, Framework = FrameworkID.NetNative)] [OuterLoop] public static void NegotiateStream_Http_With_Upn() { @@ -406,7 +406,7 @@ public static void NegotiateStream_Http_With_ExplicitUserNameAndPassword_With_Sp nameof(Explicit_Credentials_Available), nameof(Domain_Available), nameof(UPN_Available))] - [Issue(10)] + [Issue(10, Framework = FrameworkID.NetNative)] [OuterLoop] public static void NegotiateStream_Http_With_ExplicitUserNameAndPassword_With_Upn() { diff --git a/src/System.ServiceModel.Security/tests/ServiceModel/UpnEndpointIdentityTest.cs b/src/System.ServiceModel.Security/tests/ServiceModel/UpnEndpointIdentityTest.cs index c1ae2231b90d..b182d3386a40 100644 --- a/src/System.ServiceModel.Security/tests/ServiceModel/UpnEndpointIdentityTest.cs +++ b/src/System.ServiceModel.Security/tests/ServiceModel/UpnEndpointIdentityTest.cs @@ -16,7 +16,6 @@ public static class UpnEndpointIdentityTest [WcfTheory] [InlineData("")] [InlineData("test@wcf.example.com")] - [Issue(1454, Framework = FrameworkID.NetNative)] public static void Ctor_UpnName(string upn) { UpnEndpointIdentity upnEndpointEntity = new UpnEndpointIdentity(upn);