New HTTP framework to incorporate automatic insecure connection handling

jib-core/CHANGELOG.md

@@ -9,6 +9,8 @@ All notable changes to this project will be documented in this file.
 
 ### Fixed
 
+- `Containerizer#setAllowInsecureRegistries(boolean)` and the `sendCredentialsOverHttp` system property are now effective for authentication service server connections. ([#2074](https://github.com/GoogleContainerTools/jib/pull/2074)
+
 ## 0.12.0
 
 ### Added

jib-core/src/integration-test/java/com/google/cloud/tools/jib/registry/ManifestPusherIntegrationTest.java

@@ -16,14 +16,14 @@
 
 package com.google.cloud.tools.jib.registry;
 
-import com.google.api.client.http.HttpResponseException;
 import com.google.api.client.http.HttpStatusCodes;
 import com.google.cloud.tools.jib.api.DescriptorDigest;
 import com.google.cloud.tools.jib.api.RegistryException;
 import com.google.cloud.tools.jib.blob.Blob;
 import com.google.cloud.tools.jib.blob.Blobs;
 import com.google.cloud.tools.jib.event.EventHandlers;
 import com.google.cloud.tools.jib.hash.Digests;
+import com.google.cloud.tools.jib.http.ResponseException;
 import com.google.cloud.tools.jib.image.json.ManifestTemplate;
 import com.google.cloud.tools.jib.image.json.V22ManifestTemplate;
 import java.io.IOException;
@@ -58,9 +58,9 @@ public void testPush_missingBlobs() throws IOException, RegistryException {
       Assert.fail("Pushing manifest without its BLOBs should fail");
 
     } catch (RegistryErrorException ex) {
-      HttpResponseException httpResponseException = (HttpResponseException) ex.getCause();
+      ResponseException responseException = (ResponseException) ex.getCause();
       Assert.assertEquals(
-          HttpStatusCodes.STATUS_CODE_BAD_REQUEST, httpResponseException.getStatusCode());
+          HttpStatusCodes.STATUS_CODE_BAD_REQUEST, responseException.getStatusCode());
     }
   }
 

jib-core/src/main/java/com/google/cloud/tools/jib/api/InsecureRegistryException.java

@@ -23,10 +23,11 @@
  */
 public class InsecureRegistryException extends RegistryException {
 
-  public InsecureRegistryException(URL insecureUrl) {
+  public InsecureRegistryException(URL insecureUrl, Throwable cause) {
     super(
         "Failed to verify the server at "
             + insecureUrl
-            + " because only secure connections are allowed.");
+            + " because only secure connections are allowed.",
+        cause);
   }
 }

jib-core/src/main/java/com/google/cloud/tools/jib/api/RegistryUnauthorizedException.java

@@ -17,6 +17,7 @@
 package com.google.cloud.tools.jib.api;
 
 import com.google.api.client.http.HttpResponseException;
