Merge pull request from GHSA-hcxx-mp6g-6gr9

Disable sending system digest credentials unless warranted

modules/ingest-service-impl/src/main/java/org/opencastproject/ingest/impl/IngestServiceImpl.java

@@ -129,13 +129,15 @@
 import java.util.Dictionary;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Objects;
 import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
 
 import javax.management.ObjectInstance;
 
@@ -1564,18 +1566,34 @@ protected URI addContentToRepo(MediaPackage mp, String elementId, URI uri) throw
     try {
       if (uri.toString().startsWith("http")) {
         HttpGet get = new HttpGet(uri);
+        List<String> clusterUrls = new LinkedList<>();
+        try {
+          // Note that we are not checking ports here.
+          clusterUrls = organizationDirectoryService.getOrganization(uri.toURL()).getServers()
+                          .keySet()
+                          .stream()
+                          .collect(Collectors.toUnmodifiableList());
+        } catch (NotFoundException e) {
+          logger.warn("Unable to determine cluster members, will not be able to authenticate any downloads from them", e);
+        }
 
-        if (uri.getHost().matches(downloadSource)) {
-          CredentialsProvider provider = new BasicCredentialsProvider();
-          provider.setCredentials(
-              new AuthScope(AuthScope.ANY_HOST, AuthScope.ANY_PORT, AuthScope.ANY_REALM, AuthSchemes.DIGEST),
-              new UsernamePasswordCredentials(downloadUser, downloadPassword));
-           externalHttpClient = HttpClientBuilder.create()
-              .setDefaultCredentialsProvider(provider)
-              .build();
+        if (uri.toString().matches(downloadSource)) {
+          //NB: We're creating a new client here with *different* auth than the system auth creds
+          externalHttpClient = getAuthedHttpClient();
           response = externalHttpClient.execute(get);
-        } else {
+        } else if (clusterUrls.contains(uri.getScheme() + "://" + uri.getHost())) {
+          // Only using the system-level httpclient and digest credentials against our own servers
           response = httpClient.execute(get);
+        } else {
+          //NB: No auth here at all
+          externalHttpClient = getNoAuthHttpClient();
+          response = externalHttpClient.execute(get);
+        }
+
+        if (null == response) {
+          // If you get here then chances are you're using a mock httpClient which does not have appropriate
+          // mocking to respond to the URL you are feeding it.  Try adding that URL to the mock and see if that works.
+          throw new IOException("Null response object from the http client, refer to code for explanation");
         }
 
         int httpStatusCode = response.getStatusLine().getStatusCode();
@@ -1760,6 +1778,20 @@ protected OrganizationDirectoryService getOrganizationDirectoryService() {
     return organizationDirectoryService;
   }
 
+  //Used in testing
+  protected CloseableHttpClient getNoAuthHttpClient() {
+    return HttpClientBuilder.create().build();
+  }
+
+  protected CloseableHttpClient getAuthedHttpClient() {
+    HttpClientBuilder cb = HttpClientBuilder.create();
+    CredentialsProvider provider = new BasicCredentialsProvider();
+    provider.setCredentials(
+      new AuthScope(AuthScope.ANY_HOST, AuthScope.ANY_PORT, AuthScope.ANY_REALM, AuthSchemes.DIGEST),
+      new UsernamePasswordCredentials(downloadUser, downloadPassword));
+    return cb.build();
+  }
+
   private MediaPackage createSmil(MediaPackage mediaPackage) throws IOException, IngestException {
     Stream<Track> partialTracks = Stream.empty();
     for (Track track : mediaPackage.getTracks()) {

modules/ingest-service-impl/src/test/java/org/opencastproject/ingest/impl/IngestServiceImplTest.java

@@ -72,9 +72,10 @@
 import org.apache.commons.lang3.reflect.FieldUtils;
 import org.apache.http.Header;
 import org.apache.http.HttpEntity;
-import org.apache.http.HttpResponse;
 import org.apache.http.StatusLine;
+import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
+import org.apache.http.impl.client.CloseableHttpClient;
 import org.easymock.Capture;
 import org.easymock.EasyMock;
 import org.easymock.IAnswer;
@@ -93,6 +94,7 @@
 import java.io.InputStream;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.net.URL;
 import java.util.Dictionary;
 import java.util.HashMap;
 import java.util.Hashtable;
@@ -105,6 +107,9 @@ public class IngestServiceImplTest {
   private WorkflowService workflowService = null;
   private WorkflowInstance workflowInstance = null;
   private WorkingFileRepository wfr = null;
+  private CloseableHttpResponse httpResponse = null;
+  private CloseableHttpClient credClient = null;
+  private CloseableHttpClient noCredClient = null;
   private static URI baseDir;
   private static URI urlTrack;
   private static URI urlTrack1;
@@ -145,10 +150,14 @@ public static void beforeClass() throws URISyntaxException {
 
   }
 
-  @SuppressWarnings({ "unchecked", "rawtypes" })
   @Before
   public void setUp() throws Exception {
+    //This looks dumb, but is required so we can override things in testAuthWhitelist
+    setupService();
+  }
 
+  @SuppressWarnings({ "unchecked", "rawtypes" })
+  private void setupService() throws Exception {
     schedulerMediaPackage = MediaPackageParser
             .getFromXml(IOUtils.toString(getClass().getResourceAsStream("/source-manifest.xml"), "UTF-8"));
 
@@ -234,6 +243,8 @@ public void setUp() throws Exception {
     OrganizationDirectoryService organizationDirectoryService = EasyMock.createMock(OrganizationDirectoryService.class);
     EasyMock.expect(organizationDirectoryService.getOrganization((String) EasyMock.anyObject())).andReturn(organization)
             .anyTimes();
+    EasyMock.expect(organizationDirectoryService.getOrganization((URL) EasyMock.anyObject())).andReturn(organization)
+            .anyTimes();
     EasyMock.replay(organizationDirectoryService);
 
     SecurityService securityService = EasyMock.createNiceMock(SecurityService.class);
@@ -256,7 +267,7 @@ public void setUp() throws Exception {
     EasyMock.expect(contentDispositionHeader.getValue()).andReturn("attachment; filename=fname.mp4").anyTimes();
     EasyMock.replay(contentDispositionHeader);
 
-    HttpResponse httpResponse = EasyMock.createMock(HttpResponse.class);
+    httpResponse = EasyMock.createMock(CloseableHttpResponse.class);
     EasyMock.expect(httpResponse.getStatusLine()).andReturn(statusLine).anyTimes();
     EasyMock.expect(httpResponse.getFirstHeader("Content-Disposition")).andReturn(contentDispositionHeader).anyTimes();
     EasyMock.expect(httpResponse.getEntity()).andReturn(entity).anyTimes();
@@ -289,14 +300,36 @@ public Job answer() throws Throwable {
             }).anyTimes();
     EasyMock.replay(mediaInspectionService);
 
-    class MockedIngestServicve extends IngestServiceImpl {
-      protected TrustedHttpClient createStandaloneHttpClient(String user, String password) {
-        return httpClient;
-      }
+    if (null == service) {
+      service = new IngestServiceImpl() {
+
+        //These are overriden so that we get mock requests, not *actual* requests
+        @Override
+        protected CloseableHttpClient getAuthedHttpClient() {
+          CloseableHttpClient client = EasyMock.createMock(CloseableHttpClient.class);
+          try {
+            EasyMock.expect(client.execute((HttpGet) EasyMock.anyObject())).andReturn(httpResponse).anyTimes();
+            client.close();
+            EasyMock.expectLastCall().once();
+          } catch (Exception e) { }
+          EasyMock.replay(client);
+          return client;
+        }
+        @Override
+        protected CloseableHttpClient getNoAuthHttpClient() {
+          CloseableHttpClient client = EasyMock.createMock(CloseableHttpClient.class);
+          try {
+            EasyMock.expect(client.execute((HttpGet) EasyMock.anyObject())).andReturn(httpResponse).anyTimes();
+            client.close();
+            EasyMock.expectLastCall().once();
+          } catch (Exception e) { }
+          EasyMock.replay(client);
+          return client;
+        }
+      };
     }
-
-    service = new MockedIngestServicve();
     service.setHttpClient(httpClient);
+    service.setOrganizationDirectoryService(organizationDirectoryService);
     service.setWorkingFileRepository(wfr);
     service.setWorkflowService(workflowService);
     service.setSecurityService(securityService);
@@ -308,6 +341,9 @@ protected TrustedHttpClient createStandaloneHttpClient(String user, String passw
     service.setServiceRegistry(serviceRegistry);
     service.defaultWorkflowDefinionId = "sample";
     serviceRegistry.registerService(service);
+    Dictionary<String, String> p = new Hashtable<>();
+    p.put(IngestServiceImpl.DOWNLOAD_SOURCE, "http://localhost.*|http://www.test.com/.*");
+    service.updated(p);
   }
 
   @After
@@ -384,7 +420,7 @@ public void testContentDisposition() throws Exception {
     try {
       mediaPackage = service.addTrack(URI.create("http://www.test.com/testfile"), null, mediaPackage);
     } catch (Exception e) {
-      Assert.fail("Unable to read content dispostion filename!");
+      Assert.fail("Unable to read content dispostion filename: " + e.getMessage());
     }
 
     try {
@@ -395,6 +431,69 @@ public void testContentDisposition() throws Exception {
     }
   }
 
+  private void testAuthWhitelist(String url, String regex, boolean shouldFail, boolean shouldSendAuth, boolean shouldTouchMocks) throws Exception {
+    credClient = EasyMock.createNiceMock(CloseableHttpClient.class);
+    noCredClient = EasyMock.createNiceMock(CloseableHttpClient.class);
+
+    //There's one case (accessing the filesystem) where we *don't* expect the mocks to be used
+    if (shouldTouchMocks) {
+      if (shouldSendAuth) {
+        EasyMock.expect(credClient.execute(EasyMock.anyObject())).andReturn(httpResponse).once();
+        credClient.close();
+        EasyMock.expectLastCall().once();
+      } else {
+        EasyMock.expect(noCredClient.execute(EasyMock.anyObject())).andReturn(httpResponse).once();
+        noCredClient.close();
+        EasyMock.expectLastCall().once();
+      }
+    }
+    EasyMock.replay(noCredClient, credClient);
+
+    //Recreate the service so we use our own, custom mocks
+    service = new IngestServiceImpl() {
+      @Override
+      protected CloseableHttpClient getAuthedHttpClient() {
+        return credClient;
+      }
+
+      @Override
+      protected CloseableHttpClient getNoAuthHttpClient() {
+        return noCredClient;
+      }
+    };
+    setupService();
+
+    MediaPackage mediaPackage = service.createMediaPackage();
+
+    Dictionary<String, String> props = new Hashtable<>();
+    props.put(IngestServiceImpl.DOWNLOAD_SOURCE, regex);
+    service.updated(props);
+
+    try {
+      service.addTrack(URI.create(url), null, mediaPackage);
+    } catch (IOException e) {
+      if (!shouldFail) {
+        Assert.fail("Should not have failed!");
+      }
+    }
+    EasyMock.verify(credClient, noCredClient);
+    EasyMock.reset(credClient, noCredClient);
+  }
+
+  @Test
+  public void testAuthWhitelist() throws Exception {
+    //Test fetching something from something known to be inside the cluster.  This should use the default service-wide trusted client
+    testAuthWhitelist("http://localhost/testfile", "", false, true, false);
+
+    //Clear the whitelist, this should *never* send digest auth when fetching files
+    testAuthWhitelist("http://www.example.org/testfile", "", false, false, true);
+    //Non-matching regex
+    testAuthWhitelist("http://www.example.org/testfile", "http://localhost.*", true, false, true);
+    //Matching regex
+    testAuthWhitelist("http://www.example.org/testfile", "http://localhost.*|http://www.example.org/.*", false, true, true);
+  }
+
+
   @Test
   public void testSmilCreation() throws Exception {
     service.setWorkingFileRepository(new WorkingFileRepositoryImpl() {