Skip to content

Commit

Permalink
Deprecate RevocationStrategy and introduce RevocationCheckingStrategy
Browse files Browse the repository at this point in the history
The `RevocationStrategy` is an internal type, it has been superseded by `RevocationCheckingStrategy`.
  • Loading branch information
injectives committed Aug 3, 2022
1 parent e855bcc commit c220834
Show file tree
Hide file tree
Showing 10 changed files with 130 additions and 53 deletions.
33 changes: 29 additions & 4 deletions driver/src/main/java/org/neo4j/driver/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,7 @@ public enum Strategy {
private final Strategy strategy;
private final List<File> certFiles;
private boolean hostnameVerificationEnabled = true;
private RevocationStrategy revocationStrategy = RevocationStrategy.NO_CHECKS;
private RevocationCheckingStrategy revocationCheckingStrategy = RevocationCheckingStrategy.NO_CHECKS;

private TrustStrategy(Strategy strategy) {
this(strategy, Collections.emptyList());
Expand Down Expand Up @@ -882,8 +882,33 @@ public static TrustStrategy trustAllCertificates() {
/**
* The revocation strategy used for verifying certificates.
* @return this {@link TrustStrategy}'s revocation strategy
* @deprecated superseded by {@link TrustStrategy#revocationPolicy()}
*/
public RevocationCheckingStrategy revocationPolicy() {
return revocationCheckingStrategy;
}

/**
* The revocation strategy used for verifying certificates.
* @return this {@link TrustStrategy}'s revocation strategy
* @deprecated superseded by {@link TrustStrategy#revocationPolicy()}
*/
@Deprecated
public RevocationStrategy revocationStrategy() {
RevocationStrategy revocationStrategy;
switch (this.revocationCheckingStrategy) {
case VERIFY_IF_PRESENT:
revocationStrategy = RevocationStrategy.VERIFY_IF_PRESENT;
break;
case STRICT:
revocationStrategy = RevocationStrategy.STRICT;
break;
case NO_CHECKS:
revocationStrategy = RevocationStrategy.NO_CHECKS;
break;
default:
throw new IllegalStateException("Failed to map RevocationStrategy to RevocationStrategy.");
}
return revocationStrategy;
}

Expand All @@ -893,7 +918,7 @@ public RevocationStrategy revocationStrategy() {
* @return the current trust strategy
*/
public TrustStrategy withoutCertificateRevocationChecks() {
this.revocationStrategy = RevocationStrategy.NO_CHECKS;
this.revocationCheckingStrategy = RevocationCheckingStrategy.NO_CHECKS;
return this;
}

Expand All @@ -905,7 +930,7 @@ public TrustStrategy withoutCertificateRevocationChecks() {
* @return the current trust strategy
*/
public TrustStrategy withVerifyIfPresentRevocationChecks() {
this.revocationStrategy = RevocationStrategy.VERIFY_IF_PRESENT;
this.revocationCheckingStrategy = RevocationCheckingStrategy.VERIFY_IF_PRESENT;
return this;
}

Expand All @@ -919,7 +944,7 @@ public TrustStrategy withVerifyIfPresentRevocationChecks() {
* @return the current trust strategy
*/
public TrustStrategy withStrictRevocationChecks() {
this.revocationStrategy = RevocationStrategy.STRICT;
this.revocationCheckingStrategy = RevocationCheckingStrategy.STRICT;
return this;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright (c) "Neo4j"
* Neo4j Sweden AB [http://neo4j.com]
*
* This file is part of Neo4j.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.neo4j.driver;

/**
* Defines policy for revocation checks.
*/
public enum RevocationCheckingStrategy {
/** Don't do any OCSP revocation checks, regardless whether there are stapled revocation statuses or not. */
NO_CHECKS,
/** Verify OCSP revocation checks when the revocation status is stapled to the certificate, continue if not. */
VERIFY_IF_PRESENT,
/** Require stapled revocation status and verify OCSP revocation checks, fail if no revocation status is stapled to the certificate. */
STRICT;

public static boolean requiresRevocationChecking(RevocationCheckingStrategy revocationCheckingStrategy) {
return revocationCheckingStrategy.equals(STRICT) || revocationCheckingStrategy.equals(VERIFY_IF_PRESENT);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@
*/
package org.neo4j.driver.internal;

import org.neo4j.driver.RevocationCheckingStrategy;

/**
* Defines strategy for revocation checks.
*
* @deprecated superseded by {@link RevocationCheckingStrategy}
*/
@Deprecated
public enum RevocationStrategy {
/** Don't do any OCSP revocation checks, regardless whether there are stapled revocation statuses or not. */
NO_CHECKS,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.io.Serializable;
import java.security.GeneralSecurityException;
import org.neo4j.driver.Config;
import org.neo4j.driver.RevocationCheckingStrategy;
import org.neo4j.driver.exceptions.ClientException;
import org.neo4j.driver.internal.security.SecurityPlan;
import org.neo4j.driver.internal.security.SecurityPlanImpl;
Expand Down Expand Up @@ -56,6 +57,7 @@ private boolean isCustomized() {
return !(DEFAULT.encrypted() == this.encrypted() && DEFAULT.hasEqualTrustStrategy(this));
}

@SuppressWarnings("deprecation")
private boolean hasEqualTrustStrategy(SecuritySettings other) {
Config.TrustStrategy t1 = this.trustStrategy;
Config.TrustStrategy t2 = other.trustStrategy;
Expand All @@ -66,6 +68,7 @@ private boolean hasEqualTrustStrategy(SecuritySettings other) {
return t1.isHostnameVerificationEnabled() == t2.isHostnameVerificationEnabled()
&& t1.strategy() == t2.strategy()
&& t1.certFiles().equals(t2.certFiles())
&& t1.revocationPolicy() == t2.revocationPolicy()
&& t1.revocationStrategy() == t2.revocationStrategy();
}

Expand All @@ -92,9 +95,9 @@ private void assertSecuritySettingsNotUserConfigured(String uriScheme) {

private SecurityPlan createSecurityPlanFromScheme(String scheme) throws GeneralSecurityException, IOException {
if (isHighTrustScheme(scheme)) {
return SecurityPlanImpl.forSystemCASignedCertificates(true, RevocationStrategy.NO_CHECKS);
return SecurityPlanImpl.forSystemCASignedCertificates(true, RevocationCheckingStrategy.NO_CHECKS);
} else {
return SecurityPlanImpl.forAllCertificates(false, RevocationStrategy.NO_CHECKS);
return SecurityPlanImpl.forAllCertificates(false, RevocationCheckingStrategy.NO_CHECKS);
}
}

Expand All @@ -106,16 +109,16 @@ private static SecurityPlan createSecurityPlanImpl(boolean encrypted, Config.Tru
throws GeneralSecurityException, IOException {
if (encrypted) {
boolean hostnameVerificationEnabled = trustStrategy.isHostnameVerificationEnabled();
RevocationStrategy revocationStrategy = trustStrategy.revocationStrategy();
RevocationCheckingStrategy revocationCheckingStrategy = trustStrategy.revocationPolicy();
switch (trustStrategy.strategy()) {
case TRUST_CUSTOM_CA_SIGNED_CERTIFICATES:
return SecurityPlanImpl.forCustomCASignedCertificates(
trustStrategy.certFiles(), hostnameVerificationEnabled, revocationStrategy);
trustStrategy.certFiles(), hostnameVerificationEnabled, revocationCheckingStrategy);
case TRUST_SYSTEM_CA_SIGNED_CERTIFICATES:
return SecurityPlanImpl.forSystemCASignedCertificates(
hostnameVerificationEnabled, revocationStrategy);
hostnameVerificationEnabled, revocationCheckingStrategy);
case TRUST_ALL_CERTIFICATES:
return SecurityPlanImpl.forAllCertificates(hostnameVerificationEnabled, revocationStrategy);
return SecurityPlanImpl.forAllCertificates(hostnameVerificationEnabled, revocationCheckingStrategy);
default:
throw new ClientException("Unknown TLS authentication strategy: "
+ trustStrategy.strategy().name());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
package org.neo4j.driver.internal.security;

import javax.net.ssl.SSLContext;
import org.neo4j.driver.internal.RevocationStrategy;
import org.neo4j.driver.RevocationCheckingStrategy;

/**
* A SecurityPlan consists of encryption and trust details.
Expand All @@ -31,5 +31,5 @@ public interface SecurityPlan {

boolean requiresHostnameVerification();

RevocationStrategy revocationStrategy();
RevocationCheckingStrategy revocationPolicy();
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
*/
package org.neo4j.driver.internal.security;

import static org.neo4j.driver.internal.RevocationStrategy.VERIFY_IF_PRESENT;
import static org.neo4j.driver.internal.RevocationStrategy.requiresRevocationChecking;
import static org.neo4j.driver.RevocationCheckingStrategy.VERIFY_IF_PRESENT;
import static org.neo4j.driver.RevocationCheckingStrategy.requiresRevocationChecking;
import static org.neo4j.driver.internal.util.CertificateTool.loadX509Cert;

import java.io.File;
Expand All @@ -41,36 +41,39 @@
import javax.net.ssl.TrustManager;
import javax.net.ssl.TrustManagerFactory;
import javax.net.ssl.X509TrustManager;
import org.neo4j.driver.internal.RevocationStrategy;
import org.neo4j.driver.RevocationCheckingStrategy;

/**
* A SecurityPlan consists of encryption and trust details.
*/
public class SecurityPlanImpl implements SecurityPlan {
public static SecurityPlan forAllCertificates(
boolean requiresHostnameVerification, RevocationStrategy revocationStrategy)
boolean requiresHostnameVerification, RevocationCheckingStrategy revocationCheckingStrategy)
throws GeneralSecurityException {
SSLContext sslContext = SSLContext.getInstance("TLS");
sslContext.init(new KeyManager[0], new TrustManager[] {new TrustAllTrustManager()}, null);

return new SecurityPlanImpl(true, sslContext, requiresHostnameVerification, revocationStrategy);
return new SecurityPlanImpl(true, sslContext, requiresHostnameVerification, revocationCheckingStrategy);
}

public static SecurityPlan forCustomCASignedCertificates(
List<File> certFiles, boolean requiresHostnameVerification, RevocationStrategy revocationStrategy)
List<File> certFiles,
boolean requiresHostnameVerification,
RevocationCheckingStrategy revocationCheckingStrategy)
throws GeneralSecurityException, IOException {
SSLContext sslContext = configureSSLContext(certFiles, revocationStrategy);
return new SecurityPlanImpl(true, sslContext, requiresHostnameVerification, revocationStrategy);
SSLContext sslContext = configureSSLContext(certFiles, revocationCheckingStrategy);
return new SecurityPlanImpl(true, sslContext, requiresHostnameVerification, revocationCheckingStrategy);
}

public static SecurityPlan forSystemCASignedCertificates(
boolean requiresHostnameVerification, RevocationStrategy revocationStrategy)
boolean requiresHostnameVerification, RevocationCheckingStrategy revocationCheckingStrategy)
throws GeneralSecurityException, IOException {
SSLContext sslContext = configureSSLContext(Collections.emptyList(), revocationStrategy);
return new SecurityPlanImpl(true, sslContext, requiresHostnameVerification, revocationStrategy);
SSLContext sslContext = configureSSLContext(Collections.emptyList(), revocationCheckingStrategy);
return new SecurityPlanImpl(true, sslContext, requiresHostnameVerification, revocationCheckingStrategy);
}

private static SSLContext configureSSLContext(List<File> customCertFiles, RevocationStrategy revocationStrategy)
private static SSLContext configureSSLContext(
List<File> customCertFiles, RevocationCheckingStrategy revocationCheckingStrategy)
throws GeneralSecurityException, IOException {
KeyStore trustedKeyStore = KeyStore.getInstance(KeyStore.getDefaultType());
trustedKeyStore.load(null, null);
Expand All @@ -83,7 +86,7 @@ private static SSLContext configureSSLContext(List<File> customCertFiles, Revoca
}

PKIXBuilderParameters pkixBuilderParameters =
configurePKIXBuilderParameters(trustedKeyStore, revocationStrategy);
configurePKIXBuilderParameters(trustedKeyStore, revocationCheckingStrategy);

SSLContext sslContext = SSLContext.getInstance("TLS");
TrustManagerFactory trustManagerFactory =
Expand All @@ -101,11 +104,11 @@ private static SSLContext configureSSLContext(List<File> customCertFiles, Revoca
}

private static PKIXBuilderParameters configurePKIXBuilderParameters(
KeyStore trustedKeyStore, RevocationStrategy revocationStrategy)
KeyStore trustedKeyStore, RevocationCheckingStrategy revocationCheckingStrategy)
throws InvalidAlgorithmParameterException, KeyStoreException {
PKIXBuilderParameters pkixBuilderParameters = null;

if (requiresRevocationChecking(revocationStrategy)) {
if (requiresRevocationChecking(revocationCheckingStrategy)) {
// Configure certificate revocation checking (X509CertSelector() selects all certificates)
pkixBuilderParameters = new PKIXBuilderParameters(trustedKeyStore, new X509CertSelector());

Expand All @@ -115,7 +118,7 @@ private static PKIXBuilderParameters configurePKIXBuilderParameters(
// enables status_request extension in client hello
System.setProperty("jdk.tls.client.enableStatusRequestExtension", "true");

if (revocationStrategy.equals(VERIFY_IF_PRESENT)) {
if (revocationCheckingStrategy.equals(VERIFY_IF_PRESENT)) {
// enables soft-fail behaviour if no stapled response found.
Security.setProperty("ocsp.enable", "true");
}
Expand Down Expand Up @@ -146,23 +149,23 @@ private static void loadSystemCertificates(KeyStore trustedKeyStore) throws Gene
}

public static SecurityPlan insecure() {
return new SecurityPlanImpl(false, null, false, RevocationStrategy.NO_CHECKS);
return new SecurityPlanImpl(false, null, false, RevocationCheckingStrategy.NO_CHECKS);
}

private final boolean requiresEncryption;
private final SSLContext sslContext;
private final boolean requiresHostnameVerification;
private final RevocationStrategy revocationStrategy;
private final RevocationCheckingStrategy revocationCheckingStrategy;

private SecurityPlanImpl(
boolean requiresEncryption,
SSLContext sslContext,
boolean requiresHostnameVerification,
RevocationStrategy revocationStrategy) {
RevocationCheckingStrategy revocationCheckingStrategy) {
this.requiresEncryption = requiresEncryption;
this.sslContext = sslContext;
this.requiresHostnameVerification = requiresHostnameVerification;
this.revocationStrategy = revocationStrategy;
this.revocationCheckingStrategy = revocationCheckingStrategy;
}

@Override
Expand All @@ -181,8 +184,8 @@ public boolean requiresHostnameVerification() {
}

@Override
public RevocationStrategy revocationStrategy() {
return revocationStrategy;
public RevocationCheckingStrategy revocationPolicy() {
return revocationCheckingStrategy;
}

private static class TrustAllTrustManager implements X509TrustManager {
Expand Down
17 changes: 10 additions & 7 deletions driver/src/test/java/org/neo4j/driver/ConfigTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.neo4j.driver.internal.RevocationStrategy.NO_CHECKS;
import static org.neo4j.driver.internal.RevocationStrategy.STRICT;
import static org.neo4j.driver.internal.RevocationStrategy.VERIFY_IF_PRESENT;
import static org.neo4j.driver.internal.handlers.pulln.FetchSizeUtil.DEFAULT_FETCH_SIZE;

import java.io.File;
Expand All @@ -44,6 +41,7 @@
import org.junit.jupiter.params.provider.ValueSource;
import org.junit.platform.commons.support.HierarchyTraversalMode;
import org.junit.platform.commons.support.ReflectionSupport;
import org.neo4j.driver.internal.RevocationStrategy;
import org.neo4j.driver.internal.logging.ConsoleLogging;
import org.neo4j.driver.internal.logging.DevNullLogging;
import org.neo4j.driver.internal.logging.JULogging;
Expand Down Expand Up @@ -279,19 +277,24 @@ void shouldEnableAndDisableHostnameVerificationOnTrustStrategy() {
assertFalse(trustStrategy.isHostnameVerificationEnabled());
}

@SuppressWarnings("deprecation")
@Test
void shouldEnableAndDisableCertificateRevocationChecksOnTestStrategy() {
Config.TrustStrategy trustStrategy = Config.TrustStrategy.trustSystemCertificates();
assertEquals(NO_CHECKS, trustStrategy.revocationStrategy());
assertEquals(RevocationCheckingStrategy.NO_CHECKS, trustStrategy.revocationPolicy());
assertEquals(RevocationStrategy.NO_CHECKS, trustStrategy.revocationStrategy());

assertSame(trustStrategy, trustStrategy.withoutCertificateRevocationChecks());
assertEquals(NO_CHECKS, trustStrategy.revocationStrategy());
assertEquals(RevocationCheckingStrategy.NO_CHECKS, trustStrategy.revocationPolicy());
assertEquals(RevocationStrategy.NO_CHECKS, trustStrategy.revocationStrategy());

assertSame(trustStrategy, trustStrategy.withStrictRevocationChecks());
assertEquals(STRICT, trustStrategy.revocationStrategy());
assertEquals(RevocationCheckingStrategy.STRICT, trustStrategy.revocationPolicy());
assertEquals(RevocationStrategy.STRICT, trustStrategy.revocationStrategy());

assertSame(trustStrategy, trustStrategy.withVerifyIfPresentRevocationChecks());
assertEquals(VERIFY_IF_PRESENT, trustStrategy.revocationStrategy());
assertEquals(RevocationCheckingStrategy.VERIFY_IF_PRESENT, trustStrategy.revocationPolicy());
assertEquals(RevocationStrategy.VERIFY_IF_PRESENT, trustStrategy.revocationStrategy());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@
import org.junit.jupiter.api.extension.RegisterExtension;
import org.neo4j.driver.AuthToken;
import org.neo4j.driver.AuthTokens;
import org.neo4j.driver.RevocationCheckingStrategy;
import org.neo4j.driver.exceptions.AuthenticationException;
import org.neo4j.driver.exceptions.ServiceUnavailableException;
import org.neo4j.driver.internal.BoltServerAddress;
import org.neo4j.driver.internal.ConnectionSettings;
import org.neo4j.driver.internal.DefaultDomainNameResolver;
import org.neo4j.driver.internal.RevocationStrategy;
import org.neo4j.driver.internal.async.connection.BootstrapFactory;
import org.neo4j.driver.internal.async.connection.ChannelConnector;
import org.neo4j.driver.internal.async.connection.ChannelConnectorImpl;
Expand Down Expand Up @@ -222,6 +222,6 @@ private ChannelConnectorImpl newConnector(
}

private static SecurityPlan trustAllCertificates() throws GeneralSecurityException {
return SecurityPlanImpl.forAllCertificates(false, RevocationStrategy.NO_CHECKS);
return SecurityPlanImpl.forAllCertificates(false, RevocationCheckingStrategy.NO_CHECKS);
}
}
Loading

0 comments on commit c220834

Please sign in to comment.