#4289 - Ability to skip SSL certificate validation on external recommenders

inception/inception-imls-external/src/main/resources/META-INF/asciidoc/user-guide/projects_recommendation_external.adoc

@@ -16,6 +16,18 @@
 
 == External Recommender
 
-This recommender allows to use external web-services to generate predictions. For details on the
-protocol used in the communication with the external services, please refer to the developer
-documentation.
+This recommender allows to use an external web-service to generate predictions. 
+
+You can find an example implementation of several external recommenders in the link:https://github.com/inception-project/inception-external-recommender[INCEpTION External Recommender repository] on GitHub.
+
+For more details on the protocol used in the communication with the external services, please refer to the developer documentation.
+
+.HTTPS support
+The remote recommender service can be accessed via an encrypted HTTPS connection. However, this will fail unless the certificate is either signed by a well-known certificate authority or has been imported into the certificate store of the Java virtual machine. 
+
+NOTE: For testing purposes, the validation of the SSL certificate can be disabled in the 
+      external recommender settings. However, the SSL certificate will still need to contain a host 
+      name that matches the URL of the external recommender. If you also need to disable host name 
+      verification, you need to start {application-name} with the system property 
+      `jdk.internal.httpclient.disableHostnameVerification`. Note this needs to be specified **on the 
+      command line** and not in the `settings.properties` file.

inception/inception-imls-external/src/test/java/de/tudarmstadt/ukp/inception/recommendation/imls/external/v1/ExternalRecommenderSslTest.java

