Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix missing DialogPromptCertificateTrustCallback #15766

Merged
merged 2 commits into from
Mar 22, 2024
Merged

Fix missing DialogPromptCertificateTrustCallback #15766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
794115d
Fix missing DialogPromptCertificateTrustCallback
AliveDevil Mar 22, 2024
1ce338f
Fix test
AliveDevil Mar 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
72 changes: 72 additions & 0 deletions core/src/main/csharp/ch/cyberduck/core/Interactivity/DialogPromptCertificateTrustCallback.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
using ch.cyberduck.core;
using ch.cyberduck.core.exception;
using Ch.Cyberduck.Core.TaskDialog;
using java.util;
using org.apache.logging.log4j;
using System;
using System.Security.Cryptography.X509Certificates;
using System.Threading;
using static Windows.Win32.UI.WindowsAndMessaging.MESSAGEBOX_RESULT;
using X509Certificate = java.security.cert.X509Certificate;

namespace Ch.Cyberduck.Core.Interactivity;

public class DialogPromptCertificateTrustCallback : CertificateTrustCallback
{
private static readonly Logger Log = LogManager.getLogger(typeof(DialogPromptCertificateTrustCallback).FullName);
private static readonly ThreadLocal<Action> VerificationCallbackLocal = new();

public static VerificationCallbackRegistration Register(Action callback)
{
VerificationCallbackLocal.Value = callback;
return new();
}

public void prompt(string str, List l)
{
var serverCert = SystemCertificateStore.ConvertCertificate(l.get(0) as X509Certificate);
var result = TaskDialog.TaskDialog.Create()
.Title(LocaleFactory.localizedString("Certificate Error", "Keychain"))
.Instruction(LocaleFactory.localizedString("Certificate Error", "Keychain"))
.VerificationText(LocaleFactory.localizedString("Always Trust", "Keychain"), false)
.Content(str)
.CommandLinks(c =>
{
c(IDCONTINUE, LocaleFactory.localizedString("Continue", "Credentials"), false);
c(IDABORT, LocaleFactory.localizedString("Disconnect"), true);
c(IDHELP, LocaleFactory.localizedString("Show Certificate", "Keychain"), false);
})
.Callback((sender, e) =>
{
if (e is TaskDialogButtonClickedEventArgs buttonClicked)
{
if (buttonClicked.ButtonId == (int)IDHELP)
{
X509Certificate2UI.DisplayCertificate(serverCert);
return true;
}
}

return false;
})
.Show();

if (result.Button == IDABORT)
{
throw new ConnectionCanceledException();
}

if (result.VerificationChecked == true)
{
VerificationCallbackLocal.Value?.Invoke();
}
}

public ref struct VerificationCallbackRegistration
{
public void Dispose()
{
VerificationCallbackLocal.Value = null;
}
}
}
83 changes: 23 additions & 60 deletions core/src/main/csharp/ch/cyberduck/core/SystemCertificateStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,17 @@

using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Security.Cryptography.X509Certificates;
using ch.cyberduck.core;
using ch.cyberduck.core.exception;
using ch.cyberduck.core.preferences;
using Ch.Cyberduck.Core.Interactivity;
using Ch.Cyberduck.Core.Ssl;
using Ch.Cyberduck.Core.TaskDialog;
using java.io;
using java.security;
using java.security.cert;
using java.util;
using org.apache.logging.log4j;
using static Windows.Win32.UI.WindowsAndMessaging.MESSAGEBOX_RESULT;
using X509Certificate = java.security.cert.X509Certificate;

namespace Ch.Cyberduck.Core
Expand Down Expand Up @@ -88,7 +86,7 @@ public class SystemCertificateStore : CertificateStore

public bool verify(CertificateTrustCallback prompt, String hostName, List certs)
{
X509Certificate2 serverCert = ConvertCertificate(certs.iterator().next() as X509Certificate);
X509Certificate2 serverCert = ConvertCertificate(certs.get(0) as X509Certificate);
X509Chain chain = new X509Chain();
chain.ChainPolicy.RevocationMode =
PreferencesFactory.get().getBoolean("connection.ssl.x509.revocation.online")
Expand All @@ -103,9 +101,7 @@ public bool verify(CertificateTrustCallback prompt, String hostName, List certs)
}