+import com.google.cloud.tools.jib.http.ResponseException;
 
 /** Thrown when a registry request was unauthorized and therefore authentication is needed. */
 public class RegistryUnauthorizedException extends RegistryException {
@@ -32,7 +33,7 @@ public class RegistryUnauthorizedException extends RegistryException {
    * @param cause the cause
    */
   public RegistryUnauthorizedException(
-      String registry, String repository, HttpResponseException cause) {
+      String registry, String repository, ResponseException cause) {
     super("Unauthorized for " + registry + "/" + repository, cause);
     this.registry = registry;
     this.repository = repository;

jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java

@@ -17,68 +17,89 @@
 package com.google.cloud.tools.jib.http;
 
 import com.google.api.client.http.GenericUrl;
+import com.google.api.client.http.HttpHeaders;
 import com.google.api.client.http.HttpMethods;
 import com.google.api.client.http.HttpRequest;
-import com.google.api.client.http.HttpRequestFactory;
-import com.google.api.client.http.HttpResponse;
+import com.google.api.client.http.HttpResponseException;
 import com.google.api.client.http.HttpTransport;
 import com.google.api.client.http.apache.ApacheHttpTransport;
+import com.google.cloud.tools.jib.api.LogEvent;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
-import java.io.Closeable;
 import java.io.IOException;
+import java.net.ConnectException;
 import java.net.URL;
 import java.security.GeneralSecurityException;
-import java.util.function.Function;
-import javax.annotation.Nullable;
+import java.util.function.Consumer;
+import java.util.function.Supplier;
+import javax.net.ssl.SSLException;
 import org.apache.http.auth.AuthScope;
 import org.apache.http.auth.UsernamePasswordCredentials;
 import org.apache.http.impl.client.DefaultHttpClient;
 
 /**
- * Sends an HTTP {@link Request} and stores the {@link Response}. Clients should not send more than
- * one request.
+ * Thread-safe HTTP client that can automatically failover from secure HTTPS to insecure HTTPS or
+ * HTTP. Intended to be created once and shared to be called at multiple places. Callers should
+ * close the returned {@link Response}.
  *
- * <p>Example usage:
+ * <p>The failover (if enabled) in the following way:
  *
- * <pre>{@code
- * try (Connection connection = new Connection(url)) {
- *   Response response = connection.get(request);
- *   // ... process the response
- * }
- * }</pre>
+ * <ul>
+ *   <li>When a port is provided (for example {@code my-registry:5000/my-repo}):
+ *       <ol>
+ *         <li>Attempts secure HTTPS on the specified port.
+ *         <li>If (1) fails due to {@link SSLException}, re-attempts secure HTTPS on the specified
+ *             port but disabling certificate validation.
+ *         <li>If (2) fails again due to {@link SSLException}, attempts plain-HTTP on the specified
+ *             port.
+ *       </ol>
+ *   <li>When a port is not provided (for example {@code my-registry/my-repo}):
+ *       <ol>
+ *         <li>Attempts secure HTTPS on port 443 (default HTTPS port).
+ *         <li>If (1) fails due to {@link SSLException}, re-attempts secure HTTPS on port 443 but
+ *             disabling certificate validation.
+ *         <li>If (2) fails again due to {@link SSLException}, attempts plain-HTTP on port 80
+ *             (default HTTP port).
+ *         <li>Or, if (1) fails due to non-timeout {@link ConnectException}, attempts plain-HTTP on
+ *             port 80.
+ *       </ol>
+ * </ul>
+ *
+ * This failover behavior is similar to how the Docker client works:
+ * https://docs.docker.com/registry/insecure/#deploy-a-plain-http-registry
  */
-public class Connection implements Closeable {
+public class Connection { // TODO: rename to TlsFailoverHttpClient
 
-  /**
-   * Returns a factory for {@link Connection}.
-   *
-   * @return {@link Connection} factory, a function that generates a {@link Connection} to a URL
-   */
-  public static Function<URL, Connection> getConnectionFactory() {
+  private static boolean isHttpsProtocol(URL url) {
+    return "https".equals(url.getProtocol());
+  }
+
+  private static URL toHttp(URL url) {
+    GenericUrl httpUrl = new GenericUrl(url);
+    httpUrl.setScheme("http");
+    return httpUrl.toURL();
+  }
+
+  private static HttpTransport getSecureHttpTransport() {
     // Do not use NetHttpTransport. It does not process response errors properly.
     // See https://github.com/google/google-http-java-client/issues/39
     //
     // A new ApacheHttpTransport needs to be created for each connection because otherwise HTTP
     // connection persistence causes the connection to throw NoHttpResponseException.
     ApacheHttpTransport transport = new ApacheHttpTransport();
     addProxyCredentials(transport);
-    return url -> new Connection(url, transport);
+    return transport;
   }
 
-  /**
-   * Returns a factory for {@link Connection} that does not verify TLS peer verification.
-   *
-   * @throws GeneralSecurityException if unable to turn off TLS peer verification
-   * @return {@link Connection} factory, a function that generates a {@link Connection} to a URL
-   */
-  public static Function<URL, Connection> getInsecureConnectionFactory()
-      throws GeneralSecurityException {
-    // Do not use NetHttpTransport. See comments in getConnectionFactory for details.
-    ApacheHttpTransport transport =
-        new ApacheHttpTransport.Builder().doNotValidateCertificate().build();
-    addProxyCredentials(transport);
-    return url -> new Connection(url, transport);
+  private static HttpTransport getInsecureHttpTransport() {
+    try {
+      ApacheHttpTransport insecureTransport =
+          new ApacheHttpTransport.Builder().doNotValidateCertificate().build();
+      addProxyCredentials(insecureTransport);
+      return insecureTransport;
+    } catch (GeneralSecurityException ex) {
+      throw new RuntimeException("platform does not support TLS protocol", ex);
+    }
   }
 
   /**
@@ -114,87 +135,157 @@ private static void addProxyCredentials(ApacheHttpTransport transport, String pr
             new UsernamePasswordCredentials(proxyUser, proxyPassword));
   }
 
-  private HttpRequestFactory requestFactory;
-
-  @Nullable private HttpResponse httpResponse;
-
-  /** The URL to send the request to. */
-  private final GenericUrl url;
+  private final boolean enableHttpAndInsecureFailover;
+  private final boolean sendAuthorizationOverHttp;
+  private final Consumer<LogEvent> logger;
+  private final Supplier<HttpTransport> secureHttpTransportFactory;
+  private final Supplier<HttpTransport> insecureHttpTransportFactory;
 
-  /**
-   * Make sure to wrap with a try-with-resource to ensure that the connection is closed after usage.
-   *
-   * @param url the url to send the request to
-   */
-  @VisibleForTesting
-  Connection(URL url, HttpTransport transport) {
-    this.url = new GenericUrl(url);
-    requestFactory = transport.createRequestFactory();
+  public Connection(
+      boolean enableHttpAndInsecureFailover,
+      boolean sendAuthorizationOverHttp,
+      Consumer<LogEvent> logger) {
+    this(
+        enableHttpAndInsecureFailover,
+        sendAuthorizationOverHttp,
+        logger,
+        Connection::getSecureHttpTransport,
+        Connection::getInsecureHttpTransport);
   }
 
-  @Override
-  public void close() throws IOException {
-    if (httpResponse == null) {
-      return;
-    }
-
-    httpResponse.disconnect();
+  @VisibleForTesting
+  Connection(
+      boolean enableHttpAndInsecureFailover,
+      boolean sendAuthorizationOverHttp,
+      Consumer<LogEvent> logger,
+      Supplier<HttpTransport> secureHttpTransportFactory,
+      Supplier<HttpTransport> insecureHttpTransportFactory) {
+    this.enableHttpAndInsecureFailover = enableHttpAndInsecureFailover;
+    this.sendAuthorizationOverHttp = sendAuthorizationOverHttp;
+    this.logger = logger;
+    this.secureHttpTransportFactory = secureHttpTransportFactory;
+    this.insecureHttpTransportFactory = insecureHttpTransportFactory;
   }
 
   /**
    * Sends the request with method GET.
    *
+   * @param url endpoint URL
    * @param request the request to send
    * @return the response to the sent request
    * @throws IOException if sending the request fails
    */
-  public Response get(Request request) throws IOException {
-    return send(HttpMethods.GET, request);
+  public Response get(URL url, Request request) throws IOException {
+    return call(HttpMethods.GET, url, request);
   }
 
   /**
    * Sends the request with method POST.
    *
+   * @param url endpoint URL
    * @param request the request to send
    * @return the response to the sent request
    * @throws IOException if sending the request fails
    */
-  public Response post(Request request) throws IOException {
-    return send(HttpMethods.POST, request);
+  public Response post(URL url, Request request) throws IOException {
+    return call(HttpMethods.POST, url, request);
   }
 
   /**
    * Sends the request with method PUT.
    *
+   * @param url endpoint URL
    * @param request the request to send
    * @return the response to the sent request
    * @throws IOException if sending the request fails
    */
-  public Response put(Request request) throws IOException {
-    return send(HttpMethods.PUT, request);
+  public Response put(URL url, Request request) throws IOException {
+    return call(HttpMethods.PUT, url, request);
   }
 
   /**
    * Sends the request.
    *
    * @param httpMethod the HTTP request method
+   * @param url endpoint URL
    * @param request the request to send
    * @return the response to the sent request
    * @throws IOException if building the HTTP request fails.
    */
-  public Response send(String httpMethod, Request request) throws IOException {
-    Preconditions.checkState(httpResponse == null, "Connection can send only one request");
+  public Response call(String httpMethod, URL url, Request request) throws IOException {
+    if (!isHttpsProtocol(url) && !enableHttpAndInsecureFailover) {
+      throw new SSLException("insecure HTTP connection not allowed: " + url);
+    }
+
+    try {
+      return call(httpMethod, url, request, secureHttpTransportFactory.get());
+
+    } catch (SSLException ex) {
+      if (!enableHttpAndInsecureFailover) {
+        throw ex;
+      }
+
+      try {
+        logInsecureHttpsFailover(url);
+        return call(httpMethod, url, request, insecureHttpTransportFactory.get());
+
+      } catch (SSLException ignored) { // This is usually when the server is plain-HTTP.
+        logHttpFailover(url);
+        return call(httpMethod, toHttp(url), request, secureHttpTransportFactory.get());
+      }
+
+    } catch (ConnectException ex) {
+      // It is observed that Open/Oracle JDKs sometimes throw SocketTimeoutException but other times
+      // ConnectException for connection timeout. (Could be a JDK bug.) Note SocketTimeoutException
+      // does not extend ConnectException (or vice versa), and we want to be consistent to error out
+      // on timeouts: https://github.com/GoogleContainerTools/jib/issues/1895#issuecomment-527544094
+      if (ex.getMessage() != null && ex.getMessage().contains("timed out")) {
+        throw ex;
+      }
+
+      // Fall back to HTTP only if "url" had no port specified (i.e., we tried the default HTTPS
+      // port 443) and we could not connect to 443. It's worth trying port 80.
+      if (enableHttpAndInsecureFailover && isHttpsProtocol(url) && url.getPort() == -1) {
+        logHttpFailover(url);
+        return call(httpMethod, toHttp(url), request, secureHttpTransportFactory.get());
+      }
+      throw ex;
+    }
+  }
+
+  private Response call(String httpMethod, URL url, Request request, HttpTransport httpTransport)
+      throws IOException {
+    boolean clearAuthorization = !isHttpsProtocol(url) && !sendAuthorizationOverHttp;
+
+    HttpHeaders requestHeaders =
+        clearAuthorization
+            ? request.getHeaders().clone().setAuthorization((String) null) // deep clone implemented
+            : request.getHeaders();
 
     HttpRequest httpRequest =
-        requestFactory
-            .buildRequest(httpMethod, url, request.getHttpContent())
-            .setHeaders(request.getHeaders());
+        httpTransport
+            .createRequestFactory()
+            .buildRequest(httpMethod, new GenericUrl(url), request.getHttpContent())
+            .setHeaders(requestHeaders);
     if (request.getHttpTimeout() != null) {
       httpRequest.setConnectTimeout(request.getHttpTimeout());
       httpRequest.setReadTimeout(request.getHttpTimeout());
     }
 
-    httpResponse = httpRequest.execute();
-    return new Response(httpResponse);
+    try {
+      return new Response(httpRequest.execute());
+    } catch (HttpResponseException ex) {
+      throw new ResponseException(ex, clearAuthorization);
+    }
+  }
+
+  private void logHttpFailover(URL url) {
+    String log = "Failed to connect to " + url + " over HTTPS. Attempting again with HTTP.";
+    logger.accept(LogEvent.info(log));
+  }
+
+  private void logInsecureHttpsFailover(URL url) {
+    String log = "Cannot verify server at " + url + ". Attempting again with no TLS verification.";
+    logger.accept(LogEvent.info(log));
   }
 }