From 94c8e0904ad2b1a69c8d4ce5304254d8de270e4d Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Wed, 11 Oct 2023 11:54:10 +0800 Subject: [PATCH 01/24] API for the HTTP Basic Auth filter Signed-off-by: huabing zhao --- .../filters/http/basic_auth/v3/BUILD | 12 ++++++ .../http/basic_auth/v3/basic_auth.proto | 40 +++++++++++++++++++ source/extensions/extensions_metadata.yaml | 7 ++++ tools/spelling/spelling_dictionary.txt | 1 + 4 files changed, 60 insertions(+) create mode 100644 api/envoy/extensions/filters/http/basic_auth/v3/BUILD create mode 100644 api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto diff --git a/api/envoy/extensions/filters/http/basic_auth/v3/BUILD b/api/envoy/extensions/filters/http/basic_auth/v3/BUILD new file mode 100644 index 0000000000000..1c1a6f6b44235 --- /dev/null +++ b/api/envoy/extensions/filters/http/basic_auth/v3/BUILD @@ -0,0 +1,12 @@ +# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = [ + "//envoy/config/core/v3:pkg", + "@com_github_cncf_udpa//udpa/annotations:pkg", + ], +) diff --git a/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto b/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto new file mode 100644 index 0000000000000..f8cfd25110476 --- /dev/null +++ b/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto @@ -0,0 +1,40 @@ +syntax = "proto3"; + +package envoy.extensions.filters.http.basic_auth.v3; + +import "envoy/config/core/v3/base.proto"; + +import "xds/annotations/v3/status.proto"; + +import "udpa/annotations/status.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.filters.http.basic_auth.v3"; +option java_outer_classname = "BasicAuthProto"; +option java_multiple_files = true; +option go_package = "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/basic_auth/v3;basic_authv3"; +option (udpa.annotations.file_status).package_version_status = ACTIVE; +option (xds.annotations.v3.file_status).work_in_progress = true; + +// [#protodoc-title: Basic Auth] +// [#not-implemented-hide:] +// Basic Auth :ref:`configuration overview `. +// [#extension: envoy.filters.http.basic_auth] + +// Basic HTTP authentication. +// +// Example: +// +// .. code-block:: yaml +// +// htpasswd: +// inline_string: |- +// user1:hashed_user1_password +// user2:hashed_user2_password +// +message BasicAuth { + // Username-password pairs used to verify user credentials in the "Authorization" header. + // The value needs to be the htpasswd format. + // Reference to https://httpd.apache.org/docs/2.4/programs/htpasswd.html + config.core.v3.DataSource htpasswd = 1 [(udpa.annotations.sensitive) = true]; +} diff --git a/source/extensions/extensions_metadata.yaml b/source/extensions/extensions_metadata.yaml index 7c4b60b96f4a6..047432d17b4d4 100644 --- a/source/extensions/extensions_metadata.yaml +++ b/source/extensions/extensions_metadata.yaml @@ -217,6 +217,13 @@ envoy.filters.http.bandwidth_limit: status: stable type_urls: - envoy.extensions.filters.http.bandwidth_limit.v3.BandwidthLimit +envoy.filters.http.basic_auth: + categories: + - envoy.filters.http + security_posture: unknown + status: wip + type_urls: + - envoy.extensions.filters.http.basic_auth.v3.BasicAuth envoy.filters.http.buffer: categories: - envoy.filters.http diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index ef07cf25c33fb..4831c2ea4170c 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -817,6 +817,7 @@ hostnames hostset hotrestart hrefs +htpasswd huffman hystrix idempotency From 797fd75ee6e64a615a1d1f262d79ddf0fcf8fb81 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Thu, 12 Oct 2023 10:23:59 +0800 Subject: [PATCH 02/24] Basic Auth HTTP Filter implementation Signed-off-by: huabing zhao --- api/BUILD | 1 + .../filters/http/basic_auth/v3/BUILD | 1 + .../http/basic_auth/v3/basic_auth.proto | 4 +- api/versioning/BUILD | 1 + source/common/common/logger.h | 1 + source/extensions/extensions_build_config.bzl | 1 + .../extensions/filters/http/basic_auth/BUILD | 40 ++++++++ .../http/basic_auth/basic_auth_filter.cc | 96 +++++++++++++++++++ .../http/basic_auth/basic_auth_filter.h | 75 +++++++++++++++ .../filters/http/basic_auth/config.cc | 57 +++++++++++ .../filters/http/basic_auth/config.h | 27 ++++++ .../filters/http/well_known_names.h | 2 + 12 files changed, 304 insertions(+), 2 deletions(-) create mode 100644 source/extensions/filters/http/basic_auth/BUILD create mode 100644 source/extensions/filters/http/basic_auth/basic_auth_filter.cc create mode 100644 source/extensions/filters/http/basic_auth/basic_auth_filter.h create mode 100644 source/extensions/filters/http/basic_auth/config.cc create mode 100644 source/extensions/filters/http/basic_auth/config.h diff --git a/api/BUILD b/api/BUILD index 554d3fde4a5ae..2801ea0654ca0 100644 --- a/api/BUILD +++ b/api/BUILD @@ -163,6 +163,7 @@ proto_library( "//envoy/extensions/filters/http/aws_lambda/v3:pkg", "//envoy/extensions/filters/http/aws_request_signing/v3:pkg", "//envoy/extensions/filters/http/bandwidth_limit/v3:pkg", + "//envoy/extensions/filters/http/basic_auth/v3:pkg", "//envoy/extensions/filters/http/buffer/v3:pkg", "//envoy/extensions/filters/http/cache/v3:pkg", "//envoy/extensions/filters/http/cdn_loop/v3:pkg", diff --git a/api/envoy/extensions/filters/http/basic_auth/v3/BUILD b/api/envoy/extensions/filters/http/basic_auth/v3/BUILD index 1c1a6f6b44235..e9b556d681cfd 100644 --- a/api/envoy/extensions/filters/http/basic_auth/v3/BUILD +++ b/api/envoy/extensions/filters/http/basic_auth/v3/BUILD @@ -8,5 +8,6 @@ api_proto_package( deps = [ "//envoy/config/core/v3:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", + "@com_github_cncf_udpa//xds/annotations/v3:pkg", ], ) diff --git a/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto b/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto index f8cfd25110476..c389486344e9b 100644 --- a/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto +++ b/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto @@ -6,8 +6,8 @@ import "envoy/config/core/v3/base.proto"; import "xds/annotations/v3/status.proto"; +import "udpa/annotations/sensitive.proto"; import "udpa/annotations/status.proto"; -import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.extensions.filters.http.basic_auth.v3"; option java_outer_classname = "BasicAuthProto"; @@ -36,5 +36,5 @@ message BasicAuth { // Username-password pairs used to verify user credentials in the "Authorization" header. // The value needs to be the htpasswd format. // Reference to https://httpd.apache.org/docs/2.4/programs/htpasswd.html - config.core.v3.DataSource htpasswd = 1 [(udpa.annotations.sensitive) = true]; + config.core.v3.DataSource users = 1 [(udpa.annotations.sensitive) = true]; } diff --git a/api/versioning/BUILD b/api/versioning/BUILD index 43de328ff1688..ad935fa5c2ed0 100644 --- a/api/versioning/BUILD +++ b/api/versioning/BUILD @@ -101,6 +101,7 @@ proto_library( "//envoy/extensions/filters/http/aws_lambda/v3:pkg", "//envoy/extensions/filters/http/aws_request_signing/v3:pkg", "//envoy/extensions/filters/http/bandwidth_limit/v3:pkg", + "//envoy/extensions/filters/http/basic_auth/v3:pkg", "//envoy/extensions/filters/http/buffer/v3:pkg", "//envoy/extensions/filters/http/cache/v3:pkg", "//envoy/extensions/filters/http/cdn_loop/v3:pkg", diff --git a/source/common/common/logger.h b/source/common/common/logger.h index f3ad2bc01bc7d..a90e43628e1ee 100644 --- a/source/common/common/logger.h +++ b/source/common/common/logger.h @@ -39,6 +39,7 @@ const static bool should_log = true; FUNCTION(aws) \ FUNCTION(assert) \ FUNCTION(backtrace) \ + FUNCTION(basic_auth) \ FUNCTION(cache_filter) \ FUNCTION(client) \ FUNCTION(config) \ diff --git a/source/extensions/extensions_build_config.bzl b/source/extensions/extensions_build_config.bzl index 433fdd0f355e7..084df7caa1aa7 100644 --- a/source/extensions/extensions_build_config.bzl +++ b/source/extensions/extensions_build_config.bzl @@ -124,6 +124,7 @@ EXTENSIONS = { "envoy.filters.http.aws_lambda": "//source/extensions/filters/http/aws_lambda:config", "envoy.filters.http.aws_request_signing": "//source/extensions/filters/http/aws_request_signing:config", "envoy.filters.http.bandwidth_limit": "//source/extensions/filters/http/bandwidth_limit:config", + "envoy.filters.http.basic_auth": "//source/extensions/filters/http/basic_auth:config", "envoy.filters.http.buffer": "//source/extensions/filters/http/buffer:config", "envoy.filters.http.cache": "//source/extensions/filters/http/cache:config", "envoy.filters.http.cdn_loop": "//source/extensions/filters/http/cdn_loop:config", diff --git a/source/extensions/filters/http/basic_auth/BUILD b/source/extensions/filters/http/basic_auth/BUILD new file mode 100644 index 0000000000000..1e8a3a7bbcd43 --- /dev/null +++ b/source/extensions/filters/http/basic_auth/BUILD @@ -0,0 +1,40 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_extension", + "envoy_cc_library", + "envoy_extension_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_extension_package() + +envoy_cc_library( + name = "basic_auth_lib", + srcs = ["basic_auth_filter.cc"], + hdrs = ["basic_auth_filter.h"], + external_deps = ["ssl"], + deps = [ + "//envoy/server:filter_config_interface", + "//source/common/common:base64_lib", + "//source/common/config:utility_lib", + "//source/common/http:header_map_lib", + "//source/common/protobuf:utility_lib", + "//source/extensions/filters/http/common:pass_through_filter_lib", + "@envoy_api//envoy/extensions/filters/http/basic_auth/v3:pkg_cc_proto", + ], +) + +envoy_cc_extension( + name = "config", + srcs = ["config.cc"], + hdrs = ["config.h"], + deps = [ + ":basic_auth_lib", + "//envoy/registry", + "//source/common/config:datasource_lib", + "//source/common/protobuf:utility_lib", + "//source/extensions/filters/http/common:factory_base_lib", + "@envoy_api//envoy/extensions/filters/http/basic_auth/v3:pkg_cc_proto", + ], +) diff --git a/source/extensions/filters/http/basic_auth/basic_auth_filter.cc b/source/extensions/filters/http/basic_auth/basic_auth_filter.cc new file mode 100644 index 0000000000000..074966672368d --- /dev/null +++ b/source/extensions/filters/http/basic_auth/basic_auth_filter.cc @@ -0,0 +1,96 @@ +#include "source/extensions/filters/http/basic_auth/basic_auth_filter.h" + +#include + +#include "source/common/common/base64.h" +#include "source/common/http/headers.h" +#include "source/common/http/header_utility.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace BasicAuth { + +namespace { + +// Function to compute SHA1 hash +std::string computeSHA1(const std::string& password) { + unsigned char hash[SHA_DIGEST_LENGTH]; + + // Calculate the SHA-1 hash + SHA1(reinterpret_cast(password.c_str()), password.length(), hash); + + // Encode the binary hash in Base64 + std::string encodedHash = Base64::encode(reinterpret_cast(hash), SHA_DIGEST_LENGTH); + + return encodedHash; +} + +} // namespace + +FilterConfig::FilterConfig(std::vector users, const std::string& stats_prefix, + Stats::Scope& scope) + : users_(users), stats_(generateStats(stats_prefix + "basic_auth.", scope)) {} + +bool FilterConfig::validateUser(const std::string& username, const std::string& password) { + for (auto user : users_) { + if (user.name == username) { + std::string hashedPassword = computeSHA1(password); + if (hashedPassword == user.hash) { + return true; + } + return false; + } + } + return false; +} + +BasicAuthFilter::BasicAuthFilter(FilterConfigSharedPtr config) : config_(std::move(config)) {} + +Http::FilterHeadersStatus BasicAuthFilter::decodeHeaders(Http::RequestHeaderMap& headers, bool) { + ENVOY_LOG(debug, "Called Filter : {}", __func__); + + auto auth_header = headers.get(Http::CustomHeaders::get().Authorization); + if (!auth_header.empty()) { + auto auth_value = auth_header[0]->value().getStringView(); + + if (auth_value.substr(0, 6) == "Basic ") { + // Extract and decode the Base64 part of the header. + auto base64Token = auth_value.substr(6); + std::string decoded = Base64::decodeWithoutPadding(base64Token); + + // The decoded string is in the format "username:password". + size_t colonPos = decoded.find(':'); + + if (colonPos != std::string::npos) { + std::string username = decoded.substr(0, colonPos); + std::string password = decoded.substr(colonPos + 1); + + if (config_->validateUser(username, password)) { + config_->stats().allowed_.inc(); + return Http::FilterHeadersStatus::Continue; + } else { + config_->stats().denied_.inc(); + decoder_callbacks_->sendLocalReply(Http::Code::Unauthorized, "Basic Auth failed", nullptr, + absl::nullopt, ""); + return Http::FilterHeadersStatus::StopIteration; + } + } + } + } + + config_->stats().denied_.inc(); + decoder_callbacks_->sendLocalReply(Http::Code::Unauthorized, "Missing username or password", + nullptr, absl::nullopt, ""); + return Http::FilterHeadersStatus::StopIteration; +} + +void BasicAuthFilter::setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) { + ENVOY_LOG(debug, "Called Filter : {}", __func__); + decoder_callbacks_ = &callbacks; +} + +} // namespace BasicAuth +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/basic_auth/basic_auth_filter.h b/source/extensions/filters/http/basic_auth/basic_auth_filter.h new file mode 100644 index 0000000000000..85c2f66e43ebc --- /dev/null +++ b/source/extensions/filters/http/basic_auth/basic_auth_filter.h @@ -0,0 +1,75 @@ +#pragma once + +#include "envoy/stats/stats_macros.h" + +#include "source/common/common/logger.h" +#include "source/extensions/filters/http/common/pass_through_filter.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace BasicAuth { + +/** + * All Basic Auth filter stats. @see stats_macros.h + */ +#define ALL_BASIC_AUTH_STATS(COUNTER) \ + COUNTER(allowed) \ + COUNTER(denied) + +/** + * Struct definition for Basic Auth stats. @see stats_macros.h + */ +struct BasicAuthStats { + ALL_BASIC_AUTH_STATS(GENERATE_COUNTER_STRUCT) +}; + +/** + * Struct definition for username password pairs. + */ +struct User { + // the user name + std::string name; + // the hashed password, see https://httpd.apache.org/docs/2.4/misc/password_encryptions.html + std::string hash; +}; + +/** + * Configuration for the Basic Auth filter. + */ +class FilterConfig { +public: + FilterConfig(std::vector users, const std::string& stats_prefix, Stats::Scope& scope); + BasicAuthStats& stats() { return stats_; } + bool validateUser(const std::string& username, const std::string& password); + +private: + static BasicAuthStats generateStats(const std::string& prefix, Stats::Scope& scope) { + return BasicAuthStats{ALL_BASIC_AUTH_STATS(POOL_COUNTER_PREFIX(scope, prefix))}; + } + + std::vector users_; + BasicAuthStats stats_; +}; +using FilterConfigSharedPtr = std::shared_ptr; + +// The Envoy filter to process HTTP basic auth. +class BasicAuthFilter : public Http::PassThroughFilter, + public Logger::Loggable { +public: + BasicAuthFilter(FilterConfigSharedPtr config); + + // Http::StreamDecoderFilter + Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers, bool) override; + void setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) override; + +private: + // The callback function. + Http::StreamDecoderFilterCallbacks* decoder_callbacks_; + FilterConfigSharedPtr config_; +}; + +} // namespace BasicAuth +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/basic_auth/config.cc b/source/extensions/filters/http/basic_auth/config.cc new file mode 100644 index 0000000000000..92397588c37c5 --- /dev/null +++ b/source/extensions/filters/http/basic_auth/config.cc @@ -0,0 +1,57 @@ +#include "source/extensions/filters/http/basic_auth/config.h" + +#include "source/common/config/datasource.h" +#include "source/extensions/filters/http/basic_auth/basic_auth_filter.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace BasicAuth { + +using envoy::extensions::filters::http::basic_auth::v3::BasicAuth; + +std::vector readHtpasswd(std::string htpasswd) { + std::vector users; + std::istringstream htpsswd_ss(htpasswd); + std::string line; + + while (std::getline(htpsswd_ss, line)) { + size_t colonPos = line.find(':'); + + if (colonPos != std::string::npos) { + std::string name, hash; + + name = line.substr(0, colonPos); + hash = line.substr(colonPos + 1); + + if (hash.find("{SHA}") == 0) { + hash = hash.substr(5); + users.push_back({name, hash}); + continue; + } + } + + throw EnvoyException("unsupported htpasswd format: please use {SHA}"); + } + + return users; +} + +Http::FilterFactoryCb BasicAuthFilterFactory::createFilterFactoryFromProtoTyped( + const envoy::extensions::filters::http::basic_auth::v3::BasicAuth& proto_config, + const std::string& stats_prefix, Server::Configuration::FactoryContext& context) { + auto htpasswd = Config::DataSource::read(proto_config.users(), false, context.api()); + auto users = readHtpasswd(htpasswd); + FilterConfigSharedPtr config = + std::make_shared(users, stats_prefix, context.scope()); + return [users, config](Http::FilterChainFactoryCallbacks& callbacks) -> void { + callbacks.addStreamDecoderFilter(std::make_shared(config)); + }; +} + +REGISTER_FACTORY(BasicAuthFilterFactory, Server::Configuration::NamedHttpFilterConfigFactory); + +} // namespace BasicAuth +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/basic_auth/config.h b/source/extensions/filters/http/basic_auth/config.h new file mode 100644 index 0000000000000..7abebaaa789ca --- /dev/null +++ b/source/extensions/filters/http/basic_auth/config.h @@ -0,0 +1,27 @@ +#pragma once + +#include "envoy/extensions/filters/http/basic_auth/v3/basic_auth.pb.h" +#include "envoy/extensions/filters/http/basic_auth/v3/basic_auth.pb.validate.h" + +#include "source/extensions/filters/http/common/factory_base.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace BasicAuth { + +class BasicAuthFilterFactory + : public Common::FactoryBase { +public: + BasicAuthFilterFactory() : FactoryBase("envoy.filters.http.basic_auth") {} + +private: + Http::FilterFactoryCb createFilterFactoryFromProtoTyped( + const envoy::extensions::filters::http::basic_auth::v3::BasicAuth& config, + const std::string& stats_prefix, Server::Configuration::FactoryContext& context) override; +}; + +} // namespace BasicAuth +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/well_known_names.h b/source/extensions/filters/http/well_known_names.h index cfd1783b55b4c..8e33a0699c0a0 100644 --- a/source/extensions/filters/http/well_known_names.h +++ b/source/extensions/filters/http/well_known_names.h @@ -16,6 +16,8 @@ class HttpFilterNameValues { const std::string Buffer = "envoy.filters.http.buffer"; // Bandwidth limit filter const std::string BandwidthLimit = "envoy.filters.http.bandwidth_limit"; + // Basic Auth filter + const std::string BasicAuth = "envoy.filters.http.basic_auth"; // Cache filter const std::string Cache = "envoy.filters.http.cache"; // CDN Loop filter From 689604a1d2f027c3caa25e89d9ef2d23aea70209 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Mon, 16 Oct 2023 15:49:30 +0800 Subject: [PATCH 03/24] add test Signed-off-by: huabing zhao --- .../filters/http/basic_auth/config.cc | 14 +- test/extensions/filters/http/basic_auth/BUILD | 32 +++++ .../filters/http/basic_auth/config_test.cc | 127 ++++++++++++++++++ .../filters/http/basic_auth/filter_test.cc | 103 ++++++++++++++ 4 files changed, 275 insertions(+), 1 deletion(-) create mode 100644 test/extensions/filters/http/basic_auth/BUILD create mode 100644 test/extensions/filters/http/basic_auth/config_test.cc create mode 100644 test/extensions/filters/http/basic_auth/filter_test.cc diff --git a/source/extensions/filters/http/basic_auth/config.cc b/source/extensions/filters/http/basic_auth/config.cc index 92397588c37c5..f9fb075c5454c 100644 --- a/source/extensions/filters/http/basic_auth/config.cc +++ b/source/extensions/filters/http/basic_auth/config.cc @@ -10,6 +10,8 @@ namespace BasicAuth { using envoy::extensions::filters::http::basic_auth::v3::BasicAuth; +namespace { + std::vector readHtpasswd(std::string htpasswd) { std::vector users; std::istringstream htpsswd_ss(htpasswd); @@ -24,8 +26,16 @@ std::vector readHtpasswd(std::string htpasswd) { name = line.substr(0, colonPos); hash = line.substr(colonPos + 1); + if (name.length() == 0) { + throw EnvoyException("invalid user name"); + } + if (hash.find("{SHA}") == 0) { hash = hash.substr(5); + if (hash.length() != 28) { + throw EnvoyException("invalid SHA hash length"); + } + users.push_back({name, hash}); continue; } @@ -37,6 +47,8 @@ std::vector readHtpasswd(std::string htpasswd) { return users; } +} // namespace + Http::FilterFactoryCb BasicAuthFilterFactory::createFilterFactoryFromProtoTyped( const envoy::extensions::filters::http::basic_auth::v3::BasicAuth& proto_config, const std::string& stats_prefix, Server::Configuration::FactoryContext& context) { @@ -44,7 +56,7 @@ Http::FilterFactoryCb BasicAuthFilterFactory::createFilterFactoryFromProtoTyped( auto users = readHtpasswd(htpasswd); FilterConfigSharedPtr config = std::make_shared(users, stats_prefix, context.scope()); - return [users, config](Http::FilterChainFactoryCallbacks& callbacks) -> void { + return [config](Http::FilterChainFactoryCallbacks& callbacks) -> void { callbacks.addStreamDecoderFilter(std::make_shared(config)); }; } diff --git a/test/extensions/filters/http/basic_auth/BUILD b/test/extensions/filters/http/basic_auth/BUILD new file mode 100644 index 0000000000000..c506a1c8e1b02 --- /dev/null +++ b/test/extensions/filters/http/basic_auth/BUILD @@ -0,0 +1,32 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_package", +) +load( + "//test/extensions:extensions_build_system.bzl", + "envoy_extension_cc_test", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_extension_cc_test( + name = "filter_test", + srcs = ["filter_test.cc"], + extension_names = ["envoy.filters.http.basic_auth"], + deps = [ + "//source/extensions/filters/http/basic_auth:basic_auth_lib", + "//test/mocks/server:server_mocks", + ], +) + +envoy_extension_cc_test( + name = "config_test", + srcs = ["config_test.cc"], + extension_names = ["envoy.filters.http.basic_auth"], + deps = [ + "//source/extensions/filters/http/basic_auth:config", + "//test/mocks/server:server_mocks", + ], +) diff --git a/test/extensions/filters/http/basic_auth/config_test.cc b/test/extensions/filters/http/basic_auth/config_test.cc new file mode 100644 index 0000000000000..2be2d58125966 --- /dev/null +++ b/test/extensions/filters/http/basic_auth/config_test.cc @@ -0,0 +1,127 @@ +#include "source/extensions/filters/http/basic_auth/basic_auth_filter.h" +#include "source/extensions/filters/http/basic_auth/config.h" + +#include "test/mocks/server/mocks.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace BasicAuth { + +TEST(Factory, ValidConfig) { + const std::string yaml = R"( + users: + inline_string: |- + user1:{SHA}tESsBmE/yNY3lb6a0L6vVQEZNqw= + user2:{SHA}EJ9LPFDXsN9ynSmbxvjp75Bmlx8= + )"; + + BasicAuthFilterFactory factory; + ProtobufTypes::MessagePtr proto_config = factory.createEmptyRouteConfigProto(); + TestUtility::loadFromYaml(yaml, *proto_config); + + NiceMock context; + + auto callback = factory.createFilterFactoryFromProto(*proto_config, "stats", context); + Http::MockFilterChainFactoryCallbacks filter_callback; + EXPECT_CALL(filter_callback, addStreamDecoderFilter(_)); + callback(filter_callback); +} + +TEST(Factory, InvalidConfigNoColon) { + const std::string yaml = R"( + users: + inline_string: |- + user1{SHA}tESsBmE/yNY3lb6a0L6vVQEZNqw= + user2:{SHA}EJ9LPFDXsN9ynSmbxvjp75Bmlx8= + )"; + + BasicAuthFilterFactory factory; + ProtobufTypes::MessagePtr proto_config = factory.createEmptyRouteConfigProto(); + TestUtility::loadFromYaml(yaml, *proto_config); + + NiceMock context; + + EXPECT_THROW(factory.createFilterFactoryFromProto(*proto_config, "stats", context), + EnvoyException); +} + +TEST(Factory, InvalidConfigNoUser) { + const std::string yaml = R"( + users: + inline_string: |- + :{SHA}tESsBmE/yNY3lb6a0L6vVQEZNqw= + user2:{SHA}EJ9LPFDXsN9ynSmbxvjp75Bmlx8= + )"; + + BasicAuthFilterFactory factory; + ProtobufTypes::MessagePtr proto_config = factory.createEmptyRouteConfigProto(); + TestUtility::loadFromYaml(yaml, *proto_config); + + NiceMock context; + + EXPECT_THROW(factory.createFilterFactoryFromProto(*proto_config, "stats", context), + EnvoyException); +} + +TEST(Factory, InvalidConfigNoPassword) { + const std::string yaml = R"( + users: + inline_string: |- + user1: + user2:{SHA}EJ9LPFDXsN9ynSmbxvjp75Bmlx8= + )"; + + BasicAuthFilterFactory factory; + ProtobufTypes::MessagePtr proto_config = factory.createEmptyRouteConfigProto(); + TestUtility::loadFromYaml(yaml, *proto_config); + + NiceMock context; + + EXPECT_THROW(factory.createFilterFactoryFromProto(*proto_config, "stats", context), + EnvoyException); +} + +TEST(Factory, InvalidConfigNoHash) { + const std::string yaml = R"( + users: + inline_string: |- + user1:{SHA} + user2:{SHA}EJ9LPFDXsN9ynSmbxvjp75Bmlx8= + )"; + + BasicAuthFilterFactory factory; + ProtobufTypes::MessagePtr proto_config = factory.createEmptyRouteConfigProto(); + TestUtility::loadFromYaml(yaml, *proto_config); + + NiceMock context; + + EXPECT_THROW(factory.createFilterFactoryFromProto(*proto_config, "stats", context), + EnvoyException); +} + +TEST(Factory, InvalidConfigNotSHA) { + const std::string yaml = R"( + users: + inline_string: |- + user1:{SHA}tESsBmE/yNY3lb6a0L6vVQEZNqw= + user2:$apr1$0vAnUTEB$4EJJr0GR3y48WF2AiieWs. + )"; + + BasicAuthFilterFactory factory; + ProtobufTypes::MessagePtr proto_config = factory.createEmptyRouteConfigProto(); + TestUtility::loadFromYaml(yaml, *proto_config); + + NiceMock context; + + EXPECT_THROW(factory.createFilterFactoryFromProto(*proto_config, "stats", context), + EnvoyException); +} + +} // namespace BasicAuth +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/filters/http/basic_auth/filter_test.cc b/test/extensions/filters/http/basic_auth/filter_test.cc new file mode 100644 index 0000000000000..ca41cddc2eb1b --- /dev/null +++ b/test/extensions/filters/http/basic_auth/filter_test.cc @@ -0,0 +1,103 @@ +#include "envoy/extensions/filters/http/basic_auth/v3/basic_auth.pb.h" + +#include "source/extensions/filters/http/basic_auth/basic_auth_filter.h" + +#include "test/mocks/http/mocks.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace BasicAuth { + +class FilterTest : public testing::Test { +public: + FilterTest() { + std::vector users; + users.push_back({"user1", "tESsBmE/yNY3lb6a0L6vVQEZNqw="}); // user1:test1 + users.push_back({"user2", "EJ9LPFDXsN9ynSmbxvjp75Bmlx8="}); // user2:test2 + config_ = std::make_shared(users, "stats", *stats_.rootScope()); + filter_ = std::make_shared(config_); + filter_->setDecoderFilterCallbacks(decoder_filter_callbacks_); + } + + NiceMock stats_; + NiceMock decoder_filter_callbacks_; + FilterConfigSharedPtr config_; + std::shared_ptr filter_; +}; + +TEST_F(FilterTest, BasicAuth) { + // user1:test1 + Http::TestRequestHeaderMapImpl request_headers_user1{{"Authorization", "Basic dXNlcjE6dGVzdDE="}}; + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, + filter_->decodeHeaders(request_headers_user1, true)); + + // user2:test2 + Http::TestRequestHeaderMapImpl request_headers_user2{{"Authorization", "Basic dXNlcjI6dGVzdDI="}}; + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, + filter_->decodeHeaders(request_headers_user2, true)); +} + +TEST_F(FilterTest, UserNotExist) { + // user3:test2 + Http::TestRequestHeaderMapImpl request_headers_user1{{"Authorization", "Basic dXNlcjM6dGVzdDI="}}; + + EXPECT_CALL(decoder_filter_callbacks_, sendLocalReply(_, _, _, _, _)) + .WillOnce(Invoke([&](Http::Code code, absl::string_view body, + std::function, + const absl::optional grpc_status, + absl::string_view details) { + EXPECT_EQ(Http::Code::Unauthorized, code); + EXPECT_EQ("Basic Auth failed", body); + EXPECT_EQ(grpc_status, absl::nullopt); + EXPECT_EQ(details, ""); + })); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_user1, true)); +} + +TEST_F(FilterTest, InvalidPassword) { + // user1:test2 + Http::TestRequestHeaderMapImpl request_headers_user1{{"Authorization", "Basic dXNlcjE6dGVzdDI="}}; + + EXPECT_CALL(decoder_filter_callbacks_, sendLocalReply(_, _, _, _, _)) + .WillOnce(Invoke([&](Http::Code code, absl::string_view body, + std::function, + const absl::optional grpc_status, + absl::string_view details) { + EXPECT_EQ(Http::Code::Unauthorized, code); + EXPECT_EQ("Basic Auth failed", body); + EXPECT_EQ(grpc_status, absl::nullopt); + EXPECT_EQ(details, ""); + })); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_user1, true)); +} + +TEST_F(FilterTest, NoAuthHeader) { + Http::TestRequestHeaderMapImpl request_headers_user1; + + EXPECT_CALL(decoder_filter_callbacks_, sendLocalReply(_, _, _, _, _)) + .WillOnce(Invoke([&](Http::Code code, absl::string_view body, + std::function, + const absl::optional grpc_status, + absl::string_view details) { + EXPECT_EQ(Http::Code::Unauthorized, code); + EXPECT_EQ("Missing username or password", body); + EXPECT_EQ(grpc_status, absl::nullopt); + EXPECT_EQ(details, ""); + })); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_user1, true)); +} + +} // namespace BasicAuth +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy + From ea751fb77bfe07a88f9d8d94de7cd92e230bd019 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Tue, 17 Oct 2023 10:07:18 +0000 Subject: [PATCH 04/24] change user vector to hash map Signed-off-by: Huabing Zhao --- .../http/basic_auth/basic_auth_filter.cc | 20 +++++++++---------- .../http/basic_auth/basic_auth_filter.h | 9 +++++++-- .../filters/http/basic_auth/config.cc | 6 +++--- .../filters/http/basic_auth/filter_test.cc | 10 +++++----- 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/source/extensions/filters/http/basic_auth/basic_auth_filter.cc b/source/extensions/filters/http/basic_auth/basic_auth_filter.cc index 074966672368d..a494766313226 100644 --- a/source/extensions/filters/http/basic_auth/basic_auth_filter.cc +++ b/source/extensions/filters/http/basic_auth/basic_auth_filter.cc @@ -28,20 +28,20 @@ std::string computeSHA1(const std::string& password) { } // namespace -FilterConfig::FilterConfig(std::vector users, const std::string& stats_prefix, - Stats::Scope& scope) +FilterConfig::FilterConfig(UserMap users, const std::string& stats_prefix, Stats::Scope& scope) : users_(users), stats_(generateStats(stats_prefix + "basic_auth.", scope)) {} bool FilterConfig::validateUser(const std::string& username, const std::string& password) { - for (auto user : users_) { - if (user.name == username) { - std::string hashedPassword = computeSHA1(password); - if (hashedPassword == user.hash) { - return true; - } - return false; - } + const auto user = users_.find(username); + if (user == users_.end()) { + return false; } + + std::string hashedPassword = computeSHA1(password); + if (hashedPassword == user->second.hash) { + return true; + } + return false; } diff --git a/source/extensions/filters/http/basic_auth/basic_auth_filter.h b/source/extensions/filters/http/basic_auth/basic_auth_filter.h index 85c2f66e43ebc..2b6ee64c00c9b 100644 --- a/source/extensions/filters/http/basic_auth/basic_auth_filter.h +++ b/source/extensions/filters/http/basic_auth/basic_auth_filter.h @@ -1,5 +1,7 @@ #pragma once +#include "absl/container/flat_hash_map.h" + #include "envoy/stats/stats_macros.h" #include "source/common/common/logger.h" @@ -34,12 +36,14 @@ struct User { std::string hash; }; +using UserMap = absl::flat_hash_map; // username, User + /** * Configuration for the Basic Auth filter. */ class FilterConfig { public: - FilterConfig(std::vector users, const std::string& stats_prefix, Stats::Scope& scope); + FilterConfig(UserMap users, const std::string& stats_prefix, Stats::Scope& scope); BasicAuthStats& stats() { return stats_; } bool validateUser(const std::string& username, const std::string& password); @@ -48,7 +52,7 @@ class FilterConfig { return BasicAuthStats{ALL_BASIC_AUTH_STATS(POOL_COUNTER_PREFIX(scope, prefix))}; } - std::vector users_; + UserMap users_; BasicAuthStats stats_; }; using FilterConfigSharedPtr = std::shared_ptr; @@ -73,3 +77,4 @@ class BasicAuthFilter : public Http::PassThroughFilter, } // namespace HttpFilters } // namespace Extensions } // namespace Envoy + diff --git a/source/extensions/filters/http/basic_auth/config.cc b/source/extensions/filters/http/basic_auth/config.cc index f9fb075c5454c..66826e2b2f817 100644 --- a/source/extensions/filters/http/basic_auth/config.cc +++ b/source/extensions/filters/http/basic_auth/config.cc @@ -12,8 +12,8 @@ using envoy::extensions::filters::http::basic_auth::v3::BasicAuth; namespace { -std::vector readHtpasswd(std::string htpasswd) { - std::vector users; +UserMap readHtpasswd(std::string htpasswd) { + UserMap users; std::istringstream htpsswd_ss(htpasswd); std::string line; @@ -36,7 +36,7 @@ std::vector readHtpasswd(std::string htpasswd) { throw EnvoyException("invalid SHA hash length"); } - users.push_back({name, hash}); + users.insert({name, {name, hash}}); continue; } } diff --git a/test/extensions/filters/http/basic_auth/filter_test.cc b/test/extensions/filters/http/basic_auth/filter_test.cc index ca41cddc2eb1b..780e859734fba 100644 --- a/test/extensions/filters/http/basic_auth/filter_test.cc +++ b/test/extensions/filters/http/basic_auth/filter_test.cc @@ -15,9 +15,9 @@ namespace BasicAuth { class FilterTest : public testing::Test { public: FilterTest() { - std::vector users; - users.push_back({"user1", "tESsBmE/yNY3lb6a0L6vVQEZNqw="}); // user1:test1 - users.push_back({"user2", "EJ9LPFDXsN9ynSmbxvjp75Bmlx8="}); // user2:test2 + UserMap users; + users.insert({"user1", {"user1", "tESsBmE/yNY3lb6a0L6vVQEZNqw="}}); // user1:test1 + users.insert({"user2", {"user2", "EJ9LPFDXsN9ynSmbxvjp75Bmlx8="}}); // user2:test2 config_ = std::make_shared(users, "stats", *stats_.rootScope()); filter_ = std::make_shared(config_); filter_->setDecoderFilterCallbacks(decoder_filter_callbacks_); @@ -65,7 +65,7 @@ TEST_F(FilterTest, InvalidPassword) { // user1:test2 Http::TestRequestHeaderMapImpl request_headers_user1{{"Authorization", "Basic dXNlcjE6dGVzdDI="}}; - EXPECT_CALL(decoder_filter_callbacks_, sendLocalReply(_, _, _, _, _)) + EXPECT_CALL(decoder_filter_callbacks_, sendLocalReply(_, _, _, _, _)) .WillOnce(Invoke([&](Http::Code code, absl::string_view body, std::function, const absl::optional grpc_status, @@ -82,7 +82,7 @@ TEST_F(FilterTest, InvalidPassword) { TEST_F(FilterTest, NoAuthHeader) { Http::TestRequestHeaderMapImpl request_headers_user1; - EXPECT_CALL(decoder_filter_callbacks_, sendLocalReply(_, _, _, _, _)) + EXPECT_CALL(decoder_filter_callbacks_, sendLocalReply(_, _, _, _, _)) .WillOnce(Invoke([&](Http::Code code, absl::string_view body, std::function, const absl::optional grpc_status, From 51dc66360f2bf60743b0af060b18de870329f1af Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Tue, 17 Oct 2023 12:52:12 +0000 Subject: [PATCH 05/24] fix format Signed-off-by: Huabing Zhao --- CODEOWNERS | 2 ++ source/extensions/filters/http/basic_auth/BUILD | 1 - .../extensions/filters/http/basic_auth/basic_auth_filter.cc | 2 +- .../extensions/filters/http/basic_auth/basic_auth_filter.h | 5 ++--- test/extensions/filters/http/basic_auth/BUILD | 1 + test/extensions/filters/http/basic_auth/filter_test.cc | 1 - 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CODEOWNERS b/CODEOWNERS index d168f9e8b8c66..ede41bb9ff38e 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -192,6 +192,8 @@ extensions/filters/http/oauth2 @derekargueta @mattklein123 /*/extensions/filters/http/rate_limit_quota @tyxia @yanavlasov # HTTP Bandwidth Limit /*/extensions/filters/http/bandwidth_limit @nitgoy @mattklein123 @yanavlasov @tonya11en +# HTTP Basic Auth +/*/extensions/filters/http/basic_auth @zhaohuabing # Original IP detection /*/extensions/http/original_ip_detection/custom_header @alyssawilk @mattklein123 /*/extensions/http/original_ip_detection/xff @alyssawilk @mattklein123 diff --git a/source/extensions/filters/http/basic_auth/BUILD b/source/extensions/filters/http/basic_auth/BUILD index 1e8a3a7bbcd43..f610d4fee905d 100644 --- a/source/extensions/filters/http/basic_auth/BUILD +++ b/source/extensions/filters/http/basic_auth/BUILD @@ -21,7 +21,6 @@ envoy_cc_library( "//source/common/http:header_map_lib", "//source/common/protobuf:utility_lib", "//source/extensions/filters/http/common:pass_through_filter_lib", - "@envoy_api//envoy/extensions/filters/http/basic_auth/v3:pkg_cc_proto", ], ) diff --git a/source/extensions/filters/http/basic_auth/basic_auth_filter.cc b/source/extensions/filters/http/basic_auth/basic_auth_filter.cc index a494766313226..35b2cd3639a46 100644 --- a/source/extensions/filters/http/basic_auth/basic_auth_filter.cc +++ b/source/extensions/filters/http/basic_auth/basic_auth_filter.cc @@ -3,8 +3,8 @@ #include #include "source/common/common/base64.h" -#include "source/common/http/headers.h" #include "source/common/http/header_utility.h" +#include "source/common/http/headers.h" namespace Envoy { namespace Extensions { diff --git a/source/extensions/filters/http/basic_auth/basic_auth_filter.h b/source/extensions/filters/http/basic_auth/basic_auth_filter.h index 2b6ee64c00c9b..0eaadd03f56b1 100644 --- a/source/extensions/filters/http/basic_auth/basic_auth_filter.h +++ b/source/extensions/filters/http/basic_auth/basic_auth_filter.h @@ -1,12 +1,12 @@ #pragma once -#include "absl/container/flat_hash_map.h" - #include "envoy/stats/stats_macros.h" #include "source/common/common/logger.h" #include "source/extensions/filters/http/common/pass_through_filter.h" +#include "absl/container/flat_hash_map.h" + namespace Envoy { namespace Extensions { namespace HttpFilters { @@ -77,4 +77,3 @@ class BasicAuthFilter : public Http::PassThroughFilter, } // namespace HttpFilters } // namespace Extensions } // namespace Envoy - diff --git a/test/extensions/filters/http/basic_auth/BUILD b/test/extensions/filters/http/basic_auth/BUILD index c506a1c8e1b02..44e9a545daeeb 100644 --- a/test/extensions/filters/http/basic_auth/BUILD +++ b/test/extensions/filters/http/basic_auth/BUILD @@ -18,6 +18,7 @@ envoy_extension_cc_test( deps = [ "//source/extensions/filters/http/basic_auth:basic_auth_lib", "//test/mocks/server:server_mocks", + "@envoy_api//envoy/extensions/filters/http/basic_auth/v3:pkg_cc_proto", ], ) diff --git a/test/extensions/filters/http/basic_auth/filter_test.cc b/test/extensions/filters/http/basic_auth/filter_test.cc index 780e859734fba..2e316a445dde1 100644 --- a/test/extensions/filters/http/basic_auth/filter_test.cc +++ b/test/extensions/filters/http/basic_auth/filter_test.cc @@ -100,4 +100,3 @@ TEST_F(FilterTest, NoAuthHeader) { } // namespace HttpFilters } // namespace Extensions } // namespace Envoy - From 84861c3600ed5eb3b8111a61aafb46ed38831c9b Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Wed, 18 Oct 2023 08:57:27 +0800 Subject: [PATCH 06/24] change filter status to alpha Signed-off-by: huabing zhao --- api/envoy/extensions/filters/http/basic_auth/v3/BUILD | 1 - .../extensions/filters/http/basic_auth/v3/basic_auth.proto | 3 --- source/extensions/extensions_metadata.yaml | 2 +- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/api/envoy/extensions/filters/http/basic_auth/v3/BUILD b/api/envoy/extensions/filters/http/basic_auth/v3/BUILD index e9b556d681cfd..1c1a6f6b44235 100644 --- a/api/envoy/extensions/filters/http/basic_auth/v3/BUILD +++ b/api/envoy/extensions/filters/http/basic_auth/v3/BUILD @@ -8,6 +8,5 @@ api_proto_package( deps = [ "//envoy/config/core/v3:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", - "@com_github_cncf_udpa//xds/annotations/v3:pkg", ], ) diff --git a/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto b/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto index c389486344e9b..c26eee03a3824 100644 --- a/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto +++ b/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto @@ -4,8 +4,6 @@ package envoy.extensions.filters.http.basic_auth.v3; import "envoy/config/core/v3/base.proto"; -import "xds/annotations/v3/status.proto"; - import "udpa/annotations/sensitive.proto"; import "udpa/annotations/status.proto"; @@ -14,7 +12,6 @@ option java_outer_classname = "BasicAuthProto"; option java_multiple_files = true; option go_package = "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/basic_auth/v3;basic_authv3"; option (udpa.annotations.file_status).package_version_status = ACTIVE; -option (xds.annotations.v3.file_status).work_in_progress = true; // [#protodoc-title: Basic Auth] // [#not-implemented-hide:] diff --git a/source/extensions/extensions_metadata.yaml b/source/extensions/extensions_metadata.yaml index 047432d17b4d4..ef2b9f977e614 100644 --- a/source/extensions/extensions_metadata.yaml +++ b/source/extensions/extensions_metadata.yaml @@ -221,7 +221,7 @@ envoy.filters.http.basic_auth: categories: - envoy.filters.http security_posture: unknown - status: wip + status: alpha type_urls: - envoy.extensions.filters.http.basic_auth.v3.BasicAuth envoy.filters.http.buffer: From d2d26025a6165cf5e67f4945be56e062dda72bb4 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Wed, 18 Oct 2023 03:52:07 +0000 Subject: [PATCH 07/24] using declaration Signed-off-by: Huabing Zhao --- source/extensions/filters/http/basic_auth/config.cc | 7 +++---- source/extensions/filters/http/basic_auth/config.h | 12 +++++++----- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/source/extensions/filters/http/basic_auth/config.cc b/source/extensions/filters/http/basic_auth/config.cc index 66826e2b2f817..ac435f557a0c3 100644 --- a/source/extensions/filters/http/basic_auth/config.cc +++ b/source/extensions/filters/http/basic_auth/config.cc @@ -8,8 +8,6 @@ namespace Extensions { namespace HttpFilters { namespace BasicAuth { -using envoy::extensions::filters::http::basic_auth::v3::BasicAuth; - namespace { UserMap readHtpasswd(std::string htpasswd) { @@ -50,8 +48,8 @@ UserMap readHtpasswd(std::string htpasswd) { } // namespace Http::FilterFactoryCb BasicAuthFilterFactory::createFilterFactoryFromProtoTyped( - const envoy::extensions::filters::http::basic_auth::v3::BasicAuth& proto_config, - const std::string& stats_prefix, Server::Configuration::FactoryContext& context) { + const BasicAuth& proto_config, const std::string& stats_prefix, + Server::Configuration::FactoryContext& context) { auto htpasswd = Config::DataSource::read(proto_config.users(), false, context.api()); auto users = readHtpasswd(htpasswd); FilterConfigSharedPtr config = @@ -67,3 +65,4 @@ REGISTER_FACTORY(BasicAuthFilterFactory, Server::Configuration::NamedHttpFilterC } // namespace HttpFilters } // namespace Extensions } // namespace Envoy + diff --git a/source/extensions/filters/http/basic_auth/config.h b/source/extensions/filters/http/basic_auth/config.h index 7abebaaa789ca..b3bbe79a92258 100644 --- a/source/extensions/filters/http/basic_auth/config.h +++ b/source/extensions/filters/http/basic_auth/config.h @@ -10,18 +10,20 @@ namespace Extensions { namespace HttpFilters { namespace BasicAuth { -class BasicAuthFilterFactory - : public Common::FactoryBase { +using envoy::extensions::filters::http::basic_auth::v3::BasicAuth + + class BasicAuthFilterFactory : public Common::FactoryBase { public: BasicAuthFilterFactory() : FactoryBase("envoy.filters.http.basic_auth") {} private: - Http::FilterFactoryCb createFilterFactoryFromProtoTyped( - const envoy::extensions::filters::http::basic_auth::v3::BasicAuth& config, - const std::string& stats_prefix, Server::Configuration::FactoryContext& context) override; + Http::FilterFactoryCb + createFilterFactoryFromProtoTyped(const BasicAuth& config, const std::string& stats_prefix, + Server::Configuration::FactoryContext& context) override; }; } // namespace BasicAuth } // namespace HttpFilters } // namespace Extensions } // namespace Envoy + From f98bc3056b92ac713822f75e2a18c9b83b79c07f Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Wed, 18 Oct 2023 17:25:17 +0800 Subject: [PATCH 08/24] add wbpcode to code owners Signed-off-by: huabing zhao --- CODEOWNERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CODEOWNERS b/CODEOWNERS index ede41bb9ff38e..85b38d7dda368 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -193,7 +193,7 @@ extensions/filters/http/oauth2 @derekargueta @mattklein123 # HTTP Bandwidth Limit /*/extensions/filters/http/bandwidth_limit @nitgoy @mattklein123 @yanavlasov @tonya11en # HTTP Basic Auth -/*/extensions/filters/http/basic_auth @zhaohuabing +/*/extensions/filters/http/basic_auth @zhaohuabing @wbpcode # Original IP detection /*/extensions/http/original_ip_detection/custom_header @alyssawilk @mattklein123 /*/extensions/http/original_ip_detection/xff @alyssawilk @mattklein123 From ce95405798b468f4b18bd3547cef766020e0edce Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Wed, 18 Oct 2023 10:05:41 +0000 Subject: [PATCH 09/24] fix format Signed-off-by: Huabing Zhao --- source/extensions/filters/http/basic_auth/config.cc | 1 - source/extensions/filters/http/basic_auth/config.h | 1 - 2 files changed, 2 deletions(-) diff --git a/source/extensions/filters/http/basic_auth/config.cc b/source/extensions/filters/http/basic_auth/config.cc index ac435f557a0c3..683b6815b7d96 100644 --- a/source/extensions/filters/http/basic_auth/config.cc +++ b/source/extensions/filters/http/basic_auth/config.cc @@ -65,4 +65,3 @@ REGISTER_FACTORY(BasicAuthFilterFactory, Server::Configuration::NamedHttpFilterC } // namespace HttpFilters } // namespace Extensions } // namespace Envoy - diff --git a/source/extensions/filters/http/basic_auth/config.h b/source/extensions/filters/http/basic_auth/config.h index b3bbe79a92258..a43b15b9e57ed 100644 --- a/source/extensions/filters/http/basic_auth/config.h +++ b/source/extensions/filters/http/basic_auth/config.h @@ -26,4 +26,3 @@ using envoy::extensions::filters::http::basic_auth::v3::BasicAuth } // namespace HttpFilters } // namespace Extensions } // namespace Envoy - From 3a45cc6a4fa20ae5a293f5ce6953b23a88755aff Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Thu, 19 Oct 2023 08:19:57 +0800 Subject: [PATCH 10/24] add docs Signed-off-by: huabing zhao --- .../http/basic_auth/v3/basic_auth.proto | 7 ++- .../http/http_filters/basic_auth_filter.rst | 46 +++++++++++++++++++ .../http/http_filters/http_filters.rst | 1 + 3 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 docs/root/configuration/http/http_filters/basic_auth_filter.rst diff --git a/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto b/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto index c26eee03a3824..175a930f3a630 100644 --- a/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto +++ b/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto @@ -14,7 +14,6 @@ option go_package = "github.com/envoyproxy/go-control-plane/envoy/extensions/fil option (udpa.annotations.file_status).package_version_status = ACTIVE; // [#protodoc-title: Basic Auth] -// [#not-implemented-hide:] // Basic Auth :ref:`configuration overview `. // [#extension: envoy.filters.http.basic_auth] @@ -24,10 +23,10 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // // .. code-block:: yaml // -// htpasswd: +// users: // inline_string: |- -// user1:hashed_user1_password -// user2:hashed_user2_password +// user1:{SHA}hashed_user1_password +// user2:{SHA}hashed_user2_password // message BasicAuth { // Username-password pairs used to verify user credentials in the "Authorization" header. diff --git a/docs/root/configuration/http/http_filters/basic_auth_filter.rst b/docs/root/configuration/http/http_filters/basic_auth_filter.rst new file mode 100644 index 0000000000000..ebb0d1bd649dc --- /dev/null +++ b/docs/root/configuration/http/http_filters/basic_auth_filter.rst @@ -0,0 +1,46 @@ +.. _config_http_filters_basic_auth: + +Basic Auth +========== + +This HTTP filter can be used to authenticate user credentials in the HTTP Authentication heaer defined +in `RFC7617 `. + +The filter will extract the username and password from the HTTP Authentication header and verify them +against the configured username and password list. + +If the username and password are valid, the request will be forwared to the next filter in the filter chains. +If they're invalid or not provided in the HTTP request, the rqeust will be denied with a 401 Unauthorized response. + +Configuration +------------- + +* This filter should be configured with the type URL ``type.googleapis.com/envoy.extensions.filters.http.basic_auth.v3.BasicAuth``. +* :ref:`v3 API reference ` + +``users`` is a list of username-password pairs used to verify user credentials in the "Authorization" header. + The value needs to be the `htpasswd ` format. + + +An example configuration of the filter may look like the following: + +.. code-block:: yaml + + users: + inline_string: |- + user1:{SHA}hashed_user1_password + user2:{SHA}hashed_user2_password + +Note that only SHA format is currently supported. Other formats may be added in the future. + +Statistics +---------- + +The HTTP basic auth filter outputs statistics in the ``http..basic_auth.`` namespace. + +.. csv-table:: + :header: Name, Type, Description + :widths: 1, 1, 2 + + allowed, Counter, Total number of allowed requests + denied, Counter, Total number of denied requests diff --git a/docs/root/configuration/http/http_filters/http_filters.rst b/docs/root/configuration/http/http_filters/http_filters.rst index bbf38623d02fb..eb1333ad0e03c 100644 --- a/docs/root/configuration/http/http_filters/http_filters.rst +++ b/docs/root/configuration/http/http_filters/http_filters.rst @@ -11,6 +11,7 @@ HTTP filters aws_lambda_filter aws_request_signing_filter bandwidth_limit_filter + basic_auth_filter buffer_filter cache_filter cdn_loop_filter From 58a5aeb6d62bf64ac0e625169a24bc0d0d5f5dc0 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Thu, 19 Oct 2023 18:33:30 -0500 Subject: [PATCH 11/24] Update api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto Co-authored-by: code Signed-off-by: Huabing Zhao --- .../filters/http/basic_auth/v3/basic_auth.proto | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto b/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto index 175a930f3a630..df23868a42602 100644 --- a/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto +++ b/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto @@ -21,12 +21,12 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // // Example: // -// .. code-block:: yaml +// .. code-block:: yaml // -// users: -// inline_string: |- -// user1:{SHA}hashed_user1_password -// user2:{SHA}hashed_user2_password +// users: +// inline_string: |- +// user1:{SHA}hashed_user1_password +// user2:{SHA}hashed_user2_password // message BasicAuth { // Username-password pairs used to verify user credentials in the "Authorization" header. From 699c56318d01045f5b31cd085328c735af1a644c Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Fri, 20 Oct 2023 06:17:46 +0000 Subject: [PATCH 12/24] fix build Signed-off-by: Huabing Zhao --- source/extensions/filters/http/basic_auth/config.cc | 2 ++ source/extensions/filters/http/basic_auth/config.h | 11 +++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/source/extensions/filters/http/basic_auth/config.cc b/source/extensions/filters/http/basic_auth/config.cc index 683b6815b7d96..256e4fc740857 100644 --- a/source/extensions/filters/http/basic_auth/config.cc +++ b/source/extensions/filters/http/basic_auth/config.cc @@ -8,6 +8,8 @@ namespace Extensions { namespace HttpFilters { namespace BasicAuth { +using envoy::extensions::filters::http::basic_auth::v3::BasicAuth; + namespace { UserMap readHtpasswd(std::string htpasswd) { diff --git a/source/extensions/filters/http/basic_auth/config.h b/source/extensions/filters/http/basic_auth/config.h index a43b15b9e57ed..7abebaaa789ca 100644 --- a/source/extensions/filters/http/basic_auth/config.h +++ b/source/extensions/filters/http/basic_auth/config.h @@ -10,16 +10,15 @@ namespace Extensions { namespace HttpFilters { namespace BasicAuth { -using envoy::extensions::filters::http::basic_auth::v3::BasicAuth - - class BasicAuthFilterFactory : public Common::FactoryBase { +class BasicAuthFilterFactory + : public Common::FactoryBase { public: BasicAuthFilterFactory() : FactoryBase("envoy.filters.http.basic_auth") {} private: - Http::FilterFactoryCb - createFilterFactoryFromProtoTyped(const BasicAuth& config, const std::string& stats_prefix, - Server::Configuration::FactoryContext& context) override; + Http::FilterFactoryCb createFilterFactoryFromProtoTyped( + const envoy::extensions::filters::http::basic_auth::v3::BasicAuth& config, + const std::string& stats_prefix, Server::Configuration::FactoryContext& context) override; }; } // namespace BasicAuth From c0b936bf0e260fce3eb22ac874624f185586b275 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Mon, 23 Oct 2023 05:58:08 +0000 Subject: [PATCH 13/24] address comments Signed-off-by: Huabing Zhao --- source/extensions/extensions_metadata.yaml | 2 +- .../http/basic_auth/basic_auth_filter.cc | 56 +++++++++---------- .../http/basic_auth/basic_auth_filter.h | 17 +++--- .../filters/http/basic_auth/config.cc | 33 +++++------ .../filters/http/basic_auth/filter_test.cc | 6 +- 5 files changed, 58 insertions(+), 56 deletions(-) diff --git a/source/extensions/extensions_metadata.yaml b/source/extensions/extensions_metadata.yaml index ef2b9f977e614..ad04032af3735 100644 --- a/source/extensions/extensions_metadata.yaml +++ b/source/extensions/extensions_metadata.yaml @@ -220,7 +220,7 @@ envoy.filters.http.bandwidth_limit: envoy.filters.http.basic_auth: categories: - envoy.filters.http - security_posture: unknown + security_posture: robust_to_untrusted_downstream status: alpha type_urls: - envoy.extensions.filters.http.basic_auth.v3.BasicAuth diff --git a/source/extensions/filters/http/basic_auth/basic_auth_filter.cc b/source/extensions/filters/http/basic_auth/basic_auth_filter.cc index 35b2cd3639a46..102329082ff79 100644 --- a/source/extensions/filters/http/basic_auth/basic_auth_filter.cc +++ b/source/extensions/filters/http/basic_auth/basic_auth_filter.cc @@ -14,65 +14,65 @@ namespace BasicAuth { namespace { // Function to compute SHA1 hash -std::string computeSHA1(const std::string& password) { +std::string computeSHA1(absl::string_view password) { unsigned char hash[SHA_DIGEST_LENGTH]; // Calculate the SHA-1 hash - SHA1(reinterpret_cast(password.c_str()), password.length(), hash); + SHA1(reinterpret_cast(password.data()), password.length(), hash); // Encode the binary hash in Base64 - std::string encodedHash = Base64::encode(reinterpret_cast(hash), SHA_DIGEST_LENGTH); - - return encodedHash; + return Base64::encode(reinterpret_cast(hash), SHA_DIGEST_LENGTH); } } // namespace -FilterConfig::FilterConfig(UserMap users, const std::string& stats_prefix, Stats::Scope& scope) - : users_(users), stats_(generateStats(stats_prefix + "basic_auth.", scope)) {} +FilterConfig::FilterConfig(UserMapConstPtr users, const std::string& stats_prefix, + Stats::Scope& scope) + : users_(std::move(users)), stats_(generateStats(stats_prefix + "basic_auth.", scope)) {} -bool FilterConfig::validateUser(const std::string& username, const std::string& password) { - const auto user = users_.find(username); - if (user == users_.end()) { +bool FilterConfig::validateUser(absl::string_view username, absl::string_view password) const { + auto user = users_->find(username); + if (user == users_->end()) { return false; } - std::string hashedPassword = computeSHA1(password); - if (hashedPassword == user->second.hash) { + std::string hashed_password = computeSHA1(password); + if (hashed_password == user->second.hash) { return true; } return false; } -BasicAuthFilter::BasicAuthFilter(FilterConfigSharedPtr config) : config_(std::move(config)) {} +BasicAuthFilter::BasicAuthFilter(FilterConfigConstSharedPtr config) : config_(std::move(config)) {} Http::FilterHeadersStatus BasicAuthFilter::decodeHeaders(Http::RequestHeaderMap& headers, bool) { - ENVOY_LOG(debug, "Called Filter : {}", __func__); - auto auth_header = headers.get(Http::CustomHeaders::get().Authorization); if (!auth_header.empty()) { - auto auth_value = auth_header[0]->value().getStringView(); + absl::string_view auth_value = auth_header[0]->value().getStringView(); - if (auth_value.substr(0, 6) == "Basic ") { + if (absl::StartsWith(auth_value, "Basic ")) { // Extract and decode the Base64 part of the header. - auto base64Token = auth_value.substr(6); - std::string decoded = Base64::decodeWithoutPadding(base64Token); + absl::string_view base64Token = auth_value.substr(6); + const std::string decoded = Base64::decodeWithoutPadding(base64Token); // The decoded string is in the format "username:password". - size_t colonPos = decoded.find(':'); + const size_t colon_pos = decoded.find(':'); - if (colonPos != std::string::npos) { - std::string username = decoded.substr(0, colonPos); - std::string password = decoded.substr(colonPos + 1); + if (colon_pos != std::string::npos) { + absl::string_view decoded_view = decoded; + absl::string_view username = decoded_view.substr(0, colon_pos); + absl::string_view password = decoded_view.substr(colon_pos + 1); if (config_->validateUser(username, password)) { config_->stats().allowed_.inc(); return Http::FilterHeadersStatus::Continue; } else { config_->stats().denied_.inc(); - decoder_callbacks_->sendLocalReply(Http::Code::Unauthorized, "Basic Auth failed", nullptr, - absl::nullopt, ""); + decoder_callbacks_->sendLocalReply( + Http::Code::Unauthorized, + "User authentication failed. Invalid username/password combination", nullptr, + absl::nullopt, "invalid_credential_for_basic_auth"); return Http::FilterHeadersStatus::StopIteration; } } @@ -80,13 +80,13 @@ Http::FilterHeadersStatus BasicAuthFilter::decodeHeaders(Http::RequestHeaderMap& } config_->stats().denied_.inc(); - decoder_callbacks_->sendLocalReply(Http::Code::Unauthorized, "Missing username or password", - nullptr, absl::nullopt, ""); + decoder_callbacks_->sendLocalReply(Http::Code::Unauthorized, + "User authentication failed. Missing username and password", + nullptr, absl::nullopt, "no_credential_for_basic_auth"); return Http::FilterHeadersStatus::StopIteration; } void BasicAuthFilter::setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) { - ENVOY_LOG(debug, "Called Filter : {}", __func__); decoder_callbacks_ = &callbacks; } diff --git a/source/extensions/filters/http/basic_auth/basic_auth_filter.h b/source/extensions/filters/http/basic_auth/basic_auth_filter.h index 0eaadd03f56b1..2838c58ef7fa0 100644 --- a/source/extensions/filters/http/basic_auth/basic_auth_filter.h +++ b/source/extensions/filters/http/basic_auth/basic_auth_filter.h @@ -37,31 +37,32 @@ struct User { }; using UserMap = absl::flat_hash_map; // username, User +using UserMapConstPtr = std::unique_ptr; /** * Configuration for the Basic Auth filter. */ class FilterConfig { public: - FilterConfig(UserMap users, const std::string& stats_prefix, Stats::Scope& scope); - BasicAuthStats& stats() { return stats_; } - bool validateUser(const std::string& username, const std::string& password); + FilterConfig(UserMapConstPtr users, const std::string& stats_prefix, Stats::Scope& scope); + const BasicAuthStats& stats() const { return stats_; } + bool validateUser(absl::string_view username, absl::string_view password) const; private: static BasicAuthStats generateStats(const std::string& prefix, Stats::Scope& scope) { return BasicAuthStats{ALL_BASIC_AUTH_STATS(POOL_COUNTER_PREFIX(scope, prefix))}; } - UserMap users_; + UserMapConstPtr users_; BasicAuthStats stats_; }; -using FilterConfigSharedPtr = std::shared_ptr; +using FilterConfigConstSharedPtr = std::shared_ptr; // The Envoy filter to process HTTP basic auth. -class BasicAuthFilter : public Http::PassThroughFilter, +class BasicAuthFilter : public Http::PassThroughDecoderFilter, public Logger::Loggable { public: - BasicAuthFilter(FilterConfigSharedPtr config); + BasicAuthFilter(FilterConfigConstSharedPtr config); // Http::StreamDecoderFilter Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers, bool) override; @@ -70,7 +71,7 @@ class BasicAuthFilter : public Http::PassThroughFilter, private: // The callback function. Http::StreamDecoderFilterCallbacks* decoder_callbacks_; - FilterConfigSharedPtr config_; + FilterConfigConstSharedPtr config_; }; } // namespace BasicAuth diff --git a/source/extensions/filters/http/basic_auth/config.cc b/source/extensions/filters/http/basic_auth/config.cc index 256e4fc740857..8c54c4199ccbd 100644 --- a/source/extensions/filters/http/basic_auth/config.cc +++ b/source/extensions/filters/http/basic_auth/config.cc @@ -12,36 +12,37 @@ using envoy::extensions::filters::http::basic_auth::v3::BasicAuth; namespace { -UserMap readHtpasswd(std::string htpasswd) { - UserMap users; +UserMapConstPtr readHtpasswd(const std::string& htpasswd) { + std::unique_ptr users = std::make_unique(); std::istringstream htpsswd_ss(htpasswd); std::string line; while (std::getline(htpsswd_ss, line)) { - size_t colonPos = line.find(':'); + const size_t colon_pos = line.find(':'); - if (colonPos != std::string::npos) { + if (colon_pos != std::string::npos) { std::string name, hash; - name = line.substr(0, colonPos); - hash = line.substr(colonPos + 1); + name = line.substr(0, colon_pos); + hash = line.substr(colon_pos + 1); - if (name.length() == 0) { - throw EnvoyException("invalid user name"); + if (name.empty()) { + throw EnvoyException("basic auth: invalid user name"); } - if (hash.find("{SHA}") == 0) { + if (absl::StartsWith(hash, "{SHA}")) { hash = hash.substr(5); + // The base64 encoded SHA1 hash is 28 bytes long if (hash.length() != 28) { - throw EnvoyException("invalid SHA hash length"); + throw EnvoyException("basic auth: invalid SHA hash length"); } - users.insert({name, {name, hash}}); + users->insert({name, {name, hash}}); continue; } } - throw EnvoyException("unsupported htpasswd format: please use {SHA}"); + throw EnvoyException("basic auth: unsupported htpasswd format: please use {SHA}"); } return users; @@ -52,10 +53,10 @@ UserMap readHtpasswd(std::string htpasswd) { Http::FilterFactoryCb BasicAuthFilterFactory::createFilterFactoryFromProtoTyped( const BasicAuth& proto_config, const std::string& stats_prefix, Server::Configuration::FactoryContext& context) { - auto htpasswd = Config::DataSource::read(proto_config.users(), false, context.api()); - auto users = readHtpasswd(htpasswd); - FilterConfigSharedPtr config = - std::make_shared(users, stats_prefix, context.scope()); + const std::string htpasswd = Config::DataSource::read(proto_config.users(), false, context.api()); + UserMapConstPtr users = readHtpasswd(htpasswd); + FilterConfigConstSharedPtr config = + std::make_unique(std::move(users), stats_prefix, context.scope()); return [config](Http::FilterChainFactoryCallbacks& callbacks) -> void { callbacks.addStreamDecoderFilter(std::make_shared(config)); }; diff --git a/test/extensions/filters/http/basic_auth/filter_test.cc b/test/extensions/filters/http/basic_auth/filter_test.cc index 2e316a445dde1..6ccb2df1ad564 100644 --- a/test/extensions/filters/http/basic_auth/filter_test.cc +++ b/test/extensions/filters/http/basic_auth/filter_test.cc @@ -15,9 +15,9 @@ namespace BasicAuth { class FilterTest : public testing::Test { public: FilterTest() { - UserMap users; - users.insert({"user1", {"user1", "tESsBmE/yNY3lb6a0L6vVQEZNqw="}}); // user1:test1 - users.insert({"user2", {"user2", "EJ9LPFDXsN9ynSmbxvjp75Bmlx8="}}); // user2:test2 + std::unique_ptr users = std::make_unique(); + users->insert({"user1", {"user1", "tESsBmE/yNY3lb6a0L6vVQEZNqw="}}); // user1:test1 + users->insert({"user2", {"user2", "EJ9LPFDXsN9ynSmbxvjp75Bmlx8="}}); // user2:test2 config_ = std::make_shared(users, "stats", *stats_.rootScope()); filter_ = std::make_shared(config_); filter_->setDecoderFilterCallbacks(decoder_filter_callbacks_); From 7c9776b71abfcb34c30d5bc9ffc56d0a5a2cb649 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Mon, 23 Oct 2023 16:22:44 +0800 Subject: [PATCH 14/24] address comment Signed-off-by: huabing zhao --- .../filters/http/basic_auth/basic_auth_filter.cc | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/source/extensions/filters/http/basic_auth/basic_auth_filter.cc b/source/extensions/filters/http/basic_auth/basic_auth_filter.cc index 102329082ff79..b41ea8277b343 100644 --- a/source/extensions/filters/http/basic_auth/basic_auth_filter.cc +++ b/source/extensions/filters/http/basic_auth/basic_auth_filter.cc @@ -36,12 +36,7 @@ bool FilterConfig::validateUser(absl::string_view username, absl::string_view pa return false; } - std::string hashed_password = computeSHA1(password); - if (hashed_password == user->second.hash) { - return true; - } - - return false; + return computeSHA1(password) == user->scond.hash; } BasicAuthFilter::BasicAuthFilter(FilterConfigConstSharedPtr config) : config_(std::move(config)) {} From 9a90d7174996829ddd080676bbffd78eaccd87a1 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Mon, 23 Oct 2023 08:57:24 +0000 Subject: [PATCH 15/24] address comments Signed-off-by: Huabing Zhao --- .../filters/http/basic_auth/basic_auth_filter.cc | 2 +- .../filters/http/basic_auth/basic_auth_filter.h | 4 ++-- source/extensions/filters/http/basic_auth/config.cc | 8 +++++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/source/extensions/filters/http/basic_auth/basic_auth_filter.cc b/source/extensions/filters/http/basic_auth/basic_auth_filter.cc index b41ea8277b343..ae7b10e6c573a 100644 --- a/source/extensions/filters/http/basic_auth/basic_auth_filter.cc +++ b/source/extensions/filters/http/basic_auth/basic_auth_filter.cc @@ -36,7 +36,7 @@ bool FilterConfig::validateUser(absl::string_view username, absl::string_view pa return false; } - return computeSHA1(password) == user->scond.hash; + return computeSHA1(password) == user->second.hash; } BasicAuthFilter::BasicAuthFilter(FilterConfigConstSharedPtr config) : config_(std::move(config)) {} diff --git a/source/extensions/filters/http/basic_auth/basic_auth_filter.h b/source/extensions/filters/http/basic_auth/basic_auth_filter.h index 2838c58ef7fa0..d900b304eb675 100644 --- a/source/extensions/filters/http/basic_auth/basic_auth_filter.h +++ b/source/extensions/filters/http/basic_auth/basic_auth_filter.h @@ -36,8 +36,8 @@ struct User { std::string hash; }; -using UserMap = absl::flat_hash_map; // username, User -using UserMapConstPtr = std::unique_ptr; +using UserMapConstPtr = + std::unique_ptr>; // username, User /** * Configuration for the Basic Auth filter. diff --git a/source/extensions/filters/http/basic_auth/config.cc b/source/extensions/filters/http/basic_auth/config.cc index 8c54c4199ccbd..9b685c1d61c50 100644 --- a/source/extensions/filters/http/basic_auth/config.cc +++ b/source/extensions/filters/http/basic_auth/config.cc @@ -13,7 +13,8 @@ using envoy::extensions::filters::http::basic_auth::v3::BasicAuth; namespace { UserMapConstPtr readHtpasswd(const std::string& htpasswd) { - std::unique_ptr users = std::make_unique(); + std::unique_ptr> users = + std::make_unique>(); std::istringstream htpsswd_ss(htpasswd); std::string line; @@ -23,8 +24,9 @@ UserMapConstPtr readHtpasswd(const std::string& htpasswd) { if (colon_pos != std::string::npos) { std::string name, hash; - name = line.substr(0, colon_pos); - hash = line.substr(colon_pos + 1); + absl::string_view line_view = line; + name = line_view.substr(0, colon_pos); + hash = line_view.substr(colon_pos + 1); if (name.empty()) { throw EnvoyException("basic auth: invalid user name"); From 2b9d74b5c8f2903677aabb9fbc2ea977b0ca8501 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Mon, 23 Oct 2023 09:08:45 +0000 Subject: [PATCH 16/24] address comments Signed-off-by: Huabing Zhao --- source/extensions/filters/http/basic_auth/config.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/source/extensions/filters/http/basic_auth/config.cc b/source/extensions/filters/http/basic_auth/config.cc index 9b685c1d61c50..da22ed368bf0a 100644 --- a/source/extensions/filters/http/basic_auth/config.cc +++ b/source/extensions/filters/http/basic_auth/config.cc @@ -25,15 +25,16 @@ UserMapConstPtr readHtpasswd(const std::string& htpasswd) { std::string name, hash; absl::string_view line_view = line; + absl::string_view hash_view; name = line_view.substr(0, colon_pos); - hash = line_view.substr(colon_pos + 1); + hash_view = line_view.substr(colon_pos + 1); if (name.empty()) { throw EnvoyException("basic auth: invalid user name"); } - if (absl::StartsWith(hash, "{SHA}")) { - hash = hash.substr(5); + if (absl::StartsWith(hash_view, "{SHA}")) { + hash = hash_view.substr(5); // The base64 encoded SHA1 hash is 28 bytes long if (hash.length() != 28) { throw EnvoyException("basic auth: invalid SHA hash length"); From 011f5d213b980a6019b50d9730ae6a62a408d17b Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Mon, 23 Oct 2023 14:25:33 +0000 Subject: [PATCH 17/24] fix test Signed-off-by: Huabing Zhao --- test/extensions/filters/http/basic_auth/filter_test.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/extensions/filters/http/basic_auth/filter_test.cc b/test/extensions/filters/http/basic_auth/filter_test.cc index 6ccb2df1ad564..86acb7afb090b 100644 --- a/test/extensions/filters/http/basic_auth/filter_test.cc +++ b/test/extensions/filters/http/basic_auth/filter_test.cc @@ -15,7 +15,8 @@ namespace BasicAuth { class FilterTest : public testing::Test { public: FilterTest() { - std::unique_ptr users = std::make_unique(); + std::unique_ptr> users = + std::make_unique>(); users->insert({"user1", {"user1", "tESsBmE/yNY3lb6a0L6vVQEZNqw="}}); // user1:test1 users->insert({"user2", {"user2", "EJ9LPFDXsN9ynSmbxvjp75Bmlx8="}}); // user2:test2 config_ = std::make_shared(users, "stats", *stats_.rootScope()); @@ -25,7 +26,7 @@ class FilterTest : public testing::Test { NiceMock stats_; NiceMock decoder_filter_callbacks_; - FilterConfigSharedPtr config_; + FilterConfigConstSharedPtr config_; std::shared_ptr filter_; }; From 89e1517e38540f9c161650f96d460b61300d0576 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Mon, 23 Oct 2023 15:39:30 +0000 Subject: [PATCH 18/24] fix test Signed-off-by: Huabing Zhao --- .../filters/http/basic_auth/filter_test.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/extensions/filters/http/basic_auth/filter_test.cc b/test/extensions/filters/http/basic_auth/filter_test.cc index 86acb7afb090b..d8d3f3000be46 100644 --- a/test/extensions/filters/http/basic_auth/filter_test.cc +++ b/test/extensions/filters/http/basic_auth/filter_test.cc @@ -19,7 +19,7 @@ class FilterTest : public testing::Test { std::make_unique>(); users->insert({"user1", {"user1", "tESsBmE/yNY3lb6a0L6vVQEZNqw="}}); // user1:test1 users->insert({"user2", {"user2", "EJ9LPFDXsN9ynSmbxvjp75Bmlx8="}}); // user2:test2 - config_ = std::make_shared(users, "stats", *stats_.rootScope()); + config_ = std::make_unique(std::move(users), "stats", *stats_.rootScope()); filter_ = std::make_shared(config_); filter_->setDecoderFilterCallbacks(decoder_filter_callbacks_); } @@ -54,9 +54,9 @@ TEST_F(FilterTest, UserNotExist) { const absl::optional grpc_status, absl::string_view details) { EXPECT_EQ(Http::Code::Unauthorized, code); - EXPECT_EQ("Basic Auth failed", body); + EXPECT_EQ("User authentication failed. Invalid username/password combination", body); EXPECT_EQ(grpc_status, absl::nullopt); - EXPECT_EQ(details, ""); + EXPECT_EQ(details, "invalid_credential_for_basic_auth"); })); EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_user1, true)); @@ -72,9 +72,9 @@ TEST_F(FilterTest, InvalidPassword) { const absl::optional grpc_status, absl::string_view details) { EXPECT_EQ(Http::Code::Unauthorized, code); - EXPECT_EQ("Basic Auth failed", body); + EXPECT_EQ("User authentication failed. Invalid username/password combination", body); EXPECT_EQ(grpc_status, absl::nullopt); - EXPECT_EQ(details, ""); + EXPECT_EQ(details, "invalid_credential_for_basic_auth"); })); EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_user1, true)); @@ -89,9 +89,9 @@ TEST_F(FilterTest, NoAuthHeader) { const absl::optional grpc_status, absl::string_view details) { EXPECT_EQ(Http::Code::Unauthorized, code); - EXPECT_EQ("Missing username or password", body); + EXPECT_EQ("User authentication failed. Missing username and password", body); EXPECT_EQ(grpc_status, absl::nullopt); - EXPECT_EQ(details, ""); + EXPECT_EQ(details, "no_credential_for_basic_auth"); })); EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_user1, true)); From fecac2176f5da138fccf544fc0bad70fe48a901c Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Tue, 24 Oct 2023 02:06:09 +0000 Subject: [PATCH 19/24] address comment Signed-off-by: Huabing Zhao --- source/extensions/filters/http/basic_auth/config.cc | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/source/extensions/filters/http/basic_auth/config.cc b/source/extensions/filters/http/basic_auth/config.cc index da22ed368bf0a..02a3582cad693 100644 --- a/source/extensions/filters/http/basic_auth/config.cc +++ b/source/extensions/filters/http/basic_auth/config.cc @@ -22,19 +22,15 @@ UserMapConstPtr readHtpasswd(const std::string& htpasswd) { const size_t colon_pos = line.find(':'); if (colon_pos != std::string::npos) { - std::string name, hash; - - absl::string_view line_view = line; - absl::string_view hash_view; - name = line_view.substr(0, colon_pos); - hash_view = line_view.substr(colon_pos + 1); + std::string name = line.substr(0, colon_pos); + std::string hash = line.substr(colon_pos + 1); if (name.empty()) { throw EnvoyException("basic auth: invalid user name"); } - if (absl::StartsWith(hash_view, "{SHA}")) { - hash = hash_view.substr(5); + if (absl::StartsWith(hash, "{SHA}")) { + hash = hash.substr(5); // The base64 encoded SHA1 hash is 28 bytes long if (hash.length() != 28) { throw EnvoyException("basic auth: invalid SHA hash length"); From 9388e090ba8504385642910633d7883bb74a943f Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Tue, 24 Oct 2023 02:24:21 +0000 Subject: [PATCH 20/24] add release note Signed-off-by: Huabing Zhao --- changelogs/current.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index dcc37e21c9da1..c22403732bdeb 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -401,6 +401,10 @@ new_features: change: | Added ``metadata`` support for :ref:`virtual host ` and :ref:`route configuration `. +- area: filters + change: | + Added :ref:`the Basic Auth filter `, which can be used to + authenticate user credentials in the HTTP Authentication heaer defined in `RFC7617 `. deprecated: - area: tracing From 73e208bb2cd4841075b77eeb7ef97a7bdaf0b942 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Tue, 24 Oct 2023 03:39:03 +0000 Subject: [PATCH 21/24] fix format Signed-off-by: Huabing Zhao --- changelogs/current.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 1bac6c09daf18..4a9c604023004 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -26,6 +26,6 @@ new_features: - area: filters change: | Added :ref:`the Basic Auth filter `, which can be used to - authenticate user credentials in the HTTP Authentication heaer defined in `RFC7617 `. + authenticate user credentials in the HTTP Authentication heaer defined in `RFC7617 `_. deprecated: From 285d66e11eb136d6ca7175fcf2171a9aeb8f8601 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Wed, 25 Oct 2023 02:24:18 +0000 Subject: [PATCH 22/24] fix typos Signed-off-by: Huabing Zhao --- .../configuration/http/http_filters/basic_auth_filter.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/root/configuration/http/http_filters/basic_auth_filter.rst b/docs/root/configuration/http/http_filters/basic_auth_filter.rst index ebb0d1bd649dc..da8b160fd054c 100644 --- a/docs/root/configuration/http/http_filters/basic_auth_filter.rst +++ b/docs/root/configuration/http/http_filters/basic_auth_filter.rst @@ -3,14 +3,14 @@ Basic Auth ========== -This HTTP filter can be used to authenticate user credentials in the HTTP Authentication heaer defined +This HTTP filter can be used to authenticate user credentials in the HTTP Authentication header defined in `RFC7617 `. The filter will extract the username and password from the HTTP Authentication header and verify them against the configured username and password list. If the username and password are valid, the request will be forwared to the next filter in the filter chains. -If they're invalid or not provided in the HTTP request, the rqeust will be denied with a 401 Unauthorized response. +If they're invalid or not provided in the HTTP request, the request will be denied with a 401 Unauthorized response. Configuration ------------- From 09efeec6ce492632c9d8a782ec96980424a7b767 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Sat, 28 Oct 2023 11:19:09 +0800 Subject: [PATCH 23/24] integration test Signed-off-by: huabing zhao --- .../http/basic_auth/v3/basic_auth.proto | 2 +- test/extensions/filters/http/basic_auth/BUILD | 12 ++ .../basic_auth/basic_auth_integration_test.cc | 119 ++++++++++++++++++ 3 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc diff --git a/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto b/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto index df23868a42602..ac5050f9cf076 100644 --- a/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto +++ b/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto @@ -32,5 +32,5 @@ message BasicAuth { // Username-password pairs used to verify user credentials in the "Authorization" header. // The value needs to be the htpasswd format. // Reference to https://httpd.apache.org/docs/2.4/programs/htpasswd.html - config.core.v3.DataSource users = 1 [(udpa.annotations.sensitive) = true]; + config.core.v3.DataSource users = 1 [(udpa.annotations.sensitive) = true]; } diff --git a/test/extensions/filters/http/basic_auth/BUILD b/test/extensions/filters/http/basic_auth/BUILD index 44e9a545daeeb..39e573d580cdd 100644 --- a/test/extensions/filters/http/basic_auth/BUILD +++ b/test/extensions/filters/http/basic_auth/BUILD @@ -31,3 +31,15 @@ envoy_extension_cc_test( "//test/mocks/server:server_mocks", ], ) + +envoy_extension_cc_test( + name = "basic_auth_integration_test", + size = "large", + srcs = ["basic_auth_integration_test.cc"], + extension_names = ["envoy.filters.http.basic_auth"], + deps = [ + "//source/extensions/filters/http/basic_auth:config", + "//test/integration:http_protocol_integration_lib", + "//test/test_common:utility_lib", + ], +) diff --git a/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc b/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc new file mode 100644 index 0000000000000..2e70bf9efb5b7 --- /dev/null +++ b/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc @@ -0,0 +1,119 @@ +#include "test/integration/http_protocol_integration.h" +#include "test/test_common/utility.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace BasicAuth { +namespace { + +class BasicAuthIntegrationTest : public HttpProtocolIntegrationTest { +public: + void initializeFilter() { + // user1, test1 + // user2, test2 + const std::string filter_config = + R"EOF( +name: envoy.filters.http.basic_auth +typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.basic_auth.v3.BasicAuth + users: + inline_string: |- + user1:{SHA}tESsBmE/yNY3lb6a0L6vVQEZNqw= + user2:{SHA}EJ9LPFDXsN9ynSmbxvjp75Bmlx8= +)EOF"; + config_helper_.prependFilter(filter_config); + initialize(); + } +}; + +// BasicAuth integration tests that should run with all protocols +class BasicAuthIntegrationTestAllProtocols : public BasicAuthIntegrationTest {}; + +INSTANTIATE_TEST_SUITE_P( + Protocols, BasicAuthIntegrationTestAllProtocols, + testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParamsWithoutHTTP3()), + HttpProtocolIntegrationTest::protocolTestParamsToString); + +// Request with valid credential +TEST_P(BasicAuthIntegrationTestAllProtocols, ValidCredential) { + initializeFilter(); + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto response = codec_client_->makeHeaderOnlyRequest(Http::TestRequestHeaderMapImpl{ + {":method", "GET"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}, + {"Authorization", "Basic dXNlcjE6dGVzdDE="}, // user1, test1 + }); + + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().getStatusValue()); +} + +// Request without credential +TEST_P(BasicAuthIntegrationTestAllProtocols, NoCredential) { + initializeFilter(); + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto response = codec_client_->makeHeaderOnlyRequest(Http::TestRequestHeaderMapImpl{ + {":method", "GET"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}, + }); + + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("401", response->headers().getStatusValue()); + EXPECT_EQ("User authentication failed. Missing username and password", response->body()); +} + +// Request without wrong password +TEST_P(BasicAuthIntegrationTestAllProtocols, WrongPasswrod) { + initializeFilter(); + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto response = codec_client_->makeHeaderOnlyRequest(Http::TestRequestHeaderMapImpl{ + {":method", "GET"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}, + {"Authorization", "Basic dXNlcjE6dGVzdDI="}, // user1, test2 + }); + + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("401", response->headers().getStatusValue()); + EXPECT_EQ("User authentication failed. Invalid username/password combination", response->body()); +} + +// Request with none-existed user +TEST_P(BasicAuthIntegrationTestAllProtocols, NoneExistedUser) { + initializeFilter(); + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto response = codec_client_->makeHeaderOnlyRequest(Http::TestRequestHeaderMapImpl{ + {":method", "GET"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}, + {"Authorization", "Basic dXNlcjM6dGVzdDI="}, // user3, test2 + }); + + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("401", response->headers().getStatusValue()); + EXPECT_EQ("User authentication failed. Invalid username/password combination", response->body()); +} +} // namespace +} // namespace BasicAuth +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy From fb3fd170db53a8d536274d3d5d9961c3e0ab94f1 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Sun, 29 Oct 2023 12:55:58 +0000 Subject: [PATCH 24/24] fix proto format Signed-off-by: Huabing Zhao --- .../extensions/filters/http/basic_auth/v3/basic_auth.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto b/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto index ac5050f9cf076..df23868a42602 100644 --- a/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto +++ b/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto @@ -32,5 +32,5 @@ message BasicAuth { // Username-password pairs used to verify user credentials in the "Authorization" header. // The value needs to be the htpasswd format. // Reference to https://httpd.apache.org/docs/2.4/programs/htpasswd.html - config.core.v3.DataSource users = 1 [(udpa.annotations.sensitive) = true]; + config.core.v3.DataSource users = 1 [(udpa.annotations.sensitive) = true]; }