This is an automated email from the ASF dual-hosted git repository. apkhmv pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push: new 8875dc414b IGNITE-18850 Add ssl configuration validation (#1738) 8875dc414b is described below commit 8875dc414b00fe6e8f42a05eece63ac5eabad5b0 Author: Vadim Pakhnushev <8614891+valep...@users.noreply.github.com> AuthorDate: Mon Mar 6 14:20:37 2023 +0300 IGNITE-18850 Add ssl configuration validation (#1738) --- .../validation/PowerOfTwoValidatorTest.java | 4 +- .../validation/RangeValidatorTest.java | 22 ++-- .../validation/TestValidationUtil.java | 37 ++++-- .../AbstractSslConfigurationSchema.java | 2 - .../KeyStoreConfigurationValidatorImpl.java | 57 --------- .../configuration/NetworkConfigurationSchema.java | 1 + ...lidator.java => SslConfigurationValidator.java} | 4 +- .../SslConfigurationValidatorImpl.java | 76 +++++++++++ .../KeyStoreConfigurationValidatorImplTest.java | 77 ------------ .../SslConfigurationValidatorImplTest.java | 127 +++++++++++++++++++ .../network/configuration/StubSslView.java} | 44 +++++-- .../apache/ignite/internal/rest/RestComponent.java | 72 ++--------- .../configuration/RestConfigurationModule.java | 4 +- .../ignite/internal/rest/RestComponentTest.java | 139 --------------------- .../KnownDataStorageValidatorTest.java | 2 +- .../configuration/TableValidatorImplTest.java | 2 +- .../ExistingDataStorageValidatorTest.java | 6 +- 17 files changed, 303 insertions(+), 373 deletions(-) diff --git a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/validation/PowerOfTwoValidatorTest.java b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/validation/PowerOfTwoValidatorTest.java index df7f3a8934..035c795153 100644 --- a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/validation/PowerOfTwoValidatorTest.java +++ b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/validation/PowerOfTwoValidatorTest.java @@ -34,8 +34,8 @@ public class PowerOfTwoValidatorTest { PowerOfTwoValidator validator = new PowerOfTwoValidator(); - validate(validator, powerOfTwo, mockValidationContext(null, 1), null); - validate(validator, powerOfTwo, mockValidationContext(null, 1L), null); + validate(validator, powerOfTwo, mockValidationContext(null, 1)); + validate(validator, powerOfTwo, mockValidationContext(null, 1L)); } @Test diff --git a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/validation/RangeValidatorTest.java b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/validation/RangeValidatorTest.java index e27a9a5123..c7a0067ab3 100644 --- a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/validation/RangeValidatorTest.java +++ b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/validation/RangeValidatorTest.java @@ -36,23 +36,23 @@ public class RangeValidatorTest { RangeValidator validator = new RangeValidator(); - validate(validator, range0, mockValidationContext(null, 0), null); - validate(validator, range0, mockValidationContext(null, 50), null); - validate(validator, range0, mockValidationContext(null, 100), null); + validate(validator, range0, mockValidationContext(null, 0)); + validate(validator, range0, mockValidationContext(null, 50)); + validate(validator, range0, mockValidationContext(null, 100)); Range range1 = createRange(0L, null); - validate(validator, range1, mockValidationContext(null, 0), null); - validate(validator, range1, mockValidationContext(null, 50), null); - validate(validator, range1, mockValidationContext(null, 100), null); - validate(validator, range1, mockValidationContext(null, Long.MAX_VALUE), null); + validate(validator, range1, mockValidationContext(null, 0)); + validate(validator, range1, mockValidationContext(null, 50)); + validate(validator, range1, mockValidationContext(null, 100)); + validate(validator, range1, mockValidationContext(null, Long.MAX_VALUE)); Range range2 = createRange(null, 100L); - validate(validator, range2, mockValidationContext(null, 0), null); - validate(validator, range2, mockValidationContext(null, 50), null); - validate(validator, range2, mockValidationContext(null, 100), null); - validate(validator, range2, mockValidationContext(null, Long.MIN_VALUE), null); + validate(validator, range2, mockValidationContext(null, 0)); + validate(validator, range2, mockValidationContext(null, 50)); + validate(validator, range2, mockValidationContext(null, 100)); + validate(validator, range2, mockValidationContext(null, Long.MIN_VALUE)); } @Test diff --git a/modules/configuration/src/testFixtures/java/org/apache/ignite/internal/configuration/validation/TestValidationUtil.java b/modules/configuration/src/testFixtures/java/org/apache/ignite/internal/configuration/validation/TestValidationUtil.java index 1f6ca35d26..7b7d22ae50 100644 --- a/modules/configuration/src/testFixtures/java/org/apache/ignite/internal/configuration/validation/TestValidationUtil.java +++ b/modules/configuration/src/testFixtures/java/org/apache/ignite/internal/configuration/validation/TestValidationUtil.java @@ -18,17 +18,21 @@ package org.apache.ignite.internal.configuration.validation; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.empty; -import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.startsWith; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import java.lang.annotation.Annotation; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; import org.apache.ignite.configuration.validation.ValidationContext; import org.apache.ignite.configuration.validation.ValidationIssue; import org.apache.ignite.configuration.validation.Validator; +import org.hamcrest.Matcher; +import org.hamcrest.Matchers; import org.jetbrains.annotations.Nullable; import org.mockito.ArgumentCaptor; @@ -68,30 +72,49 @@ public class TestValidationUtil { return issuesCaptor; } + /** + * Checks that validation completed without errors. + * + * @param validator Checked validator. + * @param annotation Mocked annotation. + * @param ctx Mocked validation context. + */ + public static <A extends Annotation, VIEWT> void validate( + Validator<A, VIEWT> validator, + A annotation, + ValidationContext<VIEWT> ctx + ) { + validate(validator, annotation, ctx, (String[]) null); + } + /** * Performs validation checking whether there are errors. * * @param validator Checked validator. * @param annotation Mocked annotation. * @param ctx Mocked validation context. - * @param errorMessagePrefix Error prefix, if {@code null} it is expected that there will be no errors. + * @param errorMessagePrefixes Error prefixes, if {@code null} it is expected that there will be no errors. */ public static <A extends Annotation, VIEWT> void validate( Validator<A, VIEWT> validator, A annotation, ValidationContext<VIEWT> ctx, - @Nullable String errorMessagePrefix + String @Nullable ... errorMessagePrefixes ) { ArgumentCaptor<ValidationIssue> argumentCaptor = addIssuesCaptor(ctx); validator.validate(annotation, ctx); - if (errorMessagePrefix == null) { + if (errorMessagePrefixes == null) { assertThat(argumentCaptor.getAllValues(), empty()); } else { - assertThat(argumentCaptor.getAllValues(), hasSize(1)); + List<String> messages = argumentCaptor.getAllValues().stream() + .map(ValidationIssue::message).collect(Collectors.toList()); + + List<Matcher<? super String>> matchers = Arrays.stream(errorMessagePrefixes) + .map(Matchers::startsWith).collect(Collectors.toList()); - assertThat(argumentCaptor.getValue().message(), startsWith(errorMessagePrefix)); + assertThat(messages, contains(matchers)); } } } diff --git a/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/AbstractSslConfigurationSchema.java b/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/AbstractSslConfigurationSchema.java index 2baf3a7fdf..958a28732d 100644 --- a/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/AbstractSslConfigurationSchema.java +++ b/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/AbstractSslConfigurationSchema.java @@ -44,11 +44,9 @@ public class AbstractSslConfigurationSchema { /** SSL keystore configuration. */ @ConfigValue - @KeyStoreConfigurationValidator public KeyStoreConfigurationSchema keyStore; /** SSL truststore configuration. */ @ConfigValue - @KeyStoreConfigurationValidator public KeyStoreConfigurationSchema trustStore; } diff --git a/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/KeyStoreConfigurationValidatorImpl.java b/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/KeyStoreConfigurationValidatorImpl.java deleted file mode 100644 index 61ffbc7c02..0000000000 --- a/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/KeyStoreConfigurationValidatorImpl.java +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You 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.apache.ignite.internal.network.configuration; - -import static org.apache.ignite.internal.util.StringUtils.nullOrBlank; - -import org.apache.ignite.configuration.validation.ValidationContext; -import org.apache.ignite.configuration.validation.ValidationIssue; -import org.apache.ignite.configuration.validation.Validator; - -/** - * Key store configuration validator implementation. - */ -public class KeyStoreConfigurationValidatorImpl implements Validator<KeyStoreConfigurationValidator, KeyStoreView> { - - public static final KeyStoreConfigurationValidatorImpl INSTANCE = new KeyStoreConfigurationValidatorImpl(); - - @Override - public void validate(KeyStoreConfigurationValidator annotation, ValidationContext<KeyStoreView> ctx) { - KeyStoreView keyStore = ctx.getNewValue(); - String type = keyStore.type(); - String path = keyStore.path(); - String password = keyStore.password(); - if (nullOrBlank(path) && nullOrBlank(password)) { - return; - } else { - if (nullOrBlank(type)) { - ctx.addIssue(new ValidationIssue( - ctx.currentKey(), - "Key store type must not be blank" - )); - } - - if (nullOrBlank(path)) { - ctx.addIssue(new ValidationIssue( - ctx.currentKey(), - "Key store path must not be blank" - )); - } - } - } -} diff --git a/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/NetworkConfigurationSchema.java b/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/NetworkConfigurationSchema.java index 3ab3a6041e..611092171e 100644 --- a/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/NetworkConfigurationSchema.java +++ b/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/NetworkConfigurationSchema.java @@ -73,5 +73,6 @@ public class NetworkConfigurationSchema { /** SSL configuration.*/ @ConfigValue + @SslConfigurationValidator public SslConfigurationSchema ssl; } diff --git a/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/KeyStoreConfigurationValidator.java b/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/SslConfigurationValidator.java similarity index 91% copy from modules/network/src/main/java/org/apache/ignite/internal/network/configuration/KeyStoreConfigurationValidator.java copy to modules/network/src/main/java/org/apache/ignite/internal/network/configuration/SslConfigurationValidator.java index e96b9bd2be..6bd8de49cb 100644 --- a/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/KeyStoreConfigurationValidator.java +++ b/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/SslConfigurationValidator.java @@ -23,9 +23,9 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; /** - * Annotation to validate whole key store configuration. + * Annotation to validate SSL configuration. */ @Target(ElementType.FIELD) @Retention(RetentionPolicy.RUNTIME) -public @interface KeyStoreConfigurationValidator { +public @interface SslConfigurationValidator { } diff --git a/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/SslConfigurationValidatorImpl.java b/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/SslConfigurationValidatorImpl.java new file mode 100644 index 0000000000..03b8dbcb66 --- /dev/null +++ b/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/SslConfigurationValidatorImpl.java @@ -0,0 +1,76 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.ignite.internal.network.configuration; + +import static org.apache.ignite.internal.util.StringUtils.nullOrBlank; + +import io.netty.handler.ssl.ClientAuth; +import java.nio.file.Files; +import java.nio.file.InvalidPathException; +import java.nio.file.Path; +import org.apache.ignite.configuration.validation.ValidationContext; +import org.apache.ignite.configuration.validation.ValidationIssue; +import org.apache.ignite.configuration.validation.Validator; + +/** + * SSL configuration validator implementation. + */ +public class SslConfigurationValidatorImpl implements Validator<SslConfigurationValidator, SslView> { + + public static final SslConfigurationValidatorImpl INSTANCE = new SslConfigurationValidatorImpl(); + + @Override + public void validate(SslConfigurationValidator annotation, ValidationContext<SslView> ctx) { + SslView ssl = ctx.getNewValue(); + if (ssl.enabled()) { + validateKeyStore(ctx, ".keyStore", "Key store", ssl.keyStore()); + + try { + ClientAuth clientAuth = ClientAuth.valueOf(ssl.clientAuth().toUpperCase()); + if (clientAuth != ClientAuth.NONE) { + validateKeyStore(ctx, ".trustStore", "Trust store", ssl.trustStore()); + } + } catch (IllegalArgumentException e) { + ctx.addIssue(new ValidationIssue(ctx.currentKey(), "Incorrect client auth parameter " + ssl.clientAuth())); + } + } + } + + private static void validateKeyStore(ValidationContext<SslView> ctx, String keyName, String type, KeyStoreView keyStore) { + String keyStorePath = keyStore.path(); + if (nullOrBlank(keyStorePath) && nullOrBlank(keyStore.password())) { + return; + } + + if (nullOrBlank(keyStore.type())) { + ctx.addIssue(new ValidationIssue(ctx.currentKey() + keyName, type + " type must not be blank")); + } + + if (nullOrBlank(keyStorePath)) { + ctx.addIssue(new ValidationIssue(ctx.currentKey() + keyName, type + " path must not be blank")); + } else { + try { + if (!Files.exists(Path.of(keyStorePath))) { + ctx.addIssue(new ValidationIssue(ctx.currentKey() + keyName, type + " file doesn't exist at " + keyStorePath)); + } + } catch (InvalidPathException e) { + ctx.addIssue(new ValidationIssue(ctx.currentKey() + keyName, type + " file path is incorrect: " + keyStorePath)); + } + } + } +} diff --git a/modules/network/src/test/java/org/apache/ignite/internal/network/configuration/KeyStoreConfigurationValidatorImplTest.java b/modules/network/src/test/java/org/apache/ignite/internal/network/configuration/KeyStoreConfigurationValidatorImplTest.java deleted file mode 100644 index 9399660210..0000000000 --- a/modules/network/src/test/java/org/apache/ignite/internal/network/configuration/KeyStoreConfigurationValidatorImplTest.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You 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.apache.ignite.internal.network.configuration; - -import static org.apache.ignite.internal.configuration.validation.TestValidationUtil.mockValidationContext; -import static org.apache.ignite.internal.configuration.validation.TestValidationUtil.validate; -import static org.mockito.Mockito.mock; - -import org.apache.ignite.configuration.validation.ValidationContext; -import org.junit.jupiter.api.Test; - -class KeyStoreConfigurationValidatorImplTest { - - @Test - public void nullPath() { - ValidationContext<KeyStoreView> ctx = mockValidationContext( - null, - new StubKeyStoreView("PKCS12", null, "changeIt") - ); - validate(KeyStoreConfigurationValidatorImpl.INSTANCE, mock(KeyStoreConfigurationValidator.class), ctx, - "Key store path must not be blank"); - } - - @Test - public void emptyPath() { - ValidationContext<KeyStoreView> ctx = mockValidationContext( - null, - new StubKeyStoreView("PKCS12", "", "changeIt") - ); - validate(KeyStoreConfigurationValidatorImpl.INSTANCE, mock(KeyStoreConfigurationValidator.class), ctx, - "Key store path must not be blank"); - } - - @Test - public void nullType() { - ValidationContext<KeyStoreView> ctx = mockValidationContext( - null, - new StubKeyStoreView(null, "/path/to/keystore.p12", null) - ); - validate(KeyStoreConfigurationValidatorImpl.INSTANCE, mock(KeyStoreConfigurationValidator.class), ctx, - "Key store type must not be blank"); - } - - @Test - public void emptyType() { - ValidationContext<KeyStoreView> ctx = mockValidationContext( - null, - new StubKeyStoreView("", "/path/to/keystore.p12", null) - ); - validate(KeyStoreConfigurationValidatorImpl.INSTANCE, mock(KeyStoreConfigurationValidator.class), ctx, - "Key store type must not be blank"); - } - - @Test - public void validConfig() { - ValidationContext<KeyStoreView> ctx = mockValidationContext( - null, - new StubKeyStoreView("PKCS12", "/path/to/keystore.p12", null) - ); - validate(KeyStoreConfigurationValidatorImpl.INSTANCE, mock(KeyStoreConfigurationValidator.class), ctx, null); - } -} diff --git a/modules/network/src/test/java/org/apache/ignite/internal/network/configuration/SslConfigurationValidatorImplTest.java b/modules/network/src/test/java/org/apache/ignite/internal/network/configuration/SslConfigurationValidatorImplTest.java new file mode 100644 index 0000000000..b05d26d414 --- /dev/null +++ b/modules/network/src/test/java/org/apache/ignite/internal/network/configuration/SslConfigurationValidatorImplTest.java @@ -0,0 +1,127 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.ignite.internal.network.configuration; + +import static org.apache.ignite.internal.configuration.validation.TestValidationUtil.mockValidationContext; +import static org.mockito.Mockito.mock; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import org.apache.ignite.internal.configuration.validation.TestValidationUtil; +import org.apache.ignite.internal.testframework.WorkDirectory; +import org.apache.ignite.internal.testframework.WorkDirectoryExtension; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +@ExtendWith(WorkDirectoryExtension.class) +class SslConfigurationValidatorImplTest { + + @Test + public void nullKeyStorePath() { + validate(createKeyStoreConfig("PKCS12", null, "changeIt"), + "Key store path must not be blank"); + } + + @Test + public void emptyKeyStorePath() { + validate(createKeyStoreConfig("PKCS12", "", "changeIt"), + "Key store path must not be blank"); + } + + @Test + public void nullKeyStoreType() { + validate(createKeyStoreConfig(null, "/path/to/keystore.p12", null), + "Key store type must not be blank", "Key store file doesn't exist at /path/to/keystore.p12"); + } + + @Test + public void blankKeyStoreType() { + validate(createKeyStoreConfig("", "/path/to/keystore.p12", null), + "Key store type must not be blank", "Key store file doesn't exist at /path/to/keystore.p12"); + } + + @Test + public void nullTrustStorePath(@WorkDirectory Path workDir) throws IOException { + validate(createTrustStoreConfig(workDir, "PKCS12", null, "changeIt"), + "Trust store path must not be blank"); + } + + @Test + public void emptyTrustStorePath(@WorkDirectory Path workDir) throws IOException { + validate(createTrustStoreConfig(workDir, "PKCS12", "", "changeIt"), + "Trust store path must not be blank"); + } + + @Test + public void nullTrustStoreType(@WorkDirectory Path workDir) throws IOException { + validate(createTrustStoreConfig(workDir, null, "/path/to/keystore.p12", null), + "Trust store type must not be blank", "Trust store file doesn't exist at /path/to/keystore.p12"); + } + + @Test + public void blankTrustStoreType(@WorkDirectory Path workDir) throws IOException { + validate(createTrustStoreConfig(workDir, "", "/path/to/keystore.p12", null), + "Trust store type must not be blank", "Trust store file doesn't exist at /path/to/keystore.p12"); + } + + @Test + public void incorrectAuthType(@WorkDirectory Path workDir) throws IOException { + KeyStoreView keyStore = createValidKeyStoreConfig(workDir); + StubSslView sslView = new StubSslView(true, "foo", keyStore, null); + + validate(sslView, "Incorrect client auth parameter foo"); + } + + @Test + public void validKeyStoreConfig(@WorkDirectory Path workDir) throws IOException { + KeyStoreView keyStore = createValidKeyStoreConfig(workDir); + validate(new StubSslView(true, "NONE", keyStore, null), (String[]) null); + } + + @Test + public void validTrustStoreConfig(@WorkDirectory Path workDir) throws IOException { + Path trustStorePath = workDir.resolve("truststore.jks"); + Files.createFile(trustStorePath); + + validate(createTrustStoreConfig(workDir, "JKS", trustStorePath.toAbsolutePath().toString(), null), (String[]) null); + } + + private static void validate(SslView config, String ... errorMessagePrefixes) { + var ctx = mockValidationContext(null, config); + TestValidationUtil.validate(SslConfigurationValidatorImpl.INSTANCE, mock(SslConfigurationValidator.class), ctx, + errorMessagePrefixes); + } + + private static SslView createKeyStoreConfig(String type, String path, String password) { + return new StubSslView(true, "NONE", new StubKeyStoreView(type, path, password), null); + } + + private static SslView createTrustStoreConfig(Path workDir, String type, String path, String password) throws IOException { + KeyStoreView keyStore = createValidKeyStoreConfig(workDir); + KeyStoreView trustStore = new StubKeyStoreView(type, path, password); + return new StubSslView(true, "OPTIONAL", keyStore, trustStore); + } + + private static KeyStoreView createValidKeyStoreConfig(Path workDir) throws IOException { + Path keyStorePath = workDir.resolve("keystore.p12"); + Files.createFile(keyStorePath); + + return new StubKeyStoreView("PKCS12", keyStorePath.toAbsolutePath().toString(), null); + } +} diff --git a/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/KeyStoreConfigurationValidator.java b/modules/network/src/test/java/org/apache/ignite/internal/network/configuration/StubSslView.java similarity index 50% rename from modules/network/src/main/java/org/apache/ignite/internal/network/configuration/KeyStoreConfigurationValidator.java rename to modules/network/src/test/java/org/apache/ignite/internal/network/configuration/StubSslView.java index e96b9bd2be..c074054dda 100644 --- a/modules/network/src/main/java/org/apache/ignite/internal/network/configuration/KeyStoreConfigurationValidator.java +++ b/modules/network/src/test/java/org/apache/ignite/internal/network/configuration/StubSslView.java @@ -17,15 +17,39 @@ package org.apache.ignite.internal.network.configuration; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; +/** Stub of {@link SslView} for tests. */ +public class StubSslView implements SslView { -/** - * Annotation to validate whole key store configuration. - */ -@Target(ElementType.FIELD) -@Retention(RetentionPolicy.RUNTIME) -public @interface KeyStoreConfigurationValidator { + private final boolean enabled; + private final String clientAuth; + private final KeyStoreView keyStore; + private final KeyStoreView trustStore; + + /** Constructor. */ + public StubSslView(boolean enabled, String clientAuth, KeyStoreView keyStore, KeyStoreView trustStore) { + this.enabled = enabled; + this.clientAuth = clientAuth; + this.keyStore = keyStore; + this.trustStore = trustStore; + } + + @Override + public boolean enabled() { + return enabled; + } + + @Override + public String clientAuth() { + return clientAuth; + } + + @Override + public KeyStoreView keyStore() { + return keyStore; + } + + @Override + public KeyStoreView trustStore() { + return trustStore; + } } diff --git a/modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java b/modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java index 15e32a5dce..9d0e57b1e0 100644 --- a/modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java +++ b/modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java @@ -28,16 +28,14 @@ import io.netty.handler.ssl.ClientAuth; import java.net.BindException; import java.net.InetAddress; import java.net.UnknownHostException; -import java.nio.file.Files; -import java.nio.file.Path; import java.util.HashMap; import java.util.List; import java.util.Map; import org.apache.ignite.internal.logger.IgniteLogger; import org.apache.ignite.internal.logger.Loggers; import org.apache.ignite.internal.manager.IgniteComponent; +import org.apache.ignite.internal.network.configuration.KeyStoreView; import org.apache.ignite.internal.rest.configuration.RestConfiguration; -import org.apache.ignite.internal.rest.configuration.RestSslConfiguration; import org.apache.ignite.internal.rest.configuration.RestSslView; import org.apache.ignite.internal.rest.configuration.RestView; import org.apache.ignite.lang.ErrorGroups.Common; @@ -201,17 +199,11 @@ public class RestComponent implements IgniteComponent { } private Map<String, Object> serverProperties(int port, int sslPort) { - RestSslConfiguration sslCfg = restConfiguration.ssl(); - boolean sslEnabled = sslCfg.enabled().value(); + RestSslView restSslView = restConfiguration.ssl().value(); + boolean sslEnabled = restSslView.enabled(); if (sslEnabled) { - String keyStorePath = sslCfg.keyStore().path().value(); - // todo: replace with configuration-level validation https://issues.apache.org/jira/browse/IGNITE-18850 - validateKeyStorePath(keyStorePath); - - String keyStoreType = sslCfg.keyStore().type().value(); - String keyStorePassword = sslCfg.keyStore().password().value(); - + KeyStoreView keyStore = restSslView.keyStore(); boolean dualProtocol = restConfiguration.dualProtocol().value(); Map<String, Object> micronautSslConfig = Map.of( @@ -219,29 +211,23 @@ public class RestComponent implements IgniteComponent { "micronaut.server.dual-protocol", dualProtocol, "micronaut.server.ssl.port", sslPort, "micronaut.server.ssl.enabled", sslEnabled, - "micronaut.server.ssl.key-store.path", "file:" + keyStorePath, - "micronaut.server.ssl.key-store.password", keyStorePassword, - "micronaut.server.ssl.key-store.type", keyStoreType + "micronaut.server.ssl.key-store.path", "file:" + keyStore.path(), + "micronaut.server.ssl.key-store.password", keyStore.password(), + "micronaut.server.ssl.key-store.type", keyStore.type() ); - ClientAuth clientAuth = ClientAuth.valueOf(sslCfg.clientAuth().value().toUpperCase()); + ClientAuth clientAuth = ClientAuth.valueOf(restSslView.clientAuth().toUpperCase()); if (ClientAuth.NONE == clientAuth) { return micronautSslConfig; } - - String trustStorePath = sslCfg.trustStore().path().value(); - // todo: replace with configuration-level validation https://issues.apache.org/jira/browse/IGNITE-18850 - validateTrustStore(trustStorePath); - - String trustStoreType = sslCfg.trustStore().type().value(); - String trustStorePassword = sslCfg.trustStore().password().value(); + KeyStoreView trustStore = restSslView.trustStore(); Map<String, Object> micronautClientAuthConfig = Map.of( "micronaut.server.ssl.client-authentication", toMicronautClientAuth(clientAuth), - "micronaut.server.ssl.trust-store.path", "file:" + trustStorePath, - "micronaut.server.ssl.trust-store.password", trustStorePassword, - "micronaut.server.ssl.trust-store.type", trustStoreType + "micronaut.server.ssl.trust-store.path", "file:" + trustStore.path(), + "micronaut.server.ssl.trust-store.password", trustStore.password(), + "micronaut.server.ssl.trust-store.type", trustStore.type() ); HashMap<String, Object> result = new HashMap<>(); @@ -260,39 +246,7 @@ public class RestComponent implements IgniteComponent { "micronaut.security.intercept-url-map[1].access", "isAuthenticated()"); } - private static void validateKeyStorePath(String keyStorePath) { - if (keyStorePath.trim().isEmpty()) { - throw new IgniteException( - Common.SSL_CONFIGURATION_ERR, - "Trust store path is not configured. Please check your rest.ssl.keyStore.path configuration." - ); - } - - if (!Files.exists(Path.of(keyStorePath))) { - throw new IgniteException( - Common.SSL_CONFIGURATION_ERR, - "Trust store file not found: " + keyStorePath + ". Please check your rest.ssl.keyStore.path configuration." - ); - } - } - - private static void validateTrustStore(String trustStorePath) { - if (trustStorePath.trim().isEmpty()) { - throw new IgniteException( - Common.SSL_CONFIGURATION_ERR, - "Key store path is not configured. Please check your rest.ssl.trustStore.path configuration." - ); - } - - if (!Files.exists(Path.of(trustStorePath))) { - throw new IgniteException( - Common.SSL_CONFIGURATION_ERR, - "Key store file not found: " + trustStorePath + ". Please check your rest.ssl.trustStore.path configuration." - ); - } - } - - private String toMicronautClientAuth(ClientAuth clientAuth) { + private static String toMicronautClientAuth(ClientAuth clientAuth) { switch (clientAuth) { case OPTIONAL: return ClientAuthentication.WANT.name().toLowerCase(); case REQUIRE: return ClientAuthentication.NEED.name().toLowerCase(); diff --git a/modules/rest/src/main/java/org/apache/ignite/internal/rest/configuration/RestConfigurationModule.java b/modules/rest/src/main/java/org/apache/ignite/internal/rest/configuration/RestConfigurationModule.java index 9558de9f33..8b267f2f23 100644 --- a/modules/rest/src/main/java/org/apache/ignite/internal/rest/configuration/RestConfigurationModule.java +++ b/modules/rest/src/main/java/org/apache/ignite/internal/rest/configuration/RestConfigurationModule.java @@ -25,7 +25,7 @@ import org.apache.ignite.configuration.RootKey; import org.apache.ignite.configuration.annotation.ConfigurationType; import org.apache.ignite.configuration.validation.Validator; import org.apache.ignite.internal.configuration.ConfigurationModule; -import org.apache.ignite.internal.network.configuration.KeyStoreConfigurationValidatorImpl; +import org.apache.ignite.internal.network.configuration.SslConfigurationValidatorImpl; /** * {@link ConfigurationModule} for node-local configuration provided by ignite-rest. @@ -44,6 +44,6 @@ public class RestConfigurationModule implements ConfigurationModule { @Override public Set<Validator<?, ?>> validators() { - return Set.of(KeyStoreConfigurationValidatorImpl.INSTANCE); + return Set.of(SslConfigurationValidatorImpl.INSTANCE); } } diff --git a/modules/rest/src/test/java/org/apache/ignite/internal/rest/RestComponentTest.java b/modules/rest/src/test/java/org/apache/ignite/internal/rest/RestComponentTest.java deleted file mode 100644 index 4e10f283c9..0000000000 --- a/modules/rest/src/test/java/org/apache/ignite/internal/rest/RestComponentTest.java +++ /dev/null @@ -1,139 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You 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.apache.ignite.internal.rest; - - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; -import static org.junit.jupiter.api.Assertions.assertThrows; - -import io.netty.handler.ssl.util.SelfSignedCertificate; -import java.io.FileOutputStream; -import java.io.IOException; -import java.net.URI; -import java.net.http.HttpClient; -import java.net.http.HttpRequest; -import java.net.http.HttpResponse; -import java.net.http.HttpResponse.BodyHandlers; -import java.nio.file.Path; -import java.security.KeyStore; -import java.security.KeyStoreException; -import java.security.NoSuchAlgorithmException; -import java.security.cert.Certificate; -import java.security.cert.CertificateException; -import java.util.List; -import org.apache.ignite.internal.configuration.AuthenticationConfiguration; -import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension; -import org.apache.ignite.internal.configuration.testframework.InjectConfiguration; -import org.apache.ignite.internal.rest.authentication.AuthProviderFactory; -import org.apache.ignite.internal.rest.configuration.RestConfiguration; -import org.apache.ignite.internal.testframework.WorkDirectory; -import org.apache.ignite.internal.testframework.WorkDirectoryExtension; -import org.apache.ignite.lang.ErrorGroups.Common; -import org.apache.ignite.lang.IgniteException; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; - -@ExtendWith(ConfigurationExtension.class) -@ExtendWith(WorkDirectoryExtension.class) -class RestComponentTest { - - private String keyStorePath; - - @InjectConfiguration - private AuthenticationConfiguration authenticationConfiguration; - - @WorkDirectory - private Path workDir; - - private static void pingOk(RestComponent component) throws IOException, InterruptedException { - HttpClient client = HttpClient.newBuilder().build(); - HttpResponse<String> response = client.send( - HttpRequest.newBuilder() - .uri(URI.create("http://" + component.host() + ":" + component.httpPort() + "/ping")).GET() - .build(), - BodyHandlers.ofString() - ); - - assertThat(response.statusCode(), is(200)); - assertThat(response.body(), is("pong")); - } - - @BeforeEach - void setUp() { - keyStorePath = workDir.resolve("keystore.p12").toAbsolutePath().toString(); - } - - @Test - @DisplayName("REST component stars with default configuration") - void defaultConfiguration(@InjectConfiguration RestConfiguration restConfiguration) throws Exception { - // Given - RestComponent component = new RestComponent(List.of(new AuthProviderFactory(authenticationConfiguration)), restConfiguration); - - // When - component.start(); - - // Then - pingOk(component); - } - - @Test - @DisplayName("REST component does not start with ssl.enabled=true and no keystore") - void sslConfiguration(@InjectConfiguration("mock.ssl.enabled: true") RestConfiguration restConfiguration) { - // Given - RestComponent component = new RestComponent(List.of(new AuthProviderFactory(authenticationConfiguration)), restConfiguration); - - // When - IgniteException thrown = assertThrows(IgniteException.class, component::start); - - // Then - assertThat(thrown.code(), is(Common.SSL_CONFIGURATION_ERR)); - } - - @Test - @DisplayName("REST component does not start with ssl.clientAuth=require and no truststore") - void clientAuthConfiguration(@InjectConfiguration RestConfiguration restConfiguration) throws Exception { - // Given correct keystore - generateKeystore(new SelfSignedCertificate()); - restConfiguration.ssl().enabled().update(true).get(); - restConfiguration.ssl().keyStore().path().update(keyStorePath).get(); - restConfiguration.ssl().keyStore().password().update("changeit").get(); - // And clientAuth=require But no truststore - restConfiguration.ssl().clientAuth().update("require").get(); - - RestComponent component = new RestComponent(List.of(new AuthProviderFactory(authenticationConfiguration)), restConfiguration); - - // When - IgniteException thrown = assertThrows(IgniteException.class, component::start); - - // Then - assertThat(thrown.code(), is(Common.SSL_CONFIGURATION_ERR)); - } - - private void generateKeystore(SelfSignedCertificate cert) - throws KeyStoreException, IOException, NoSuchAlgorithmException, CertificateException { - KeyStore ks = KeyStore.getInstance("PKCS12"); - ks.load(null, null); - ks.setKeyEntry("key", cert.key(), null, new Certificate[]{cert.cert()}); - try (FileOutputStream fos = new FileOutputStream(keyStorePath)) { - ks.store(fos, "changeit".toCharArray()); - } - } -} diff --git a/modules/schema/src/test/java/org/apache/ignite/internal/schema/configuration/KnownDataStorageValidatorTest.java b/modules/schema/src/test/java/org/apache/ignite/internal/schema/configuration/KnownDataStorageValidatorTest.java index 5f699ada8b..a3f5ada423 100644 --- a/modules/schema/src/test/java/org/apache/ignite/internal/schema/configuration/KnownDataStorageValidatorTest.java +++ b/modules/schema/src/test/java/org/apache/ignite/internal/schema/configuration/KnownDataStorageValidatorTest.java @@ -62,6 +62,6 @@ public class KnownDataStorageValidatorTest { DataStorageView value = dataStorageConfig.value(); - validate(new KnownDataStorageValidator(), annotation, mockValidationContext(value, value), null); + validate(new KnownDataStorageValidator(), annotation, mockValidationContext(value, value)); } } diff --git a/modules/schema/src/test/java/org/apache/ignite/internal/schema/configuration/TableValidatorImplTest.java b/modules/schema/src/test/java/org/apache/ignite/internal/schema/configuration/TableValidatorImplTest.java index 1542e7ca11..a2d8aede54 100644 --- a/modules/schema/src/test/java/org/apache/ignite/internal/schema/configuration/TableValidatorImplTest.java +++ b/modules/schema/src/test/java/org/apache/ignite/internal/schema/configuration/TableValidatorImplTest.java @@ -35,6 +35,6 @@ public class TableValidatorImplTest extends AbstractTableIndexValidatorTest { public void testNoIssues() { ValidationContext<NamedListView<TableView>> ctx = mockValidationContext(null, tablesCfg.tables().value()); - validate(TableValidatorImpl.INSTANCE, mock(TableValidator.class), ctx, null); + validate(TableValidatorImpl.INSTANCE, mock(TableValidator.class), ctx); } } diff --git a/modules/storage-api/src/test/java/org/apache/ignite/internal/storage/configuration/ExistingDataStorageValidatorTest.java b/modules/storage-api/src/test/java/org/apache/ignite/internal/storage/configuration/ExistingDataStorageValidatorTest.java index ad1324428e..14132418ee 100644 --- a/modules/storage-api/src/test/java/org/apache/ignite/internal/storage/configuration/ExistingDataStorageValidatorTest.java +++ b/modules/storage-api/src/test/java/org/apache/ignite/internal/storage/configuration/ExistingDataStorageValidatorTest.java @@ -62,9 +62,9 @@ public class ExistingDataStorageValidatorTest { createMockedStorageEngineFactory(dataStorage2) )); - validate(validator, annotation, mockValidationContext(UNKNOWN_DATA_STORAGE, UNKNOWN_DATA_STORAGE), null); - validate(validator, annotation, mockValidationContext(dataStorage1, dataStorage1), null); - validate(validator, annotation, mockValidationContext(dataStorage2, dataStorage2), null); + validate(validator, annotation, mockValidationContext(UNKNOWN_DATA_STORAGE, UNKNOWN_DATA_STORAGE)); + validate(validator, annotation, mockValidationContext(dataStorage1, dataStorage1)); + validate(validator, annotation, mockValidationContext(dataStorage2, dataStorage2)); } private DataStorageModule createMockedStorageEngineFactory(String name) {