From 282ddb85c7b3556bea98c572bc99e35476ad0cc9 Mon Sep 17 00:00:00 2001 From: Sean Kaim Date: Mon, 13 Mar 2017 17:11:21 -0400 Subject: [PATCH 1/7] code clean up Fix - CA2214: Do not call overridable methods in constructors Relates to #249 --- .../Protocol/Http/Connection.Protocol.HTTP.cs | 12 ++--- .../Http/Connection.Protocol.HTTPBase.cs | 52 ++----------------- .../Http/Connection.Protocol.HTTPS.cs | 11 ++-- 3 files changed, 10 insertions(+), 65 deletions(-) diff --git a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTP.cs b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTP.cs index 5c87f26e..cbdaca2f 100644 --- a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTP.cs +++ b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTP.cs @@ -5,15 +5,9 @@ namespace mRemoteNG.Connection.Protocol.Http public ProtocolHTTP(RenderingEngine RenderingEngine) : base(RenderingEngine) { - } - - public override void NewExtended() - { - base.NewExtended(); - - httpOrS = "http"; - defaultPort = (int)Defaults.Port; - } + httpOrS = "http"; + defaultPort = (int)Defaults.Port; + } public enum Defaults { diff --git a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPBase.cs b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPBase.cs index 78362d5a..2dca9030 100644 --- a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPBase.cs +++ b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPBase.cs @@ -13,13 +13,14 @@ namespace mRemoteNG.Connection.Protocol.Http { #region Private Properties private Control wBrowser; - public string httpOrS; - public int defaultPort; + protected string httpOrS; + protected int defaultPort; private string tabTitle; #endregion #region Public Methods - public HTTPBase(RenderingEngine RenderingEngine) + + protected HTTPBase(RenderingEngine RenderingEngine) { try { @@ -34,18 +35,12 @@ namespace mRemoteNG.Connection.Protocol.Http { Control = new WebBrowser(); } - - NewExtended(); } catch (Exception ex) { Runtime.MessageCollector.AddExceptionStackTrace(Language.strHttpConnectionFailed, ex); } } - - public virtual void NewExtended() - { - } public override bool Initialize() { @@ -178,24 +173,6 @@ namespace mRemoteNG.Connection.Protocol.Http objWebBrowser.Navigated -= wBrowser_Navigated; } -#if false - private void wBrowser_NewWindow3(ref object ppDisp, ref bool Cancel, uint dwFlags, string bstrUrlContext, string bstrUrl) - { - if ((dwFlags & (long)NWMF.NWMF_OVERRIDEKEY) > 0) - { - Cancel = false; - } - else - { - Cancel = true; - } - } - - private void wBrowser_LastTabRemoved(object sender) - { - Close(); - } -#endif private void wBrowser_DocumentTitleChanged(object sender, EventArgs e) { try @@ -301,27 +278,6 @@ namespace mRemoteNG.Connection.Protocol.Http Gecko = 2 } -#if false - private enum NWMF - { - // ReSharper disable InconsistentNaming - NWMF_UNLOADING = 0x1, - NWMF_USERINITED = 0x2, - NWMF_FIRST = 0x4, - NWMF_OVERRIDEKEY = 0x8, - NWMF_SHOWHELP = 0x10, - NWMF_HTMLDIALOG = 0x20, - NWMF_FROMDIALOGCHILD = 0x40, - NWMF_USERREQUESTED = 0x80, - NWMF_USERALLOWED = 0x100, - NWMF_FORCEWINDOW = 0x10000, - NWMF_FORCETAB = 0x20000, - NWMF_SUGGESTWINDOW = 0x40000, - NWMF_SUGGESTTAB = 0x80000, - NWMF_INACTIVETAB = 0x100000 - // ReSharper restore InconsistentNaming - } -#endif #endregion } } diff --git a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.cs b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.cs index a8cff0f1..08f28a92 100644 --- a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.cs +++ b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.cs @@ -5,14 +5,9 @@ namespace mRemoteNG.Connection.Protocol.Http public ProtocolHTTPS(RenderingEngine RenderingEngine) : base(RenderingEngine) { - } - - public override void NewExtended() - { - base.NewExtended(); - httpOrS = "https"; - defaultPort = (int)Defaults.Port; - } + httpOrS = "https"; + defaultPort = (int)Defaults.Port; + } public enum Defaults { From a9db6a04734f3a22695000974f57a31b8ac1da94 Mon Sep 17 00:00:00 2001 From: Sean Kaim Date: Mon, 13 Mar 2017 17:26:52 -0400 Subject: [PATCH 2/7] More code cleanup --- .../Protocol/Http/Connection.Protocol.HTTPBase.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPBase.cs b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPBase.cs index 2dca9030..70301427 100644 --- a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPBase.cs +++ b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPBase.cs @@ -5,8 +5,6 @@ using mRemoteNG.Tools; using mRemoteNG.App; using TabPage = Crownwood.Magic.Controls.TabPage; -//using SHDocVw; - namespace mRemoteNG.Connection.Protocol.Http { public class HTTPBase : ProtocolBase @@ -66,13 +64,11 @@ namespace mRemoteNG.Connection.Protocol.Http if (GeckoBrowser != null) { GeckoBrowser.DocumentTitleChanged += geckoBrowser_DocumentTitleChanged; - //GeckoBrowser.Tab.LastTabRemoved += wBrowser_LastTabRemoved; } } else { var objWebBrowser = (WebBrowser)wBrowser; - //SHDocVw.WebBrowserClass objAxWebBrowser = (SHDocVw.WebBrowserClass)objWebBrowser.ActiveXInstance; objWebBrowser.ScrollBarsEnabled = true; // http://stackoverflow.com/questions/4655662/how-to-ignore-script-errors-in-webbrowser @@ -80,8 +76,6 @@ namespace mRemoteNG.Connection.Protocol.Http objWebBrowser.Navigated += wBrowser_Navigated; objWebBrowser.DocumentTitleChanged += wBrowser_DocumentTitleChanged; - //objWebBrowser.NewWindow3 += wBrowser_NewWindow3; - //objAxWebBrowser.NewWindow3 += wBrowser_NewWindow3; } return true; From cc13707feaca86e78d226b2a84d0300f56c3aa99 Mon Sep 17 00:00:00 2001 From: Sean Kaim Date: Mon, 13 Mar 2017 17:29:17 -0400 Subject: [PATCH 3/7] Better error handling --- .../Connection/Protocol/Http/Connection.Protocol.HTTPBase.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPBase.cs b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPBase.cs index 70301427..9e18eb59 100644 --- a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPBase.cs +++ b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPBase.cs @@ -65,6 +65,10 @@ namespace mRemoteNG.Connection.Protocol.Http { GeckoBrowser.DocumentTitleChanged += geckoBrowser_DocumentTitleChanged; } + else + { + throw new Exception("Failed to initialize Gecko Rendering Engine."); + } } else { From 20fb2698288db3d5824079cca9eb4c2442ad3944 Mon Sep 17 00:00:00 2001 From: Sean Kaim Date: Mon, 13 Mar 2017 18:05:07 -0400 Subject: [PATCH 4/7] Initial prompt for allow untrusted cert --- .../Http/Connection.Protocol.HTTPBase.cs | 1 + .../Connection.Protocol.HTTPS.CertEvent.cs | 60 +++++++++++++++++++ mRemoteV1/mRemoteV1.csproj | 1 + 3 files changed, 62 insertions(+) create mode 100644 mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.CertEvent.cs diff --git a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPBase.cs b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPBase.cs index 9e18eb59..75206bac 100644 --- a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPBase.cs +++ b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPBase.cs @@ -64,6 +64,7 @@ namespace mRemoteNG.Connection.Protocol.Http if (GeckoBrowser != null) { GeckoBrowser.DocumentTitleChanged += geckoBrowser_DocumentTitleChanged; + GeckoBrowser.NSSError += CertEvent.GeckoBrowser_NSSError; } else { diff --git a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.CertEvent.cs b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.CertEvent.cs new file mode 100644 index 00000000..2b375f20 --- /dev/null +++ b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.CertEvent.cs @@ -0,0 +1,60 @@ +using Gecko; +using Gecko.Events; +using mRemoteNG.App.Info; +using mRemoteNG.UI.TaskDialog; +// ReSharper disable RedundantAssignment + +namespace mRemoteNG.Connection.Protocol.Http +{ + internal abstract class CertEvent + { + + //Refernce: https://bitbucket.org/geckofx/geckofx-33.0/issues/90/invalid-security-certificate-error-on + internal static void GeckoBrowser_NSSError(object sender, GeckoNSSErrorEventArgs e) + { + if (!e.Message.Contains("Certificate")) + { + e.Handled = true; + return; + } + + string[] commandButtons = + { + $"Allow Once", // 0 + $"Allow Always", // 1 + $"Don't Allow" // 2 + }; + + CTaskDialog.ShowTaskDialogBox(null, GeneralAppInfo.ProductName, $"Allow Insecure Certificate?", + string.Format($"Allow Insecure Certificate?", GeneralAppInfo.ProductName), + "", "", "", "", string.Join(" | ", commandButtons), ETaskDialogButtons.None, ESysIcons.Question, + ESysIcons.Question); + + + var allow = false; + var always = false; + // ReSharper disable once SwitchStatementMissingSomeCases + switch (CTaskDialog.CommandButtonResult) + { + case 0: + allow = true; + always = false; + break; + case 1: + allow = true; + always = true; + break; + case 2: + allow = false; + always = false; + break; + } + + // "always" might not work: https://bitbucket.org/geckofx/geckofx-45.0/issues/152/remembervalidityoverride-doesnt-save-in + if(allow) + CertOverrideService.GetService().RememberValidityOverride(e.Uri, e.Certificate, CertOverride.Mismatch | CertOverride.Time | CertOverride.Untrusted, always); + e.Handled = true; + ((GeckoWebBrowser)sender).Navigate(e.Uri.AbsoluteUri); + } + } +} diff --git a/mRemoteV1/mRemoteV1.csproj b/mRemoteV1/mRemoteV1.csproj index d1bedbb4..837d216e 100644 --- a/mRemoteV1/mRemoteV1.csproj +++ b/mRemoteV1/mRemoteV1.csproj @@ -205,6 +205,7 @@ + From f2934f8453fa8ce587ead13cdee08575f77164f0 Mon Sep 17 00:00:00 2001 From: Sean Kaim Date: Tue, 14 Mar 2017 10:23:15 -0400 Subject: [PATCH 5/7] update message, change unhandled messages to false --- .../Protocol/Http/Connection.Protocol.HTTPS.CertEvent.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.CertEvent.cs b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.CertEvent.cs index 2b375f20..08f7fbdc 100644 --- a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.CertEvent.cs +++ b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.CertEvent.cs @@ -14,7 +14,7 @@ namespace mRemoteNG.Connection.Protocol.Http { if (!e.Message.Contains("Certificate")) { - e.Handled = true; + e.Handled = false; return; } @@ -26,7 +26,7 @@ namespace mRemoteNG.Connection.Protocol.Http }; CTaskDialog.ShowTaskDialogBox(null, GeneralAppInfo.ProductName, $"Allow Insecure Certificate?", - string.Format($"Allow Insecure Certificate?", GeneralAppInfo.ProductName), + string.Format($"Allow Insecure Certificate for URL: {0}?", e.Uri.AbsoluteUri), "", "", "", "", string.Join(" | ", commandButtons), ETaskDialogButtons.None, ESysIcons.Question, ESysIcons.Question); From d2e33ee42335c4b1a1b6442d52e6a057dfc90d0b Mon Sep 17 00:00:00 2001 From: Sean Kaim Date: Tue, 14 Mar 2017 11:58:51 -0400 Subject: [PATCH 6/7] I was doing some backwards thinking it appears... --- .../Connection.Protocol.HTTPS.CertEvent.cs | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.CertEvent.cs b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.CertEvent.cs index 08f7fbdc..37ed32a7 100644 --- a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.CertEvent.cs +++ b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.CertEvent.cs @@ -1,6 +1,8 @@ using Gecko; using Gecko.Events; +using mRemoteNG.App; using mRemoteNG.App.Info; +using mRemoteNG.Messages; using mRemoteNG.UI.TaskDialog; // ReSharper disable RedundantAssignment @@ -8,12 +10,16 @@ namespace mRemoteNG.Connection.Protocol.Http { internal abstract class CertEvent { - //Refernce: https://bitbucket.org/geckofx/geckofx-33.0/issues/90/invalid-security-certificate-error-on internal static void GeckoBrowser_NSSError(object sender, GeckoNSSErrorEventArgs e) { - if (!e.Message.Contains("Certificate")) + /* some messages say "Certificate", some say "certificate" + * I'm guessing that this is going to be a localization issue... + * Log a message so we can try to find a better solution if problems are reported in the future... + */ + if (!e.Message.ToLower().Contains("certificate")) { + Runtime.MessageCollector.AddMessage(MessageClass.WarningMsg, $"Unhandled NSSError: {e.Message}"); e.Handled = false; return; } @@ -26,34 +32,38 @@ namespace mRemoteNG.Connection.Protocol.Http }; CTaskDialog.ShowTaskDialogBox(null, GeneralAppInfo.ProductName, $"Allow Insecure Certificate?", - string.Format($"Allow Insecure Certificate for URL: {0}?", e.Uri.AbsoluteUri), + string.Format("Allow Insecure Certificate for URL: {0}?", e.Uri.AbsoluteUri), "", "", "", "", string.Join(" | ", commandButtons), ETaskDialogButtons.None, ESysIcons.Question, ESysIcons.Question); var allow = false; - var always = false; + var temporary = true; // ReSharper disable once SwitchStatementMissingSomeCases switch (CTaskDialog.CommandButtonResult) { case 0: allow = true; - always = false; + temporary = true; break; case 1: allow = true; - always = true; + temporary = false; break; case 2: allow = false; - always = false; + temporary = true; // just to be safe break; } - // "always" might not work: https://bitbucket.org/geckofx/geckofx-45.0/issues/152/remembervalidityoverride-doesnt-save-in - if(allow) - CertOverrideService.GetService().RememberValidityOverride(e.Uri, e.Certificate, CertOverride.Mismatch | CertOverride.Time | CertOverride.Untrusted, always); + /* "temporary == false" (aka always) might not work: + * https://bitbucket.org/geckofx/geckofx-45.0/issues/152/remembervalidityoverride-doesnt-save-in + * However, my testing was successful in Gecko 45.0.22 + */ + if (allow) + CertOverrideService.GetService().RememberValidityOverride(e.Uri, e.Certificate, CertOverride.Mismatch | CertOverride.Time | CertOverride.Untrusted, temporary); e.Handled = true; + // navigate even if we don't trust the cert. This will allow Gecko the return errors to the user. ((GeckoWebBrowser)sender).Navigate(e.Uri.AbsoluteUri); } } From bd1e62abc1d5163e62216cd96d43d1d52924a6f5 Mon Sep 17 00:00:00 2001 From: Sean Kaim Date: Tue, 14 Mar 2017 12:20:20 -0400 Subject: [PATCH 7/7] localization message support, logging, formatting --- .../Connection.Protocol.HTTPS.CertEvent.cs | 32 +++++++------ .../Resources/Language/Language.Designer.cs | 45 +++++++++++++++++++ mRemoteV1/Resources/Language/Language.resx | 15 +++++++ 3 files changed, 78 insertions(+), 14 deletions(-) diff --git a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.CertEvent.cs b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.CertEvent.cs index 37ed32a7..086d8b49 100644 --- a/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.CertEvent.cs +++ b/mRemoteV1/Connection/Protocol/Http/Connection.Protocol.HTTPS.CertEvent.cs @@ -26,16 +26,14 @@ namespace mRemoteNG.Connection.Protocol.Http string[] commandButtons = { - $"Allow Once", // 0 - $"Allow Always", // 1 - $"Don't Allow" // 2 + Language.strHttpsInsecureAllowOnce, // 0 + Language.strHttpsInsecureAllowAlways, // 1 + Language.strHttpsInsecureDontAllow // 2 }; - CTaskDialog.ShowTaskDialogBox(null, GeneralAppInfo.ProductName, $"Allow Insecure Certificate?", - string.Format("Allow Insecure Certificate for URL: {0}?", e.Uri.AbsoluteUri), - "", "", "", "", string.Join(" | ", commandButtons), ETaskDialogButtons.None, ESysIcons.Question, - ESysIcons.Question); - + CTaskDialog.ShowTaskDialogBox(null, GeneralAppInfo.ProductName, Language.strHttpsInsecurePromptTitle, + string.Format(Language.strHttpsInsecurePrompt, e.Uri.AbsoluteUri), "", "", "", "", + string.Join(" | ", commandButtons), ETaskDialogButtons.None, ESysIcons.Question, ESysIcons.Question); var allow = false; var temporary = true; @@ -55,15 +53,21 @@ namespace mRemoteNG.Connection.Protocol.Http temporary = true; // just to be safe break; } + + if (!allow) + { + Runtime.MessageCollector.AddMessage(MessageClass.WarningMsg, $"User did not allow navigation to {e.Uri.AbsoluteUri} with an insecure certificate: {e.Message}"); + return; + } /* "temporary == false" (aka always) might not work: - * https://bitbucket.org/geckofx/geckofx-45.0/issues/152/remembervalidityoverride-doesnt-save-in - * However, my testing was successful in Gecko 45.0.22 - */ - if (allow) - CertOverrideService.GetService().RememberValidityOverride(e.Uri, e.Certificate, CertOverride.Mismatch | CertOverride.Time | CertOverride.Untrusted, temporary); + * https://bitbucket.org/geckofx/geckofx-45.0/issues/152/remembervalidityoverride-doesnt-save-in + * However, my testing was successful in Gecko 45.0.22 + */ + CertOverrideService.GetService().RememberValidityOverride(e.Uri, e.Certificate, + CertOverride.Mismatch | CertOverride.Time | CertOverride.Untrusted, temporary); + e.Handled = true; - // navigate even if we don't trust the cert. This will allow Gecko the return errors to the user. ((GeckoWebBrowser)sender).Navigate(e.Uri.AbsoluteUri); } } diff --git a/mRemoteV1/Resources/Language/Language.Designer.cs b/mRemoteV1/Resources/Language/Language.Designer.cs index b00d9f8c..2b6403ae 100644 --- a/mRemoteV1/Resources/Language/Language.Designer.cs +++ b/mRemoteV1/Resources/Language/Language.Designer.cs @@ -2199,6 +2199,51 @@ namespace mRemoteNG { } } + /// + /// Looks up a localized string similar to Allow Always. + /// + internal static string strHttpsInsecureAllowAlways { + get { + return ResourceManager.GetString("strHttpsInsecureAllowAlways", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Allow Once. + /// + internal static string strHttpsInsecureAllowOnce { + get { + return ResourceManager.GetString("strHttpsInsecureAllowOnce", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Don't Allow. + /// + internal static string strHttpsInsecureDontAllow { + get { + return ResourceManager.GetString("strHttpsInsecureDontAllow", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Allow Insecure Certificate for URL: {0}?. + /// + internal static string strHttpsInsecurePrompt { + get { + return ResourceManager.GetString("strHttpsInsecurePrompt", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Allow Insecure Certificate?. + /// + internal static string strHttpsInsecurePromptTitle { + get { + return ResourceManager.GetString("strHttpsInsecurePromptTitle", resourceCulture); + } + } + /// /// Looks up a localized string similar to ICA. /// diff --git a/mRemoteV1/Resources/Language/Language.resx b/mRemoteV1/Resources/Language/Language.resx index 2774da8a..99c877f5 100644 --- a/mRemoteV1/Resources/Language/Language.resx +++ b/mRemoteV1/Resources/Language/Language.resx @@ -2508,4 +2508,19 @@ mRemoteNG will now quit and begin with the installation. Assigned Credential + + Allow Always + + + Allow Once + + + Don't Allow + + + Allow Insecure Certificate for URL: {0}? + + + Allow Insecure Certificate? + \ No newline at end of file