envoyproxy · mattklein123 · Jan 9, 2019 · Jan 9, 2019 · Jan 9, 2019 · Jan 9, 2019
diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst
@@ -4,6 +4,7 @@ Version history
 1.10.0 (pending)
 ================
 * access log: added a new flag for upstream retry count exceeded.
+* admin: the admin server can now be accessed via HTTP/2 (prior knowledge).
 * buffer: fix vulnerabilities when allocation fails
 * config: removed deprecated_v1 sds_config from :ref:`Bootstrap config <config_overview_v2_bootstrap>`.
 * config: removed REST_LEGACY as a valid :ref:`ApiType <envoy_api_field_core.ApiConfigSource.api_type>`.

diff --git a/include/envoy/server/admin.h b/include/envoy/server/admin.h
@@ -39,6 +39,11 @@ class AdminStream {
    */
   virtual Http::StreamDecoderFilterCallbacks& getDecoderFilterCallbacks() const PURE;
 
+  /**
+   * @return const Buffer::Instance* the fully buffered admin request if applicable.
+   */
+  virtual const Buffer::Instance* getRequestBody() const PURE;
+
   /**
    * @return Http::HeaderMap& to be used by handler to parse header information sent with the
    * request.

diff --git a/include/envoy/singleton/BUILD b/include/envoy/singleton/BUILD
@@ -18,5 +18,6 @@ envoy_cc_library(
     hdrs = ["manager.h"],
     deps = [
         ":instance_interface",
+        "//include/envoy/registry",
     ],
 )
diff --git a/include/envoy/singleton/manager.h b/include/envoy/singleton/manager.h
@@ -5,6 +5,7 @@
 #include <string>
 
 #include "envoy/common/pure.h"
+#include "envoy/registry/registry.h"
 #include "envoy/singleton/instance.h"
 
 namespace Envoy {

diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc
@@ -8,6 +8,8 @@
 #include "common/common/empty_string.h"
 #include "common/common/utility.h"
 #include "common/http/headers.h"
+#include "common/http/http1/codec_impl.h"
+#include "common/http/http2/codec_impl.h"
 #include "common/http/utility.h"
 #include "common/network/utility.h"
 #include "common/runtime/uuid_util.h"
@@ -18,10 +20,40 @@
 namespace Envoy {
 namespace Http {
 
+std::string ConnectionManagerUtility::determineNextProtocol(Network::Connection& connection,
+                                                            const Buffer::Instance& data) {
+  if (!connection.nextProtocol().empty()) {
+    return connection.nextProtocol();
+  }
+
+  // See if the data we have so far shows the HTTP/2 prefix. We ignore the case where someone sends
+  // us the first few bytes of the HTTP/2 prefix since in all public cases we use SSL/ALPN. For
+  // internal cases this should practically never happen.
+  if (-1 != data.search(Http2::CLIENT_MAGIC_PREFIX.c_str(), Http2::CLIENT_MAGIC_PREFIX.size(), 0)) {
+    return Http2::ALPN_STRING;
+  }
+
+  return "";
+}
+
+ServerConnectionPtr ConnectionManagerUtility::autoCreateCodec(Network::Connection& connection,
+                                                              const Buffer::Instance& data,
+                                                              ServerConnectionCallbacks& callbacks,
+                                                              Stats::Scope& scope,
+                                                              const Http1Settings& http1_settings,
+                                                              const Http2Settings& http2_settings) {
+  if (determineNextProtocol(connection, data) == Http2::ALPN_STRING) {
+    return ServerConnectionPtr{
+        new Http2::ServerConnectionImpl(connection, callbacks, scope, http2_settings)};
+  } else {
+    return ServerConnectionPtr{
+        new Http1::ServerConnectionImpl(connection, callbacks, http1_settings)};
+  }
+}
+
 Network::Address::InstanceConstSharedPtr ConnectionManagerUtility::mutateRequestHeaders(
-    Http::HeaderMap& request_headers, Network::Connection& connection,
-    ConnectionManagerConfig& config, const Router::Config& route_config,
-    Runtime::RandomGenerator& random, Runtime::Loader& runtime,
+    HeaderMap& request_headers, Network::Connection& connection, ConnectionManagerConfig& config,
+    const Router::Config& route_config, Runtime::RandomGenerator& random, Runtime::Loader& runtime,
     const LocalInfo::LocalInfo& local_info) {
   // If this is a Upgrade request, do not remove the Connection and Upgrade headers,
   // as we forward them verbatim to the upstream hosts.
@@ -139,7 +171,7 @@ Network::Address::InstanceConstSharedPtr ConnectionManagerUtility::mutateRequest
     request_headers.removeEnvoyForceTrace();
     request_headers.removeEnvoyIpTags();
 
-    for (const Http::LowerCaseString& header : route_config.internalOnlyHeaders()) {
+    for (const LowerCaseString& header : route_config.internalOnlyHeaders()) {
       request_headers.remove(header);
     }
   }
@@ -184,7 +216,7 @@ Network::Address::InstanceConstSharedPtr ConnectionManagerUtility::mutateRequest
   return final_remote_address;
 }
 
-void ConnectionManagerUtility::mutateTracingRequestHeader(Http::HeaderMap& request_headers,
+void ConnectionManagerUtility::mutateTracingRequestHeader(HeaderMap& request_headers,
                                                           Runtime::Loader& runtime,
                                                           ConnectionManagerConfig& config) {
   if (!config.tracingConfig() || !request_headers.RequestId()) {
@@ -221,22 +253,22 @@ void ConnectionManagerUtility::mutateTracingRequestHeader(Http::HeaderMap& reque
   request_headers.RequestId()->value(x_request_id);
 }
 
-void ConnectionManagerUtility::mutateXfccRequestHeader(Http::HeaderMap& request_headers,
+void ConnectionManagerUtility::mutateXfccRequestHeader(HeaderMap& request_headers,
                                                        Network::Connection& connection,
                                                        ConnectionManagerConfig& config) {
   // When AlwaysForwardOnly is set, always forward the XFCC header without modification.
-  if (config.forwardClientCert() == Http::ForwardClientCertType::AlwaysForwardOnly) {
+  if (config.forwardClientCert() == ForwardClientCertType::AlwaysForwardOnly) {
     return;
   }
   // When Sanitize is set, or the connection is not mutual TLS, remove the XFCC header.
-  if (config.forwardClientCert() == Http::ForwardClientCertType::Sanitize ||
+  if (config.forwardClientCert() == ForwardClientCertType::Sanitize ||
       !(connection.ssl() && connection.ssl()->peerCertificatePresented())) {
     request_headers.removeForwardedClientCert();
     return;
   }
 
   // When ForwardOnly is set, always forward the XFCC header without modification.
-  if (config.forwardClientCert() == Http::ForwardClientCertType::ForwardOnly) {
+  if (config.forwardClientCert() == ForwardClientCertType::ForwardOnly) {
     return;
   }
 
@@ -246,8 +278,8 @@ void ConnectionManagerUtility::mutateXfccRequestHeader(Http::HeaderMap& request_
   std::vector<std::string> client_cert_details;
   // When AppendForward or SanitizeSet is set, the client certificate information should be set into
   // the XFCC header.
-  if (config.forwardClientCert() == Http::ForwardClientCertType::AppendForward ||
-      config.forwardClientCert() == Http::ForwardClientCertType::SanitizeSet) {
+  if (config.forwardClientCert() == ForwardClientCertType::AppendForward ||
+      config.forwardClientCert() == ForwardClientCertType::SanitizeSet) {
     const std::string uri_san_local_cert = connection.ssl()->uriSanLocalCertificate();
     if (!uri_san_local_cert.empty()) {
       client_cert_details.push_back("By=" + uri_san_local_cert);
@@ -258,23 +290,23 @@ void ConnectionManagerUtility::mutateXfccRequestHeader(Http::HeaderMap& request_
     }
     for (const auto& detail : config.setCurrentClientCertDetails()) {
       switch (detail) {
-      case Http::ClientCertDetailsType::Cert: {
+      case ClientCertDetailsType::Cert: {
         const std::string peer_cert = connection.ssl()->urlEncodedPemEncodedPeerCertificate();
         if (!peer_cert.empty()) {
           client_cert_details.push_back("Cert=\"" + peer_cert + "\"");
         }
         break;
       }
-      case Http::ClientCertDetailsType::Subject:
+      case ClientCertDetailsType::Subject:
         // The "Subject" key still exists even if the subject is empty.
         client_cert_details.push_back("Subject=\"" + connection.ssl()->subjectPeerCertificate() +
                                       "\"");
         break;
-      case Http::ClientCertDetailsType::URI:
+      case ClientCertDetailsType::URI:
         // The "URI" key still exists even if the URI is empty.
         client_cert_details.push_back("URI=" + connection.ssl()->uriSanPeerCertificate());
         break;
-      case Http::ClientCertDetailsType::DNS: {
+      case ClientCertDetailsType::DNS: {
         const std::vector<std::string> dns_sans = connection.ssl()->dnsSansPeerCertificate();
         if (!dns_sans.empty()) {
           for (const std::string& dns : dns_sans) {
@@ -288,18 +320,18 @@ void ConnectionManagerUtility::mutateXfccRequestHeader(Http::HeaderMap& request_
   }
 
   const std::string client_cert_details_str = absl::StrJoin(client_cert_details, ";");
-  if (config.forwardClientCert() == Http::ForwardClientCertType::AppendForward) {
+  if (config.forwardClientCert() == ForwardClientCertType::AppendForward) {
     HeaderMapImpl::appendToHeader(request_headers.insertForwardedClientCert().value(),
                                   client_cert_details_str);
-  } else if (config.forwardClientCert() == Http::ForwardClientCertType::SanitizeSet) {
+  } else if (config.forwardClientCert() == ForwardClientCertType::SanitizeSet) {
     request_headers.insertForwardedClientCert().value(client_cert_details_str);
   } else {
     NOT_REACHED_GCOVR_EXCL_LINE;
   }
 }
 
-void ConnectionManagerUtility::mutateResponseHeaders(Http::HeaderMap& response_headers,
-                                                     const Http::HeaderMap* request_headers,
+void ConnectionManagerUtility::mutateResponseHeaders(HeaderMap& response_headers,
+                                                     const HeaderMap* request_headers,
                                                      const std::string& via) {
   if (request_headers != nullptr && Utility::isUpgrade(*request_headers) &&
       Utility::isUpgrade(response_headers)) {

diff --git a/source/common/http/conn_manager_utility.h b/source/common/http/conn_manager_utility.h
@@ -16,6 +16,28 @@ namespace Http {
  */
 class ConnectionManagerUtility {
 public:
+  /**
+   * Determine the next protocol to used based both on ALPN as well as protocol inspection.
+   * @param connection supplies the connection to determine a protocol for.
+   * @param data supplies the currently available read data on the connection.
+   */
+  static std::string determineNextProtocol(Network::Connection& connection,
+                                           const Buffer::Instance& data);
+
+  /**
+   * Create an HTTP codec given the connection and the beginning of the incoming data.
+   * @param connection supplies the connection.
+   * @param data supplies the initial data supplied by the client.
+   * @param callbacks supplies the codec callbacks.
+   * @param scope supplies the stats scope for codec stats.
+   * @param http1_settings supplies the HTTP/1 settings to use if HTTP/1 is chosen.
+   * @param http2_settings supplies the HTTP/2 settings to use if HTTP/2 is chosen.
+   */
+  static ServerConnectionPtr
+  autoCreateCodec(Network::Connection& connection, const Buffer::Instance& data,
+                  ServerConnectionCallbacks& callbacks, Stats::Scope& scope,
+                  const Http1Settings& http1_settings, const Http2Settings& http2_settings);
+
   /**
    * Mutates request headers in various ways. This functionality is broken out because of its
    * complexity for ease of testing. See the method itself for detailed comments on what
@@ -28,23 +50,22 @@ class ConnectionManagerUtility {
    *         existence of the x-forwarded-for header. Again see the method for more details.
    */
   static Network::Address::InstanceConstSharedPtr
-  mutateRequestHeaders(Http::HeaderMap& request_headers, Network::Connection& connection,
+  mutateRequestHeaders(HeaderMap& request_headers, Network::Connection& connection,
                        ConnectionManagerConfig& config, const Router::Config& route_config,
                        Runtime::RandomGenerator& random, Runtime::Loader& runtime,
                        const LocalInfo::LocalInfo& local_info);
 
-  static void mutateResponseHeaders(Http::HeaderMap& response_headers,
-                                    const Http::HeaderMap* request_headers, const std::string& via);
+  static void mutateResponseHeaders(HeaderMap& response_headers, const HeaderMap* request_headers,
+                                    const std::string& via);
 
 private:
   /**
    * Mutate request headers if request needs to be traced.
    */
-  static void mutateTracingRequestHeader(Http::HeaderMap& request_headers, Runtime::Loader& runtime,
+  static void mutateTracingRequestHeader(HeaderMap& request_headers, Runtime::Loader& runtime,
                                          ConnectionManagerConfig& config);
 
-  static void mutateXfccRequestHeader(Http::HeaderMap& request_headers,
-                                      Network::Connection& connection,
+  static void mutateXfccRequestHeader(HeaderMap& request_headers, Network::Connection& connection,
                                       ConnectionManagerConfig& config);
 };
 

diff --git a/source/common/http/header_utility.cc b/source/common/http/header_utility.cc
@@ -66,7 +66,7 @@ HeaderUtility::HeaderData::HeaderData(const Json::Object& config)
 
 bool HeaderUtility::matchHeaders(const Http::HeaderMap& request_headers,
                                  const std::vector<HeaderData>& config_headers) {
-  // TODO (rodaine): Should this really allow empty headers to always match?
+  // No headers to match is considered a match.
   if (!config_headers.empty()) {
     for (const HeaderData& cfg_header_data : config_headers) {
       if (!matchHeaders(request_headers, cfg_header_data)) {

diff --git a/source/common/http/header_utility.h b/source/common/http/header_utility.h
@@ -38,7 +38,7 @@ class HeaderUtility {
    * @param request_headers supplies the headers from the request.
    * @param config_headers supplies the list of configured header conditions on which to match.
    * @return bool true if all the headers (and values) in the config_headers are found in the
-   *         request_headers
+   *         request_headers. If no config_headers are specified, returns true.
    */
   static bool matchHeaders(const Http::HeaderMap& request_headers,
                            const std::vector<HeaderData>& config_headers);

diff --git a/source/common/http/http2/codec_impl.h b/source/common/http/http2/codec_impl.h
@@ -11,17 +11,15 @@
 #include "envoy/network/connection.h"
 #include "envoy/stats/scope.h"
 
-//#include "envoy/stats/stats_macros.h"
-
 #include "common/buffer/buffer_impl.h"
 #include "common/buffer/watermark_buffer.h"
 #include "common/common/linked_object.h"
 #include "common/common/logger.h"
 #include "common/http/codec_helper.h"
 #include "common/http/header_map_impl.h"
-#include "common/http/utility.h"
 #include "common/http/http2/metadata_decoder.h"
 #include "common/http/http2/metadata_encoder.h"
+#include "common/http/utility.h"
 
 #include "absl/types/optional.h"
 #include "nghttp2/nghttp2.h"

diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc
@@ -14,6 +14,7 @@
 #include "common/common/fmt.h"
 #include "common/config/filter_json.h"
 #include "common/config/utility.h"
+#include "common/http/conn_manager_utility.h"
 #include "common/http/date_provider_impl.h"
 #include "common/http/default_server_string.h"
 #include "common/http/http1/codec_impl.h"
@@ -117,24 +118,6 @@ static Registry::RegisterFactory<HttpConnectionManagerFilterConfigFactory,
                                  Server::Configuration::NamedNetworkFilterConfigFactory>
     registered_;
 
-std::string
-HttpConnectionManagerConfigUtility::determineNextProtocol(Network::Connection& connection,
-                                                          const Buffer::Instance& data) {
-  if (!connection.nextProtocol().empty()) {
-    return connection.nextProtocol();
-  }
-
-  // See if the data we have so far shows the HTTP/2 prefix. We ignore the case where someone sends
-  // us the first few bytes of the HTTP/2 prefix since in all public cases we use SSL/ALPN. For
-  // internal cases this should practically never happen.
-  if (-1 != data.search(Http::Http2::CLIENT_MAGIC_PREFIX.c_str(),
-                        Http::Http2::CLIENT_MAGIC_PREFIX.size(), 0)) {
-    return Http::Http2::ALPN_STRING;
-  }
-
-  return "";
-}
-
 InternalAddressConfig::InternalAddressConfig(
     const envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager::
         InternalAddressConfig& config)
@@ -344,14 +327,8 @@ HttpConnectionManagerConfig::createCodec(Network::Connection& connection,
     return Http::ServerConnectionPtr{new Http::Http2::ServerConnectionImpl(
         connection, callbacks, context_.scope(), http2_settings_)};
   case CodecType::AUTO:
-    if (HttpConnectionManagerConfigUtility::determineNextProtocol(connection, data) ==
-        Http::Http2::ALPN_STRING) {
-      return Http::ServerConnectionPtr{new Http::Http2::ServerConnectionImpl(
-          connection, callbacks, context_.scope(), http2_settings_)};
-    } else {
-      return Http::ServerConnectionPtr{
-          new Http::Http1::ServerConnectionImpl(connection, callbacks, http1_settings_)};
-    }
+    return Http::ConnectionManagerUtility::autoCreateCodec(
+        connection, data, callbacks, context_.scope(), http1_settings_, http2_settings_);
   }
 
   NOT_REACHED_GCOVR_EXCL_LINE;

diff --git a/source/extensions/filters/network/http_connection_manager/config.h b/source/extensions/filters/network/http_connection_manager/config.h
@@ -46,20 +46,6 @@ class HttpConnectionManagerFilterConfigFactory
       Server::Configuration::FactoryContext& context) override;
 };
 
-/**
- * Utilities for the HTTP connection manager that facilitate testing.
- */
-class HttpConnectionManagerConfigUtility {
-public:
-  /**
-   * Determine the next protocol to used based both on ALPN as well as protocol inspection.
-   * @param connection supplies the connection to determine a protocol for.
-   * @param data supplies the currently available read data on the connection.
-   */
-  static std::string determineNextProtocol(Network::Connection& connection,
-                                           const Buffer::Instance& data);
-};
-
 /**
  * Determines if an address is internal based on user provided config.
  */

diff --git a/source/server/http/BUILD b/source/server/http/BUILD
@@ -47,7 +47,6 @@ envoy_cc_library(
         "//source/common/http:header_map_lib",
         "//source/common/http:headers_lib",
         "//source/common/http:utility_lib",
-        "//source/common/http/http1:codec_lib",
         "//source/common/memory:stats_lib",
         "//source/common/network:listen_socket_lib",
         "//source/common/network:raw_buffer_socket_lib",