chain.Build(serverCert);

bool isException = CheckForException(hostName, serverCert);
if (isException)
if (CheckForException(hostName, serverCert))
{
// Exceptions always have precedence
return true;
Expand All @@ -114,7 +110,7 @@ public bool verify(CertificateTrustCallback prompt, String hostName, List certs)
string errorFromChainStatus = GetErrorFromChainStatus(chain, hostName);
bool certError = null != errorFromChainStatus;
bool hostnameMismatch = hostName != null &&
!HostnameVerifier.CheckServerIdentity(certs.iterator().next() as X509Certificate,
!HostnameVerifier.CheckServerIdentity(certs.get(0) as X509Certificate,
serverCert, hostName);

// check if host name matches
Expand All @@ -128,59 +124,26 @@ public bool verify(CertificateTrustCallback prompt, String hostName, List certs)

if (null != errorFromChainStatus)
{
// Title: LocaleFactory.localizedString("Certificate Error", "Keychain")
// Main Instruction: LocaleFactory.localizedString("Certificate Error", "Keychain")
// Content: errorFromChainStatus
// Verification Text: LocaleFactory.localizedString("Always Trust", "Keychain")
// CommandButtons: { LocaleFactory.localizedString("Continue", "Credentials"), LocaleFactory.localizedString("Disconnect"), LocaleFactory.localizedString("Show Certificate", "Keychain") }
// ShowCancelButton: false
// Main Icon: Warning
// Footer Icon: Information

var result = TaskDialog.TaskDialog.Create()
.Title(LocaleFactory.localizedString("Certificate Error", "Keychain"))
.Instruction(LocaleFactory.localizedString("Certificate Error", "Keychain"))
.VerificationText(LocaleFactory.localizedString("Always Trust", "Keychain"), false)
.Content(errorFromChainStatus)
.CommandLinks(c =>
{
c(IDCONTINUE, LocaleFactory.localizedString("Continue", "Credentials"), false);
c(IDABORT, LocaleFactory.localizedString("Disconnect"), true);
c(IDHELP, LocaleFactory.localizedString("Show Certificate", "Keychain"), false);
})
.Callback((sender, e) =>
{
if (e is TaskDialogButtonClickedEventArgs buttonClicked)
{
if (buttonClicked.ButtonId == (int)IDHELP)
{
X509Certificate2UI.DisplayCertificate(serverCert);
return true;
}
}

return false;
})
.Show();
if (result.Button == IDCONTINUE)
// Force use of ThreadLocal, otherwise we can't persist X.certificate.accept
using (DialogPromptCertificateTrustCallback.Register(() =>
{
if (result.VerificationChecked == true)
if (certError)
{
if (certError)
{
//todo can we use the Trusted People and Third Party Certificate Authority Store? Currently X509Chain is the problem.
AddCertificate(serverCert, StoreName.Root);
}

PreferencesFactory.get()
.setProperty(hostName + ".certificate.accept", serverCert.Thumbprint);
AddCertificate(serverCert, StoreName.Root);
}

return true;
}
else
PreferencesFactory.get()
.setProperty(hostName + ".certificate.accept", serverCert.Thumbprint);
}))
{
return false;
try
{
prompt.prompt(errorFromChainStatus, certs);
}
catch (ConnectionCanceledException)
{
return false;
}
}
}

Expand All @@ -192,12 +155,12 @@ public static IReadOnlyCollection<string> ListAliases()
using X509Store store = new(StoreName.My, StoreLocation.CurrentUser);
HashSet<string> certs = new();
store.Open(OpenFlags.ReadOnly);
foreach(X509Certificate2 certificate in store.Certificates)
foreach (X509Certificate2 certificate in store.Certificates)
{
var alias = string.IsNullOrWhiteSpace(certificate.FriendlyName)
? certificate.GetNameInfo(X509NameType.SimpleName, false)
: certificate.FriendlyName;
if(!certs.Add(alias) && Log.isDebugEnabled())
if (!certs.Add(alias) && Log.isDebugEnabled())
{
Log.debug($"Skipping duplicate alias \"{alias}\"");
}
Expand Down Expand Up @@ -264,7 +227,7 @@ private string GetErrorFromChainStatus(X509Chain chain, string hostName)
string.Format(LocaleFactory.localizedString(
"The certificate for this server has expired. You might be connecting to a server that is pretending to be {0} which could put your confidential information at risk. Would you like to connect to the server anyway?",
"Keychain"), hostName);
return error;
break;
}

if (((status.Status & X509ChainStatusFlags.UntrustedRoot) == X509ChainStatusFlags.UntrustedRoot) ||
Expand All @@ -275,7 +238,7 @@ private string GetErrorFromChainStatus(X509Chain chain, string hostName)
string.Format(LocaleFactory.localizedString(
"The certificate for this server was signed by an unknown certifying authority. You might be connecting to a server that is pretending to be {0} which could put your confidential information at risk. Would you like to connect to the server anyway?",
"Keychain"), hostName);
return error;
break;
}

//all other errors we map to !CSSM_CERT_STATUS_IS_IN_ANCHORS
Expand Down
26 changes: 5 additions & 21 deletions core/src/test/csharp/ch/cyberduck/core/SystemCertificateStoreTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

using System;
using System.Text;
using ch.cyberduck.core;
using ch.cyberduck.core.preferences;
using Ch.Cyberduck.Core;
using java.io;
Expand Down Expand Up @@ -58,20 +59,12 @@ public void ExpiredSelfSignedCertificate()
const string hostName = "foo.secure.example.com";
List certs = new ArrayList();
certs.add(cert);
try
{
//no exception registered yet
new SystemCertificateStore().verify(null, hostName, certs);
Assert.Fail();
}
catch (EntryPointNotFoundException exception)
{
}
Assert.False(new SystemCertificateStore().verify(new DisabledCertificateTrustCallback(), hostName, certs));
//register exception
PreferencesFactory.get()
.setProperty(hostName + ".certificate.accept",
SystemCertificateStore.ConvertCertificate(cert).Thumbprint);
Assert.IsTrue(new SystemCertificateStore().verify(null, hostName, certs));
Assert.IsTrue(new SystemCertificateStore().verify(new DisabledCertificateTrustCallback(), hostName, certs));
}