@@ -0,0 +1,216 @@
+/*
+ * Licensed to the Technische Universität Darmstadt under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The Technische Universität Darmstadt 
+ * 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.
+ *  
+ * 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 de.tudarmstadt.ukp.inception.recommendation.imls.external.v1;
+
+import static de.tudarmstadt.ukp.inception.annotation.storage.CasMetadataUtils.getInternalTypeSystem;
+import static java.util.Arrays.asList;
+import static org.apache.uima.fit.factory.TypeSystemDescriptionFactory.createTypeSystemDescription;
+import static org.apache.uima.util.CasCreationUtils.mergeTypeSystems;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+
+import java.util.Arrays;
+import java.util.List;
+
+import org.apache.uima.cas.CAS;
+import org.apache.uima.fit.factory.JCasFactory;
+import org.apache.uima.jcas.JCas;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import de.tudarmstadt.ukp.clarin.webanno.api.type.CASMetadata;
+import de.tudarmstadt.ukp.clarin.webanno.model.AnchoringMode;
+import de.tudarmstadt.ukp.clarin.webanno.model.AnnotationFeature;
+import de.tudarmstadt.ukp.clarin.webanno.model.AnnotationLayer;
+import de.tudarmstadt.ukp.inception.annotation.storage.CasStorageSession;
+import de.tudarmstadt.ukp.inception.recommendation.api.model.Recommender;
+import de.tudarmstadt.ukp.inception.recommendation.api.recommender.RecommendationException;
+import de.tudarmstadt.ukp.inception.recommendation.api.recommender.RecommenderContext;
+import de.tudarmstadt.ukp.inception.recommendation.imls.external.v1.config.ExternalRecommenderPropertiesImpl;
+
+public class ExternalRecommenderSslTest
+{
+    private static final String TYPE = "de.tudarmstadt.ukp.dkpro.core.api.ner.type.NamedEntity";
+
+    private static final String USER_NAME = "test_user";
+    private static final long PROJECT_ID = 42L;
+    private static final boolean CROSS_SENTENCE = true;
+    private static final AnchoringMode ANCHORING_MODE = AnchoringMode.TOKENS;
+
+    private Recommender recommender;
+    private RecommenderContext context;
+    private ExternalRecommender sut;
+    private ExternalRecommenderTraits traits;
+    private CasStorageSession casStorageSession;
+    private List<CAS> data;
+
+    @BeforeEach
+    public void setUp() throws Exception
+    {
+        casStorageSession = CasStorageSession.open();
+        recommender = buildRecommender();
+        context = new RecommenderContext();
+
+        traits = new ExternalRecommenderTraits();
+
+        JCas jcas = JCasFactory.createJCas(
+                mergeTypeSystems(asList(createTypeSystemDescription(), getInternalTypeSystem())));
+        addCasMetadata(jcas, 1l);
+        data = Arrays.asList(jcas.getCas());
+    }
+
+    @AfterEach
+    public void tearDown() throws Exception
+    {
+        casStorageSession.close();
+    }
+
+    @Test
+    void thatDisablingCertificateValidationWorks_expired()
+    {
+        traits.setRemoteUrl("https://expired.badssl.com/");
+
+        traits.setVerifyCertificates(true);
+        sut = new ExternalRecommender(new ExternalRecommenderPropertiesImpl(), recommender, traits);
+        assertThatExceptionOfType(RecommendationException.class) //
+                .isThrownBy(() -> sut.train(context, data)) //
+                .withMessageContaining("PKIX path validation failed");
+
+        traits.setVerifyCertificates(false);
+        sut = new ExternalRecommender(new ExternalRecommenderPropertiesImpl(), recommender, traits);
+        assertThatExceptionOfType(RecommendationException.class) //
+                .isThrownBy(() -> sut.train(context, data)) //
+                .withMessageContaining("404 Not Found");
+    }
+
+    @Test
+    void thatDisablingCertificateValidationWorks_wrongHost()
+    {
+        traits.setRemoteUrl("https://wrong.host.badssl.com/");
+
+        traits.setVerifyCertificates(true);
+        sut = new ExternalRecommender(new ExternalRecommenderPropertiesImpl(), recommender, traits);
+        assertThatExceptionOfType(RecommendationException.class) //
+                .isThrownBy(() -> sut.train(context, data)) //
+                .withMessageContaining("No subject alternative DNS name matching");
+
+        // Disabling certificate validation does not disable host checking for recommenders.
+        // Instead the VM would need to be started with {@code
+        // -Djdk.internal.httpclient.disableHostnameVerification}
+        // System.setProperty("", "true");
+        // // traits.setVerifyCertificates(false);
+        // sut = new ExternalRecommender(new ExternalRecommenderPropertiesImpl(), recommender,
+        // traits);
+        // assertThatExceptionOfType(RecommendationException.class) //
+        // .isThrownBy(() -> sut.train(context, data)) //
+        // .withMessageContaining("404 Not Found");
+    }
+
+    @Test
+    void thatDisablingCertificateValidationWorks_selfSigned()
+    {
+        traits.setRemoteUrl("https://self-signed.badssl.com/");
+
+        traits.setVerifyCertificates(true);
+        sut = new ExternalRecommender(new ExternalRecommenderPropertiesImpl(), recommender, traits);
+        assertThatExceptionOfType(RecommendationException.class) //
+                .isThrownBy(() -> sut.train(context, data)) //
+                .withMessageContaining("PKIX path building failed");
+
+        traits.setVerifyCertificates(false);
+        sut = new ExternalRecommender(new ExternalRecommenderPropertiesImpl(), recommender, traits);
+        assertThatExceptionOfType(RecommendationException.class) //
+                .isThrownBy(() -> sut.train(context, data)) //
+                .withMessageContaining("404 Not Found");
+    }
+
+    @Test
+    void thatDisablingCertificateValidationWorks_untrusted()
+    {
+        traits.setRemoteUrl("https://untrusted-root.badssl.com/");
+
+        traits.setVerifyCertificates(true);
+        sut = new ExternalRecommender(new ExternalRecommenderPropertiesImpl(), recommender, traits);
+        assertThatExceptionOfType(RecommendationException.class) //
+                .isThrownBy(() -> sut.train(context, data)) //
+                .withMessageContaining("PKIX path building failed");
+
+        traits.setVerifyCertificates(false);
+        sut = new ExternalRecommender(new ExternalRecommenderPropertiesImpl(), recommender, traits);
+        assertThatExceptionOfType(RecommendationException.class) //
+                .isThrownBy(() -> sut.train(context, data)) //
+                .withMessageContaining("404 Not Found");
+    }
+
+    @Test
+    void thatDisablingCertificateValidationWorks_revoked()
+    {
+        traits.setRemoteUrl("https://revoked.badssl.com/");
+
+        traits.setVerifyCertificates(true);
+        sut = new ExternalRecommender(new ExternalRecommenderPropertiesImpl(), recommender, traits);
+        assertThatExceptionOfType(RecommendationException.class) //
+                .isThrownBy(() -> sut.train(context, data)) //
+                .withMessageContaining("PKIX path validation failed");
+
+        traits.setVerifyCertificates(false);
+        sut = new ExternalRecommender(new ExternalRecommenderPropertiesImpl(), recommender, traits);
+        assertThatExceptionOfType(RecommendationException.class) //
+                .isThrownBy(() -> sut.train(context, data)) //
+                .withMessageContaining("404 Not Found");
+    }
+
+    @Test
+    void thatCertificateValidationWorks()
+    {
+        traits.setRemoteUrl("https://tls-v1-2.badssl.com:1012/");
+
+        traits.setVerifyCertificates(true);
+        sut = new ExternalRecommender(new ExternalRecommenderPropertiesImpl(), recommender, traits);
+        assertThatExceptionOfType(RecommendationException.class) //
+                .isThrownBy(() -> sut.train(context, data)) //
+                .withMessageContaining("404 Not Found");
+    }
+
+    private static Recommender buildRecommender()
+    {
+        AnnotationLayer layer = new AnnotationLayer();
+        layer.setName(TYPE);
+        layer.setCrossSentence(CROSS_SENTENCE);
+        layer.setAnchoringMode(ANCHORING_MODE);
+
+        AnnotationFeature feature = new AnnotationFeature();
+        feature.setName("value");
+
+        Recommender recommender = new Recommender();
+        recommender.setLayer(layer);
+        recommender.setFeature(feature);
+        recommender.setMaxRecommendations(3);
+
+        return recommender;
+    }
+
+    private void addCasMetadata(JCas aJCas, long aDocumentId)
+    {
+        CASMetadata cmd = new CASMetadata(aJCas);
+        cmd.setUsername(USER_NAME);
+        cmd.setProjectId(PROJECT_ID);
+        cmd.setSourceDocumentId(aDocumentId);
+        aJCas.addFsToIndexes(cmd);
+    }
+}

inception/inception-remote/src/main/java/de/tudarmstadt/ukp/clarin/webanno/webapp/remoteapi/webhooks/WebhookService.java

@@ -30,7 +30,9 @@
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLSession;
 
 import org.apache.commons.codec.digest.DigestUtils;
 import org.apache.commons.lang3.tuple.Pair;
@@ -101,13 +103,16 @@ public WebhookService(WebhooksConfiguration aConfiguration,
         restTemplateBuilder = aRestTemplateBuilder;
 
         TrustStrategy acceptingTrustStrategy = (X509Certificate[] chain, String authType) -> true;
+        HostnameVerifier verifier = (String aHostname, SSLSession aSession) -> true;
 
         SSLContext sslContext = org.apache.http.ssl.SSLContexts.custom()
                 .loadTrustMaterial(null, acceptingTrustStrategy).build();
 
-        SSLConnectionSocketFactory csf = new SSLConnectionSocketFactory(sslContext);
+        SSLConnectionSocketFactory csf = new SSLConnectionSocketFactory(sslContext, verifier);
 
-        CloseableHttpClient httpClient = HttpClients.custom().setSSLSocketFactory(csf).build();
+        CloseableHttpClient httpClient = HttpClients.custom() //
+                .setSSLSocketFactory(csf) //
+                .build();
 
         nonValidatingRequestFactory = new HttpComponentsClientHttpRequestFactory();
         nonValidatingRequestFactory.setHttpClient(httpClient);
@@ -209,7 +214,7 @@ private void dispatch(String topic, Object message)
         }
     }
 
-    private void sendNotification(String topic, Object message, Webhook hook) throws IOException
+    void sendNotification(String topic, Object message, Webhook hook) throws IOException
     {
         log.trace("Sending webhook message on topic [{}] to [{}]", topic, hook.getUrl());
 

inception/inception-remote/src/test/java/de/tudarmstadt/ukp/clarin/webanno/webapp/remoteapi/webhooks/WebhookServiceTest.java

@@ -23,6 +23,7 @@
 import static de.tudarmstadt.ukp.clarin.webanno.webapp.remoteapi.webhooks.WebhookService.X_AERO_NOTIFICATION;
 import static java.util.Arrays.asList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
 
 import java.lang.invoke.MethodHandles;
@@ -55,6 +56,8 @@
 import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RequestHeader;
 import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.client.HttpClientErrorException;
+import org.springframework.web.client.ResourceAccessException;
 
 import de.tudarmstadt.ukp.clarin.webanno.api.event.AnnotationStateChangeEvent;
 import de.tudarmstadt.ukp.clarin.webanno.api.event.DocumentStateChangedEvent;
@@ -102,6 +105,7 @@ public class WebhookServiceTest
     private @Autowired ApplicationEventPublisher applicationEventPublisher;
     private @Autowired WebhooksConfiguration webhooksConfiguration;
     private @Autowired TestService testService;
+    private @Autowired WebhookService webhookService;
 
     private Project project;
     private SourceDocument doc;
@@ -221,6 +225,97 @@ public void thatDeliveryIsRetried()
                 .containsExactly(new AnnotationStateChangeMessage(event));
     }
 
+    @Test
+    void thatDisablingCertificateValidationWorks_expired()
+    {
+        hook.setUrl("https://expired.badssl.com/");
+
+        hook.setVerifyCertificates(true);
+        assertThatExceptionOfType(ResourceAccessException.class) //
+                .isThrownBy(() -> webhookService.sendNotification("test", "test", hook)) //
+                .withMessageContaining("PKIX path validation failed");
+
+        hook.setVerifyCertificates(false);
+        assertThatExceptionOfType(HttpClientErrorException.class) //
+                .isThrownBy(() -> webhookService.sendNotification("test", "test", hook)) //
+                .withMessageContaining("405 Not Allowed");
+    }
+
+    @Test
+    void thatDisablingCertificateValidationWorks_wrongHost()
+    {
+        hook.setUrl("https://wrong.host.badssl.com/");
+
+        hook.setVerifyCertificates(true);
+        assertThatExceptionOfType(ResourceAccessException.class) //
+                .isThrownBy(() -> webhookService.sendNotification("test", "test", hook)) //
+                .withMessageContaining("match any of the subject alternative names");
+
+        hook.setVerifyCertificates(false);
+        assertThatExceptionOfType(HttpClientErrorException.class) //
+                .isThrownBy(() -> webhookService.sendNotification("test", "test", hook)) //
+                .withMessageContaining("405 Not Allowed");
+    }
+
+    @Test
+    void thatDisablingCertificateValidationWorks_selfSigned()
+    {
+        hook.setUrl("https://self-signed.badssl.com/");
+
+        hook.setVerifyCertificates(true);
+        assertThatExceptionOfType(ResourceAccessException.class) //
+                .isThrownBy(() -> webhookService.sendNotification("test", "test", hook)) //
+                .withMessageContaining("PKIX path building failed");
+
+        hook.setVerifyCertificates(false);
+        assertThatExceptionOfType(HttpClientErrorException.class) //
+                .isThrownBy(() -> webhookService.sendNotification("test", "test", hook)) //
+                .withMessageContaining("405 Not Allowed");
+    }
+
+    @Test
+    void thatDisablingCertificateValidationWorks_untrusted()
+    {
+        hook.setUrl("https://untrusted-root.badssl.com/");
+
+        hook.setVerifyCertificates(true);
+        assertThatExceptionOfType(ResourceAccessException.class) //
+                .isThrownBy(() -> webhookService.sendNotification("test", "test", hook)) //
+                .withMessageContaining("PKIX path building failed");
+
+        hook.setVerifyCertificates(false);
+        assertThatExceptionOfType(HttpClientErrorException.class) //
+                .isThrownBy(() -> webhookService.sendNotification("test", "test", hook)) //
+                .withMessageContaining("405 Not Allowed");
+    }
+
+    @Test
+    void thatDisablingCertificateValidationWorks_revoked()
+    {
+        hook.setUrl("https://revoked.badssl.com/");
+
+        hook.setVerifyCertificates(true);
+        assertThatExceptionOfType(ResourceAccessException.class) //
+                .isThrownBy(() -> webhookService.sendNotification("test", "test", hook)) //
+                .withMessageContaining("PKIX path validation failed");
+
+        hook.setVerifyCertificates(false);
+        assertThatExceptionOfType(HttpClientErrorException.class) //
+                .isThrownBy(() -> webhookService.sendNotification("test", "test", hook)) //
+                .withMessageContaining("405 Not Allowed");
+    }
+
+    @Test
+    void thatCertificateValidationWorks()
+    {
+        hook.setUrl("https://tls-v1-2.badssl.com:1012/");
+
+        hook.setVerifyCertificates(true);
+        assertThatExceptionOfType(HttpClientErrorException.class) //
+                .isThrownBy(() -> webhookService.sendNotification("test", "test", hook)) //
+                .withMessageContaining("405 Not Allowed");
+    }
+
     @RequestMapping("/test")
     @Controller
     public static class TestService