From 1792e2c1d677c5a70b2a3efe5738b16257843211 Mon Sep 17 00:00:00 2001 From: Lex Li Date: Sat, 29 Jul 2017 21:56:06 -0400 Subject: [PATCH] Fixed a few SNI related issues for IIS 7 Express. --- JexusManager.Shared/BindingUtility.cs | 189 +++++++++--------- JexusManager/Dialogs/NewSiteDialog.cs | 7 +- JexusManager/Features/Main/BindingDialog.cs | 49 +++-- JexusManager/Features/Main/BindingsDialog.cs | 4 +- JexusManager/Features/Main/SslDiagDialog.cs | 6 +- Microsoft.Web.Administration/Binding.cs | 22 +- .../BindingCollection.cs | 4 +- Microsoft.Web.Administration/Helper.cs | 24 +++ .../IFeatureDetection.cs | 7 + .../IisExpressServerManager.cs | 15 +- .../IisServerManager.cs | 2 + Microsoft.Web.Administration/JexusHelper.cs | 2 + .../Microsoft.Web.Administration.csproj | 1 + Microsoft.Web.Administration/ServerManager.cs | 5 +- Tests/TestCases.cs | 4 + 15 files changed, 207 insertions(+), 134 deletions(-) create mode 100644 Microsoft.Web.Administration/IFeatureDetection.cs diff --git a/JexusManager.Shared/BindingUtility.cs b/JexusManager.Shared/BindingUtility.cs index 51986118..d4c57d18 100644 --- a/JexusManager.Shared/BindingUtility.cs +++ b/JexusManager.Shared/BindingUtility.cs @@ -25,16 +25,20 @@ public static class BindingUtility internal static void Reinitialize(this Binding original, Binding binding) { - if (original.GetIsSni() && (!binding.GetIsSni() || original.Host != binding.Host)) + if (original.Parent.Parent.Server.SupportsSni) { - original.CleanUpSni(); + if (original.GetIsSni() && (!binding.GetIsSni() || original.Host != binding.Host)) + { + original.CleanUpSni(); + } + + original.SslFlags = binding.SslFlags; } original.BindingInformation = binding.BindingInformation; original.Protocol = binding.Protocol; original.CertificateHash = binding.CertificateHash; original.CertificateStoreName = binding.CertificateStoreName; - original.SslFlags = binding.SslFlags; binding.Delete(); } @@ -45,114 +49,119 @@ internal static string FixCertificateMapping(this Binding binding, X509Certifica return string.Empty; } - if (binding.GetIsSni()) + if (binding.Parent.Parent.Server.SupportsSni) { - if (certificate2.GetNameInfo(X509NameType.DnsName, false) != binding.Host) + if (binding.GetIsSni()) { - return "SNI mode requires host name matches common name of the certificate"; - } + if (certificate2.GetNameInfo(X509NameType.DnsName, false) != binding.Host) + { + return "SNI mode requires host name matches common name of the certificate"; + } - // handle SNI - var sni = NativeMethods.QuerySslSniInfo(new Tuple(binding.Host, binding.EndPoint.Port)); - if (sni == null) - { - try + // handle SNI + var sni = NativeMethods.QuerySslSniInfo(new Tuple(binding.Host, + binding.EndPoint.Port)); + if (sni == null) { - // register mapping - using (var process = new Process()) + try { - var start = process.StartInfo; - start.Verb = "runas"; - start.FileName = "cmd"; - start.Arguments = string.Format( - "/c \"\"{2}\" /h:\"{0}\" /s:{1}\" /i:{3} /a:{4} /o:{5} /x:{6}", - Hex.ToHexString(binding.CertificateHash), - binding.CertificateStoreName, - Path.Combine(Environment.CurrentDirectory, "certificateinstaller.exe"), - AppIdIisExpress, - binding.EndPoint.Address, - binding.EndPoint.Port, - binding.Host); - start.CreateNoWindow = true; - start.WindowStyle = ProcessWindowStyle.Hidden; - process.Start(); - process.WaitForExit(); - - if (process.ExitCode != 0) + // register mapping + using (var process = new Process()) { - return "Register new certificate failed: access is denied"; + var start = process.StartInfo; + start.Verb = "runas"; + start.FileName = "cmd"; + start.Arguments = string.Format( + "/c \"\"{2}\" /h:\"{0}\" /s:{1}\" /i:{3} /a:{4} /o:{5} /x:{6}", + Hex.ToHexString(binding.CertificateHash), + binding.CertificateStoreName, + Path.Combine(Environment.CurrentDirectory, "certificateinstaller.exe"), + AppIdIisExpress, + binding.EndPoint.Address, + binding.EndPoint.Port, + binding.Host); + start.CreateNoWindow = true; + start.WindowStyle = ProcessWindowStyle.Hidden; + process.Start(); + process.WaitForExit(); + + if (process.ExitCode != 0) + { + return "Register new certificate failed: access is denied"; + } + + return string.Empty; } - - return string.Empty; + } + catch (Exception) + { + // elevation is cancelled. + return "Register new certificate failed: operation is cancelled"; } } - catch (Exception) - { - // elevation is cancelled. - return "Register new certificate failed: operation is cancelled"; - } - } - if (!sni.Hash.SequenceEqual(binding.CertificateHash)) - { - // TODO: fix the error message. - var result = - MessageBox.Show( - "At least one other site is using the same HTTPS binding and the binding is configured with a different certificate. Are you sure that you want to reuse this HTTPS binding and reassign the other site or sites to use the new certificate?", - "TODO", - MessageBoxButtons.YesNo, - MessageBoxIcon.Question, - MessageBoxDefaultButton.Button1); - if (result != DialogResult.Yes) + if (!sni.Hash.SequenceEqual(binding.CertificateHash)) { - return "Certificate hash does not match. Please use the certificate that matches HTTPS binding"; - } + // TODO: fix the error message. + var result = + MessageBox.Show( + "At least one other site is using the same HTTPS binding and the binding is configured with a different certificate. Are you sure that you want to reuse this HTTPS binding and reassign the other site or sites to use the new certificate?", + "TODO", + MessageBoxButtons.YesNo, + MessageBoxIcon.Question, + MessageBoxDefaultButton.Button1); + if (result != DialogResult.Yes) + { + return + "Certificate hash does not match. Please use the certificate that matches HTTPS binding"; + } - try - { - // register mapping - using (var process = new Process()) + try { - var start = process.StartInfo; - start.Verb = "runas"; - start.FileName = "cmd"; - start.Arguments = string.Format( - "/c \"\"{2}\" /h:\"{0}\" /s:{1}\" /i:{3} /a:{4} /o:{5} /x:{6}", - Hex.ToHexString(binding.CertificateHash), - binding.CertificateStoreName, - Path.Combine(Environment.CurrentDirectory, "certificateinstaller.exe"), - AppIdIisExpress, - binding.EndPoint.Address, - binding.EndPoint.Port, - binding.Host); - start.CreateNoWindow = true; - start.WindowStyle = ProcessWindowStyle.Hidden; - process.Start(); - process.WaitForExit(); - - if (process.ExitCode != 0) + // register mapping + using (var process = new Process()) { - return "Register new certificate failed: access is denied"; + var start = process.StartInfo; + start.Verb = "runas"; + start.FileName = "cmd"; + start.Arguments = string.Format( + "/c \"\"{2}\" /h:\"{0}\" /s:{1}\" /i:{3} /a:{4} /o:{5} /x:{6}", + Hex.ToHexString(binding.CertificateHash), + binding.CertificateStoreName, + Path.Combine(Environment.CurrentDirectory, "certificateinstaller.exe"), + AppIdIisExpress, + binding.EndPoint.Address, + binding.EndPoint.Port, + binding.Host); + start.CreateNoWindow = true; + start.WindowStyle = ProcessWindowStyle.Hidden; + process.Start(); + process.WaitForExit(); + + if (process.ExitCode != 0) + { + return "Register new certificate failed: access is denied"; + } + + return string.Empty; } - - return string.Empty; + } + catch (Exception) + { + // elevation is cancelled. + return "Register new certificate failed: operation is cancelled"; } } - catch (Exception) + + if (!string.Equals(sni.StoreName, binding.CertificateStoreName, StringComparison.OrdinalIgnoreCase)) { - // elevation is cancelled. - return "Register new certificate failed: operation is cancelled"; + // TODO: can this happen? + return + "Certificate store name does not match. Please use the certificate that matches HTTPS binding"; } - } - if (!string.Equals(sni.StoreName, binding.CertificateStoreName, StringComparison.OrdinalIgnoreCase)) - { - // TODO: can this happen? - return - "Certificate store name does not match. Please use the certificate that matches HTTPS binding"; + return string.Empty; } - - return string.Empty; } // handle IP based diff --git a/JexusManager/Dialogs/NewSiteDialog.cs b/JexusManager/Dialogs/NewSiteDialog.cs index 77eb495b..f1be07e5 100644 --- a/JexusManager/Dialogs/NewSiteDialog.cs +++ b/JexusManager/Dialogs/NewSiteDialog.cs @@ -30,12 +30,17 @@ public NewSiteDialog(IServiceProvider serviceProvider, SiteCollection collection InitializeComponent(); cbType.SelectedIndex = 0; _collection = collection; + if (collection.Parent == null) + { + throw new InvalidOperationException("null server for site collection"); + } + btnBrowse.Visible = collection.Parent.IsLocalhost; txtPool.Text = collection.Parent.ApplicationDefaults.ApplicationPoolName; btnChoose.Enabled = collection.Parent.Mode != WorkingMode.Jexus; txtHost.Text = collection.Parent.Mode == WorkingMode.IisExpress ? "localhost" : string.Empty; DialogHelper.LoadAddresses(cbAddress); - if (Environment.OSVersion.Version < new Version(6, 2)) + if (!collection.Parent.SupportsSni) { cbSniRequired.Enabled = false; } diff --git a/JexusManager/Features/Main/BindingDialog.cs b/JexusManager/Features/Main/BindingDialog.cs index 73925d95..b032cafa 100644 --- a/JexusManager/Features/Main/BindingDialog.cs +++ b/JexusManager/Features/Main/BindingDialog.cs @@ -20,36 +20,38 @@ namespace JexusManager.Features.Main public sealed partial class BindingDialog : DialogForm { - private Binding _binding; private readonly Site _site; public BindingDialog(IServiceProvider serviceProvider, Binding binding, Site site) : base(serviceProvider) { InitializeComponent(); - _binding = binding; + Binding = binding; _site = site; - Text = _binding == null ? "Create Site Binding" : "Edit Site Binding"; + Text = Binding == null ? "Create Site Binding" : "Edit Site Binding"; DialogHelper.LoadAddresses(cbAddress); txtPort.Text = "80"; cbType.SelectedIndex = 0; - if (Environment.OSVersion.Version < new Version(6, 2)) + if (!Binding.Parent.Parent.Server.SupportsSni) { cbSniRequired.Enabled = false; } - if (_binding == null) + if (Binding == null) { txtHost.Text = site.Server.Mode == WorkingMode.IisExpress ? "localhost" : string.Empty; return; } - cbType.Text = _binding.Protocol; - cbType.Enabled = _binding == null; - cbAddress.Text = _binding.EndPoint.Address.AddressToCombo(); - txtPort.Text = _binding.EndPoint.Port.ToString(); - txtHost.Text = _binding.Host.HostToDisplay(); - cbSniRequired.Checked = _binding.GetIsSni(); + cbType.Text = Binding.Protocol; + cbType.Enabled = Binding == null; + cbAddress.Text = Binding.EndPoint.Address.AddressToCombo(); + txtPort.Text = Binding.EndPoint.Port.ToString(); + txtHost.Text = Binding.Host.HostToDisplay(); + if (Binding.Parent.Parent.Server.SupportsSni) + { + cbSniRequired.Checked = Binding.GetIsSni(); + } } private void CbTypeSelectedIndexChanged(object sender, EventArgs e) @@ -118,18 +120,16 @@ private void BtnOkClick(object sender, EventArgs e) var host = txtHost.Text.DisplayToHost(); var binding = new Binding( cbType.Text, - string.Format("{0}:{1}:{2}", address.AddressToDisplay(), port, host.HostToDisplay()), + $"{address.AddressToDisplay()}:{port}:{host.HostToDisplay()}", cbType.Text == "https" ? certificate?.Certificate.GetCertHash() : new byte[0], cbType.Text == "https" ? certificate?.Store : null, cbSniRequired.Checked ? SslFlags.Sni : SslFlags.None, _site.Bindings); - var matched = _site.Parent.FindDuplicate(binding, _site, _binding); + var matched = _site.Parent.FindDuplicate(binding, _site, Binding); if (matched == true) { var result = ShowMessage( - string.Format( - "The binding '{0}' is assigned to another site. If you assign the same binding to this site, you will only be able to start one of the sites. Are you sure that you want to add this duplicate binding?", - _binding), + $"The binding '{Binding}' is assigned to another site. If you assign the same binding to this site, you will only be able to start one of the sites. Are you sure that you want to add this duplicate binding?", MessageBoxButtons.YesNo, MessageBoxIcon.Question, MessageBoxDefaultButton.Button1); @@ -149,21 +149,21 @@ private void BtnOkClick(object sender, EventArgs e) return; } - if (_binding == null) + if (Binding == null) { - _binding = binding; + Binding = binding; } else { - _binding.Reinitialize(binding); + Binding.Reinitialize(binding); } if (_site.Server.Mode == WorkingMode.IisExpress) { - var result = _binding.FixCertificateMapping(certificate?.Certificate); + var result = Binding.FixCertificateMapping(certificate?.Certificate); if (!string.IsNullOrEmpty(result)) { - MessageBox.Show(string.Format("The binding '{0}' is invalid: {1}", _binding, result), Text, MessageBoxButtons.OK, MessageBoxIcon.Error); + MessageBox.Show($"The binding '{Binding}' is invalid: {result}", Text, MessageBoxButtons.OK, MessageBoxIcon.Error); return; } } @@ -171,10 +171,7 @@ private void BtnOkClick(object sender, EventArgs e) DialogResult = DialogResult.OK; } - internal Binding Binding - { - get { return _binding; } - } + internal Binding Binding { get; private set; } private void CbAddressTextChanged(object sender, EventArgs e) { @@ -203,7 +200,7 @@ private void BindingDialogHelpButtonClicked(object sender, CancelEventArgs e) private void BindingDialogLoad(object sender, EventArgs e) { var service = (IConfigurationService)GetService(typeof(IConfigurationService)); - DialogHelper.LoadCertificates(cbCertificates, _binding?.CertificateHash, service); + DialogHelper.LoadCertificates(cbCertificates, Binding?.CertificateHash, service); } private void CbCertificatesSelectedIndexChanged(object sender, EventArgs e) diff --git a/JexusManager/Features/Main/BindingsDialog.cs b/JexusManager/Features/Main/BindingsDialog.cs index 9de133bb..4d7ddbe8 100644 --- a/JexusManager/Features/Main/BindingsDialog.cs +++ b/JexusManager/Features/Main/BindingsDialog.cs @@ -61,7 +61,9 @@ private void ListView1SelectedIndexChanged(object sender, EventArgs e) return; } - var toElevate = selected && ((Binding)listView1.SelectedItems[0].Tag).GetIsSni(); + var binding = (Binding)listView1.SelectedItems[0].Tag; + var supportsSni = binding.Parent.Parent.Server.SupportsSni; + var toElevate = selected && (!supportsSni || (supportsSni && binding.GetIsSni())); if (toElevate) { JexusManager.NativeMethods.TryAddShieldToButton(btnRemove); diff --git a/JexusManager/Features/Main/SslDiagDialog.cs b/JexusManager/Features/Main/SslDiagDialog.cs index a1bb5db9..0299be00 100644 --- a/JexusManager/Features/Main/SslDiagDialog.cs +++ b/JexusManager/Features/Main/SslDiagDialog.cs @@ -67,7 +67,11 @@ private void BtnGenerateClick(object sender, System.EventArgs e) { var hashString = Hex.ToHexString(binding.CertificateHash); Debug($"SSLCertHash: {hashString}"); - Debug($"SSL Flags: {binding.SslFlags}"); + if (site.Server.SupportsSni) + { + Debug($"SSL Flags: {binding.SslFlags}"); + } + Debug("Testing EndPoint: 127.0.0.1"); var personal = new X509Store(binding.CertificateStoreName, StoreLocation.LocalMachine); diff --git a/Microsoft.Web.Administration/Binding.cs b/Microsoft.Web.Administration/Binding.cs index 24f37a09..74f1639c 100644 --- a/Microsoft.Web.Administration/Binding.cs +++ b/Microsoft.Web.Administration/Binding.cs @@ -28,7 +28,11 @@ internal Binding(string protocol, string bindingInformation, byte[] hash, string Protocol = protocol; CertificateHash = hash; CertificateStoreName = store; - SslFlags = flags; + if (parent.Parent.Server.SupportsSni) + { + SslFlags = flags; + } + Parent = parent; } @@ -92,14 +96,18 @@ private void Initialize() return; } - if (this.GetIsSni()) + if (Parent.Parent.Server.SupportsSni) { - var sni = NativeMethods.QuerySslSniInfo(new Tuple(_host, _endPoint.Port)); - if (sni != null) + if (this.GetIsSni()) { - CertificateHash = sni.Hash; - CertificateStoreName = sni.StoreName; - return; + var sni = NativeMethods.QuerySslSniInfo(new Tuple(_host, _endPoint.Port)); + if (sni != null) + { + CertificateHash = sni.Hash; + CertificateStoreName = sni.StoreName; + SslFlags = SslFlags.Sni; + return; + } } } diff --git a/Microsoft.Web.Administration/BindingCollection.cs b/Microsoft.Web.Administration/BindingCollection.cs index 798f538b..844a647a 100644 --- a/Microsoft.Web.Administration/BindingCollection.cs +++ b/Microsoft.Web.Administration/BindingCollection.cs @@ -28,7 +28,9 @@ internal BindingCollection(ConfigurationElement element, Site parent) public Binding Add(string bindingInformation, string bindingProtocol) { - throw new NotImplementedException(); + var item = new Binding(bindingProtocol, bindingInformation, null, null, SslFlags.None, this); + InternalAdd(item); + return item; } public Binding Add(string bindingInformation, byte[] certificateHash, string certificateStoreName) diff --git a/Microsoft.Web.Administration/Helper.cs b/Microsoft.Web.Administration/Helper.cs index 4bd64be7..5af8ba77 100644 --- a/Microsoft.Web.Administration/Helper.cs +++ b/Microsoft.Web.Administration/Helper.cs @@ -3,6 +3,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Diagnostics; using System.IO; namespace Microsoft.Web.Administration @@ -31,5 +32,28 @@ public static bool IsRunningOnMono() { return Type.GetType("Mono.Runtime") != null; } + + public static Version GetIisExpressVersion() + { + var fileName = + Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles), + "IIS Express", + "iisexpress.exe"); + if (!File.Exists(fileName)) + { + fileName = Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86), + "IIS Express", + "iisexpress.exe"); + } + + if (File.Exists(fileName)) + { + return new Version(FileVersionInfo.GetVersionInfo(fileName).ProductVersion); + } + + throw new InvalidOperationException("Cannot detect IIS Express installation."); + } } } diff --git a/Microsoft.Web.Administration/IFeatureDetection.cs b/Microsoft.Web.Administration/IFeatureDetection.cs new file mode 100644 index 00000000..3b32dd9a --- /dev/null +++ b/Microsoft.Web.Administration/IFeatureDetection.cs @@ -0,0 +1,7 @@ +namespace Microsoft.Web.Administration +{ + internal interface IFeatureDetection + { + bool SupportsSni { get; } + } +} diff --git a/Microsoft.Web.Administration/IisExpressServerManager.cs b/Microsoft.Web.Administration/IisExpressServerManager.cs index d8845921..9f81afbc 100644 --- a/Microsoft.Web.Administration/IisExpressServerManager.cs +++ b/Microsoft.Web.Administration/IisExpressServerManager.cs @@ -16,17 +16,22 @@ namespace Microsoft.Web.Administration { public sealed class IisExpressServerManager : ServerManager { - public IisExpressServerManager(bool readOnly, string applicationHostConfigurationPath) - : base(readOnly, applicationHostConfigurationPath) - { - Mode = WorkingMode.IisExpress; - } + public Version Version { get; } + + public override bool SupportsSni => Version >= new Version(8, 0) && Environment.OSVersion.Version >= new Version(6, 2); public IisExpressServerManager(string applicationHostConfigurationPath) : this(false, applicationHostConfigurationPath) { } + public IisExpressServerManager(bool readOnly, string applicationHostConfigurationPath) + : base(readOnly, applicationHostConfigurationPath) + { + Mode = WorkingMode.IisExpress; + Version = Helper.GetIisExpressVersion(); + } + internal override bool GetSiteState(Site site) { using (var process = new Process()) diff --git a/Microsoft.Web.Administration/IisServerManager.cs b/Microsoft.Web.Administration/IisServerManager.cs index e22c4dc1..d69748b5 100644 --- a/Microsoft.Web.Administration/IisServerManager.cs +++ b/Microsoft.Web.Administration/IisServerManager.cs @@ -17,6 +17,8 @@ namespace Microsoft.Web.Administration /// public sealed class IisServerManager : ServerManager { + public override bool SupportsSni => Environment.OSVersion.Version >= new Version(6, 2); + public IisServerManager() : this(null, true) { diff --git a/Microsoft.Web.Administration/JexusHelper.cs b/Microsoft.Web.Administration/JexusHelper.cs index f8a9950b..0b3d8a20 100644 --- a/Microsoft.Web.Administration/JexusHelper.cs +++ b/Microsoft.Web.Administration/JexusHelper.cs @@ -28,6 +28,8 @@ public partial class JexusServerManager internal string Credentials { get; set; } + public override bool SupportsSni => false; + public async Task SaveKeyAsync(string key) { using (var client = GetClient()) diff --git a/Microsoft.Web.Administration/Microsoft.Web.Administration.csproj b/Microsoft.Web.Administration/Microsoft.Web.Administration.csproj index ebb0f301..9630815f 100644 --- a/Microsoft.Web.Administration/Microsoft.Web.Administration.csproj +++ b/Microsoft.Web.Administration/Microsoft.Web.Administration.csproj @@ -66,6 +66,7 @@ + diff --git a/Microsoft.Web.Administration/ServerManager.cs b/Microsoft.Web.Administration/ServerManager.cs index cfb1a8d0..271ffbfe 100644 --- a/Microsoft.Web.Administration/ServerManager.cs +++ b/Microsoft.Web.Administration/ServerManager.cs @@ -6,11 +6,10 @@ using System.Collections.Generic; using System.IO; using System.Linq; -using System.Threading.Tasks; namespace Microsoft.Web.Administration { - public abstract class ServerManager + public abstract class ServerManager : IFeatureDetection { private Configuration _applicationHost; @@ -48,6 +47,8 @@ internal string Title } } + public abstract bool SupportsSni { get; } + public WorkingMode Mode { get; protected set; } public ServerManager() diff --git a/Tests/TestCases.cs b/Tests/TestCases.cs index 28007f23..131787aa 100644 --- a/Tests/TestCases.cs +++ b/Tests/TestCases.cs @@ -95,6 +95,10 @@ public static void TestIisExpress(ServerManager server) var sslSite = server.Sites[1]; var sslBinding = sslSite.Bindings[1]; Assert.Equal(SslFlags.Sni, sslBinding.SslFlags); + sslSite.Bindings.RemoveAt(1); + sslSite.Bindings.Add(":443:localhost", "https"); + sslBinding = sslSite.Bindings[1]; + Assert.Equal(SslFlags.None, sslBinding.SslFlags); var app = site.Applications[0]; Assert.True(site.Applications.AllowsAdd);