[Test]
Expand Down Expand Up @@ -147,21 +140,12 @@ public void UntrustedSelfSignedCertificate()
certs.add(hostCert);
certs.add(caCert);

try
{
//no exception registered yet
new SystemCertificateStore().verify(null, hostName, certs);
Assert.Fail();
}
catch (EntryPointNotFoundException exception)
{
Console.WriteLine("TEST");
}
Assert.False(new SystemCertificateStore().verify(new DisabledCertificateTrustCallback(), hostName, certs));
//register exception
PreferencesFactory.get()
.setProperty(hostName + ".certificate.accept",
SystemCertificateStore.ConvertCertificate(hostCert).Thumbprint);
Assert.IsTrue(new SystemCertificateStore().verify(null, hostName, certs));
Assert.IsTrue(new SystemCertificateStore().verify(new DisabledCertificateTrustCallback(), hostName, certs));
}
}
}
2 changes: 2 additions & 0 deletions windows/src/main/csharp/ch/cyberduck/ui/core/preferences/ApplicationPreferences.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
using Ch.Cyberduck.Core.Diagnostics;
using Ch.Cyberduck.Core.Editor;
using Ch.Cyberduck.Core.I18n;
using Ch.Cyberduck.Core.Interactivity;
using Ch.Cyberduck.Core.Local;
using Ch.Cyberduck.Core.Preferences;
using Ch.Cyberduck.Core.Proxy;
Expand Down Expand Up @@ -105,6 +106,7 @@ protected override void setFactories()
this.setDefault("factory.badgelabeler.class", typeof(TaskbarApplicationBadgeLabeler).AssemblyQualifiedName);
this.setDefault("factory.filedescriptor.class", typeof(Win32FileDescriptor).AssemblyQualifiedName);
this.setDefault("factory.schemehandler.class", typeof(URLSchemeHandlerConfiguration).AssemblyQualifiedName);
this.setDefault("factory.certificatetrustcallback.class", typeof(DialogPromptCertificateTrustCallback).AssemblyQualifiedName);

if (Cyberduck.Core.Utils.IsWin10FallCreatorsUpdate)
{
Expand Down