Remove ConfirmingHostnameVerifier and unused settings from CertificateManager

It turned out to be non-obvious how to properly save hostname and
expiration date exceptions in settings, so these checks were disabled
in the first place and the corresponding settings were hidden in the
configurable. CertificateManager.HOSTNAME_VERIFIER was left in
CertificateManager my mistake, and, unfortunately, has acquired some
users since then.
This commit is contained in:
Mikhail Golubev
2016-09-29 18:17:41 +03:00
parent 3f562affe7
commit a7bcd40725
8 changed files with 12 additions and 180 deletions
@@ -3,7 +3,7 @@
<grid id="27dc6" binding="myRootPanel" layout-manager="GridLayoutManager" row-count="4" column-count="1" same-size-horizontally="false" same-size-vertically="false" hgap="-1" vgap="-1">
<margin top="0" left="0" bottom="0" right="0"/>
<constraints>
<xy x="20" y="20" width="606" height="400"/>
<xy x="20" y="20" width="1012" height="400"/>
</constraints>
<properties/>
<border type="none"/>
@@ -19,7 +19,7 @@
<border type="etched" title="Accepted certificates"/>
<children/>
</grid>
<grid id="16d1c" layout-manager="GridLayoutManager" row-count="1" column-count="4" same-size-horizontally="false" same-size-vertically="false" hgap="-1" vgap="-1">
<grid id="16d1c" layout-manager="GridLayoutManager" row-count="1" column-count="2" same-size-horizontally="false" same-size-vertically="false" hgap="-1" vgap="-1">
<margin top="0" left="0" bottom="0" right="0"/>
<constraints>
<grid row="0" column="0" row-span="1" col-span="1" vsize-policy="1" hsize-policy="5" anchor="0" fill="3" indent="0" use-parent-layout="false"/>
@@ -30,25 +30,9 @@
</clientProperties>
<border type="none"/>
<children>
<component id="89f9b" class="com.intellij.ui.components.JBCheckBox" binding="myCheckHostname">
<constraints>
<grid row="0" column="1" row-span="1" col-span="1" vsize-policy="0" hsize-policy="0" anchor="8" fill="0" indent="0" use-parent-layout="false"/>
</constraints>
<properties>
<text value="Check &amp;hostname"/>
</properties>
</component>
<component id="fd372" class="com.intellij.ui.components.JBCheckBox" binding="myCheckValidityPeriod">
<constraints>
<grid row="0" column="2" row-span="1" col-span="1" vsize-policy="0" hsize-policy="0" anchor="8" fill="0" indent="0" use-parent-layout="false"/>
</constraints>
<properties>
<text value="Check &amp;validity period"/>
</properties>
</component>
<hspacer id="30a6f">
<constraints>
<grid row="0" column="3" row-span="1" col-span="1" vsize-policy="1" hsize-policy="6" anchor="0" fill="1" indent="0" use-parent-layout="false"/>
<grid row="0" column="1" row-span="1" col-span="1" vsize-policy="1" hsize-policy="6" anchor="0" fill="1" indent="0" use-parent-layout="false"/>
</constraints>
</hspacer>
<component id="2b8c2" class="com.intellij.ui.components.JBCheckBox" binding="myAcceptAutomatically">
@@ -22,14 +22,12 @@ import com.intellij.openapi.options.ConfigurationException;
import com.intellij.openapi.options.SearchableConfigurable;
import com.intellij.openapi.ui.Messages;
import com.intellij.openapi.util.Disposer;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.ui.AnActionButton;
import com.intellij.ui.AnActionButtonRunnable;
import com.intellij.ui.ToolbarDecorator;
import com.intellij.ui.components.JBCheckBox;
import com.intellij.ui.components.JBScrollPane;
import com.intellij.ui.treeStructure.Tree;
import com.intellij.util.Consumer;
import com.intellij.util.containers.HashSet;
import com.intellij.util.ui.UIUtil;
import org.jetbrains.annotations.Nls;
@@ -59,8 +57,6 @@ public class CertificateConfigurable implements SearchableConfigurable, Configur
private JPanel myRootPanel;
private JBCheckBox myAcceptAutomatically;
private JBCheckBox myCheckHostname;
private JBCheckBox myCheckValidityPeriod;
private JPanel myCertificatesListPanel;
private JPanel myDetailsPanel;
@@ -75,10 +71,6 @@ public class CertificateConfigurable implements SearchableConfigurable, Configur
myTree = new Tree();
myTreeBuilder = new CertificateTreeBuilder(myTree);
// are not fully functional by now
myCheckHostname.setVisible(false);
myCheckValidityPeriod.setVisible(false);
myTrustManager = CertificateManager.getInstance().getCustomTrustManager();
// show newly added certificates
myTrustManager.addListener(this);
@@ -187,8 +179,6 @@ public class CertificateConfigurable implements SearchableConfigurable, Configur
public boolean isModified() {
CertificateManager.Config state = CertificateManager.getInstance().getState();
return myAcceptAutomatically.isSelected() != state.ACCEPT_AUTOMATICALLY ||
myCheckHostname.isSelected() != state.CHECK_HOSTNAME ||
myCheckValidityPeriod.isSelected() != state.CHECK_VALIDITY ||
!myCertificates.equals(new HashSet<>(myTrustManager.getCertificates()));
}
@@ -216,8 +206,6 @@ public class CertificateConfigurable implements SearchableConfigurable, Configur
CertificateManager.Config state = CertificateManager.getInstance().getState();
state.ACCEPT_AUTOMATICALLY = myAcceptAutomatically.isSelected();
state.CHECK_HOSTNAME = myCheckHostname.isSelected();
state.CHECK_VALIDITY = myCheckValidityPeriod.isSelected();
}
@Override
@@ -242,8 +230,6 @@ public class CertificateConfigurable implements SearchableConfigurable, Configur
CertificateManager.Config state = CertificateManager.getInstance().getState();
myAcceptAutomatically.setSelected(state.ACCEPT_AUTOMATICALLY);
myCheckHostname.setSelected(state.CHECK_HOSTNAME);
myCheckValidityPeriod.setSelected(state.CHECK_VALIDITY);
}
@Override
@@ -28,9 +28,6 @@ import com.intellij.openapi.util.io.FileUtil;
import com.intellij.openapi.util.io.StreamUtil;
import com.intellij.openapi.util.registry.Registry;
import com.intellij.util.xmlb.XmlSerializerUtil;
import com.intellij.util.xmlb.annotations.AbstractCollection;
import com.intellij.util.xmlb.annotations.Property;
import com.intellij.util.xmlb.annotations.Tag;
import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
import org.jetbrains.annotations.NonNls;
import org.jetbrains.annotations.NotNull;
@@ -45,7 +42,6 @@ import java.security.KeyManagementException;
import java.security.KeyStore;
import java.security.KeyStoreException;
import java.security.NoSuchAlgorithmException;
import java.util.LinkedHashSet;
import java.util.concurrent.Callable;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
@@ -93,10 +89,15 @@ public class CertificateManager implements PersistentStateComponent<CertificateM
private static final Logger LOG = Logger.getInstance(CertificateManager.class);
/**
* Special version of hostname verifier, that asks user whether he accepts certificate, which subject's common name
* doesn't match requested hostname.
* Note that deprecated {@link org.apache.http.conn.ssl.BrowserCompatHostnameVerifier} is used intentionally here
* since external clients might expect implementor of {@link org.apache.http.conn.ssl.X509HostnameVerifier} and
* {@link org.apache.http.conn.ssl.DefaultHostnameVerifier} is not.
*
* @deprecated To be removed in IDEA 18. Use specific host name verifiers from httpclient-4.x instead.
*/
public static final HostnameVerifier HOSTNAME_VERIFIER = new ConfirmingHostnameVerifier(SSLConnectionSocketFactory.BROWSER_COMPATIBLE_HOSTNAME_VERIFIER);
@Deprecated
public static final HostnameVerifier HOSTNAME_VERIFIER = SSLConnectionSocketFactory.BROWSER_COMPATIBLE_HOSTNAME_VERIFIER;
/**
* Used to check whether dialog is visible to prevent possible deadlock, e.g. when some external resource is loaded by
* {@link java.awt.MediaTracker}.
@@ -352,20 +353,6 @@ public class CertificateManager implements PersistentStateComponent<CertificateM
}
public static class Config {
/**
* Ensure that request's hostname matches certificate's common name (CN).
*/
public boolean CHECK_HOSTNAME = false;
/**
* Ensure that certificate is neither expired nor not yet eligible.
*/
public boolean CHECK_VALIDITY = false;
@Tag("expired")
@Property(surroundWithTag = false)
@AbstractCollection(elementTag = "commonName")
public LinkedHashSet<String> BROKEN_CERTIFICATES = new LinkedHashSet<>();
/**
* Do not show the dialog and accept untrusted certificates automatically.
*/
@@ -30,14 +30,6 @@ public class CertificateWarningDialog extends DialogWrapper {
throw new UnsupportedOperationException("Not supported");
}
public static CertificateWarningDialog createHostnameMismatchWarning(@NotNull X509Certificate certificate,
@NotNull String hostname) {
String message = String.format("Server's certificate common name doesn't match hostname in URL: '%s' != '%s'",
new CertificateWrapper(certificate).getSubjectField(CertificateWrapper.CommonField.COMMON_NAME),
hostname);
return new CertificateWarningDialog(certificate, "Invalid hostname", message);
}
private JPanel myRootPanel;
private JLabel myWarningSign;
private JPanel myCertificateInfoPanel;
@@ -1,109 +0,0 @@
package com.intellij.util.net.ssl;
import com.intellij.openapi.ui.DialogWrapper;
import org.apache.http.conn.ssl.X509HostnameVerifier;
import org.jetbrains.annotations.NotNull;
import javax.net.ssl.SSLException;
import javax.net.ssl.SSLSession;
import javax.net.ssl.SSLSocket;
import java.io.IOException;
import java.io.InputStream;
import java.security.cert.Certificate;
import java.security.cert.X509Certificate;
import java.util.concurrent.Callable;
/**
* @author Mikhail Golubev
*/ /*
* BrowserCompatibleHostnameVerifier has all final/package-private methods, so direct
* inheritance from it makes no sense. Inheriting from AbstractVerifier also makes no sense, because
* the only method I can override verify(String, String[], String[]), and I have no access to certificate for the dialog
* in this case. Why the heck verify(String, X509Certificate) is final? It basically means, that I should copy half of the
* AbstractVerifier here, which gives me a strong feel of stupidity.
*
* I submit a bug about this problem https://issues.apache.org/jira/browse/HTTPCLIENT-1449 and it seems to be
* resolved in httpclient4.4.
*/
class ConfirmingHostnameVerifier implements X509HostnameVerifier {
private final X509HostnameVerifier myVerifier;
public ConfirmingHostnameVerifier(@NotNull X509HostnameVerifier verifier) {
myVerifier = verifier;
}
// Copied from httpclient 4.2 sources, read class level commentary for explanation.
@Override
public void verify(String host, SSLSocket ssl) throws IOException {
if (host == null) {
throw new NullPointerException("host to verify is null");
}
SSLSession session = ssl.getSession();
if (session == null) {
// In our experience this only happens under IBM 1.4.x when
// spurious (unrelated) certificates show up in the server'
// chain. Hopefully this will unearth the real problem:
final InputStream in = ssl.getInputStream();
in.available();
// If ssl.getInputStream().available() didn't cause an
// exception, maybe at least now the session is available?
session = ssl.getSession();
if (session == null) {
// If it's still null, probably a startHandshake() will
// unearth the real problem.
ssl.startHandshake();
// Okay, if we still haven't managed to cause an exception,
// might as well go for the NPE. Or maybe we're okay now?
session = ssl.getSession();
}
}
final Certificate[] certs = session.getPeerCertificates();
final X509Certificate x509 = (X509Certificate)certs[0];
verify(host, x509);
}
@Override
public void verify(final String host, final X509Certificate cert) throws SSLException {
if (!CertificateManager.getInstance().getState().CHECK_HOSTNAME) {
return;
}
try {
myVerifier.verify(host, cert);
}
catch (SSLException e) {
//noinspection ConstantConditions
if (!accepted(host, cert)) {
throw e;
}
// TODO: inclusion in some kind of persistent settings
// Read/Write lock to protect storage?
}
}
private static boolean accepted(final String host, final X509Certificate cert) {
return CertificateManager.showAcceptDialog(() -> CertificateWarningDialog.createHostnameMismatchWarning(cert, host));
}
// Copied from httpclient 4.2 sources, read class level commentary for explanation.
@Override
public boolean verify(String host, SSLSession session) {
try {
final Certificate[] certs = session.getPeerCertificates();
final X509Certificate x509 = (X509Certificate)certs[0];
verify(host, x509);
return true;
}
catch (final SSLException e) {
return false;
}
}
@Override
public void verify(String host, String[] cns, String[] subjectAlts) throws SSLException {
// actually never used, because it's only used in verify(final String host, final X509Certificate cert)
myVerifier.verify(host, cns, subjectAlts);
}
}
@@ -28,7 +28,6 @@ import com.intellij.util.PathUtilRt;
import com.intellij.util.Url;
import com.intellij.util.io.HttpRequests;
import com.intellij.util.io.RequestBuilder;
import com.intellij.util.net.ssl.CertificateManager;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.ide.BuiltInServerManager;
@@ -65,7 +64,6 @@ public class DefaultRemoteContentProvider extends RemoteContentProvider {
addRequestTuner(url, HttpRequests.request(url.toExternalForm()))
.connectTimeout(60 * 1000)
.productNameAsUserAgent()
.hostNameVerifier(CertificateManager.HOSTNAME_VERIFIER)
.connect(new HttpRequests.RequestProcessor<Object>() {
@Override
public Object process(@NotNull HttpRequests.Request request) throws IOException {
@@ -27,7 +27,6 @@ import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.protocol.HttpClientContext;
import org.apache.http.client.utils.URIBuilder;
import org.apache.http.config.ConnectionConfig;
import org.apache.http.conn.ssl.X509HostnameVerifier;
import org.apache.http.impl.auth.BasicScheme;
import org.apache.http.impl.client.BasicCredentialsProvider;
import org.apache.http.impl.client.CloseableHttpClient;
@@ -62,8 +61,7 @@ class GithubConnectionBuilder {
.setDefaultRequestConfig(createRequestConfig())
.setDefaultConnectionConfig(createConnectionConfig())
.setDefaultHeaders(createHeaders())
.setSslcontext(CertificateManager.getInstance().getSslContext())
.setHostnameVerifier((X509HostnameVerifier)CertificateManager.HOSTNAME_VERIFIER);
.setSslcontext(CertificateManager.getInstance().getSslContext());
setupCredentialsProvider(builder);
@@ -19,7 +19,6 @@ import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.HttpRequestBase;
import org.apache.http.client.protocol.HttpClientContext;
import org.apache.http.conn.routing.HttpRoute;
import org.apache.http.conn.ssl.X509HostnameVerifier;
import org.apache.http.impl.auth.BasicScheme;
import org.apache.http.impl.client.BasicCredentialsProvider;
import org.apache.http.impl.client.HttpClientBuilder;
@@ -63,9 +62,6 @@ public abstract class NewBaseRepositoryImpl extends BaseRepository {
HttpClientBuilder builder = HttpClients.custom()
.setDefaultRequestConfig(createRequestConfig())
.setSslcontext(CertificateManager.getInstance().getSslContext())
// TODO: use custom one for additional certificate check
//.setHostnameVerifier(SSLConnectionSocketFactory.ALLOW_ALL_HOSTNAME_VERIFIER)
.setHostnameVerifier((X509HostnameVerifier)CertificateManager.HOSTNAME_VERIFIER)
.setDefaultCredentialsProvider(createCredentialsProvider())
.addInterceptorFirst(PREEMPTIVE_BASIC_AUTH)
.addInterceptorLast(createRequestInterceptor());