rbac: add some debug logging. (#3744)

Signed-off-by: Yangmin Zhu <ymzhu@google.com>
envoyproxy · Jun 28, 2018 · 4865c9c · 4865c9c
1 parent ab86abc
commit 4865c9c
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 13 deletions.
diff --git a/source/common/common/logger.h b/source/common/common/logger.h
@@ -26,27 +26,28 @@ namespace Logger {
   FUNCTION(client)               \
   FUNCTION(config)               \
   FUNCTION(connection)           \
-  FUNCTION(misc)                 \
   FUNCTION(file)                 \
   FUNCTION(filter)               \
+  FUNCTION(grpc)                 \
   FUNCTION(hc)                   \
   FUNCTION(health_checker)       \
   FUNCTION(http)                 \
   FUNCTION(http2)                \
   FUNCTION(hystrix)              \
   FUNCTION(lua)                  \
   FUNCTION(main)                 \
+  FUNCTION(misc)                 \
   FUNCTION(mongo)                \
   FUNCTION(pool)                 \
+  FUNCTION(rbac)                 \
   FUNCTION(redis)                \
   FUNCTION(router)               \
   FUNCTION(runtime)              \
+  FUNCTION(stats)                \
   FUNCTION(testing)              \
+  FUNCTION(thrift)               \
   FUNCTION(tracing)              \
-  FUNCTION(upstream)             \
-  FUNCTION(grpc)                 \
-  FUNCTION(stats)                \
-  FUNCTION(thrift)
+  FUNCTION(upstream)
 
 enum class Id {
   ALL_LOGGER_IDS(GENERATE_ENUM)

diff --git a/source/extensions/filters/http/rbac/rbac_filter.cc b/source/extensions/filters/http/rbac/rbac_filter.cc
@@ -65,13 +65,27 @@ RoleBasedAccessControlRouteSpecificFilterConfig::RoleBasedAccessControlRouteSpec
 
 Http::FilterHeadersStatus RoleBasedAccessControlFilter::decodeHeaders(Http::HeaderMap& headers,
                                                                       bool) {
+  ENVOY_LOG(
+      debug,
+      "checking request: remoteAddress: {}, localAddress: {}, ssl: {}, headers: {}, "
+      "dynamicMetadata: {}",
+      callbacks_->connection()->remoteAddress()->asString(),
+      callbacks_->connection()->localAddress()->asString(),
+      callbacks_->connection()->ssl()
+          ? "uriSanPeerCertificate: " + callbacks_->connection()->ssl()->uriSanPeerCertificate() +
+                ", subjectPeerCertificate: " +
+                callbacks_->connection()->ssl()->subjectPeerCertificate()
+          : "none",
+      headers, callbacks_->requestInfo().dynamicMetadata().DebugString());
   const absl::optional<Filters::Common::RBAC::RoleBasedAccessControlEngineImpl>& shadow_engine =
       config_->engine(callbacks_->route(), EnforcementMode::Shadow);
   if (shadow_engine.has_value()) {
     if (shadow_engine->allowed(*callbacks_->connection(), headers,
                                callbacks_->requestInfo().dynamicMetadata())) {
+      ENVOY_LOG(debug, "shadow allowed");
       config_->stats().shadow_allowed_.inc();
     } else {
+      ENVOY_LOG(debug, "shadow denied");
       config_->stats().shadow_denied_.inc();
     }
   }
@@ -81,15 +95,18 @@ Http::FilterHeadersStatus RoleBasedAccessControlFilter::decodeHeaders(Http::Head
   if (engine.has_value()) {
     if (engine->allowed(*callbacks_->connection(), headers,
                         callbacks_->requestInfo().dynamicMetadata())) {
+      ENVOY_LOG(debug, "enforced allowed");
       config_->stats().allowed_.inc();
       return Http::FilterHeadersStatus::Continue;
     } else {
+      ENVOY_LOG(debug, "enforced denied");
       callbacks_->sendLocalReply(Http::Code::Forbidden, "RBAC: access denied", nullptr);
       config_->stats().denied_.inc();
       return Http::FilterHeadersStatus::StopIteration;
     }
   }
 
+  ENVOY_LOG(debug, "no engine, allowed by default");
   return Http::FilterHeadersStatus::Continue;
 }
 

diff --git a/source/extensions/filters/http/rbac/rbac_filter.h b/source/extensions/filters/http/rbac/rbac_filter.h
@@ -6,6 +6,8 @@
 #include "envoy/http/filter.h"
 #include "envoy/stats/stats_macros.h"
 
+#include "common/common/logger.h"
+
 #include "extensions/filters/common/rbac/engine_impl.h"
 
 namespace Envoy {
@@ -80,7 +82,8 @@ typedef std::shared_ptr<RoleBasedAccessControlFilterConfig>
 /**
  * A filter that provides role-based access control authorization for HTTP requests.
  */
-class RoleBasedAccessControlFilter : public Http::StreamDecoderFilter {
+class RoleBasedAccessControlFilter : public Http::StreamDecoderFilter,
+                                     public Logger::Loggable<Logger::Id::rbac> {
 public:
   RoleBasedAccessControlFilter(RoleBasedAccessControlFilterConfigSharedPtr config)
       : config_(config) {}

diff --git a/test/extensions/filters/http/rbac/rbac_filter_test.cc b/test/extensions/filters/http/rbac/rbac_filter_test.cc
@@ -46,13 +46,9 @@ class RoleBasedAccessControlFilterTest : public testing::Test {
     filter_.setDecoderFilterCallbacks(callbacks_);
   }
 
-  void setDestinationPort(uint16_t port, int times = 2) {
+  void setDestinationPort(uint16_t port) {
     address_ = Envoy::Network::Utility::parseInternetAddress("1.2.3.4", port, false);
-    auto& expect = EXPECT_CALL(connection_, localAddress());
-    if (times > 0) {
-      expect.Times(times);
-    }
-    expect.WillRepeatedly(ReturnRef(address_));
+    ON_CALL(connection_, localAddress()).WillByDefault(ReturnRef(address_));
   }
 
   NiceMock<Http::MockStreamDecoderFilterCallbacks> callbacks_;
@@ -94,7 +90,7 @@ TEST_F(RoleBasedAccessControlFilterTest, Denied) {
 }
 
 TEST_F(RoleBasedAccessControlFilterTest, RouteLocalOverride) {
-  setDestinationPort(456, 0);
+  setDestinationPort(456);
 
   envoy::config::filter::http::rbac::v2::RBACPerRoute route_config;
   route_config.mutable_rbac()->mutable_rules()->set_action(