yifan-c commented on code in PR #137: URL: https://github.com/apache/cassandra-sidecar/pull/137#discussion_r1796038108
########## vertx-auth-mtls/src/main/java/io/vertx/ext/auth/mtls/impl/SpiffeIdentityExtractor.java: ########## @@ -0,0 +1,82 @@ +/* + * 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 io.vertx.ext.auth.mtls.impl; + +import io.vertx.ext.auth.authentication.CredentialValidationException; +import io.vertx.ext.auth.mtls.CertificateIdentityExtractor; + +import java.security.cert.Certificate; +import java.security.cert.CertificateParsingException; +import java.security.cert.X509Certificate; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; + +/** + * {@link CertificateIdentityExtractor} implementation for SPIFFE certificates for extracting SPIFFE identity. + * SPIFFE is a URI, present as part of SAN of client certificates, it uniquely identifies client. + */ +public class SpiffeIdentityExtractor implements CertificateIdentityExtractor +{ + private static final int SUBJECT_ALT_NAME_URI_TYPE = 6; + private static final String SPIFFE_PREFIX = "spiffe://"; + + @Override + public String identity(Certificate[] certificateChain) throws CredentialValidationException, CertificateParsingException + { + X509Certificate[] castedCerts = castCertsToX509(certificateChain); + if (castedCerts.length == 0) + { + throw new CredentialValidationException("No based X509Certificate found for validating"); + } + X509Certificate privateCert = castedCerts[0]; + + Collection<List<?>> subjectAltNames = privateCert.getSubjectAlternativeNames(); + if (subjectAltNames != null && !subjectAltNames.isEmpty()) + { + for (List<?> item : subjectAltNames) + { + Integer type = (Integer) item.get(0); + String identity = (String) item.get(1); + if (type == SUBJECT_ALT_NAME_URI_TYPE && identity.startsWith(SPIFFE_PREFIX)) + { + return identity; + } + } + } + + throw new CredentialValidationException("Unable to extract valid Spiffe ID from certificate"); + } + + /** + * Filters instances of {@link X509Certificate} certificates and returns the certificate chain as + * {@link X509Certificate} certificates. + * + * @param certificateChain client certificate chain + * @return an array of {@link X509Certificate} certificates + */ + private X509Certificate[] castCertsToX509(Certificate[] certificateChain) throws CredentialValidationException + { + if (certificateChain == null || certificateChain.length == 0) + { + throw new CredentialValidationException("Certificate chain shared is empty"); + } + return Arrays.stream(certificateChain).filter(certificate -> certificate instanceof X509Certificate).toArray(X509Certificate[]::new); + } Review Comment: I would suggest doing this instead. You can avoid the throw statement. ```suggestion private List<X509Certificate> castCertsToX509(Certificate[] certificateChain) throws CredentialValidationException { List<X509Certificate> x509Certs = new ArrayList<>(); for (Certificate cert : certificateChain) { if (cert instanceof X509Certificate) { x509Certs.add((X509Certificate) cert); } } return x509Certs; } ``` In fact, it ########## vertx-auth-mtls/src/main/java/io/vertx/ext/auth/mtls/impl/MutualTlsAuthenticationProvider.java: ########## @@ -0,0 +1,93 @@ +/* + * 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 io.vertx.ext.auth.mtls.impl; + +import io.vertx.core.AsyncResult; +import io.vertx.core.Future; +import io.vertx.core.Handler; +import io.vertx.core.json.JsonObject; +import io.vertx.ext.auth.User; +import io.vertx.ext.auth.authentication.AuthenticationProvider; +import io.vertx.ext.auth.authentication.CertificateCredentials; +import io.vertx.ext.auth.authentication.Credentials; +import io.vertx.ext.auth.mtls.CertificateIdentityExtractor; +import io.vertx.ext.auth.mtls.CertificateIdentityValidator; +import io.vertx.ext.auth.mtls.CertificateValidator; + +import java.security.cert.Certificate; + +/** + * {@link AuthenticationProvider} implementation for mTLS (MutualTLS) authentication. With mTLS authentication + * both server and client exchange certificates and validates each other's certificates. + */ +public class MutualTlsAuthenticationProvider implements AuthenticationProvider +{ + private final CertificateValidator certificateValidator; + private final CertificateIdentityExtractor identityExtractor; + private final CertificateIdentityValidator identityValidator; + + public MutualTlsAuthenticationProvider(CertificateValidator certificateValidator, + CertificateIdentityExtractor identityExtractor, + CertificateIdentityValidator identityValidator) + { + this.certificateValidator = certificateValidator; + this.identityExtractor = identityExtractor; + this.identityValidator = identityValidator; + } + + @Override + public Future<User> authenticate(Credentials credentials) + { + if (!(credentials instanceof CertificateCredentials)) + { + return Future.failedFuture("CertificateCredentials expected for mTLS authentication"); + } + + CertificateCredentials certificateCredentials = (CertificateCredentials) credentials; + String identity; + try + { + certificateValidator.isValidCertificate(certificateCredentials); + identity = identityExtractor.identity(certificateCredentials.certificateChain().toArray(new Certificate[0])); + } + catch (Exception e) + { + return Future.failedFuture("Invalid certificate passed"); Review Comment: do not swallow the exception. ########## vertx-auth-mtls/src/main/java/io/vertx/ext/auth/mtls/impl/SpiffeIdentityValidator.java: ########## @@ -0,0 +1,62 @@ +/* + * 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 io.vertx.ext.auth.mtls.impl; + +import io.vertx.ext.auth.mtls.CertificateIdentityValidator; + +/** + * {@link CertificateIdentityValidator} implementation for SPIFFE certificates for validating SPIFFE identities. + * SPIFFE is a URI, present as part of SAN of client certificates, it uniquely identifies client. + */ +public class SpiffeIdentityValidator implements CertificateIdentityValidator +{ + private static final String SPIFFE_ID_PREFIX = "spiffe://"; Review Comment: The same constant is define in multiple classes. Another sign that those classes should be a single one. ########## vertx-auth-mtls/src/main/java/io/vertx/ext/auth/mtls/impl/SpiffeIdentityExtractor.java: ########## @@ -0,0 +1,82 @@ +/* + * 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 io.vertx.ext.auth.mtls.impl; + +import io.vertx.ext.auth.authentication.CredentialValidationException; +import io.vertx.ext.auth.mtls.CertificateIdentityExtractor; + +import java.security.cert.Certificate; +import java.security.cert.CertificateParsingException; +import java.security.cert.X509Certificate; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; + +/** + * {@link CertificateIdentityExtractor} implementation for SPIFFE certificates for extracting SPIFFE identity. + * SPIFFE is a URI, present as part of SAN of client certificates, it uniquely identifies client. + */ +public class SpiffeIdentityExtractor implements CertificateIdentityExtractor +{ + private static final int SUBJECT_ALT_NAME_URI_TYPE = 6; + private static final String SPIFFE_PREFIX = "spiffe://"; + + @Override + public String identity(Certificate[] certificateChain) throws CredentialValidationException, CertificateParsingException + { + X509Certificate[] castedCerts = castCertsToX509(certificateChain); + if (castedCerts.length == 0) + { + throw new CredentialValidationException("No based X509Certificate found for validating"); Review Comment: Is the word `based` added to the error message by accident? ########## vertx-auth-mtls/src/main/java/io/vertx/ext/auth/mtls/impl/SpiffeIdentityExtractor.java: ########## @@ -0,0 +1,82 @@ +/* + * 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 io.vertx.ext.auth.mtls.impl; + +import io.vertx.ext.auth.authentication.CredentialValidationException; +import io.vertx.ext.auth.mtls.CertificateIdentityExtractor; + +import java.security.cert.Certificate; +import java.security.cert.CertificateParsingException; +import java.security.cert.X509Certificate; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; + +/** + * {@link CertificateIdentityExtractor} implementation for SPIFFE certificates for extracting SPIFFE identity. + * SPIFFE is a URI, present as part of SAN of client certificates, it uniquely identifies client. + */ +public class SpiffeIdentityExtractor implements CertificateIdentityExtractor +{ + private static final int SUBJECT_ALT_NAME_URI_TYPE = 6; + private static final String SPIFFE_PREFIX = "spiffe://"; + + @Override + public String identity(Certificate[] certificateChain) throws CredentialValidationException, CertificateParsingException + { + X509Certificate[] castedCerts = castCertsToX509(certificateChain); + if (castedCerts.length == 0) + { + throw new CredentialValidationException("No based X509Certificate found for validating"); + } + X509Certificate privateCert = castedCerts[0]; Review Comment: why checking only the first cert from the array? ########## vertx-auth-mtls/src/main/java/io/vertx/ext/auth/authentication/CertificateCredentials.java: ########## @@ -0,0 +1,92 @@ +/* + * 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 io.vertx.ext.auth.authentication; + +import io.vertx.core.http.HttpServerRequest; +import io.vertx.core.json.JsonObject; + +import java.security.cert.Certificate; +import java.util.Collections; +import java.util.List; +import javax.net.ssl.SSLPeerUnverifiedException; + +/** + * Certificates based {@link Credentials} implementation, carries user's certificates, which can be used for + * authenticating or authorizing users. + */ +public class CertificateCredentials implements Credentials Review Comment: Do you need to implement `applyHttpChallenge` and `toHttpAuthorization` methods? It is hard to see the need since this patch only adds the code that is not used. ########## vertx-auth-mtls/src/main/java/io/vertx/ext/auth/mtls/CertificateIdentityValidator.java: ########## @@ -0,0 +1,33 @@ +/* + * 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 io.vertx.ext.auth.mtls; + +/** + * {@link CertificateIdentityValidator} interface can be extended to implement custom certificate identity validator. + */ +public interface CertificateIdentityValidator +{ + /** + * Verifies that identity extracted from certificate is a valid identity. + * + * @param identity {@code String} string extracted from certificate, uniquely represents client + * @return {@code true} if the identity is valid, {@code false otherwise} + */ + boolean isValidIdentity(String identity); Review Comment: Prefer throwing exception instead of returning boolean for the validation. The exception conveys better context when it throws. ```suggestion void validateIdentity(String identity) throws InvalidIdentityException; ``` ########## vertx-auth-mtls/build.gradle: ########## @@ -0,0 +1,82 @@ +/* + * 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. + */ + +import java.nio.file.Paths + +plugins { + id('java-library') + id('idea') + id('maven-publish') +} + +group 'org.apache.cassandra.sidecar' +version project.version + +sourceCompatibility = 1.8 + +repositories { + mavenCentral() +} + +test { + useJUnitPlatform() + maxParallelForks = Runtime.runtime.availableProcessors().intdiv(2) ?: 1 + reports { + junitXml.enabled = true + def destDir = Paths.get(rootProject.rootDir.absolutePath, "build", "test-results", "vertx-auth-mtls").toFile() + println("Destination directory for vertx-auth-mtls tests: ${destDir}") + junitXml.destination = destDir + html.enabled = true + } +} + +configurations { + all*.exclude(group: 'ch.qos.logback') +} + +dependencies { + implementation(group: 'io.vertx', name: 'vertx-auth-common', version: '4.5.8') + testImplementation(group: 'io.vertx', name: 'vertx-junit5', version: '4.5.8') + testImplementation group: 'org.assertj', name: 'assertj-core', version: '3.26.3' + testImplementation group: 'org.mockito', name: 'mockito-core', version: '2.1.0' + testImplementation group: 'org.bouncycastle', name: 'bcpkix-jdk15on', version: '1.70' Review Comment: nit: use the same style ```suggestion testImplementation(group: 'org.assertj', name: 'assertj-core', version: '3.26.3') testImplementation(group: 'org.mockito', name: 'mockito-core', version: '2.1.0') testImplementation(group: 'org.bouncycastle', name: 'bcpkix-jdk15on', version: '1.70') ``` ########## vertx-auth-mtls/src/main/java/io/vertx/ext/auth/mtls/impl/MutualTlsAuthenticationProvider.java: ########## @@ -0,0 +1,93 @@ +/* + * 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 io.vertx.ext.auth.mtls.impl; + +import io.vertx.core.AsyncResult; +import io.vertx.core.Future; +import io.vertx.core.Handler; +import io.vertx.core.json.JsonObject; +import io.vertx.ext.auth.User; +import io.vertx.ext.auth.authentication.AuthenticationProvider; +import io.vertx.ext.auth.authentication.CertificateCredentials; +import io.vertx.ext.auth.authentication.Credentials; +import io.vertx.ext.auth.mtls.CertificateIdentityExtractor; +import io.vertx.ext.auth.mtls.CertificateIdentityValidator; +import io.vertx.ext.auth.mtls.CertificateValidator; + +import java.security.cert.Certificate; + +/** + * {@link AuthenticationProvider} implementation for mTLS (MutualTLS) authentication. With mTLS authentication + * both server and client exchange certificates and validates each other's certificates. + */ +public class MutualTlsAuthenticationProvider implements AuthenticationProvider +{ + private final CertificateValidator certificateValidator; + private final CertificateIdentityExtractor identityExtractor; + private final CertificateIdentityValidator identityValidator; + + public MutualTlsAuthenticationProvider(CertificateValidator certificateValidator, + CertificateIdentityExtractor identityExtractor, + CertificateIdentityValidator identityValidator) + { + this.certificateValidator = certificateValidator; + this.identityExtractor = identityExtractor; + this.identityValidator = identityValidator; + } + + @Override + public Future<User> authenticate(Credentials credentials) + { + if (!(credentials instanceof CertificateCredentials)) + { + return Future.failedFuture("CertificateCredentials expected for mTLS authentication"); + } + + CertificateCredentials certificateCredentials = (CertificateCredentials) credentials; + String identity; + try + { + certificateValidator.isValidCertificate(certificateCredentials); + identity = identityExtractor.identity(certificateCredentials.certificateChain().toArray(new Certificate[0])); + } + catch (Exception e) + { + return Future.failedFuture("Invalid certificate passed"); + } + + if (identity == null || identity.isEmpty()) + { + return Future.failedFuture("Could not extract identity from certificate"); + } Review Comment: This block can be deleted, if you update the API to return non-null value only. ########## vertx-auth-mtls/src/main/java/io/vertx/ext/auth/mtls/impl/SpiffeIdentityExtractor.java: ########## @@ -0,0 +1,82 @@ +/* + * 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 io.vertx.ext.auth.mtls.impl; + +import io.vertx.ext.auth.authentication.CredentialValidationException; +import io.vertx.ext.auth.mtls.CertificateIdentityExtractor; + +import java.security.cert.Certificate; +import java.security.cert.CertificateParsingException; +import java.security.cert.X509Certificate; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; + +/** + * {@link CertificateIdentityExtractor} implementation for SPIFFE certificates for extracting SPIFFE identity. + * SPIFFE is a URI, present as part of SAN of client certificates, it uniquely identifies client. + */ +public class SpiffeIdentityExtractor implements CertificateIdentityExtractor +{ + private static final int SUBJECT_ALT_NAME_URI_TYPE = 6; Review Comment: Consider add a comment to further explain what is 6. According to https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.6, type 6 is for URI. ########## vertx-auth-mtls/src/main/java/io/vertx/ext/auth/mtls/CertificateIdentityExtractor.java: ########## @@ -0,0 +1,47 @@ +/* + * 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 io.vertx.ext.auth.mtls; + +import io.vertx.ext.auth.authentication.CredentialValidationException; + +import java.security.cert.Certificate; +import java.security.cert.CertificateParsingException; + +/** + * Extracts a valid identity from certificate chain provided. + */ +public interface CertificateIdentityExtractor +{ + /** + * Extracts identity out of {@code Certificate[]} certificate chain. If required, this identity can later be used + * for authorizing the user's resource level permissions. + * + * <p>An example of identity could be the following: + * <ul> + * <li>an identifier in SAN of the certificate like SPIFFE + * <li>CN of the certificate + * <li>any other fields in the certificate can be combined and be used as identifier of the certificate + * </ul> + * + * @param certificateChain certificate chain of user + * @return identity {@code String} extracted from certificate + * @throws CredentialValidationException when identity cannot be extracted Review Comment: missing javadoc for `CertificateParsingException` ########## vertx-auth-mtls/src/main/java/io/vertx/ext/auth/mtls/impl/MutualTlsAuthenticationProvider.java: ########## @@ -0,0 +1,93 @@ +/* + * 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 io.vertx.ext.auth.mtls.impl; + +import io.vertx.core.AsyncResult; +import io.vertx.core.Future; +import io.vertx.core.Handler; +import io.vertx.core.json.JsonObject; +import io.vertx.ext.auth.User; +import io.vertx.ext.auth.authentication.AuthenticationProvider; +import io.vertx.ext.auth.authentication.CertificateCredentials; +import io.vertx.ext.auth.authentication.Credentials; +import io.vertx.ext.auth.mtls.CertificateIdentityExtractor; +import io.vertx.ext.auth.mtls.CertificateIdentityValidator; +import io.vertx.ext.auth.mtls.CertificateValidator; + +import java.security.cert.Certificate; + +/** + * {@link AuthenticationProvider} implementation for mTLS (MutualTLS) authentication. With mTLS authentication + * both server and client exchange certificates and validates each other's certificates. + */ +public class MutualTlsAuthenticationProvider implements AuthenticationProvider +{ + private final CertificateValidator certificateValidator; + private final CertificateIdentityExtractor identityExtractor; + private final CertificateIdentityValidator identityValidator; + + public MutualTlsAuthenticationProvider(CertificateValidator certificateValidator, + CertificateIdentityExtractor identityExtractor, + CertificateIdentityValidator identityValidator) + { + this.certificateValidator = certificateValidator; + this.identityExtractor = identityExtractor; + this.identityValidator = identityValidator; + } + + @Override + public Future<User> authenticate(Credentials credentials) + { + if (!(credentials instanceof CertificateCredentials)) + { + return Future.failedFuture("CertificateCredentials expected for mTLS authentication"); + } + + CertificateCredentials certificateCredentials = (CertificateCredentials) credentials; + String identity; + try + { + certificateValidator.isValidCertificate(certificateCredentials); + identity = identityExtractor.identity(certificateCredentials.certificateChain().toArray(new Certificate[0])); + } + catch (Exception e) + { + return Future.failedFuture("Invalid certificate passed"); + } + + if (identity == null || identity.isEmpty()) + { + return Future.failedFuture("Could not extract identity from certificate"); + } + if (!identityValidator.isValidIdentity(identity)) + { + return Future.failedFuture("Certificate identity is not a valid identity"); + } + return Future.succeededFuture(User.fromName(identity)); + } + + /** + * @deprecated use {@code authenticate(Credentials credentials, Handler<AsyncResult<User>> resultHandler)} instead Review Comment: 1. IIRC, codestyle plugin does not like `@deprecated` 2. the message `use {@code authenticate(Credentials credentials, Handler<AsyncResult<User>> resultHandler)} instead` is incorrect. It is referring to itself. You probably want to have `use {@code authenticate(Credentials credentials} instead` ########## vertx-auth-mtls/src/main/java/io/vertx/ext/auth/mtls/CertificateIdentityExtractor.java: ########## @@ -0,0 +1,47 @@ +/* + * 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 io.vertx.ext.auth.mtls; + +import io.vertx.ext.auth.authentication.CredentialValidationException; + +import java.security.cert.Certificate; +import java.security.cert.CertificateParsingException; + +/** + * Extracts a valid identity from certificate chain provided. + */ +public interface CertificateIdentityExtractor +{ + /** + * Extracts identity out of {@code Certificate[]} certificate chain. If required, this identity can later be used + * for authorizing the user's resource level permissions. + * + * <p>An example of identity could be the following: + * <ul> + * <li>an identifier in SAN of the certificate like SPIFFE + * <li>CN of the certificate + * <li>any other fields in the certificate can be combined and be used as identifier of the certificate + * </ul> + * + * @param certificateChain certificate chain of user + * @return identity {@code String} extracted from certificate + * @throws CredentialValidationException when identity cannot be extracted + */ + String identity(Certificate[] certificateChain) throws CredentialValidationException, CertificateParsingException; Review Comment: 1. Can we use `List<Certificate>` instead of array? The only callsite, i.e. `MutualTlsAuthenticationProvider#authenticate` converts from List to array to call the method. 2. How about we refine the contract of this API that it only returns non-null string? When it cannot extract any identity, it throws the `CredentialValidationException`. So there is no scenario, that the API would return null. In other words, this method either extracts some identity or throws. ########## vertx-auth-mtls/src/main/java/io/vertx/ext/auth/mtls/impl/MutualTlsAuthenticationProvider.java: ########## @@ -0,0 +1,93 @@ +/* + * 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 io.vertx.ext.auth.mtls.impl; + +import io.vertx.core.AsyncResult; +import io.vertx.core.Future; +import io.vertx.core.Handler; +import io.vertx.core.json.JsonObject; +import io.vertx.ext.auth.User; +import io.vertx.ext.auth.authentication.AuthenticationProvider; +import io.vertx.ext.auth.authentication.CertificateCredentials; +import io.vertx.ext.auth.authentication.Credentials; +import io.vertx.ext.auth.mtls.CertificateIdentityExtractor; +import io.vertx.ext.auth.mtls.CertificateIdentityValidator; +import io.vertx.ext.auth.mtls.CertificateValidator; + +import java.security.cert.Certificate; + +/** + * {@link AuthenticationProvider} implementation for mTLS (MutualTLS) authentication. With mTLS authentication + * both server and client exchange certificates and validates each other's certificates. + */ +public class MutualTlsAuthenticationProvider implements AuthenticationProvider +{ + private final CertificateValidator certificateValidator; + private final CertificateIdentityExtractor identityExtractor; + private final CertificateIdentityValidator identityValidator; + + public MutualTlsAuthenticationProvider(CertificateValidator certificateValidator, + CertificateIdentityExtractor identityExtractor, + CertificateIdentityValidator identityValidator) + { + this.certificateValidator = certificateValidator; + this.identityExtractor = identityExtractor; + this.identityValidator = identityValidator; + } + + @Override + public Future<User> authenticate(Credentials credentials) + { + if (!(credentials instanceof CertificateCredentials)) + { + return Future.failedFuture("CertificateCredentials expected for mTLS authentication"); + } + + CertificateCredentials certificateCredentials = (CertificateCredentials) credentials; + String identity; + try + { + certificateValidator.isValidCertificate(certificateCredentials); + identity = identityExtractor.identity(certificateCredentials.certificateChain().toArray(new Certificate[0])); + } + catch (Exception e) + { + return Future.failedFuture("Invalid certificate passed"); + } + + if (identity == null || identity.isEmpty()) + { + return Future.failedFuture("Could not extract identity from certificate"); + } + if (!identityValidator.isValidIdentity(identity)) + { + return Future.failedFuture("Certificate identity is not a valid identity"); + } Review Comment: You can merge the validation code with the try-catch block above, once you made the interface to throw exception, instead of returning boolean. ########## vertx-auth-mtls/src/main/java/io/vertx/ext/auth/mtls/impl/CertificateValidatorImpl.java: ########## @@ -0,0 +1,189 @@ +/* + * 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 io.vertx.ext.auth.mtls.impl; + +import io.vertx.ext.auth.authentication.CertificateCredentials; +import io.vertx.ext.auth.authentication.CredentialValidationException; +import io.vertx.ext.auth.mtls.CertificateValidator; + +import javax.naming.InvalidNameException; +import javax.naming.NamingException; +import javax.naming.directory.Attribute; +import javax.naming.directory.Attributes; +import javax.naming.ldap.LdapName; +import javax.naming.ldap.Rdn; +import java.security.cert.Certificate; +import java.security.cert.CertificateExpiredException; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +/** + * {@link CertificateValidator} implementation that can be used for validating certificates. + */ +public class CertificateValidatorImpl implements CertificateValidator +{ + private final Set<String> trustedCNs; + private final String trustedIssuerOrganization; + private final String trustedIssuerOrganizationUnit; + private final String trustedIssuerCountry; + + public CertificateValidatorImpl() + { + this.trustedCNs = Collections.emptySet(); + this.trustedIssuerOrganization = null; + this.trustedIssuerOrganizationUnit = null; + this.trustedIssuerCountry = null; + } + Review Comment: not used ########## vertx-auth-mtls/src/main/java/io/vertx/ext/auth/mtls/CertificateValidator.java: ########## @@ -0,0 +1,47 @@ +/* + * 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 io.vertx.ext.auth.mtls; + +import io.vertx.ext.auth.authentication.CertificateCredentials; + +import java.security.cert.CertificateParsingException; + +/** + * Interface for validating certificates for mutual TLS authentication. + * <p> + * This interface can be implemented to provide logic for validating various fields from Certificates. + */ +public interface CertificateValidator Review Comment: I feel that all three interfaces `CertificateValidator`, `CertificateIdentityExtractor` and `CertificateIdentityValidator` can merge together to reduce cognitive complexity. They are tightly coupled logically, even if they are 3 individual interfaces as of now. For example, you always pair `SpiffeIdentityExtractor` with `SpiffeIdentityValidator`. For another example, it is probably never a valid scenario to have this combination, `AllowAllCertValidator + SpiffeIdentityExtractor + AllowAllIndentityValidator`, but the `MutualTlsAuthenticationProvider` constructor would allow it. ########## vertx-auth-mtls/src/main/java/io/vertx/ext/auth/mtls/impl/CertificateValidatorImpl.java: ########## @@ -0,0 +1,189 @@ +/* + * 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 io.vertx.ext.auth.mtls.impl; + +import io.vertx.ext.auth.authentication.CertificateCredentials; +import io.vertx.ext.auth.authentication.CredentialValidationException; +import io.vertx.ext.auth.mtls.CertificateValidator; + +import javax.naming.InvalidNameException; +import javax.naming.NamingException; +import javax.naming.directory.Attribute; +import javax.naming.directory.Attributes; +import javax.naming.ldap.LdapName; +import javax.naming.ldap.Rdn; +import java.security.cert.Certificate; +import java.security.cert.CertificateExpiredException; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +/** + * {@link CertificateValidator} implementation that can be used for validating certificates. + */ +public class CertificateValidatorImpl implements CertificateValidator +{ + private final Set<String> trustedCNs; + private final String trustedIssuerOrganization; + private final String trustedIssuerOrganizationUnit; + private final String trustedIssuerCountry; + + public CertificateValidatorImpl() + { + this.trustedCNs = Collections.emptySet(); + this.trustedIssuerOrganization = null; + this.trustedIssuerOrganizationUnit = null; + this.trustedIssuerCountry = null; + } + + public CertificateValidatorImpl(Set<String> trustedCNs, + String trustedIssuerOrganization, + String trustedIssuerOrganizationUnit, + String trustedIssuerCountry) + { + this.trustedCNs = Collections.unmodifiableSet(trustedCNs); + this.trustedIssuerOrganization = trustedIssuerOrganization; + this.trustedIssuerOrganizationUnit = trustedIssuerOrganizationUnit; + this.trustedIssuerCountry = trustedIssuerCountry; + } + + @Override + public boolean isValidCertificate(CertificateCredentials credentials) + { + credentials.checkValid(); + Certificate certificate = credentials.certificateChain().get(0); + if (certificate instanceof X509Certificate) + { + X509Certificate castedCert = (X509Certificate) certificate; + if (!isValidIssuer(castedCert)) + { + return false; + } + + try + { + castedCert.checkValidity(); + return true; + } + catch (CertificateExpiredException e) + { + throw new CredentialValidationException("Expired certificates shared for authentication"); + } + catch (Exception e) + { + return false; + } + } + return false; Review Comment: It requires the first certificate to be a `X509Certificate`, otherwise returns false? Also, why only the first cert? ########## vertx-auth-mtls/src/main/java/io/vertx/ext/auth/mtls/CertificateValidator.java: ########## @@ -0,0 +1,47 @@ +/* + * 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 io.vertx.ext.auth.mtls; + +import io.vertx.ext.auth.authentication.CertificateCredentials; + +import java.security.cert.CertificateParsingException; + +/** + * Interface for validating certificates for mutual TLS authentication. + * <p> + * This interface can be implemented to provide logic for validating various fields from Certificates. + */ +public interface CertificateValidator +{ + /** + * Perform any checks that are to be performed on the certificate before authenticating user. + * + * <p>For example: + * <ul> + * <li>Verifying CA information + * <li>Checking CN information + * <li>Validating Issuer information + * <li>Checking organization information etc + * </ul> + * + * @param credentials user certificate credentials shared + * @return {@code true} if the credentials are valid, {@code false} otherwise + */ + boolean isValidCertificate(CertificateCredentials credentials) throws CertificateParsingException; Review Comment: if it throws exception, you do not need to return boolean. If it is valid, no throw; it throws otherwise. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
