diff --git a/.circleci/Dockerfile b/.circleci/Dockerfile index 5e2c409f79c..1b331f529fc 100644 --- a/.circleci/Dockerfile +++ b/.circleci/Dockerfile @@ -10,24 +10,24 @@ FROM circleci/openjdk:latest # clang is used for TSAN and ASAN tests RUN sudo sh -c 'curl http://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add -' -RUN sudo sh -c 'echo "deb http://apt.llvm.org/stretch/ llvm-toolchain-stretch-5.0 main" > /etc/apt/sources.list.d/llvm.list' +RUN sudo sh -c 'echo "deb http://apt.llvm.org/stretch/ llvm-toolchain-stretch-6.0 main" > /etc/apt/sources.list.d/llvm.list' RUN sudo apt-get update && \ sudo apt-get -y install \ - wget software-properties-common make cmake python python-pip \ + wget software-properties-common make cmake python python-pip pkg-config \ zlib1g-dev bash-completion bc libtool automake zip time g++-6 gcc-6 \ - clang-5.0 rsync + clang-6.0 clang-format-6.0 rsync ninja-build # ~100M, depends on g++, zlib1g-dev, bash-completions -RUN curl -Lo /tmp/bazel.deb https://github.com/bazelbuild/bazel/releases/download/0.11.0/bazel_0.11.0-linux-x86_64.deb && \ +RUN curl -Lo /tmp/bazel.deb https://github.com/bazelbuild/bazel/releases/download/0.15.2/bazel_0.15.2-linux-x86_64.deb && \ sudo dpkg -i /tmp/bazel.deb && rm /tmp/bazel.deb # Instead of "apt-get -y install golang" RUN cd /tmp && \ - wget https://redirector.gvt1.com/edgedl/go/go1.9.2.linux-amd64.tar.gz && \ + wget https://redirector.gvt1.com/edgedl/go/go1.10.3.linux-amd64.tar.gz && \ sudo rm -rf /usr/local/go && \ - sudo tar -C /usr/local -xzf go1.9.2.linux-amd64.tar.gz && \ + sudo tar -C /usr/local -xzf go1.10.3.linux-amd64.tar.gz && \ sudo chown -R circleci /usr/local/go && \ sudo ln -s /usr/local/go/bin/go /usr/local/bin diff --git a/.circleci/Makefile b/.circleci/Makefile index fcbb7ef827e..aa7ddab12a2 100644 --- a/.circleci/Makefile +++ b/.circleci/Makefile @@ -2,7 +2,7 @@ HUB ?= PROJECT ?= istio # Using same naming convention as istio/istio -VERSION ?= go1.9-bazel0.11 +VERSION ?= go1.10-bazel0.15-clang6.0 IMG ?= ci # Build a local image, can be used for testing with circleci command line. diff --git a/.circleci/config.yml b/.circleci/config.yml index c256a3b5bfe..db59eae9322 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -3,7 +3,7 @@ version: 2 jobs: build: docker: - - image: istio/ci:go1.9-bazel0.11 + - image: istio/ci:go1.10-bazel0.15-clang6.0 environment: - BAZEL_TEST_ARGS: "--test_env=ENVOY_IP_TEST_VERSIONS=v4only --test_output=all" resource_class: xlarge @@ -11,54 +11,89 @@ jobs: - checkout - restore_cache: keys: - - bazel-cache-{{ checksum "WORKSPACE" }} - - restore_cache: - keys: - - repo-cache-{{ checksum "WORKSPACE" }} + - linux_fastbuild-bazel-cache-{{ checksum "WORKSPACE" }} # To build docker containers or run tests in a docker - setup_remote_docker - run: rm ~/.gitconfig - run: make check - - run: make deb BAZEL_BUILD_ARGS="-j 4" + - run: make deb - run: make test - - run: make test_asan - - run: make test_tsan - - save_cache: - key: repo-cache-{{ checksum "WORKSPACE" }} - paths: - - /home/circleci/.repo - save_cache: - key: bazel-cache-{{ checksum "WORKSPACE" }} + key: linux_fastbuild-bazel-cache-{{ checksum "WORKSPACE" }} paths: - /home/circleci/.cache/bazel - store_artifacts: path: /home/circleci/project/bazel-bin/tools/deb/istio-proxy.deb destination: /proxy/deb - store_artifacts: - path: /home/circleci/project/bazel-bin/src/envoy/mixer/envoy + path: /home/circleci/project/bazel-bin/src/envoy/envoy destination: /proxy/bin + linux_asan: + docker: + - image: istio/ci:go1.10-bazel0.15-clang6.0 + environment: + - BAZEL_TEST_ARGS: "--test_env=ENVOY_IP_TEST_VERSIONS=v4only --test_output=all" + resource_class: xlarge + steps: + - checkout + - restore_cache: + keys: + - linux_asan-bazel-cache-{{ checksum "WORKSPACE" }} + # To build docker containers or run tests in a docker + - setup_remote_docker + - run: rm ~/.gitconfig + - run: make test_asan + - save_cache: + key: linux_asan-bazel-cache-{{ checksum "WORKSPACE" }} + paths: + - /home/circleci/.cache/bazel + linux_tsan: + docker: + - image: istio/ci:go1.10-bazel0.15-clang6.0 + environment: + - BAZEL_TEST_ARGS: "--test_env=ENVOY_IP_TEST_VERSIONS=v4only --test_output=all" + resource_class: xlarge + steps: + - checkout + - restore_cache: + keys: + - linux_tsan-bazel-cache-{{ checksum "WORKSPACE" }} + # To build docker containers or run tests in a docker + - setup_remote_docker + - run: rm ~/.gitconfig + - run: make test_tsan + - save_cache: + key: linux_tsan-bazel-cache-{{ checksum "WORKSPACE" }} + paths: + - /home/circleci/.cache/bazel macos: macos: xcode: "9.3.0" environment: + - BAZEL_STARTUP_ARGS: "--output_base /Users/distiller/.cache/bazel" - BAZEL_TEST_ARGS: "--test_env=ENVOY_IP_TEST_VERSIONS=v4only --test_output=all" + - CC: clang + - CXX: clang++ steps: - run: sudo ntpdate -vu time.apple.com - - run: brew install automake bazel cmake coreutils go libtool wget + - run: brew install bazel cmake coreutils go libtool ninja wget - checkout - restore_cache: keys: - - bazel-cache-{{ checksum "WORKSPACE" }}-macos + - macos_fastbuild_v2-bazel-cache-{{ checksum "WORKSPACE" }} - run: rm ~/.gitconfig + - run: make build_envoy - run: make test - save_cache: - key: bazel-cache-{{ checksum "WORKSPACE" }}-macos + key: macos_fastbuild_v2-bazel-cache-{{ checksum "WORKSPACE" }} paths: - - /home/circleci/.cache/bazel + - /Users/distiller/.cache/bazel workflows: version: 2 all: jobs: - build + - linux_asan + - linux_tsan - macos diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c38680b0241..17cc87489fb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,5 +1,5 @@ # Contribution guidelines So, you want to hack on the Istio proxy? Yay! Please refer to Istio's overall -[contribution guidelines](https://github.com/istio/istio/blob/master/DEV-GUIDE.md) +[contribution guidelines](https://github.com/istio/community/blob/master/CONTRIBUTING.md) to find out how you can help. diff --git a/Makefile b/Makefile index 0c0216b171e..501d2ebb14f 100644 --- a/Makefile +++ b/Makefile @@ -17,30 +17,42 @@ TOP := $(shell dirname $(realpath $(lastword $(MAKEFILE_LIST)))) SHELL := /bin/bash LOCAL_ARTIFACTS_DIR ?= $(abspath artifacts) ARTIFACTS_DIR ?= $(LOCAL_ARTIFACTS_DIR) -BAZEL_STARTUP_ARGS ?= --batch +BAZEL_STARTUP_ARGS ?= BAZEL_BUILD_ARGS ?= BAZEL_TEST_ARGS ?= HUB ?= TAG ?= +ifeq "$(origin CC)" "default" +CC := clang-6.0 +endif +ifeq "$(origin CXX)" "default" +CXX := clang++-6.0 +endif build: - @bazel $(BAZEL_STARTUP_ARGS) build $(BAZEL_BUILD_ARGS) //... + CC=$(CC) CXX=$(CXX) bazel $(BAZEL_STARTUP_ARGS) build $(BAZEL_BUILD_ARGS) //... + @bazel shutdown # Build only envoy - fast build_envoy: - bazel $(BAZEL_STARTUP_ARGS) build $(BAZEL_BUILD_ARGS) //src/envoy/mixer:envoy + CC=$(CC) CXX=$(CXX) bazel $(BAZEL_STARTUP_ARGS) build $(BAZEL_BUILD_ARGS) //src/envoy:envoy + @bazel shutdown clean: @bazel clean + @bazel shutdown test: - bazel $(BAZEL_STARTUP_ARGS) test $(BAZEL_TEST_ARGS) //... + CC=$(CC) CXX=$(CXX) bazel $(BAZEL_STARTUP_ARGS) test $(BAZEL_TEST_ARGS) //... + @bazel shutdown test_asan: - CC=clang-5.0 CXX=clang++-5.0 bazel $(BAZEL_STARTUP_ARGS) test $(BAZEL_TEST_ARGS) --config=clang-asan //... + CC=$(CC) CXX=$(CXX) bazel $(BAZEL_STARTUP_ARGS) test $(BAZEL_TEST_ARGS) --config=clang-asan //... + @bazel shutdown test_tsan: - CC=clang-5.0 CXX=clang++-5.0 bazel $(BAZEL_STARTUP_ARGS) test $(BAZEL_TEST_ARGS) --config=clang-tsan //... + CC=$(CC) CXX=$(CXX) bazel $(BAZEL_STARTUP_ARGS) test $(BAZEL_TEST_ARGS) --config=clang-tsan //... + @bazel shutdown check: @script/check-license-headers @@ -50,7 +62,8 @@ artifacts: build @script/push-debian.sh -c opt -p $(ARTIFACTS_DIR) deb: - @bazel build tools/deb:istio-proxy ${BAZEL_BUILD_ARGS} + CC=$(CC) CXX=$(CXX) bazel $(BAZEL_STARTUP_ARGS) build $(BAZEL_BUILD_ARGS) //tools/deb:istio-proxy + @bazel shutdown .PHONY: build clean test check artifacts diff --git a/WORKSPACE b/WORKSPACE index 03e70ac0e9c..31a0f42481d 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -30,12 +30,12 @@ bind( ) # When updating envoy sha manually please update the sha in istio.deps file also -ENVOY_SHA = "93d96b52b47ac2f296379c15b03946cb33c58252" +ENVOY_SHA = "aa8053b1e8f7d02e1bb0c2a44e78688643248ad1" http_archive( name = "envoy", strip_prefix = "envoy-" + ENVOY_SHA, - url = "https://github.com/envoyproxy/envoy/archive/" + ENVOY_SHA + ".zip", + url = "https://github.com/istio/envoy/archive/" + ENVOY_SHA + ".zip", ) load("@envoy//bazel:repositories.bzl", "envoy_dependencies") diff --git a/include/istio/control/http/check_data.h b/include/istio/control/http/check_data.h index fc8d204aa88..a05211888c1 100644 --- a/include/istio/control/http/check_data.h +++ b/include/istio/control/http/check_data.h @@ -19,7 +19,7 @@ #include #include -#include "src/istio/authn/context.pb.h" +#include "google/protobuf/struct.pb.h" namespace istio { namespace control { @@ -38,8 +38,8 @@ class CheckData { // Get downstream tcp connection ip and port. virtual bool GetSourceIpPort(std::string *ip, int *port) const = 0; - // If SSL is used, get origin user name. - virtual bool GetSourceUser(std::string *user) const = 0; + // If SSL is used, get peer or local certificate SAN URI. + virtual bool GetPrincipal(bool peer, std::string *user) const = 0; // Get request HTTP headers virtual std::map GetRequestHeaders() const = 0; @@ -47,6 +47,9 @@ class CheckData { // Returns true if connection is mutual TLS enabled. virtual bool IsMutualTLS() const = 0; + // Get requested server name, SNI in case of TLS + virtual bool GetRequestedServerName(std::string *name) const = 0; + // These headers are extracted into top level attributes. // This is for standard HTTP headers. It supports both HTTP/1.1 and HTTP2 // They can be retrieved at O(1) speed by environment (Envoy). @@ -78,14 +81,18 @@ class CheckData { virtual bool FindCookie(const std::string &name, std::string *value) const = 0; - // If the request has a JWT token and it is verified, get its payload as - // string map, and return true. Otherwise return false. - virtual bool GetJWTPayload( - std::map *payload) const = 0; + // Returns a pointer to the authentication result from request info dynamic + // metadata, if available. Otherwise, returns nullptr. + virtual const ::google::protobuf::Struct *GetAuthenticationResult() const = 0; + + // Get request url path, which strips query part from the http path header. + // Return true if url path is found, otherwise return false. + virtual bool GetUrlPath(std::string *url_path) const = 0; - // If the request has authentication result in header, parses data into the - // output result; returns true if success. Otherwise, returns false. - virtual bool GetAuthenticationResult(istio::authn::Result *result) const = 0; + // Get request queries with string map format. Return true if query params are + // found, otherwise return false. + virtual bool GetRequestQueryParams( + std::map *query_params) const = 0; }; // An interfact to update request HTTP headers with Istio attributes. diff --git a/include/istio/control/http/report_data.h b/include/istio/control/http/report_data.h index 3876f336ac3..11b1e7e9f59 100644 --- a/include/istio/control/http/report_data.h +++ b/include/istio/control/http/report_data.h @@ -40,12 +40,20 @@ class ReportData { uint64_t response_body_size; std::chrono::nanoseconds duration; int response_code; + std::string response_flags; }; virtual void GetReportInfo(ReportInfo* info) const = 0; // Get destination ip/port. virtual bool GetDestinationIpPort(std::string* ip, int* port) const = 0; + // Get Rbac attributes. + struct RbacReportInfo { + std::string permissive_resp_code; + std::string permissive_policy_id; + }; + virtual bool GetRbacReportInfo(RbacReportInfo* report_info) const = 0; + // Get upstream host UID. This value overrides the value in the report bag. virtual bool GetDestinationUID(std::string* uid) const = 0; diff --git a/include/istio/control/tcp/check_data.h b/include/istio/control/tcp/check_data.h index 991b6f8d421..73cc2ab14de 100644 --- a/include/istio/control/tcp/check_data.h +++ b/include/istio/control/tcp/check_data.h @@ -31,12 +31,15 @@ class CheckData { // Get downstream tcp connection ip and port. virtual bool GetSourceIpPort(std::string* ip, int* port) const = 0; - // If SSL is used, get origin user name. - virtual bool GetSourceUser(std::string* user) const = 0; + // If SSL is used, get peer or local certificate SAN URI. + virtual bool GetPrincipal(bool peer, std::string* user) const = 0; // Returns true if connection is mutual TLS enabled. virtual bool IsMutualTLS() const = 0; + // Get requested server name, SNI in case of TLS + virtual bool GetRequestedServerName(std::string* name) const = 0; + // Get downstream tcp connection id. virtual std::string GetConnectionId() const = 0; }; diff --git a/include/istio/control/tcp/report_data.h b/include/istio/control/tcp/report_data.h index f4f7d3a3b8a..7535b5904ba 100644 --- a/include/istio/control/tcp/report_data.h +++ b/include/istio/control/tcp/report_data.h @@ -42,6 +42,14 @@ class ReportData { // Get upstream host UID. This value overrides the value in the report bag. virtual bool GetDestinationUID(std::string* uid) const = 0; + + // ConnectionEvent is used to indicates the tcp connection event in Report + // call. + enum ConnectionEvent { + OPEN = 0, + CLOSE, + CONTINUE, + }; }; } // namespace tcp diff --git a/include/istio/control/tcp/request_handler.h b/include/istio/control/tcp/request_handler.h index 493af0d940d..2fb0bedf60a 100644 --- a/include/istio/control/tcp/request_handler.h +++ b/include/istio/control/tcp/request_handler.h @@ -36,16 +36,8 @@ class RequestHandler { CheckData* check_data, ::istio::mixerclient::CheckDoneFunc on_done) = 0; // Make report call. - // This can be called multiple times for long connection. - // TODO(JimmyCYJ): Let TCP filter use - // void Report(ReportData* report_data, bool is_final_report), and deprecate - // this method. - virtual void Report(ReportData* report_data) = 0; - - // Make report call. - // If is_final_report is true, report all attributes. Otherwise, report delta - // attributes. - virtual void Report(ReportData* report_data, bool is_final_report) = 0; + virtual void Report(ReportData* report_data, + ReportData::ConnectionEvent event) = 0; }; } // namespace tcp diff --git a/include/istio/utils/BUILD b/include/istio/utils/BUILD index f343c5a51e7..92bb7e084bd 100644 --- a/include/istio/utils/BUILD +++ b/include/istio/utils/BUILD @@ -41,4 +41,4 @@ cc_library( "attribute_names.h", ], visibility = ["//visibility:public"], -) +) \ No newline at end of file diff --git a/include/istio/utils/attribute_names.h b/include/istio/utils/attribute_names.h index d7c00fd3bb2..84e7415837d 100644 --- a/include/istio/utils/attribute_names.h +++ b/include/istio/utils/attribute_names.h @@ -27,6 +27,7 @@ struct AttributeName { // https://github.com/istio/istio/issues/4689 static const char kSourceUser[]; static const char kSourcePrincipal[]; + static const char kDestinationPrincipal[]; static const char kRequestHeaders[]; static const char kRequestHost[]; @@ -34,6 +35,8 @@ struct AttributeName { static const char kRequestPath[]; static const char kRequestReferer[]; static const char kRequestScheme[]; + static const char kRequestUrlPath[]; + static const char kRequestQueryParams[]; static const char kRequestBodySize[]; // Total size of request received, including request headers, body, and // trailers. @@ -59,12 +62,14 @@ struct AttributeName { static const char kDestinationIp[]; static const char kDestinationPort[]; static const char kDestinationUID[]; + static const char kOriginIp[]; static const char kConnectionReceviedBytes[]; static const char kConnectionReceviedTotalBytes[]; static const char kConnectionSendBytes[]; static const char kConnectionSendTotalBytes[]; static const char kConnectionDuration[]; static const char kConnectionMtls[]; + static const char kConnectionRequestedServerName[]; static const char kConnectionId[]; // Record TCP connection status: open, continue, close static const char kConnectionEvent[]; @@ -72,6 +77,7 @@ struct AttributeName { // Context attributes static const char kContextProtocol[]; static const char kContextTime[]; + static const char kContextProxyErrorCode[]; // Check error code and message. static const char kCheckErrorCode[]; @@ -84,12 +90,16 @@ struct AttributeName { // Authentication attributes static const char kRequestAuthPrincipal[]; static const char kRequestAuthAudiences[]; + static const char kRequestAuthGroups[]; static const char kRequestAuthPresenter[]; static const char kRequestAuthClaims[]; static const char kRequestAuthRawClaims[]; static const char kResponseGrpcStatus[]; static const char kResponseGrpcMessage[]; + + static const char kRbacPermissiveResponseCode[]; + static const char kRbacPermissivePolicyId[]; }; } // namespace utils diff --git a/include/istio/utils/attributes_builder.h b/include/istio/utils/attributes_builder.h index da124977414..eae9836f4d0 100644 --- a/include/istio/utils/attributes_builder.h +++ b/include/istio/utils/attributes_builder.h @@ -20,7 +20,7 @@ #include #include -#include "google/protobuf/map.h" +#include "google/protobuf/struct.pb.h" #include "mixer/v1/attributes.pb.h" namespace istio { @@ -89,18 +89,24 @@ class AttributesBuilder { } } - void AddProtobufStringMap( - const std::string& key, - const google::protobuf::Map& string_map) { - if (string_map.empty()) { + void AddProtoStructStringMap(const std::string& key, + const google::protobuf::Struct& struct_map) { + if (struct_map.fields().empty()) { return; } auto entries = (*attributes_->mutable_attributes())[key] .mutable_string_map_value() ->mutable_entries(); entries->clear(); - for (const auto& map_it : string_map) { - (*entries)[map_it.first] = map_it.second; + for (const auto& field : struct_map.fields()) { + // Ignore all fields that are not string. + switch (field.second.kind_case()) { + case google::protobuf::Value::kStringValue: + (*entries)[field.first] = field.second.string_value(); + break; + default: + break; + } } } diff --git a/istio.deps b/istio.deps index 29f5ed5b453..e4d5ea71c25 100644 --- a/istio.deps +++ b/istio.deps @@ -4,13 +4,13 @@ "name": "ISTIO_API", "repoName": "api", "file": "repositories.bzl", - "lastStableSHA": "2d2fd0bdcf2ce26f5d96615e05974913b3d260c5", + "lastStableSHA": "214c7598afb74f7f4dea49f77e45832c49382a15" }, { "_comment": "", "name": "ENVOY_SHA", "repoName": "envoyproxy/envoy", "file": "WORKSPACE", - "lastStableSHA": "93d96b52b47ac2f296379c15b03946cb33c58252" + "lastStableSHA": "73bd3d95cd0b6a23de0b6357f1b3065b9014651a" } ] diff --git a/repositories.bzl b/repositories.bzl index 8a32737c803..719413dab4a 100644 --- a/repositories.bzl +++ b/repositories.bzl @@ -113,7 +113,7 @@ cc_library( actual = "@googletest_git//:googletest_prod", ) -ISTIO_API = "2d8fbdb9495e737b10f6ac5f64e344517b0e7b10" +ISTIO_API = "214c7598afb74f7f4dea49f77e45832c49382a15" def mixerapi_repositories(bind=True): BUILD = """ @@ -168,6 +168,7 @@ cc_proto_library( srcs = glob( ["envoy/config/filter/http/authn/v2alpha1/*.proto", "authentication/v1alpha1/*.proto", + "common/v1alpha1/*.proto", ], ), default_runtime = "//external:protobuf", diff --git a/script/check-style b/script/check-style index 53cada67c79..d9cf3fce522 100755 --- a/script/check-style +++ b/script/check-style @@ -18,7 +18,7 @@ # ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -CLANG_VERSION_REQUIRED="5.0.0" +CLANG_VERSION_REQUIRED="6.0.0" CLANG_FORMAT=$(which clang-format-${CLANG_VERSION_REQUIRED%.*}) if [[ ! -x "${CLANG_FORMAT}" ]]; then # Install required clang version to a folder and cache it. @@ -28,7 +28,7 @@ if [[ ! -x "${CLANG_FORMAT}" ]]; then if [ "$(uname)" == "Darwin" ]; then CLANG_BIN="x86_64-apple-darwin.tar.xz" elif [ "$(expr substr $(uname -s) 1 5)" == "Linux" ]; then - CLANG_BIN="linux-x86_64-ubuntu14.04.tar.xz" + CLANG_BIN="x86_64-linux-gnu-ubuntu-14.04.tar.xz" else echo "Unsupported environment." ; exit 1 ; fi diff --git a/script/pre-commit b/script/pre-commit index e0a5db52927..a87c08396c7 100755 --- a/script/pre-commit +++ b/script/pre-commit @@ -38,7 +38,7 @@ if [[ ! -x "${CLANG_FORMAT}" ]]; then fi CLANG_FORMAT_VERSION="$(${CLANG_FORMAT} -version | cut -d ' ' -f 3)" -CLANG_FORMAT_VERSION_REQUIRED="5.0.0" +CLANG_FORMAT_VERSION_REQUIRED="6.0.0" if ! [[ "${CLANG_FORMAT_VERSION}" =~ "${CLANG_FORMAT_VERSION_REQUIRED}" ]]; then echo "Skipping: clang-format ${CLANG_FORMAT_VERSION_REQUIRED} required." exit 0 diff --git a/script/release-binary b/script/release-binary index cf5dc5385ab..ebde3fc43c3 100755 --- a/script/release-binary +++ b/script/release-binary @@ -17,6 +17,11 @@ ################################################################################ # set -ex + +# Use clang for the release builds. +CC=${CC:-clang-6.0} +CXX=${CXX:-clang++-6.0} + # The bucket name to store proxy binary DST="gs://istio-build/proxy" @@ -54,7 +59,7 @@ gsutil stat "${DST}/${BINARY_NAME}" \ || echo 'Building a new binary.' # Build the release binary with symbol -bazel --batch build --config=release-symbol //src/envoy:envoy_tar +CC=${CC} CXX=${CXX} bazel build --config=release-symbol //src/envoy:envoy_tar BAZEL_TARGET="bazel-bin/src/envoy/envoy_tar.tar.gz" cp -f "${BAZEL_TARGET}" "${BINARY_NAME}" sha256sum "${BINARY_NAME}" > "${SHA256_NAME}" @@ -67,7 +72,7 @@ BINARY_NAME="envoy-alpha-${SHA}.tar.gz" SHA256_NAME="envoy-alpha-${SHA}.sha256" # Build the release binary -bazel --batch build --config=release //src/envoy:envoy_tar +CC=${CC} CXX=${CXX} bazel build --config=release //src/envoy:envoy_tar BAZEL_TARGET="bazel-bin/src/envoy/envoy_tar.tar.gz" cp -f "${BAZEL_TARGET}" "${BINARY_NAME}" sha256sum "${BINARY_NAME}" > "${SHA256_NAME}" @@ -78,7 +83,7 @@ gsutil cp "${BINARY_NAME}" "${SHA256_NAME}" "${DST}/" BINARY_NAME="istio-proxy-${SHA}.deb" SHA256_NAME="istio-proxy-${SHA}.sha256" -bazel --batch build --config=release //tools/deb:istio-proxy +CC=${CC} CXX=${CXX} bazel build --config=release //tools/deb:istio-proxy BAZEL_TARGET="bazel-bin/tools/deb/istio-proxy.deb" cp -f "${BAZEL_TARGET}" "${BINARY_NAME}" sha256sum "${BINARY_NAME}" > "${SHA256_NAME}" @@ -91,7 +96,7 @@ gsutil cp "${BINARY_NAME}" "${SHA256_NAME}" "${DST}/" # Build the debug binary BINARY_NAME="envoy-debug-${SHA}.tar.gz" SHA256_NAME="envoy-debug-${SHA}.sha256" -bazel --batch build -c dbg //src/envoy:envoy_tar +CC=${CC} CXX=${CXX} bazel build -c dbg //src/envoy:envoy_tar BAZEL_TARGET="bazel-bin/src/envoy/envoy_tar.tar.gz" cp -f "${BAZEL_TARGET}" "${BINARY_NAME}" sha256sum "${BINARY_NAME}" > "${SHA256_NAME}" @@ -102,7 +107,7 @@ gsutil cp "${BINARY_NAME}" "${SHA256_NAME}" "${DST}/" BINARY_NAME="istio-proxy-debug-${SHA}.deb" SHA256_NAME="istio-proxy-debug-${SHA}.sha256" -bazel --batch build -c dbg //tools/deb:istio-proxy +CC=${CC} CXX=${CXX} bazel build -c dbg //tools/deb:istio-proxy BAZEL_TARGET="bazel-bin/tools/deb/istio-proxy.deb" cp -f "${BAZEL_TARGET}" "${BINARY_NAME}" sha256sum "${BINARY_NAME}" > "${SHA256_NAME}" @@ -110,3 +115,5 @@ sha256sum "${BINARY_NAME}" > "${SHA256_NAME}" # Copy it to the bucket. echo "Copying ${BINARY_NAME} ${SHA256_NAME} to ${DST}/" gsutil cp "${BINARY_NAME}" "${SHA256_NAME}" "${DST}/" + +bazel shutdown diff --git a/src/envoy/alts/tsi_transport_socket.h b/src/envoy/alts/tsi_transport_socket.h index cd26b60d3b3..629cb6f9e5b 100644 --- a/src/envoy/alts/tsi_transport_socket.h +++ b/src/envoy/alts/tsi_transport_socket.h @@ -57,7 +57,6 @@ class TsiSocket : public Network::TransportSocket, Envoy::Network::TransportSocketCallbacks& callbacks) override; std::string protocol() const override; bool canFlushClose() override { return handshake_complete_; } - Envoy::Ssl::Connection* ssl() override { return nullptr; } const Envoy::Ssl::Connection* ssl() const override { return nullptr; } Network::IoResult doWrite(Buffer::Instance& buffer, bool end_stream) override; void closeSocket(Network::ConnectionEvent event) override; diff --git a/src/envoy/http/authn/BUILD b/src/envoy/http/authn/BUILD index 6c9efa35324..3f4c3f296cd 100644 --- a/src/envoy/http/authn/BUILD +++ b/src/envoy/http/authn/BUILD @@ -46,6 +46,7 @@ envoy_cc_library( "//src/envoy/http/jwt_auth:jwt_lib", "//src/envoy/utils:utils_lib", "//src/istio/authn:context_proto", + "//src/envoy/utils:filter_names_lib", ], ) @@ -66,6 +67,7 @@ envoy_cc_library( "//src/envoy/utils:utils_lib", "//src/istio/authn:context_proto", "@envoy//source/exe:envoy_common_lib", + "//src/envoy/utils:filter_names_lib", ], ) @@ -85,6 +87,7 @@ envoy_cc_test( deps = [ ":authenticator", ":test_utils", + "@envoy//test/mocks/http:http_mocks", "@envoy//test/test_common:utility_lib", ], ) @@ -97,6 +100,7 @@ envoy_cc_test( ":authenticator", ":test_utils", "@envoy//test/mocks/network:network_mocks", + "//src/envoy/utils:filter_names_lib", "@envoy//test/mocks/ssl:ssl_mocks", "@envoy//test/test_common:utility_lib", ], @@ -158,6 +162,8 @@ envoy_cc_test( repository = "@envoy", deps = [ ":filter_lib", - "@envoy//test/integration:http_integration_lib", + "@envoy//source/common/common:utility_lib", + "@envoy//test/integration:http_protocol_integration_lib", + "//src/envoy/utils:filter_names_lib", ], ) diff --git a/src/envoy/http/authn/authenticator_base.cc b/src/envoy/http/authn/authenticator_base.cc index c392acbaa5d..9262d249e25 100644 --- a/src/envoy/http/authn/authenticator_base.cc +++ b/src/envoy/http/authn/authenticator_base.cc @@ -15,7 +15,9 @@ #include "src/envoy/http/authn/authenticator_base.h" #include "common/common/assert.h" +#include "common/config/metadata.h" #include "src/envoy/http/authn/authn_utils.h" +#include "src/envoy/utils/filter_names.h" #include "src/envoy/utils/utils.h" using istio::authn::Payload; @@ -43,7 +45,8 @@ bool AuthenticatorBase::validateX509(const iaapi::MutualTls& mtls, const bool has_user = connection->ssl() != nullptr && connection->ssl()->peerCertificatePresented() && - Utils::GetSourceUser(connection, payload->mutable_x509()->mutable_user()); + Utils::GetPrincipal(connection, true, + payload->mutable_x509()->mutable_user()); ENVOY_LOG(debug, "validateX509 mode {}: ssl={}, has_user={}", iaapi::MutualTls::Mode_Name(mtls.mode()), @@ -58,26 +61,16 @@ bool AuthenticatorBase::validateX509(const iaapi::MutualTls& mtls, case iaapi::MutualTls::STRICT: return has_user; default: - NOT_REACHED; + NOT_REACHED_GCOVR_EXCL_LINE; } } bool AuthenticatorBase::validateJwt(const iaapi::Jwt& jwt, Payload* payload) { - Envoy::Http::HeaderMap& header = *filter_context()->headers(); - - auto iter = - filter_context()->filter_config().jwt_output_payload_locations().find( - jwt.issuer()); - if (iter == - filter_context()->filter_config().jwt_output_payload_locations().end()) { - ENVOY_LOG(warn, "No JWT payload header location is found for the issuer {}", - jwt.issuer()); - return false; + std::string jwt_payload; + if (filter_context()->getJwtPayload(jwt.issuer(), &jwt_payload)) { + return AuthnUtils::ProcessJwtPayload(jwt_payload, payload->mutable_jwt()); } - - LowerCaseString header_key(iter->second); - return AuthnUtils::GetJWTPayloadFromHeaders(header, header_key, - payload->mutable_jwt()); + return false; } } // namespace AuthN diff --git a/src/envoy/http/authn/authenticator_base_test.cc b/src/envoy/http/authn/authenticator_base_test.cc index 454e40e19c4..068e5db1edb 100644 --- a/src/envoy/http/authn/authenticator_base_test.cc +++ b/src/envoy/http/authn/authenticator_base_test.cc @@ -16,9 +16,11 @@ #include "src/envoy/http/authn/authenticator_base.h" #include "common/common/base64.h" #include "common/protobuf/protobuf.h" +#include "envoy/api/v2/core/base.pb.h" #include "envoy/config/filter/http/authn/v2alpha1/config.pb.h" #include "gmock/gmock.h" #include "src/envoy/http/authn/test_utils.h" +#include "src/envoy/utils/filter_names.h" #include "test/mocks/network/mocks.h" #include "test/mocks/ssl/mocks.h" @@ -27,6 +29,7 @@ using istio::authn::Payload; using istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig; using testing::NiceMock; using testing::Return; +using testing::StrictMock; namespace iaapi = istio::authentication::v1alpha1; @@ -36,7 +39,6 @@ namespace Istio { namespace AuthN { namespace { -const std::string kSecIstioAuthUserInfoHeaderKey = "sec-istio-auth-userinfo"; const std::string kSecIstioAuthUserinfoHeaderValue = R"( { @@ -60,13 +62,13 @@ class ValidateX509Test : public testing::TestWithParam, public: virtual ~ValidateX509Test() {} - Http::TestHeaderMapImpl request_headers_{}; NiceMock connection_{}; NiceMock ssl_{}; FilterConfig filter_config_{}; - FilterContext filter_context_{&request_headers_, &connection_, - istio::envoy::config::filter::http::authn:: - v2alpha1::FilterConfig::default_instance()}; + FilterContext filter_context_{ + envoy::api::v2::core::Metadata::default_instance(), &connection_, + istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig:: + default_instance()}; MockAuthenticatorBase authenticator_{&filter_context_}; @@ -162,30 +164,19 @@ class ValidateJwtTest : public testing::Test, public: virtual ~ValidateJwtTest() {} - Http::TestHeaderMapImpl request_headers_{}; + // StrictMock request_info_{}; + envoy::api::v2::core::Metadata dynamic_metadata_; NiceMock connection_{}; - NiceMock ssl_{}; + // NiceMock ssl_{}; FilterConfig filter_config_{}; - FilterContext filter_context_{&request_headers_, &connection_, - istio::envoy::config::filter::http::authn:: - v2alpha1::FilterConfig::default_instance()}; - + FilterContext filter_context_{dynamic_metadata_, &connection_, + filter_config_}; MockAuthenticatorBase authenticator_{&filter_context_}; void SetUp() override { payload_ = new Payload(); } void TearDown() override { delete (payload_); } - Http::TestHeaderMapImpl CreateTestHeaderMap(const std::string& header_key, - const std::string& header_value) { - // The base64 encoding is done through Base64::encode(). - // If the test input has special chars, may need to use the counterpart of - // Base64UrlDecode(). - std::string value_base64 = - Base64::encode(header_value.c_str(), header_value.size()); - return Http::TestHeaderMapImpl{{header_key, value_base64}}; - } - protected: iaapi::MutualTls mtls_params_; iaapi::Jwt jwt_; @@ -193,8 +184,7 @@ class ValidateJwtTest : public testing::Test, Payload default_payload_; }; -// TODO: more tests for Jwt. -TEST_F(ValidateJwtTest, ValidateJwtWithNoIstioAuthnConfig) { +TEST_F(ValidateJwtTest, NoIstioAuthnConfig) { jwt_.set_issuer("issuer@foo.com"); // authenticator_ has empty Istio authn config // When there is empty Istio authn config, validateJwt() should return @@ -206,7 +196,6 @@ TEST_F(ValidateJwtTest, ValidateJwtWithNoIstioAuthnConfig) { TEST_F(ValidateJwtTest, NoIssuer) { // no issuer in jwt google::protobuf::util::JsonParseOptions options; - FilterConfig filter_config; JsonStringToMessage( R"({ "jwt_output_payload_locations": @@ -215,11 +204,7 @@ TEST_F(ValidateJwtTest, NoIssuer) { } } )", - &filter_config, options); - Http::TestHeaderMapImpl empty_request_headers{}; - FilterContext filter_context{&empty_request_headers, &connection_, - filter_config}; - MockAuthenticatorBase authenticator{&filter_context}; + &filter_config_, options); // When there is no issuer in the JWT config, validateJwt() should return // nullptr and failure. @@ -227,12 +212,9 @@ TEST_F(ValidateJwtTest, NoIssuer) { EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_)); } -TEST_F(ValidateJwtTest, EmptyJwtOutputPayloadLocations) { +TEST_F(ValidateJwtTest, OutputPayloadLocationNotDefine) { jwt_.set_issuer("issuer@foo.com"); - Http::TestHeaderMapImpl request_headers_with_jwt = CreateTestHeaderMap( - kSecIstioAuthUserInfoHeaderKey, kSecIstioAuthUserinfoHeaderValue); google::protobuf::util::JsonParseOptions options; - FilterConfig filter_config; JsonStringToMessage( R"({ "jwt_output_payload_locations": @@ -240,10 +222,8 @@ TEST_F(ValidateJwtTest, EmptyJwtOutputPayloadLocations) { } } )", - &filter_config, options); - FilterContext filter_context{&request_headers_with_jwt, &connection_, - filter_config}; - MockAuthenticatorBase authenticator{&filter_context}; + &filter_config_, options); + // authenticator has empty jwt_output_payload_locations in Istio authn config // When there is no matching jwt_output_payload_locations for the issuer in // the Istio authn config, validateJwt() should return nullptr and failure. @@ -251,47 +231,43 @@ TEST_F(ValidateJwtTest, EmptyJwtOutputPayloadLocations) { EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_)); } -TEST_F(ValidateJwtTest, NoJwtInHeader) { +TEST_F(ValidateJwtTest, NoJwtPayloadOutput) { jwt_.set_issuer("issuer@foo.com"); - google::protobuf::util::JsonParseOptions options; - FilterConfig filter_config; - JsonStringToMessage( - R"({ - "jwt_output_payload_locations": - { - "issuer@foo.com": "sec-istio-auth-jwt-output" - } - } - )", - &filter_config, options); - Http::TestHeaderMapImpl empty_request_headers{}; - FilterContext filter_context{&empty_request_headers, &connection_, - filter_config}; - MockAuthenticatorBase authenticator{&filter_context}; - // When there is no JWT in the HTTP header, validateJwt() should return - // nullptr and failure. + + // When there is no JWT in request info dynamic metadata, validateJwt() should + // return nullptr and failure. EXPECT_FALSE(authenticator_.validateJwt(jwt_, payload_)); EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_)); } -TEST_F(ValidateJwtTest, JwtInHeader) { +TEST_F(ValidateJwtTest, HasJwtPayloadOutputButNoDataForKey) { jwt_.set_issuer("issuer@foo.com"); - Http::TestHeaderMapImpl request_headers_with_jwt = CreateTestHeaderMap( - "sec-istio-auth-jwt-output", kSecIstioAuthUserinfoHeaderValue); - google::protobuf::util::JsonParseOptions options; - FilterConfig filter_config; - JsonStringToMessage( - R"({ - "jwt_output_payload_locations": - { - "issuer@foo.com": "sec-istio-auth-jwt-output" - } - } - )", - &filter_config, options); - FilterContext filter_context{&request_headers_with_jwt, &connection_, - filter_config}; - MockAuthenticatorBase authenticator{&filter_context}; + + (*dynamic_metadata_.mutable_filter_metadata())[Utils::IstioFilterName::kJwt] + .MergeFrom(MessageUtil::keyValueStruct("foo", "bar")); + + // When there is no JWT payload for given issuer in request info dynamic + // metadata, validateJwt() should return nullptr and failure. + EXPECT_FALSE(authenticator_.validateJwt(jwt_, payload_)); + EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_)); +} + +TEST_F(ValidateJwtTest, JwtPayloadAvailableWithBadData) { + jwt_.set_issuer("issuer@foo.com"); + (*dynamic_metadata_.mutable_filter_metadata())[Utils::IstioFilterName::kJwt] + .MergeFrom(MessageUtil::keyValueStruct("issuer@foo.com", "bad-data")); + // EXPECT_CALL(request_info_, dynamicMetadata()); + + EXPECT_FALSE(authenticator_.validateJwt(jwt_, payload_)); + EXPECT_TRUE(MessageDifferencer::Equivalent(*payload_, default_payload_)); +} + +TEST_F(ValidateJwtTest, JwtPayloadAvailable) { + jwt_.set_issuer("issuer@foo.com"); + (*dynamic_metadata_.mutable_filter_metadata())[Utils::IstioFilterName::kJwt] + .MergeFrom(MessageUtil::keyValueStruct("issuer@foo.com", + kSecIstioAuthUserinfoHeaderValue)); + Payload expected_payload; JsonStringToMessage( R"({ @@ -309,9 +285,9 @@ TEST_F(ValidateJwtTest, JwtInHeader) { } } )", - &expected_payload, options); + &expected_payload, google::protobuf::util::JsonParseOptions{}); - EXPECT_TRUE(authenticator.validateJwt(jwt_, payload_)); + EXPECT_TRUE(authenticator_.validateJwt(jwt_, payload_)); EXPECT_TRUE(MessageDifferencer::Equals(expected_payload, *payload_)); } diff --git a/src/envoy/http/authn/authn_utils.cc b/src/envoy/http/authn/authn_utils.cc index 78c232e3108..952114b9810 100644 --- a/src/envoy/http/authn/authn_utils.cc +++ b/src/envoy/http/authn/authn_utils.cc @@ -25,6 +25,9 @@ namespace { // The JWT audience key name static const std::string kJwtAudienceKey = "aud"; +// The JWT groups key name +static const std::string kJwtGroupsKey = "groups"; + // Extract JWT audience into the JwtPayload. // This function should to be called after the claims are extracted. void ExtractJwtAudience( @@ -48,29 +51,34 @@ void ExtractJwtAudience( // Not convertable to string array } } -}; // namespace -// Retrieve the JwtPayload from the HTTP headers with the key -bool AuthnUtils::GetJWTPayloadFromHeaders( - const HeaderMap& headers, const LowerCaseString& jwt_payload_key, +// Extract JWT groups into the JwtPayload. +// This function should to be called after the claims are extracted. +void ExtractJwtGroups( + const Envoy::Json::Object& obj, + const ::google::protobuf::Map< ::std::string, ::std::string>& claims, istio::authn::JwtPayload* payload) { - const HeaderEntry* entry = headers.get(jwt_payload_key); - if (!entry) { - ENVOY_LOG(debug, "No JWT payload {} in the header", jwt_payload_key.get()); - return false; + const std::string& key = kJwtGroupsKey; + // "groups" can be either string array or string. + // First, try as string + if (claims.count(key) > 0) { + payload->add_groups(claims.at(key)); + return; } - std::string value(entry->value().c_str(), entry->value().size()); - // JwtAuth::Base64UrlDecode() is different from Base64::decode(). - std::string payload_str = JwtAuth::Base64UrlDecode(value); - // Return an empty string if Base64 decode fails. - if (payload_str.empty()) { - ENVOY_LOG(error, "Invalid {} header, invalid base64: {}", - jwt_payload_key.get(), value); - return false; + // Next, try as string array + try { + std::vector group_vector = obj.getStringArray(key); + for (const std::string group : group_vector) { + payload->add_groups(group); + } + } catch (Json::Exception& e) { + // Not convertable to string array } - *payload->mutable_raw_claims() = payload_str; - ::google::protobuf::Map< ::std::string, ::std::string>* claims = - payload->mutable_claims(); +} +}; // namespace + +bool AuthnUtils::ProcessJwtPayload(const std::string& payload_str, + istio::authn::JwtPayload* payload) { Envoy::Json::ObjectSharedPtr json_obj; try { json_obj = Json::Factory::loadFromString(payload_str); @@ -80,6 +88,10 @@ bool AuthnUtils::GetJWTPayloadFromHeaders( return false; } + *payload->mutable_raw_claims() = payload_str; + ::google::protobuf::Map< ::std::string, ::std::string>* claims = + payload->mutable_claims(); + // Extract claims json_obj->iterate( [payload](const std::string& key, const Json::Object& obj) -> bool { @@ -98,6 +110,8 @@ bool AuthnUtils::GetJWTPayloadFromHeaders( // Extract audience // ExtractJwtAudience() should be called after claims are extracted. ExtractJwtAudience(*json_obj, payload->claims(), payload); + // ExtractJwtGroups() should be called after claims are extracted. + ExtractJwtGroups(*json_obj, payload->claims(), payload); // Build user if (claims->count("iss") > 0 && claims->count("sub") > 0) { payload->set_user((*claims)["iss"] + "/" + (*claims)["sub"]); diff --git a/src/envoy/http/authn/authn_utils.h b/src/envoy/http/authn/authn_utils.h index e156f5eb2c3..023b03e8f10 100644 --- a/src/envoy/http/authn/authn_utils.h +++ b/src/envoy/http/authn/authn_utils.h @@ -28,12 +28,11 @@ namespace AuthN { // AuthnUtils class provides utility functions used for authentication. class AuthnUtils : public Logger::Loggable { public: - // Retrieve the JWT payload from the HTTP header into the output payload map - // Return true if parsing the header payload key succeeds. - // Otherwise, return false. - static bool GetJWTPayloadFromHeaders(const HeaderMap& headers, - const LowerCaseString& jwt_payload_key, - istio::authn::JwtPayload* payload); + // Parse JWT payload string (which typically is the output from jwt filter) + // and populate JwtPayload object. Return true if input string can be parsed + // successfully. Otherwise, return false. + static bool ProcessJwtPayload(const std::string& jwt_payload_str, + istio::authn::JwtPayload* payload); }; } // namespace AuthN diff --git a/src/envoy/http/authn/authn_utils_test.cc b/src/envoy/http/authn/authn_utils_test.cc index 3663476ab0d..710a30e5bda 100644 --- a/src/envoy/http/authn/authn_utils_test.cc +++ b/src/envoy/http/authn/authn_utils_test.cc @@ -27,7 +27,6 @@ namespace Istio { namespace AuthN { namespace { -const LowerCaseString kSecIstioAuthUserInfoHeaderKey("sec-istio-auth-userinfo"); const std::string kSecIstioAuthUserinfoHeaderValue = R"( { @@ -58,20 +57,8 @@ const std::string kSecIstioAuthUserInfoHeaderWithTwoAudValue = } )"; -Http::TestHeaderMapImpl CreateTestHeaderMap(const LowerCaseString& header_key, - const std::string& header_value) { - // The base64 encoding is done through Base64::encode(). - // If the test input has special chars, may need to use the counterpart of - // Base64UrlDecode(). - std::string value_base64 = - Base64::encode(header_value.c_str(), header_value.size()); - return Http::TestHeaderMapImpl{{header_key.get(), value_base64}}; -} - TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderTest) { JwtPayload payload, expected_payload; - Http::TestHeaderMapImpl request_headers_with_jwt = CreateTestHeaderMap( - kSecIstioAuthUserInfoHeaderKey, kSecIstioAuthUserinfoHeaderValue); ASSERT_TRUE(Protobuf::TextFormat::ParseFromString( R"( user: "issuer@foo.com/sub@foo.com" @@ -95,19 +82,16 @@ TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderTest) { raw_claims: ")" + StringUtil::escape(kSecIstioAuthUserinfoHeaderValue) + R"(")", &expected_payload)); - // The payload returned from GetJWTPayloadFromHeaders() should be the same as + // The payload returned from ProcessJwtPayload() should be the same as // the expected. - bool ret = AuthnUtils::GetJWTPayloadFromHeaders( - request_headers_with_jwt, kSecIstioAuthUserInfoHeaderKey, &payload); + bool ret = + AuthnUtils::ProcessJwtPayload(kSecIstioAuthUserinfoHeaderValue, &payload); EXPECT_TRUE(ret); EXPECT_TRUE(MessageDifferencer::Equals(expected_payload, payload)); } -TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderWithNoAudTest) { +TEST(AuthnUtilsTest, ProcessJwtPayloadWithNoAudTest) { JwtPayload payload, expected_payload; - Http::TestHeaderMapImpl request_headers_with_jwt = - CreateTestHeaderMap(kSecIstioAuthUserInfoHeaderKey, - kSecIstioAuthUserInfoHeaderWithNoAudValue); ASSERT_TRUE(Protobuf::TextFormat::ParseFromString( R"( user: "issuer@foo.com/sub@foo.com" @@ -127,20 +111,17 @@ TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderWithNoAudTest) { StringUtil::escape(kSecIstioAuthUserInfoHeaderWithNoAudValue) + R"(")", &expected_payload)); - // The payload returned from GetJWTPayloadFromHeaders() should be the same as + // The payload returned from ProcessJwtPayload() should be the same as // the expected. When there is no aud, the aud is not saved in the payload // and claims. - bool ret = AuthnUtils::GetJWTPayloadFromHeaders( - request_headers_with_jwt, kSecIstioAuthUserInfoHeaderKey, &payload); + bool ret = AuthnUtils::ProcessJwtPayload( + kSecIstioAuthUserInfoHeaderWithNoAudValue, &payload); EXPECT_TRUE(ret); EXPECT_TRUE(MessageDifferencer::Equals(expected_payload, payload)); } -TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderWithTwoAudTest) { +TEST(AuthnUtilsTest, ProcessJwtPayloadWithTwoAudTest) { JwtPayload payload, expected_payload; - Http::TestHeaderMapImpl request_headers_with_jwt = - CreateTestHeaderMap(kSecIstioAuthUserInfoHeaderKey, - kSecIstioAuthUserInfoHeaderWithTwoAudValue); ASSERT_TRUE(Protobuf::TextFormat::ParseFromString( R"( user: "issuer@foo.com/sub@foo.com" @@ -163,11 +144,11 @@ TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderWithTwoAudTest) { R"(")", &expected_payload)); - // The payload returned from GetJWTPayloadFromHeaders() should be the same as + // The payload returned from ProcessJwtPayload() should be the same as // the expected. When the aud is a string array, the aud is not saved in the // claims. - bool ret = AuthnUtils::GetJWTPayloadFromHeaders( - request_headers_with_jwt, kSecIstioAuthUserInfoHeaderKey, &payload); + bool ret = AuthnUtils::ProcessJwtPayload( + kSecIstioAuthUserInfoHeaderWithTwoAudValue, &payload); EXPECT_TRUE(ret); EXPECT_TRUE(MessageDifferencer::Equals(expected_payload, payload)); } diff --git a/src/envoy/http/authn/filter_context.cc b/src/envoy/http/authn/filter_context.cc index ac72ae8bc89..38bf952db00 100644 --- a/src/envoy/http/authn/filter_context.cc +++ b/src/envoy/http/authn/filter_context.cc @@ -14,6 +14,7 @@ */ #include "src/envoy/http/authn/filter_context.h" +#include "src/envoy/utils/filter_names.h" #include "src/envoy/utils/utils.h" using istio::authn::Payload; @@ -71,6 +72,29 @@ void FilterContext::setPrincipal(const iaapi::PrincipalBinding& binding) { } } +bool FilterContext::getJwtPayload(const std::string& issuer, + std::string* payload) const { + const auto filter_it = + dynamic_metadata_.filter_metadata().find(Utils::IstioFilterName::kJwt); + if (filter_it == dynamic_metadata_.filter_metadata().end()) { + ENVOY_LOG(debug, "No dynamic_metadata found for filter {}", + Utils::IstioFilterName::kJwt); + return false; + } + + const auto& data_struct = filter_it->second; + const auto entry_it = data_struct.fields().find(issuer); + if (entry_it == data_struct.fields().end()) { + return false; + } + if (entry_it->second.string_value().empty()) { + return false; + } + + *payload = entry_it->second.string_value(); + return true; +} + } // namespace AuthN } // namespace Istio } // namespace Http diff --git a/src/envoy/http/authn/filter_context.h b/src/envoy/http/authn/filter_context.h index 7907479d55f..0b5e060f98e 100644 --- a/src/envoy/http/authn/filter_context.h +++ b/src/envoy/http/authn/filter_context.h @@ -17,8 +17,8 @@ #include "authentication/v1alpha1/policy.pb.h" #include "common/common/logger.h" +#include "envoy/api/v2/core/base.pb.h" #include "envoy/config/filter/http/authn/v2alpha1/config.pb.h" -#include "envoy/http/header_map.h" #include "envoy/network/connection.h" #include "src/istio/authn/context.pb.h" @@ -27,15 +27,16 @@ namespace Http { namespace Istio { namespace AuthN { -// FilterContext holds inputs, such as request header and connection and -// result data for authentication process. +// FilterContext holds inputs, such as request dynamic metadata and connection +// and result data for authentication process. class FilterContext : public Logger::Loggable { public: FilterContext( - HeaderMap* headers, const Network::Connection* connection, + const envoy::api::v2::core::Metadata& dynamic_metadata, + const Network::Connection* connection, const istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig& filter_config) - : headers_(headers), + : dynamic_metadata_(dynamic_metadata), connection_(connection), filter_config_(filter_config) {} virtual ~FilterContext() {} @@ -56,8 +57,6 @@ class FilterContext : public Logger::Loggable { // Returns the authentication result. const istio::authn::Result& authenticationResult() { return result_; } - // Accessor to headers. - HeaderMap* headers() { return headers_; } // Accessor to connection const Network::Connection* connection() { return connection_; } // Accessor to the filter config @@ -66,9 +65,15 @@ class FilterContext : public Logger::Loggable { return filter_config_; } + // Gets JWT payload (output from JWT filter) for given issuer. If non-empty + // payload found, returns true and set the output payload string. Otherwise, + // returns false. + bool getJwtPayload(const std::string& issuer, std::string* payload) const; + private: - // Pointer to the headers of the request. - HeaderMap* headers_; + // Const reference to request info dynamic metadata. This provides data that + // output from other filters, e.g JWT. + const envoy::api::v2::core::Metadata& dynamic_metadata_; // Pointer to network connection of the request. const Network::Connection* connection_; diff --git a/src/envoy/http/authn/filter_context_test.cc b/src/envoy/http/authn/filter_context_test.cc index c00f3514ea0..6fb89866e15 100644 --- a/src/envoy/http/authn/filter_context_test.cc +++ b/src/envoy/http/authn/filter_context_test.cc @@ -14,6 +14,7 @@ */ #include "src/envoy/http/authn/filter_context.h" +#include "envoy/api/v2/core/base.pb.h" #include "src/envoy/http/authn/test_utils.h" #include "test/test_common/utility.h" @@ -32,9 +33,9 @@ class FilterContextTest : public testing::Test { public: virtual ~FilterContextTest() {} - // This test suit does not use headers nor connection, so ok to use null for - // them. - FilterContext filter_context_{nullptr, nullptr, + envoy::api::v2::core::Metadata metadata_; + // This test suit does not use connection, so ok to use null for it. + FilterContext filter_context_{metadata_, nullptr, istio::envoy::config::filter::http::authn:: v2alpha1::FilterConfig::default_instance()}; diff --git a/src/envoy/http/authn/http_filter.cc b/src/envoy/http/authn/http_filter.cc index 27e14b8a088..f1f372fa960 100644 --- a/src/envoy/http/authn/http_filter.cc +++ b/src/envoy/http/authn/http_filter.cc @@ -20,6 +20,7 @@ #include "src/envoy/http/authn/origin_authenticator.h" #include "src/envoy/http/authn/peer_authenticator.h" #include "src/envoy/utils/authn.h" +#include "src/envoy/utils/filter_names.h" #include "src/envoy/utils/utils.h" using istio::authn::Payload; @@ -41,21 +42,20 @@ void AuthenticationFilter::onDestroy() { ENVOY_LOG(debug, "Called AuthenticationFilter : {}", __func__); } -FilterHeadersStatus AuthenticationFilter::decodeHeaders(HeaderMap& headers, - bool) { +FilterHeadersStatus AuthenticationFilter::decodeHeaders(HeaderMap&, bool) { ENVOY_LOG(debug, "AuthenticationFilter::decodeHeaders with config\n{}", filter_config_.DebugString()); state_ = State::PROCESSING; filter_context_.reset(new Istio::AuthN::FilterContext( - &headers, decoder_callbacks_->connection(), filter_config_)); + decoder_callbacks_->requestInfo().dynamicMetadata(), + decoder_callbacks_->connection(), filter_config_)); Payload payload; if (!filter_config_.policy().peer_is_optional() && !createPeerAuthenticator(filter_context_.get())->run(&payload)) { rejectRequest("Peer authentication failed."); - removeJwtPayloadFromHeaders(); return FilterHeadersStatus::StopIteration; } @@ -63,10 +63,6 @@ FilterHeadersStatus AuthenticationFilter::decodeHeaders(HeaderMap& headers, filter_config_.policy().origin_is_optional() || createOriginAuthenticator(filter_context_.get())->run(&payload); - // After Istio authn, the JWT headers consumed by Istio authn should be - // removed. - removeJwtPayloadFromHeaders(); - if (!success) { rejectRequest("Origin authentication failed."); return FilterHeadersStatus::StopIteration; @@ -74,28 +70,19 @@ FilterHeadersStatus AuthenticationFilter::decodeHeaders(HeaderMap& headers, // Put authentication result to headers. if (filter_context_ != nullptr) { - // TODO(yangminzhu): Remove the header and only use the metadata to pass the - // attributes. - Utils::Authentication::SaveResultToHeader( - filter_context_->authenticationResult(), filter_context_->headers()); - - // Save auth results in the metadata, could be later used by RBAC filter. + // Save auth results in the metadata, could be used later by RBAC and/or + // mixer filter. ProtobufWkt::Struct data; Utils::Authentication::SaveAuthAttributesToStruct( filter_context_->authenticationResult(), data); - decoder_callbacks_->requestInfo().setDynamicMetadata("istio_authn", data); + decoder_callbacks_->requestInfo().setDynamicMetadata( + Utils::IstioFilterName::kAuthentication, data); ENVOY_LOG(debug, "Saved Dynamic Metadata:\n{}", data.DebugString()); } state_ = State::COMPLETE; return FilterHeadersStatus::Continue; } -void AuthenticationFilter::removeJwtPayloadFromHeaders() { - for (auto const iter : filter_config_.jwt_output_payload_locations()) { - filter_context_->headers()->remove(LowerCaseString(iter.second)); - } -} - FilterDataStatus AuthenticationFilter::decodeData(Buffer::Instance&, bool) { if (state_ == State::PROCESSING) { return FilterDataStatus::StopIterationAndWatermark; diff --git a/src/envoy/http/authn/http_filter.h b/src/envoy/http/authn/http_filter.h index c7a29a02a8e..a710cfc0559 100644 --- a/src/envoy/http/authn/http_filter.h +++ b/src/envoy/http/authn/http_filter.h @@ -63,9 +63,6 @@ class AuthenticationFilter : public StreamDecoderFilter, createOriginAuthenticator(Istio::AuthN::FilterContext* filter_context); - // Removes output JWT valiation from headers. - void removeJwtPayloadFromHeaders(); - private: // Store the config. const istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig& diff --git a/src/envoy/http/authn/http_filter_factory.cc b/src/envoy/http/authn/http_filter_factory.cc index 3fa6182f504..221cc78a6e5 100644 --- a/src/envoy/http/authn/http_filter_factory.cc +++ b/src/envoy/http/authn/http_filter_factory.cc @@ -18,6 +18,7 @@ #include "envoy/server/filter_config.h" #include "google/protobuf/util/json_util.h" #include "src/envoy/http/authn/http_filter.h" +#include "src/envoy/utils/filter_names.h" #include "src/envoy/utils/utils.h" using istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig; @@ -26,11 +27,6 @@ namespace Envoy { namespace Server { namespace Configuration { -namespace { -// The name for the Istio authentication filter. -const std::string kAuthnFactoryName("istio_authn"); -} // namespace - class AuthnFilterConfig : public NamedHttpFilterConfigFactory, public Logger::Loggable { public: @@ -65,7 +61,9 @@ class AuthnFilterConfig : public NamedHttpFilterConfigFactory, return ProtobufTypes::MessagePtr{new FilterConfig}; } - std::string name() override { return kAuthnFactoryName; } + std::string name() override { + return Utils::IstioFilterName::kAuthentication; + } private: Http::FilterFactoryCb createFilterFactory(const FilterConfig& config_pb) { diff --git a/src/envoy/http/authn/http_filter_integration_test.cc b/src/envoy/http/authn/http_filter_integration_test.cc index 80416e135e0..a404e07d2aa 100644 --- a/src/envoy/http/authn/http_filter_integration_test.cc +++ b/src/envoy/http/authn/http_filter_integration_test.cc @@ -14,8 +14,12 @@ */ #include "common/common/base64.h" +#include "common/common/utility.h" +#include "extensions/filters/http/well_known_names.h" +#include "fmt/printf.h" +#include "src/envoy/utils/filter_names.h" #include "src/istio/authn/context.pb.h" -#include "test/integration/http_integration.h" +#include "test/integration/http_protocol_integration.h" using google::protobuf::util::MessageDifferencer; using istio::authn::Payload; @@ -23,65 +27,72 @@ using istio::authn::Result; namespace Envoy { namespace { -const std::string kSecIstioAuthUserInfoHeaderKey = "sec-istio-auth-userinfo"; -const std::string kSecIstioAuthUserinfoHeaderValue = - "eyJpc3MiOiI2Mjg2NDU3NDE4ODEtbm9hYml1MjNmNWE4bThvdmQ4dWN2Njk4bGo3OH" - "Z2MGxAZGV2ZWxvcGVyLmdzZXJ2aWNlYWNjb3VudC5jb20iLCJzdWIiOiI2Mjg2NDU3" - "NDE4ODEtbm9hYml1MjNmNWE4bThvdmQ4dWN2Njk4bGo3OHZ2MGxAZGV2ZWxvcGVyLm" - "dzZXJ2aWNlYWNjb3VudC5jb20iLCJhdWQiOiJib29rc3RvcmUtZXNwLWVjaG8uY2xv" - "dWRlbmRwb2ludHNhcGlzLmNvbSIsImlhdCI6MTUxMjc1NDIwNSwiZXhwIjo1MTEyNz" - "U0MjA1fQ=="; -const Envoy::Http::LowerCaseString kSecIstioAuthnPayloadHeaderKey( + +static const Envoy::Http::LowerCaseString kSecIstioAuthnPayloadHeaderKey( "sec-istio-authn-payload"); -class AuthenticationFilterIntegrationTest - : public HttpIntegrationTest, - public testing::TestWithParam { - public: - AuthenticationFilterIntegrationTest() - : HttpIntegrationTest(Http::CodecClient::Type::HTTP1, GetParam()), - default_request_headers_{ - {":method", "GET"}, {":path", "/"}, {":authority", "host"}}, - request_headers_with_jwt_{{":method", "GET"}, - {":path", "/"}, - {":authority", "host"}, - {kSecIstioAuthUserInfoHeaderKey, - kSecIstioAuthUserinfoHeaderValue}} {} - - void SetUp() override { - fake_upstreams_.emplace_back( - new FakeUpstream(0, FakeHttpConnection::Type::HTTP1, version_)); - registerPort("upstream_0", - fake_upstreams_.back()->localAddress()->ip()->port()); - fake_upstreams_.emplace_back( - new FakeUpstream(0, FakeHttpConnection::Type::HTTP1, version_)); - registerPort("upstream_1", - fake_upstreams_.back()->localAddress()->ip()->port()); - } - - void TearDown() override { - test_server_.reset(); - fake_upstream_connection_.reset(); - fake_upstreams_.clear(); - } - - protected: - Http::TestHeaderMapImpl default_request_headers_; - Http::TestHeaderMapImpl request_headers_with_jwt_; -}; +// Default request for testing. +static const Http::TestHeaderMapImpl kSimpleRequestHeader{{ + {":method", "GET"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-forwarded-for", "10.0.0.1"}, +}}; + +// Keep the same as issuer in the policy below. +static const char kJwtIssuer[] = "some@issuer"; + +static const char kAuthnFilterWithJwt[] = R"( + name: istio_authn + config: + policy: + origins: + - jwt: + issuer: some@issuer + jwks_uri: http://localhost:8081/)"; + +// Payload data to inject. Note the iss claim intentionally set different from +// kJwtIssuer. +static const char kMockJwtPayload[] = + "{\"iss\":\"https://example.com\"," + "\"sub\":\"test@example.com\",\"exp\":2001001001," + "\"aud\":\"example_service\"}"; +// Returns a simple header-to-metadata filter config that can be used to inject +// data into request info dynamic metadata for testing. +std::string MakeHeaderToMetadataConfig() { + return fmt::sprintf( + R"( + name: %s + config: + request_rules: + - header: x-mock-metadata-injection + on_header_missing: + metadata_namespace: %s + key: %s + value: "%s" + type: STRING)", + Extensions::HttpFilters::HttpFilterNames::get().HeaderToMetadata, + Utils::IstioFilterName::kJwt, kJwtIssuer, + StringUtil::escape(kMockJwtPayload)); +} + +typedef HttpProtocolIntegrationTest AuthenticationFilterIntegrationTest; INSTANTIATE_TEST_CASE_P( - IpVersions, AuthenticationFilterIntegrationTest, - testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); + Protocols, AuthenticationFilterIntegrationTest, + testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams()), + HttpProtocolIntegrationTest::protocolTestParamsToString); TEST_P(AuthenticationFilterIntegrationTest, EmptyPolicy) { - createTestServer("src/envoy/http/authn/testdata/envoy_empty.conf", {"http"}); + config_helper_.addFilter("name: istio_authn"); + initialize(); codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); - auto response = - codec_client_->makeHeaderOnlyRequest(default_request_headers_); - // Wait for request to upstream[0] (backend) - waitForNextUpstreamRequest(0); + auto response = codec_client_->makeHeaderOnlyRequest(kSimpleRequestHeader); + // Wait for request to upstream (backend) + waitForNextUpstreamRequest(); + // Send backend response. upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, true); @@ -92,15 +103,19 @@ TEST_P(AuthenticationFilterIntegrationTest, EmptyPolicy) { } TEST_P(AuthenticationFilterIntegrationTest, SourceMTlsFail) { - createTestServer("src/envoy/http/authn/testdata/envoy_peer_mtls.conf", - {"http"}); + config_helper_.addFilter(R"( + name: istio_authn + config: + policy: + peers: + - mtls: {})"); + initialize(); // AuthN filter use MTls, but request doesn't have certificate, request // would be rejected. codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); - auto response = - codec_client_->makeHeaderOnlyRequest(default_request_headers_); + auto response = codec_client_->makeHeaderOnlyRequest(kSimpleRequestHeader); // Request is rejected, there will be no upstream request (thus no // waitForNextUpstreamRequest). @@ -112,16 +127,14 @@ TEST_P(AuthenticationFilterIntegrationTest, SourceMTlsFail) { // TODO (diemtvu/lei-tang): add test for MTls success. TEST_P(AuthenticationFilterIntegrationTest, OriginJwtRequiredHeaderNoJwtFail) { - createTestServer( - "src/envoy/http/authn/testdata/envoy_origin_jwt_authn_only.conf", - {"http"}); + config_helper_.addFilter(kAuthnFilterWithJwt); + initialize(); // The AuthN filter requires JWT, but request doesn't have JWT, request // would be rejected. codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); - auto response = - codec_client_->makeHeaderOnlyRequest(default_request_headers_); + auto response = codec_client_->makeHeaderOnlyRequest(kSimpleRequestHeader); // Request is rejected, there will be no upstream request (thus no // waitForNextUpstreamRequest). @@ -131,19 +144,18 @@ TEST_P(AuthenticationFilterIntegrationTest, OriginJwtRequiredHeaderNoJwtFail) { } TEST_P(AuthenticationFilterIntegrationTest, CheckValidJwtPassAuthentication) { - createTestServer( - "src/envoy/http/authn/testdata/envoy_origin_jwt_authn_only.conf", - {"http"}); + config_helper_.addFilter(kAuthnFilterWithJwt); + config_helper_.addFilter(MakeHeaderToMetadataConfig()); + initialize(); // The AuthN filter requires JWT. The http request contains validated JWT and // the authentication should succeed. codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); - auto response = - codec_client_->makeHeaderOnlyRequest(request_headers_with_jwt_); + auto response = codec_client_->makeHeaderOnlyRequest(kSimpleRequestHeader); - // Wait for request to upstream[0] (backend) - waitForNextUpstreamRequest(0); + // Wait for request to upstream (backend) + waitForNextUpstreamRequest(); // Send backend response. upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, true); @@ -153,100 +165,5 @@ TEST_P(AuthenticationFilterIntegrationTest, CheckValidJwtPassAuthentication) { EXPECT_STREQ("200", response->headers().Status()->value().c_str()); } -TEST_P(AuthenticationFilterIntegrationTest, CheckConsumedJwtHeadersAreRemoved) { - const Envoy::Http::LowerCaseString header_location( - "location-to-read-jwt-result"); - const std::string jwt_header = - R"( - { - "iss": "issuer@foo.com", - "sub": "sub@foo.com", - "aud": "aud1", - "non-string-will-be-ignored": 1512754205, - "some-other-string-claims": "some-claims-kept" - } - )"; - std::string jwt_header_base64 = - Base64::encode(jwt_header.c_str(), jwt_header.size()); - Http::TestHeaderMapImpl request_headers_with_jwt_at_specified_location{ - {":method", "GET"}, - {":path", "/"}, - {":authority", "host"}, - {"location-to-read-jwt-result", jwt_header_base64}}; - // In this config, the JWT verification result for "issuer@foo.com" is in the - // header "location-to-read-jwt-result" - createTestServer( - "src/envoy/http/authn/testdata/" - "envoy_jwt_with_output_header_location.conf", - {"http"}); - // The AuthN filter requires JWT and the http request contains validated JWT. - // In this case, the authentication should succeed and an authn result - // should be generated. - codec_client_ = - makeHttpConnection(makeClientConnection((lookupPort("http")))); - auto response = codec_client_->makeHeaderOnlyRequest( - request_headers_with_jwt_at_specified_location); - - // Wait for request to upstream[0] (backend) - waitForNextUpstreamRequest(0); - response->waitForEndStream(); - - // After Istio authn, the JWT headers consumed by Istio authn should have - // been removed. - EXPECT_TRUE(nullptr == upstream_request_->headers().get(header_location)); -} - -TEST_P(AuthenticationFilterIntegrationTest, CheckAuthnResultIsExpected) { - createTestServer( - "src/envoy/http/authn/testdata/envoy_origin_jwt_authn_only.conf", - {"http"}); - - // The AuthN filter requires JWT and the http request contains validated JWT. - // In this case, the authentication should succeed and an authn result - // should be generated. - codec_client_ = - makeHttpConnection(makeClientConnection((lookupPort("http")))); - auto response = - codec_client_->makeHeaderOnlyRequest(request_headers_with_jwt_); - - // Wait for request to upstream[0] (backend) - waitForNextUpstreamRequest(0); - response->waitForEndStream(); - - // Authn result should be as expected - const Envoy::Http::HeaderString &header_value = - upstream_request_->headers().get(kSecIstioAuthnPayloadHeaderKey)->value(); - std::string value_base64(header_value.c_str(), header_value.size()); - const std::string value = Base64::decode(value_base64); - Result result; - google::protobuf::util::JsonParseOptions options; - Result expected_result; - - bool parse_ret = result.ParseFromString(value); - EXPECT_TRUE(parse_ret); - JsonStringToMessage( - R"( - { - "origin": { - "user": "628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com/628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com", - "audiences": [ - "bookstore-esp-echo.cloudendpointsapis.com" - ], - "presenter": "", - "claims": { - "aud": "bookstore-esp-echo.cloudendpointsapis.com", - "iss": "628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com", - "sub": "628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com" - }, - raw_claims: "{\"iss\":\"628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com\",\"sub\":\"628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com\",\"aud\":\"bookstore-esp-echo.cloudendpointsapis.com\",\"iat\":1512754205,\"exp\":5112754205}" - } - } - )", - &expected_result, options); - // Note: TestUtility::protoEqual() uses SerializeAsString() and the output - // is non-deterministic. Thus, MessageDifferencer::Equals() is used. - EXPECT_TRUE(MessageDifferencer::Equals(expected_result, result)); -} - } // namespace } // namespace Envoy diff --git a/src/envoy/http/authn/http_filter_test.cc b/src/envoy/http/authn/http_filter_test.cc index a71d60aeb56..08376bfb9a3 100644 --- a/src/envoy/http/authn/http_filter_test.cc +++ b/src/envoy/http/authn/http_filter_test.cc @@ -16,6 +16,7 @@ #include "src/envoy/http/authn/http_filter.h" #include "common/common/base64.h" #include "common/http/header_map_impl.h" +#include "common/request_info/request_info_impl.h" #include "envoy/config/filter/http/authn/v2alpha1/config.pb.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -30,10 +31,12 @@ using Envoy::Http::Istio::AuthN::FilterContext; using istio::authn::Payload; using istio::authn::Result; using istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig; +using testing::_; +using testing::AtLeast; using testing::Invoke; using testing::NiceMock; +using testing::ReturnRef; using testing::StrictMock; -using testing::_; namespace iaapi = istio::authentication::v1alpha1; @@ -116,6 +119,10 @@ TEST_F(AuthenticationFilterTest, PeerFail) { EXPECT_CALL(filter_, createPeerAuthenticator(_)) .Times(1) .WillOnce(Invoke(createAlwaysFailAuthenticator)); + RequestInfo::RequestInfoImpl request_info(Http::Protocol::Http2); + EXPECT_CALL(decoder_callbacks_, requestInfo()) + .Times(AtLeast(1)) + .WillRepeatedly(ReturnRef(request_info)); EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _)) .Times(1) .WillOnce(testing::Invoke([](Http::HeaderMap &headers, bool) { @@ -123,7 +130,8 @@ TEST_F(AuthenticationFilterTest, PeerFail) { })); EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(request_headers_, true)); - EXPECT_FALSE(Utils::Authentication::HasResultInHeader(request_headers_)); + EXPECT_FALSE(Utils::Authentication::GetResultFromMetadata( + request_info.dynamicMetadata())); } TEST_F(AuthenticationFilterTest, PeerPassOrginFail) { @@ -135,6 +143,10 @@ TEST_F(AuthenticationFilterTest, PeerPassOrginFail) { EXPECT_CALL(filter_, createOriginAuthenticator(_)) .Times(1) .WillOnce(Invoke(createAlwaysFailAuthenticator)); + RequestInfo::RequestInfoImpl request_info(Http::Protocol::Http2); + EXPECT_CALL(decoder_callbacks_, requestInfo()) + .Times(AtLeast(1)) + .WillRepeatedly(ReturnRef(request_info)); EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _)) .Times(1) .WillOnce(testing::Invoke([](Http::HeaderMap &headers, bool) { @@ -142,7 +154,8 @@ TEST_F(AuthenticationFilterTest, PeerPassOrginFail) { })); EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(request_headers_, true)); - EXPECT_FALSE(Utils::Authentication::HasResultInHeader(request_headers_)); + EXPECT_FALSE(Utils::Authentication::GetResultFromMetadata( + request_info.dynamicMetadata())); } TEST_F(AuthenticationFilterTest, AllPass) { @@ -152,13 +165,35 @@ TEST_F(AuthenticationFilterTest, AllPass) { EXPECT_CALL(filter_, createOriginAuthenticator(_)) .Times(1) .WillOnce(Invoke(createAlwaysPassAuthenticator)); + RequestInfo::RequestInfoImpl request_info(Http::Protocol::Http2); + EXPECT_CALL(decoder_callbacks_, requestInfo()) + .Times(AtLeast(1)) + .WillRepeatedly(ReturnRef(request_info)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(request_headers_, true)); - Result authn; - EXPECT_TRUE( - Utils::Authentication::FetchResultFromHeader(request_headers_, &authn)); - EXPECT_TRUE(TestUtility::protoEqual( - TestUtilities::AuthNResultFromString(R"(peer_user: "foo")"), authn)); + + EXPECT_EQ(1, request_info.dynamicMetadata().filter_metadata_size()); + const auto *data = Utils::Authentication::GetResultFromMetadata( + request_info.dynamicMetadata()); + ASSERT_TRUE(data); + + ProtobufWkt::Struct expected_data; + ASSERT_TRUE(Protobuf::TextFormat::ParseFromString(R"( + fields { + key: "source.principal" + value { + string_value: "foo" + } + } + fields { + key: "source.user" + value { + string_value: "foo" + } + })", + &expected_data)); + EXPECT_TRUE(TestUtility::protoEqual(expected_data, *data)); } TEST_F(AuthenticationFilterTest, IgnoreBothFail) { diff --git a/src/envoy/http/authn/origin_authenticator_test.cc b/src/envoy/http/authn/origin_authenticator_test.cc index 8aeaa85215f..a2a2cd7e858 100644 --- a/src/envoy/http/authn/origin_authenticator_test.cc +++ b/src/envoy/http/authn/origin_authenticator_test.cc @@ -15,8 +15,8 @@ #include "src/envoy/http/authn/origin_authenticator.h" #include "authentication/v1alpha1/policy.pb.h" -#include "common/http/header_map_impl.h" #include "common/protobuf/protobuf.h" +#include "envoy/api/v2/core/base.pb.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include "src/envoy/http/authn/test_utils.h" @@ -27,13 +27,13 @@ namespace iaapi = istio::authentication::v1alpha1; using istio::authn::Payload; using istio::authn::Result; +using testing::_; using testing::DoAll; using testing::MockFunction; using testing::NiceMock; using testing::Return; using testing::SetArgPointee; using testing::StrictMock; -using testing::_; namespace Envoy { namespace Http { @@ -90,8 +90,7 @@ class MockOriginAuthenticator : public OriginAuthenticator { class OriginAuthenticatorTest : public testing::TestWithParam { public: - OriginAuthenticatorTest() - : request_headers_{{":method", "GET"}, {":path", "/"}} {} + OriginAuthenticatorTest() {} virtual ~OriginAuthenticatorTest() {} void SetUp() override { @@ -121,10 +120,11 @@ class OriginAuthenticatorTest : public testing::TestWithParam { protected: std::unique_ptr> authenticator_; - Http::TestHeaderMapImpl request_headers_; - FilterContext filter_context_{&request_headers_, nullptr, - istio::envoy::config::filter::http::authn:: - v2alpha1::FilterConfig::default_instance()}; + // envoy::api::v2::core::Metadata metadata_; + FilterContext filter_context_{ + envoy::api::v2::core::Metadata::default_instance(), nullptr, + istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig:: + default_instance()}; iaapi::Policy policy_; Payload* payload_; diff --git a/src/envoy/http/authn/peer_authenticator_test.cc b/src/envoy/http/authn/peer_authenticator_test.cc index d0cc95b6f6a..cce09dbdc99 100644 --- a/src/envoy/http/authn/peer_authenticator_test.cc +++ b/src/envoy/http/authn/peer_authenticator_test.cc @@ -15,8 +15,8 @@ #include "src/envoy/http/authn/peer_authenticator.h" #include "authentication/v1alpha1/policy.pb.h" -#include "common/http/header_map_impl.h" #include "common/protobuf/protobuf.h" +#include "envoy/api/v2/core/base.pb.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include "src/envoy/http/authn/test_utils.h" @@ -26,13 +26,13 @@ namespace iaapi = istio::authentication::v1alpha1; using istio::authn::Payload; +using testing::_; using testing::DoAll; using testing::MockFunction; using testing::NiceMock; using testing::Return; using testing::SetArgPointee; using testing::StrictMock; -using testing::_; namespace Envoy { namespace Http { @@ -52,8 +52,7 @@ class MockPeerAuthenticator : public PeerAuthenticator { class PeerAuthenticatorTest : public testing::Test { public: - PeerAuthenticatorTest() - : request_headers_{{":method", "GET"}, {":path", "/"}} {} + PeerAuthenticatorTest() {} virtual ~PeerAuthenticatorTest() {} void createAuthenticator() { @@ -67,10 +66,10 @@ class PeerAuthenticatorTest : public testing::Test { protected: std::unique_ptr> authenticator_; - Http::TestHeaderMapImpl request_headers_; - FilterContext filter_context_{&request_headers_, nullptr, - istio::envoy::config::filter::http::authn:: - v2alpha1::FilterConfig::default_instance()}; + FilterContext filter_context_{ + envoy::api::v2::core::Metadata::default_instance(), nullptr, + istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig:: + default_instance()}; iaapi::Policy policy_; Payload* payload_; diff --git a/src/envoy/http/authn/testdata/envoy_empty.conf b/src/envoy/http/authn/testdata/envoy_empty.conf deleted file mode 100644 index c9f244f91ce..00000000000 --- a/src/envoy/http/authn/testdata/envoy_empty.conf +++ /dev/null @@ -1,85 +0,0 @@ -{ - "listeners": [ - { - "address": "tcp://{{ ip_loopback_address }}:0", - "bind_to_port": true, - "filters": [ - { - "type": "read", - "name": "http_connection_manager", - "config": { - "codec_type": "auto", - "stat_prefix": "ingress_http", - "route_config": { - "virtual_hosts": [ - { - "name": "backend", - "domains": ["*"], - "routes": [ - { - "prefix": "/", - "cluster": "backend_service" - } - ] - } - ] - }, - "access_log": [ - { - "path": "/dev/null" - } - ], - "filters": [ - { - "type": "decoder", - "name": "istio_authn", - "config": {} - }, - { - "type": "decoder", - "name": "router", - "config": {} - } - ] - } - } - ] - } - ], - "admin": { - "access_log_path": "/dev/null", - "address": "tcp://{{ ip_loopback_address }}:0" - }, - "cluster_manager": { - "clusters": [ - { - "name": "backend_service", - "connect_timeout_ms": 5000, - "type": "static", - "lb_type": "round_robin", - "hosts": [ - { - "url": "tcp://{{ ip_loopback_address }}:{{ upstream_0 }}" - } - ] - }, - { - "name": "example_issuer", - "connect_timeout_ms": 5000, - "type": "static", - "circuit_breakers": { - "default": { - "max_pending_requests": 10000, - "max_requests": 10000 - } - }, - "lb_type": "round_robin", - "hosts": [ - { - "url": "tcp://{{ ip_loopback_address }}:{{ upstream_1 }}" - } - ] - } - ] - } -} diff --git a/src/envoy/http/authn/testdata/envoy_jwt_with_output_header_location.conf b/src/envoy/http/authn/testdata/envoy_jwt_with_output_header_location.conf deleted file mode 100644 index 0a0c6e8cdd8..00000000000 --- a/src/envoy/http/authn/testdata/envoy_jwt_with_output_header_location.conf +++ /dev/null @@ -1,99 +0,0 @@ -{ - "listeners": [ - { - "address": "tcp://{{ ip_loopback_address }}:0", - "bind_to_port": true, - "filters": [ - { - "type": "read", - "name": "http_connection_manager", - "config": { - "codec_type": "auto", - "stat_prefix": "ingress_http", - "route_config": { - "virtual_hosts": [ - { - "name": "backend", - "domains": ["*"], - "routes": [ - { - "prefix": "/", - "cluster": "backend_service" - } - ] - } - ] - }, - "access_log": [ - { - "path": "/dev/null" - } - ], - "filters": [ - { - "type": "decoder", - "name": "istio_authn", - "config": { - "policy": { - "origins": [ - { - "jwt": { - "issuer": "issuer@foo.com", - "jwks_uri": "http://localhost:8081/" - } - } - ] - }, - "jwt_output_payload_locations": { - "issuer@foo.com": "location-to-read-jwt-result" - } - } - }, - { - "type": "decoder", - "name": "router", - "config": {} - } - ] - } - } - ] - } - ], - "admin": { - "access_log_path": "/dev/null", - "address": "tcp://{{ ip_loopback_address }}:0" - }, - "cluster_manager": { - "clusters": [ - { - "name": "backend_service", - "connect_timeout_ms": 5000, - "type": "static", - "lb_type": "round_robin", - "hosts": [ - { - "url": "tcp://{{ ip_loopback_address }}:{{ upstream_0 }}" - } - ] - }, - { - "name": "example_issuer", - "connect_timeout_ms": 5000, - "type": "static", - "circuit_breakers": { - "default": { - "max_pending_requests": 10000, - "max_requests": 10000 - } - }, - "lb_type": "round_robin", - "hosts": [ - { - "url": "tcp://{{ ip_loopback_address }}:{{ upstream_1 }}" - } - ] - } - ] - } -} diff --git a/src/envoy/http/authn/testdata/envoy_origin_jwt_authn_only.conf b/src/envoy/http/authn/testdata/envoy_origin_jwt_authn_only.conf deleted file mode 100644 index 5806fec16d2..00000000000 --- a/src/envoy/http/authn/testdata/envoy_origin_jwt_authn_only.conf +++ /dev/null @@ -1,99 +0,0 @@ -{ - "listeners": [ - { - "address": "tcp://{{ ip_loopback_address }}:0", - "bind_to_port": true, - "filters": [ - { - "type": "read", - "name": "http_connection_manager", - "config": { - "codec_type": "auto", - "stat_prefix": "ingress_http", - "route_config": { - "virtual_hosts": [ - { - "name": "backend", - "domains": ["*"], - "routes": [ - { - "prefix": "/", - "cluster": "backend_service" - } - ] - } - ] - }, - "access_log": [ - { - "path": "/dev/null" - } - ], - "filters": [ - { - "type": "decoder", - "name": "istio_authn", - "config": { - "policy": { - "origins": [ - { - "jwt": { - "issuer": "628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com", - "jwks_uri": "http://localhost:8081/" - } - } - ] - }, - "jwt_output_payload_locations": { - "628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com": "sec-istio-auth-userinfo" - } - } - }, - { - "type": "decoder", - "name": "router", - "config": {} - } - ] - } - } - ] - } - ], - "admin": { - "access_log_path": "/dev/null", - "address": "tcp://{{ ip_loopback_address }}:0" - }, - "cluster_manager": { - "clusters": [ - { - "name": "backend_service", - "connect_timeout_ms": 5000, - "type": "static", - "lb_type": "round_robin", - "hosts": [ - { - "url": "tcp://{{ ip_loopback_address }}:{{ upstream_0 }}" - } - ] - }, - { - "name": "example_issuer", - "connect_timeout_ms": 5000, - "type": "static", - "circuit_breakers": { - "default": { - "max_pending_requests": 10000, - "max_requests": 10000 - } - }, - "lb_type": "round_robin", - "hosts": [ - { - "url": "tcp://{{ ip_loopback_address }}:{{ upstream_1 }}" - } - ] - } - ] - } -} diff --git a/src/envoy/http/authn/testdata/envoy_peer_mtls.conf b/src/envoy/http/authn/testdata/envoy_peer_mtls.conf deleted file mode 100644 index 993de548707..00000000000 --- a/src/envoy/http/authn/testdata/envoy_peer_mtls.conf +++ /dev/null @@ -1,94 +0,0 @@ -{ - "listeners": [ - { - "address": "tcp://{{ ip_loopback_address }}:0", - "bind_to_port": true, - "filters": [ - { - "type": "read", - "name": "http_connection_manager", - "config": { - "codec_type": "auto", - "stat_prefix": "ingress_http", - "route_config": { - "virtual_hosts": [ - { - "name": "backend", - "domains": ["*"], - "routes": [ - { - "prefix": "/", - "cluster": "backend_service" - } - ] - } - ] - }, - "access_log": [ - { - "path": "/dev/null" - } - ], - "filters": [ - { - "type": "decoder", - "name": "istio_authn", - "config": { - "policy": { - "peers": [ - { - "mtls": { - } - } - ] - } - } - }, - { - "type": "decoder", - "name": "router", - "config": {} - } - ] - } - } - ] - } - ], - "admin": { - "access_log_path": "/dev/null", - "address": "tcp://{{ ip_loopback_address }}:0" - }, - "cluster_manager": { - "clusters": [ - { - "name": "backend_service", - "connect_timeout_ms": 5000, - "type": "static", - "lb_type": "round_robin", - "hosts": [ - { - "url": "tcp://{{ ip_loopback_address }}:{{ upstream_0 }}" - } - ] - }, - { - "name": "example_issuer", - "connect_timeout_ms": 5000, - "type": "static", - "circuit_breakers": { - "default": { - "max_pending_requests": 10000, - "max_requests": 10000 - } - }, - "lb_type": "round_robin", - "hosts": [ - { - "url": "tcp://{{ ip_loopback_address }}:{{ upstream_1 }}" - } - ] - } - ] - } -} diff --git a/src/envoy/http/jwt_auth/BUILD b/src/envoy/http/jwt_auth/BUILD index a0f80bf43e4..eca24ac187f 100644 --- a/src/envoy/http/jwt_auth/BUILD +++ b/src/envoy/http/jwt_auth/BUILD @@ -70,6 +70,7 @@ envoy_cc_library( deps = [ ":jwt_authenticator_lib", "@envoy//source/exe:envoy_common_lib", + "//src/envoy/utils:filter_names_lib", ], ) @@ -80,6 +81,7 @@ envoy_cc_library( deps = [ ":http_filter_lib", "@envoy//source/exe:envoy_common_lib", + "//src/envoy/utils:filter_names_lib", ], ) diff --git a/src/envoy/http/jwt_auth/auth_store.h b/src/envoy/http/jwt_auth/auth_store.h index bc4286622d9..2410929e4eb 100644 --- a/src/envoy/http/jwt_auth/auth_store.h +++ b/src/envoy/http/jwt_auth/auth_store.h @@ -71,7 +71,7 @@ class JwtAuthStoreFactory : public Logger::Loggable { [this](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr { return std::make_shared(config_); }); - ENVOY_LOG(info, "Loaded JwtAuthConfig: {}", + ENVOY_LOG(debug, "Loaded JwtAuthConfig: {}", MessageUtil::getJsonStringFromMessage(config_, true)); } diff --git a/src/envoy/http/jwt_auth/http_filter.cc b/src/envoy/http/jwt_auth/http_filter.cc index 1b7c35b621a..40a50e9ffb5 100644 --- a/src/envoy/http/jwt_auth/http_filter.cc +++ b/src/envoy/http/jwt_auth/http_filter.cc @@ -18,6 +18,7 @@ #include "common/http/message_impl.h" #include "common/http/utility.h" #include "envoy/http/async_client.h" +#include "src/envoy/utils/filter_names.h" #include #include @@ -31,22 +32,13 @@ JwtVerificationFilter::JwtVerificationFilter(Upstream::ClusterManager& cm, JwtVerificationFilter::~JwtVerificationFilter() {} -void JwtVerificationFilter::onDestroy() { - ENVOY_LOG(debug, "Called JwtVerificationFilter : {}", __func__); - jwt_auth_.onDestroy(); -} +void JwtVerificationFilter::onDestroy() { jwt_auth_.onDestroy(); } FilterHeadersStatus JwtVerificationFilter::decodeHeaders(HeaderMap& headers, bool) { - ENVOY_LOG(debug, "Called JwtVerificationFilter : {}", __func__); state_ = Calling; stopped_ = false; - // Sanitize the JWT verification result in the HTTP headers - // TODO (lei-tang): when the JWT verification result is in a configurable - // header, need to sanitize based on the configuration. - headers.remove(JwtAuth::JwtAuthenticator::JwtPayloadKey()); - // Verify the JWT token, onDone() will be called when completed. jwt_auth_.Verify(headers, this); @@ -59,8 +51,8 @@ FilterHeadersStatus JwtVerificationFilter::decodeHeaders(HeaderMap& headers, } void JwtVerificationFilter::onDone(const JwtAuth::Status& status) { - ENVOY_LOG(debug, "Called JwtVerificationFilter : check complete {}", - int(status)); + ENVOY_LOG(debug, "JwtVerificationFilter::onDone with status {}", + JwtAuth::StatusToString(status)); // This stream has been reset, abort the callback. if (state_ == Responded) { return; @@ -81,8 +73,13 @@ void JwtVerificationFilter::onDone(const JwtAuth::Status& status) { } } +void JwtVerificationFilter::savePayload(const std::string& key, + const std::string& payload) { + decoder_callbacks_->requestInfo().setDynamicMetadata( + Utils::IstioFilterName::kJwt, MessageUtil::keyValueStruct(key, payload)); +} + FilterDataStatus JwtVerificationFilter::decodeData(Buffer::Instance&, bool) { - ENVOY_LOG(debug, "Called JwtVerificationFilter : {}", __func__); if (state_ == Calling) { return FilterDataStatus::StopIterationAndWatermark; } @@ -90,7 +87,6 @@ FilterDataStatus JwtVerificationFilter::decodeData(Buffer::Instance&, bool) { } FilterTrailersStatus JwtVerificationFilter::decodeTrailers(HeaderMap&) { - ENVOY_LOG(debug, "Called JwtVerificationFilter : {}", __func__); if (state_ == Calling) { return FilterTrailersStatus::StopIteration; } @@ -99,7 +95,6 @@ FilterTrailersStatus JwtVerificationFilter::decodeTrailers(HeaderMap&) { void JwtVerificationFilter::setDecoderFilterCallbacks( StreamDecoderFilterCallbacks& callbacks) { - ENVOY_LOG(debug, "Called JwtVerificationFilter : {}", __func__); decoder_callbacks_ = &callbacks; } diff --git a/src/envoy/http/jwt_auth/http_filter.h b/src/envoy/http/jwt_auth/http_filter.h index 838605faa6c..bcf8c583a54 100644 --- a/src/envoy/http/jwt_auth/http_filter.h +++ b/src/envoy/http/jwt_auth/http_filter.h @@ -47,6 +47,10 @@ class JwtVerificationFilter : public StreamDecoderFilter, // To be called when its Verify() call is completed. void onDone(const JwtAuth::Status& status) override; + // the function for JwtAuth::Authenticator::Callbacks interface. + // To be called when Jwt validation success to save payload for future use. + void savePayload(const std::string& key, const std::string& payload) override; + // The callback funcion. StreamDecoderFilterCallbacks* decoder_callbacks_; // The auth object. diff --git a/src/envoy/http/jwt_auth/http_filter_factory.cc b/src/envoy/http/jwt_auth/http_filter_factory.cc index 7c3b11fd86d..06daf039fb0 100644 --- a/src/envoy/http/jwt_auth/http_filter_factory.cc +++ b/src/envoy/http/jwt_auth/http_filter_factory.cc @@ -17,6 +17,7 @@ #include "google/protobuf/util/json_util.h" #include "src/envoy/http/jwt_auth/auth_store.h" #include "src/envoy/http/jwt_auth/http_filter.h" +#include "src/envoy/utils/filter_names.h" using ::istio::envoy::config::filter::http::jwt_auth::v2alpha1:: JwtAuthentication; @@ -46,7 +47,7 @@ class JwtVerificationFilterConfig : public NamedHttpFilterConfigFactory { return ProtobufTypes::MessagePtr{new JwtAuthentication}; } - std::string name() override { return "jwt-auth"; } + std::string name() override { return Utils::IstioFilterName::kJwt; } private: Http::FilterFactoryCb createFilter(const JwtAuthentication& proto_config, diff --git a/src/envoy/http/jwt_auth/integration_test/envoy.conf.jwk b/src/envoy/http/jwt_auth/integration_test/envoy.conf.jwk index 21a8d136fc2..0262d21357e 100644 --- a/src/envoy/http/jwt_auth/integration_test/envoy.conf.jwk +++ b/src/envoy/http/jwt_auth/integration_test/envoy.conf.jwk @@ -46,7 +46,7 @@ "cluster": "example_issuer" } }, - "forward_payload_header": "sec-istio-auth-userinfo" + "forward_payload_header": "test-jwt-payload-output" } ] } diff --git a/src/envoy/http/jwt_auth/integration_test/envoy_allow_missing_or_failed_jwt.conf.jwk b/src/envoy/http/jwt_auth/integration_test/envoy_allow_missing_or_failed_jwt.conf.jwk index 91746d58809..172fa3f7a2c 100644 --- a/src/envoy/http/jwt_auth/integration_test/envoy_allow_missing_or_failed_jwt.conf.jwk +++ b/src/envoy/http/jwt_auth/integration_test/envoy_allow_missing_or_failed_jwt.conf.jwk @@ -45,7 +45,8 @@ "uri": "http://example.com/foobar_cert", "cluster": "example_issuer" } - } + }, + "forward_payload_header": "test-jwt-payload-output" } ], "allow_missing_or_failed": true diff --git a/src/envoy/http/jwt_auth/integration_test/http_filter_integration_test.cc b/src/envoy/http/jwt_auth/integration_test/http_filter_integration_test.cc index d606ee1b9c5..55bd663b95c 100644 --- a/src/envoy/http/jwt_auth/integration_test/http_filter_integration_test.cc +++ b/src/envoy/http/jwt_auth/integration_test/http_filter_integration_test.cc @@ -19,9 +19,10 @@ namespace Envoy { namespace { -// The HTTP header key for the JWT verification result +// The HTTP header key for the JWT verification result. Should be the same as +// the one define for forward_payload_header in envoy.conf.jwk const Http::LowerCaseString kJwtVerificationResultHeaderKey( - "sec-istio-auth-userinfo"); + "test-jwt-payload-output"); // {"iss":"https://example.com","sub":"test@example.com","aud":"example_service","exp":2001001001} const std::string kJwtVerificationResult = "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVz" @@ -135,13 +136,14 @@ class JwtVerificationFilterIntegrationTest // Empty issuer_response_body indicates issuer will not be called. // Mock a response from an issuer server. if (!issuer_response_body.empty()) { - fake_upstream_connection_issuer = - fake_upstreams_[1]->waitForHttpConnection(*dispatcher_); - request_stream_issuer = - fake_upstream_connection_issuer->waitForNewStream(*dispatcher_); - request_stream_issuer->waitForEndStream(*dispatcher_); - - request_stream_issuer->encodeHeaders(issuer_response_headers, false); + ASSERT_TRUE(fake_upstreams_[1]->waitForHttpConnection( + *dispatcher_, fake_upstream_connection_issuer)); + ASSERT_TRUE(fake_upstream_connection_issuer->waitForNewStream( + *dispatcher_, request_stream_issuer)); + ASSERT_TRUE(request_stream_issuer->waitForEndStream(*dispatcher_)); + + request_stream_issuer->encodeHeaders( + Http::HeaderMapImpl(issuer_response_headers), false); Buffer::OwnedImpl body(issuer_response_body); request_stream_issuer->encodeData(body, true); } @@ -149,12 +151,11 @@ class JwtVerificationFilterIntegrationTest // Valid JWT case. // Check if the request sent to the backend includes the expected one. if (verification_success) { - fake_upstream_connection_backend = - fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); - request_stream_backend = - fake_upstream_connection_backend->waitForNewStream(*dispatcher_); - request_stream_backend->waitForEndStream(*dispatcher_); - + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection( + *dispatcher_, fake_upstream_connection_backend)); + ASSERT_TRUE(fake_upstream_connection_backend->waitForNewStream( + *dispatcher_, request_stream_backend)); + ASSERT_TRUE(request_stream_backend->waitForEndStream(*dispatcher_)); EXPECT_TRUE(request_stream_backend->complete()); ExpectHeaderIncluded(expected_headers, request_stream_backend->headers()); @@ -179,12 +180,12 @@ class JwtVerificationFilterIntegrationTest codec_client->close(); if (!issuer_response_body.empty()) { - fake_upstream_connection_issuer->close(); - fake_upstream_connection_issuer->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection_issuer->close()); + ASSERT_TRUE(fake_upstream_connection_issuer->waitForDisconnect()); } if (verification_success) { - fake_upstream_connection_backend->close(); - fake_upstream_connection_backend->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection_backend->close()); + ASSERT_TRUE(fake_upstream_connection_backend->waitForDisconnect()); } } @@ -259,11 +260,13 @@ TEST_P(JwtVerificationFilterIntegrationTestWithJwks, RSASuccess1) { "xfP590ACPyXrivtsxg"; auto expected_headers = BaseRequestHeaders(); - expected_headers.addCopy( - "sec-istio-auth-userinfo", - "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVz" - "dEBleGFtcGxlLmNvbSIsImF1ZCI6ImV4YW1wbGVfc2VydmljZSIs" - "ImV4cCI6MjAwMTAwMTAwMX0"); + // TODO: JWT payload is not longer output to header. Find way to verify that + // data equivalent to this is added to dynamicMetadata. + // expected_headers.addCopy( + // kJwtVerificationResultHeaderKey, + // "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVz" + // "dEBleGFtcGxlLmNvbSIsImF1ZCI6ImV4YW1wbGVfc2VydmljZSIs" + // "ImV4cCI6MjAwMTAwMTAwMX0"); TestVerification(createHeaders(kJwtNoKid), "", createIssuerHeaders(), kPublicKeyRSA, true, expected_headers, ""); @@ -282,11 +285,13 @@ TEST_P(JwtVerificationFilterIntegrationTestWithJwks, ES256Success1) { "T9ubWvRvNGGYOTuJ8T17Db68Qk3T8UNTK5lzfR_mw"; auto expected_headers = BaseRequestHeaders(); - expected_headers.addCopy("sec-istio-auth-userinfo", - "eyJpc3MiOiJo" - "dHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtc" - "GxlLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiZXhhbX" - "BsZV9zZXJ2aWNlIn0"); + // TODO: JWT payload is not longer output to header. Find way to verify that + // data equivalent to this is added to dynamicMetadata. + // expected_headers.addCopy(kJwtVerificationResultHeaderKey, + // "eyJpc3MiOiJo" + // "dHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtc" + // "GxlLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiZXhhbX" + // "BsZV9zZXJ2aWNlIn0"); TestVerification(createHeaders(kJwtEC), "", createIssuerHeaders(), kPublicKeyEC, true, expected_headers, ""); @@ -372,22 +377,17 @@ TEST_P(JwtVerificationFilterIntegrationTestWithInjectedJwtResult, codec_client = makeHttpConnection(lookupPort("http")); // Send a request to Envoy. response = codec_client->makeHeaderOnlyRequest(headers); - fake_upstream_connection_backend = - fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); - request_stream_backend = - fake_upstream_connection_backend->waitForNewStream(*dispatcher_); - request_stream_backend->waitForEndStream(*dispatcher_); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection( + *dispatcher_, fake_upstream_connection_backend)); + ASSERT_TRUE(fake_upstream_connection_backend->waitForNewStream( + *dispatcher_, request_stream_backend)); + ASSERT_TRUE(request_stream_backend->waitForEndStream(*dispatcher_)); EXPECT_TRUE(request_stream_backend->complete()); - // With sanitization, the headers received by the backend should not - // contain the injected JWT verification header. - EXPECT_TRUE(request_stream_backend->headers().get( - kJwtVerificationResultHeaderKey) == nullptr); - response->waitForEndStream(); codec_client->close(); - fake_upstream_connection_backend->close(); - fake_upstream_connection_backend->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection_backend->close()); + ASSERT_TRUE(fake_upstream_connection_backend->waitForDisconnect()); } } // namespace Envoy diff --git a/src/envoy/http/jwt_auth/jwt_authenticator.cc b/src/envoy/http/jwt_auth/jwt_authenticator.cc index b535d31f9a3..c47b8b6d036 100644 --- a/src/envoy/http/jwt_auth/jwt_authenticator.cc +++ b/src/envoy/http/jwt_auth/jwt_authenticator.cc @@ -195,17 +195,11 @@ void JwtAuthenticator::VerifyKey(const PubkeyCacheItem& issuer_item) { return; } - // TODO(lei-tang): remove this backward compatibility. - // Tracking issue: https://github.com/istio/istio/issues/4744 - headers_->addReferenceKey(kJwtPayloadKey, jwt_->PayloadStrBase64Url()); - - if (!issuer_item.jwt_config().forward_payload_header().empty()) { - const LowerCaseString key( - issuer_item.jwt_config().forward_payload_header()); - if (key.get() != kJwtPayloadKey.get()) { - headers_->addCopy(key, jwt_->PayloadStrBase64Url()); - } - } + // TODO: can we save as proto or json object directly? + // User the issuer as the entry key for simplicity. The forward_payload_header + // field can be removed or replace by a boolean (to make `save` is + // conditional) + callback_->savePayload(issuer_item.jwt_config().issuer(), jwt_->PayloadStr()); if (!issuer_item.jwt_config().forward()) { // Remove JWT from headers. diff --git a/src/envoy/http/jwt_auth/jwt_authenticator.h b/src/envoy/http/jwt_auth/jwt_authenticator.h index 748b5752799..2796cf1ce01 100644 --- a/src/envoy/http/jwt_auth/jwt_authenticator.h +++ b/src/envoy/http/jwt_auth/jwt_authenticator.h @@ -36,6 +36,8 @@ class JwtAuthenticator : public Logger::Loggable, public: virtual ~Callbacks() {} virtual void onDone(const Status& status) PURE; + virtual void savePayload(const std::string& key, + const std::string& payload) PURE; }; void Verify(HeaderMap& headers, Callbacks* callback); diff --git a/src/envoy/http/jwt_auth/jwt_authenticator_test.cc b/src/envoy/http/jwt_auth/jwt_authenticator_test.cc index b5ec6ca974f..fd35224e7fe 100644 --- a/src/envoy/http/jwt_auth/jwt_authenticator_test.cc +++ b/src/envoy/http/jwt_auth/jwt_authenticator_test.cc @@ -22,9 +22,9 @@ using ::istio::envoy::config::filter::http::jwt_auth::v2alpha1:: JwtAuthentication; +using ::testing::_; using ::testing::Invoke; using ::testing::NiceMock; -using ::testing::_; namespace Envoy { namespace Http { @@ -88,6 +88,8 @@ const std::string kPublicKey = " \"kid\": \"b3319a147514df7ee5e4bcdee51350cc890cc89e\"" "}]}"; +// Keep this same as issuer field in the config below. +const char kJwtIssuer[] = "https://example.com"; // A good JSON config. const char kExampleConfig[] = R"( { @@ -108,7 +110,7 @@ const char kExampleConfig[] = R"( "seconds": 600 } }, - "forward_payload_header": "sec-istio-auth-userinfo" + "forward_payload_header": "test-output" } ] } @@ -214,6 +216,12 @@ const std::string kGoodToken = "EprqSZUzi_ZzzYzqBNVhIJujcNWij7JRra2sXXiSAfKjtxHQoxrX8n4V1ySWJ3_1T" "H_cJcdfS_RKP7YgXRWC0L16PNF5K7iqRqmjKALNe83ZFnFIw"; +// Payload output for kGoodToken. +const std::string kGoodTokenPayload = + "{\"iss\":\"https://" + "example.com\",\"sub\":\"test@example.com\",\"exp\":2001001001," + "\"aud\":\"example_service\"}"; + // Payload: // {"iss":"https://example.com","sub":"test@example.com","aud":"http://example_service/","exp":2001001001} const std::string kGoodTokenAudHasProtocolScheme = @@ -226,6 +234,12 @@ const std::string kGoodTokenAudHasProtocolScheme = "EpfDR3lAXSaug1NE22zX_tm0d9JnC5ZrIk3kwmPJPrnAS2_9RKTQW2e2skpAT8dUV" "T5aSpQxJmWIkyp4PKWmH6h4H2INS7hWyASZdX4oW-R0PMy3FAd8D6Y8740A"; +// Payload output for kGoodTokenAudHasProtocolScheme. +const std::string kGoodTokenAudHasProtocolSchemePayload = + "{\"iss\":\"https://" + "example.com\",\"sub\":\"test@example.com\",\"exp\":2001001001," + "\"aud\":\"http://example_service/\"}"; + // Payload: // {"iss":"https://example.com","sub":"test@example.com","aud":"https://example_service1/","exp":2001001001} const std::string kGoodTokenAudService1 = @@ -238,6 +252,12 @@ const std::string kGoodTokenAudService1 = "nzvwse8DINafa5kOhBmQcrIADiOyTVC1IqcOvaftVcS4MTkTeCyzfsqcNQ-VeNPKY" "3e6wTe9brxbii-IPZFNY-1osQNnfCtYpEDjfvMjwHTielF-b55xq_tUwuqaaQ"; +// Payload output for kGoodTokenAudService1. +const std::string kGoodTokenAudService1Payload = + "{\"iss\":\"https://" + "example.com\",\"sub\":\"test@example.com\",\"exp\":2001001001," + "\"aud\":\"https://example_service1/\"}"; + // Payload: // {"iss":"https://example.com","sub":"test@example.com","aud":"http://example_service2","exp":2001001001} const std::string kGoodTokenAudService2 = @@ -250,11 +270,18 @@ const std::string kGoodTokenAudService2 = "Thwt7f3DTmHOMBeO_3xrLOOZgNtuXipqupkp9sb-DcCRdSokoFpGSTibvV_8RwkQo" "W2fdqw_ZD7WOe4sTcK27Uma9exclisHVxzJJbQOW82WdPQGicYaR_EajYzA"; +// Payload output for kGoodTokenAudService2. +const std::string kGoodTokenAudService2Payload = + "{\"iss\":\"https://" + "example.com\",\"sub\":\"test@example.com\",\"exp\":2001001001," + "\"aud\":\"http://example_service2\"}"; } // namespace class MockJwtAuthenticatorCallbacks : public JwtAuthenticator::Callbacks { public: MOCK_METHOD1(onDone, void(const Status &status)); + MOCK_METHOD2(savePayload, + void(const std::string &key, const std::string &payload)); }; class JwtAuthenticatorTest : public ::testing::Test { @@ -316,13 +343,9 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTandCache) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); - + EXPECT_CALL(mock_cb, savePayload(kJwtIssuer, kGoodTokenPayload)); auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_("sec-istio-auth-userinfo"), - "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcG" - "xlLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiZXhhbXBsZV9zZXJ2" - "aWNlIn0"); // Verify the token is removed. EXPECT_FALSE(headers.Authorization()); } @@ -347,13 +370,10 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTPubkeyNoAlg) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); + EXPECT_CALL(mock_cb, savePayload(kJwtIssuer, kGoodTokenPayload)); auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_("sec-istio-auth-userinfo"), - "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcG" - "xlLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiZXhhbXBsZV9zZXJ2" - "aWNlIn0"); // Verify the token is removed. EXPECT_FALSE(headers.Authorization()); @@ -380,13 +400,10 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTPubkeyNoKid) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); + EXPECT_CALL(mock_cb, savePayload(kJwtIssuer, kGoodTokenPayload)); auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_("sec-istio-auth-userinfo"), - "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcG" - "xlLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiZXhhbXBsZV9zZXJ2" - "aWNlIn0"); // Verify the token is removed. EXPECT_FALSE(headers.Authorization()); @@ -406,13 +423,11 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTAudService) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); + EXPECT_CALL(mock_cb, + savePayload(kJwtIssuer, kGoodTokenAudHasProtocolSchemePayload)); auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_("sec-istio-auth-userinfo"), - "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcGx" - "lLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiaHR0cDovL2V4YW1wbG" - "Vfc2VydmljZS8ifQ"); // Verify the token is removed. EXPECT_FALSE(headers.Authorization()); @@ -432,13 +447,10 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTAudService1) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); + EXPECT_CALL(mock_cb, savePayload(kJwtIssuer, kGoodTokenAudService1Payload)); auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_("sec-istio-auth-userinfo"), - "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcGx" - "lLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiaHR0cHM6Ly9leGFtcG" - "xlX3NlcnZpY2UxLyJ9"); // Verify the token is removed. EXPECT_FALSE(headers.Authorization()); @@ -458,13 +470,10 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTAudService2) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); + EXPECT_CALL(mock_cb, savePayload(kJwtIssuer, kGoodTokenAudService2Payload)); auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_("sec-istio-auth-userinfo"), - "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcGx" - "lLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiaHR0cDovL2V4YW1wbG" - "Vfc2VydmljZTIifQ"); // Verify the token is removed. EXPECT_FALSE(headers.Authorization()); @@ -487,6 +496,7 @@ TEST_F(JwtAuthenticatorTest, TestForwardJwt) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); + EXPECT_CALL(mock_cb, savePayload(kJwtIssuer, kGoodTokenPayload)); auth_->Verify(headers, &mock_cb); @@ -718,7 +728,9 @@ TEST_F(JwtAuthenticatorTest, TestOnDestroy) { } TEST_F(JwtAuthenticatorTest, TestNoForwardPayloadHeader) { - // In this config, there is no forward_payload_header + // The flag (forward_payload_header) is deprecated and have no impact. The + // current behavior is always save JWT payload to request info (dynamic + // metadata). In this config, there is no forward_payload_header. SetupConfig(kExampleConfigWithoutForwardPayloadHeader); MockUpstream mock_pubkey(mock_cm_, kPublicKey); auto headers = TestHeaderMapImpl{{"Authorization", "Bearer " + kGoodToken}}; @@ -726,13 +738,12 @@ TEST_F(JwtAuthenticatorTest, TestNoForwardPayloadHeader) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); + // Note savePayload is still being called, as explain above. + EXPECT_CALL(mock_cb, savePayload(kJwtIssuer, kGoodTokenPayload)); auth_->Verify(headers, &mock_cb); - // Test when forward_payload_header is not set, the output should still - // contain the sec-istio-auth-userinfo header for backward compatibility. - EXPECT_TRUE(headers.has("sec-istio-auth-userinfo")); - // In addition, the sec-istio-auth-userinfo header should be the only header - EXPECT_EQ(headers.size(), 1); + // Test when forward_payload_header is not set, nothing added to headers. + EXPECT_EQ(headers.size(), 0); } TEST_F(JwtAuthenticatorTest, TestInlineJwks) { @@ -753,6 +764,7 @@ TEST_F(JwtAuthenticatorTest, TestInlineJwks) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); + EXPECT_CALL(mock_cb, savePayload(kJwtIssuer, kGoodTokenPayload)); auth_->Verify(headers, &mock_cb); EXPECT_EQ(mock_pubkey.called_count(), 0); diff --git a/src/envoy/http/jwt_auth/token_extractor_test.cc b/src/envoy/http/jwt_auth/token_extractor_test.cc index f4befee63b3..8c5dac0d69f 100644 --- a/src/envoy/http/jwt_auth/token_extractor_test.cc +++ b/src/envoy/http/jwt_auth/token_extractor_test.cc @@ -19,9 +19,9 @@ using ::istio::envoy::config::filter::http::jwt_auth::v2alpha1:: JwtAuthentication; +using ::testing::_; using ::testing::Invoke; using ::testing::NiceMock; -using ::testing::_; namespace Envoy { namespace Http { diff --git a/src/envoy/http/mixer/README.md b/src/envoy/http/mixer/README.md index d4bcb57f792..efd37c73fb5 100644 --- a/src/envoy/http/mixer/README.md +++ b/src/envoy/http/mixer/README.md @@ -12,7 +12,7 @@ Please make sure to modify both the original one and the copy together if necess ## Build Mixer server * Follow https://github.com/istio/istio/blob/master/mixer/doc/running-local-mixer.md to run a Mixer server locally. -Follow https://github.com/istio/istio/blob/master/DEV-GUIDE.md to build Istio, which includes building Mixer server. +Follow https://github.com/istio/istio/wiki/Dev-Guide to build Istio, which includes building Mixer server. ## Build Envoy proxy diff --git a/src/envoy/http/mixer/check_data.cc b/src/envoy/http/mixer/check_data.cc index 4726a4be7ae..0cc991e1381 100644 --- a/src/envoy/http/mixer/check_data.cc +++ b/src/envoy/http/mixer/check_data.cc @@ -38,8 +38,9 @@ const std::set RequestHeaderExclusives = { } // namespace CheckData::CheckData(const HeaderMap& headers, + const envoy::api::v2::core::Metadata& metadata, const Network::Connection* connection) - : headers_(headers), connection_(connection) { + : headers_(headers), metadata_(metadata), connection_(connection) { if (headers_.Path()) { query_params_ = Utility::parseQueryString(std::string( headers_.Path()->value().c_str(), headers_.Path()->value().size())); @@ -65,8 +66,8 @@ bool CheckData::GetSourceIpPort(std::string* ip, int* port) const { return false; } -bool CheckData::GetSourceUser(std::string* user) const { - return Utils::GetSourceUser(connection_, user); +bool CheckData::GetPrincipal(bool peer, std::string* user) const { + return Utils::GetPrincipal(connection_, peer, user); } std::map CheckData::GetRequestHeaders() const { @@ -77,6 +78,10 @@ std::map CheckData::GetRequestHeaders() const { bool CheckData::IsMutualTLS() const { return Utils::IsMutualTLS(connection_); } +bool CheckData::GetRequestedServerName(std::string* name) const { + return Utils::GetRequestedServerName(connection_, name); +} + bool CheckData::FindHeaderByType(HttpCheckData::HeaderType header_type, std::string* value) const { switch (header_type) { @@ -162,42 +167,31 @@ bool CheckData::FindCookie(const std::string& name, std::string* value) const { return false; } -bool CheckData::GetJWTPayload( - std::map* payload) const { - const HeaderEntry* entry = - headers_.get(JwtAuth::JwtAuthenticator::JwtPayloadKey()); - if (!entry) { - return false; - } - std::string value(entry->value().c_str(), entry->value().size()); - std::string payload_str = JwtAuth::Base64UrlDecode(value); - // Return an empty string if Base64 decode fails. - if (payload_str.empty()) { - ENVOY_LOG(error, "Invalid {} header, invalid base64: {}", - JwtAuth::JwtAuthenticator::JwtPayloadKey().get(), value); +const ::google::protobuf::Struct* CheckData::GetAuthenticationResult() const { + return Utils::Authentication::GetResultFromMetadata(metadata_); +} + +bool CheckData::GetUrlPath(std::string* url_path) const { + if (!headers_.Path()) { return false; } - try { - auto json_obj = Json::Factory::loadFromString(payload_str); - json_obj->iterate( - [payload](const std::string& key, const Json::Object& obj) -> bool { - // will throw execption if value type is not string. - try { - (*payload)[key] = obj.asString(); - } catch (...) { - } - return true; - }); - } catch (...) { - ENVOY_LOG(error, "Invalid {} header, invalid json: {}", - JwtAuth::JwtAuthenticator::JwtPayloadKey().get(), payload_str); - return false; + const HeaderString& path = headers_.Path()->value(); + const char* query_start = Utility::findQueryStringStart(path); + if (query_start != nullptr) { + *url_path = std::string(path.c_str(), query_start - path.c_str()); + } else { + *url_path = std::string(path.c_str(), path.size()); } return true; } -bool CheckData::GetAuthenticationResult(istio::authn::Result* result) const { - return Utils::Authentication::FetchResultFromHeader(headers_, result); +bool CheckData::GetRequestQueryParams( + std::map* query_params) const { + if (!headers_.Path()) { + return false; + } + *query_params = Utility::parseQueryString(headers_.Path()->value().c_str()); + return true; } } // namespace Mixer diff --git a/src/envoy/http/mixer/check_data.h b/src/envoy/http/mixer/check_data.h index b03c3a1e7f2..88a36ad41f9 100644 --- a/src/envoy/http/mixer/check_data.h +++ b/src/envoy/http/mixer/check_data.h @@ -17,7 +17,9 @@ #include "common/common/logger.h" #include "common/http/utility.h" +#include "envoy/api/v2/core/base.pb.h" #include "envoy/http/header_map.h" +#include "google/protobuf/struct.pb.h" #include "include/istio/control/http/controller.h" #include "src/istio/authn/context.pb.h" @@ -28,7 +30,9 @@ namespace Mixer { class CheckData : public ::istio::control::http::CheckData, public Logger::Loggable { public: - CheckData(const HeaderMap& headers, const Network::Connection* connection); + CheckData(const HeaderMap& headers, + const envoy::api::v2::core::Metadata& metadata, + const Network::Connection* connection); // Find "x-istio-attributes" headers, if found base64 decode // its value and remove it from the headers. @@ -36,12 +40,14 @@ class CheckData : public ::istio::control::http::CheckData, bool GetSourceIpPort(std::string* ip, int* port) const override; - bool GetSourceUser(std::string* user) const override; + bool GetPrincipal(bool peer, std::string* user) const override; std::map GetRequestHeaders() const override; bool IsMutualTLS() const override; + bool GetRequestedServerName(std::string* name) const override; + bool FindHeaderByType( ::istio::control::http::CheckData::HeaderType header_type, std::string* value) const override; @@ -54,13 +60,16 @@ class CheckData : public ::istio::control::http::CheckData, bool FindCookie(const std::string& name, std::string* value) const override; - bool GetJWTPayload( - std::map* payload) const override; + const ::google::protobuf::Struct* GetAuthenticationResult() const override; + + bool GetUrlPath(std::string* url_path) const override; - bool GetAuthenticationResult(istio::authn::Result* result) const override; + bool GetRequestQueryParams( + std::map* query_params) const override; private: const HeaderMap& headers_; + const envoy::api::v2::core::Metadata& metadata_; const Network::Connection* connection_; Utility::QueryParams query_params_; }; diff --git a/src/envoy/http/mixer/filter.cc b/src/envoy/http/mixer/filter.cc index ca08d36e145..8c55fd4552a 100644 --- a/src/envoy/http/mixer/filter.cc +++ b/src/envoy/http/mixer/filter.cc @@ -131,7 +131,9 @@ FilterHeadersStatus Filter::decodeHeaders(HeaderMap& headers, bool) { state_ = Calling; initiating_call_ = true; - CheckData check_data(headers, decoder_callbacks_->connection()); + CheckData check_data(headers, + decoder_callbacks_->requestInfo().dynamicMetadata(), + decoder_callbacks_->connection()); Utils::HeaderUpdate header_update(&headers); headers_ = &headers; cancel_check_ = handler_->Check( @@ -209,11 +211,6 @@ void Filter::completeCheck(const CheckResponseInfo& info) { auto status = info.response_status; ENVOY_LOG(debug, "Called Mixer::Filter : check complete {}", status.ToString()); - // Remove Istio authentication header after Check() is completed - if (nullptr != headers_) { - Envoy::Utils::Authentication::ClearResultInHeader(headers_); - } - // This stream has been reset, abort the callback. if (state_ == Responded) { return; @@ -281,7 +278,8 @@ void Filter::log(const HeaderMap* request_headers, ReadPerRouteConfig(request_info.routeEntry(), &config); handler_ = control_.controller()->CreateRequestHandler(config); - CheckData check_data(*request_headers, nullptr); + CheckData check_data(*request_headers, request_info.dynamicMetadata(), + nullptr); handler_->ExtractRequestAttributes(&check_data); } // response trailer header is not counted to response total size. diff --git a/src/envoy/http/mixer/report_data.h b/src/envoy/http/mixer/report_data.h index b3c8ee0a38b..86e9a8e3d62 100644 --- a/src/envoy/http/mixer/report_data.h +++ b/src/envoy/http/mixer/report_data.h @@ -15,8 +15,11 @@ #pragma once +#include "common/common/logger.h" +#include "common/request_info/utility.h" #include "envoy/http/header_map.h" #include "envoy/request_info/request_info.h" +#include "extensions/filters/http/well_known_names.h" #include "include/istio/control/http/controller.h" #include "src/envoy/utils/utils.h" @@ -24,6 +27,9 @@ namespace Envoy { namespace Http { namespace Mixer { namespace { +const std::string kRbacPermissivePolicyIDField = "shadow_effective_policyID"; +const std::string kRbacPermissiveRespCodeField = "shadow_response_code"; + // Set of headers excluded from response.headers attribute. const std::set ResponseHeaderExclusives = {}; @@ -43,7 +49,8 @@ bool ExtractGrpcStatus(const HeaderMap *headers, } // namespace -class ReportData : public ::istio::control::http::ReportData { +class ReportData : public ::istio::control::http::ReportData, + public Logger::Loggable { const HeaderMap *headers_; const HeaderMap *trailers_; const RequestInfo::RequestInfo &info_; @@ -88,6 +95,8 @@ class ReportData : public ::istio::control::http::ReportData { // responseCode is for the backend response. If it is not valid, the request // is rejected by Envoy. Set the response code for such requests as 500. data->response_code = info_.responseCode().value_or(500); + + data->response_flags = RequestInfo::ResponseFlagUtils::toShortString(info_); } bool GetDestinationIpPort(std::string *str_ip, int *port) const override { @@ -100,7 +109,7 @@ class ReportData : public ::istio::control::http::ReportData { bool GetDestinationUID(std::string *uid) const override { if (info_.upstreamHost()) { - return Utils::GetDestinationUID(info_.upstreamHost()->metadata(), uid); + return Utils::GetDestinationUID(*info_.upstreamHost()->metadata(), uid); } return false; } @@ -111,6 +120,42 @@ class ReportData : public ::istio::control::http::ReportData { return ExtractGrpcStatus(trailers_, status) || ExtractGrpcStatus(headers_, status); } + + // Get Rbac related attributes. + bool GetRbacReportInfo(RbacReportInfo *report_info) const override { + const auto filter_meta = info_.dynamicMetadata().filter_metadata(); + const auto filter_it = + filter_meta.find(Extensions::HttpFilters::HttpFilterNames::get().Rbac); + if (filter_it == filter_meta.end()) { + ENVOY_LOG(debug, "No dynamic_metadata found for filter {}", + Extensions::HttpFilters::HttpFilterNames::get().Rbac); + return false; + } + + const auto &data_struct = filter_it->second; + const auto resp_code_it = + data_struct.fields().find(kRbacPermissiveRespCodeField); + if (resp_code_it != data_struct.fields().end()) { + report_info->permissive_resp_code = resp_code_it->second.string_value(); + } else { + ENVOY_LOG(debug, "No {} field found in filter {} dynamic_metadata", + kRbacPermissiveRespCodeField, + Extensions::HttpFilters::HttpFilterNames::get().Rbac); + } + + const auto policy_id_it = + data_struct.fields().find(kRbacPermissivePolicyIDField); + if (policy_id_it != data_struct.fields().end()) { + report_info->permissive_policy_id = policy_id_it->second.string_value(); + } else { + ENVOY_LOG(debug, "No {} field found in filter {} dynamic_metadata", + kRbacPermissivePolicyIDField, + Extensions::HttpFilters::HttpFilterNames::get().Rbac); + } + + return !report_info->permissive_resp_code.empty() || + !report_info->permissive_policy_id.empty(); + } }; } // namespace Mixer diff --git a/src/envoy/tcp/mixer/filter.cc b/src/envoy/tcp/mixer/filter.cc index 128e06ef715..105db9d19b6 100644 --- a/src/envoy/tcp/mixer/filter.cc +++ b/src/envoy/tcp/mixer/filter.cc @@ -117,6 +117,7 @@ void Filter::completeCheck(const Status& status) { if (!calling_check_) { filter_callbacks_->continueReading(); } + handler_->Report(this, ConnectionEvent::OPEN); report_timer_ = control_.dispatcher().createTimer([this]() { OnReportTimer(); }); report_timer_->enableTimer(control_.config().report_interval_ms()); @@ -139,7 +140,7 @@ void Filter::onEvent(Network::ConnectionEvent event) { if (report_timer_) { report_timer_->disableTimer(); } - handler_->Report(this, /* is_final_report */ true); + handler_->Report(this, ConnectionEvent::CLOSE); } cancelCheck(); } @@ -149,14 +150,18 @@ bool Filter::GetSourceIpPort(std::string* str_ip, int* port) const { return Utils::GetIpPort(filter_callbacks_->connection().remoteAddress()->ip(), str_ip, port); } -bool Filter::GetSourceUser(std::string* user) const { - return Utils::GetSourceUser(&filter_callbacks_->connection(), user); +bool Filter::GetPrincipal(bool peer, std::string* user) const { + return Utils::GetPrincipal(&filter_callbacks_->connection(), peer, user); } bool Filter::IsMutualTLS() const { return Utils::IsMutualTLS(&filter_callbacks_->connection()); } +bool Filter::GetRequestedServerName(std::string* name) const { + return Utils::GetRequestedServerName(&filter_callbacks_->connection(), name); +} + bool Filter::GetDestinationIpPort(std::string* str_ip, int* port) const { if (filter_callbacks_->upstreamHost() && filter_callbacks_->upstreamHost()->address()) { @@ -168,7 +173,7 @@ bool Filter::GetDestinationIpPort(std::string* str_ip, int* port) const { bool Filter::GetDestinationUID(std::string* uid) const { if (filter_callbacks_->upstreamHost()) { return Utils::GetDestinationUID( - filter_callbacks_->upstreamHost()->metadata(), uid); + *filter_callbacks_->upstreamHost()->metadata(), uid); } return false; } @@ -189,7 +194,7 @@ std::string Filter::GetConnectionId() const { } void Filter::OnReportTimer() { - handler_->Report(this, /* is_final_report */ false); + handler_->Report(this, ConnectionEvent::CONTINUE); report_timer_->enableTimer(control_.config().report_interval_ms()); } diff --git a/src/envoy/tcp/mixer/filter.h b/src/envoy/tcp/mixer/filter.h index 2fa8b8fe382..a06daa9f852 100644 --- a/src/envoy/tcp/mixer/filter.h +++ b/src/envoy/tcp/mixer/filter.h @@ -51,8 +51,9 @@ class Filter : public Network::Filter, // CheckData virtual functions. bool GetSourceIpPort(std::string* str_ip, int* port) const override; - bool GetSourceUser(std::string* user) const override; + bool GetPrincipal(bool peer, std::string* user) const override; bool IsMutualTLS() const override; + bool GetRequestedServerName(std::string* name) const override; // ReportData virtual functions. bool GetDestinationIpPort(std::string* str_ip, int* port) const override; diff --git a/src/envoy/utils/BUILD b/src/envoy/utils/BUILD index 4b30761be45..adf671a1b20 100644 --- a/src/envoy/utils/BUILD +++ b/src/envoy/utils/BUILD @@ -35,6 +35,7 @@ envoy_cc_library( "//include/istio/utils:attribute_names_header", "//src/istio/authn:context_proto", "//src/istio/utils:attribute_names_lib", + ":filter_names_lib", "@envoy//source/exe:envoy_common_lib", ], ) @@ -89,3 +90,15 @@ envoy_cc_test( "@envoy//test/test_common:utility_lib", ], ) + + +cc_library( + name = "filter_names_lib", + srcs = [ + "filter_names.cc", + ], + hdrs = [ + "filter_names.h", + ], + visibility = ["//visibility:public"], +) \ No newline at end of file diff --git a/src/envoy/utils/authn.cc b/src/envoy/utils/authn.cc index a9b03bd67df..a28f8ceba8f 100644 --- a/src/envoy/utils/authn.cc +++ b/src/envoy/utils/authn.cc @@ -16,6 +16,7 @@ #include "src/envoy/utils/authn.h" #include "common/common/base64.h" #include "include/istio/utils/attribute_names.h" +#include "src/envoy/utils/filter_names.h" #include "src/istio/authn/context.pb.h" using istio::authn::Result; @@ -24,10 +25,6 @@ namespace Envoy { namespace Utils { namespace { -// The HTTP header to save authentication result. -const Http::LowerCaseString kAuthenticationOutputHeaderLocation( - "sec-istio-authn-payload"); - // Helper function to set a key/value pair into Struct. static void setKeyValue(::google::protobuf::Struct& data, std::string key, std::string value) { @@ -36,23 +33,9 @@ static void setKeyValue(::google::protobuf::Struct& data, std::string key, } // namespace -bool Authentication::SaveResultToHeader(const istio::authn::Result& result, - Http::HeaderMap* headers) { - if (HasResultInHeader(*headers)) { - ENVOY_LOG(warn, - "Authentication result already exist in header. Cannot save"); - return false; - } - - std::string payload_data; - result.SerializeToString(&payload_data); - headers->addCopy(kAuthenticationOutputHeaderLocation, - Base64::encode(payload_data.c_str(), payload_data.size())); - return true; -} - void Authentication::SaveAuthAttributesToStruct( const istio::authn::Result& result, ::google::protobuf::Struct& data) { + // TODO(diemvu): Refactor istio::authn::Result this conversion can be removed. if (!result.principal().empty()) { setKeyValue(data, istio::utils::AttributeName::kRequestAuthPrincipal, result.principal()); @@ -74,6 +57,15 @@ void Authentication::SaveAuthAttributesToStruct( setKeyValue(data, istio::utils::AttributeName::kRequestAuthAudiences, origin.audiences(0)); } + if (!origin.groups().empty()) { + ::google::protobuf::ListValue* value; + value = (*data.mutable_fields()) + [istio::utils::AttributeName::kRequestAuthGroups] + .mutable_list_value(); + for (int i = 0; i < origin.groups().size(); i++) { + value->add_values()->set_string_value(origin.groups(i)); + } + } if (!origin.presenter().empty()) { setKeyValue(data, istio::utils::AttributeName::kRequestAuthPresenter, origin.presenter()); @@ -93,26 +85,14 @@ void Authentication::SaveAuthAttributesToStruct( } } -bool Authentication::FetchResultFromHeader(const Http::HeaderMap& headers, - istio::authn::Result* result) { - const auto entry = headers.get(kAuthenticationOutputHeaderLocation); - if (entry == nullptr) { - return false; +const ProtobufWkt::Struct* Authentication::GetResultFromMetadata( + const envoy::api::v2::core::Metadata& metadata) { + const auto& iter = + metadata.filter_metadata().find(Utils::IstioFilterName::kAuthentication); + if (iter == metadata.filter_metadata().end()) { + return nullptr; } - std::string value(entry->value().c_str(), entry->value().size()); - return result->ParseFromString(Base64::decode(value)); -} - -void Authentication::ClearResultInHeader(Http::HeaderMap* headers) { - headers->remove(kAuthenticationOutputHeaderLocation); -} - -bool Authentication::HasResultInHeader(const Http::HeaderMap& headers) { - return headers.get(kAuthenticationOutputHeaderLocation) != nullptr; -} - -const Http::LowerCaseString& Authentication::GetHeaderLocation() { - return kAuthenticationOutputHeaderLocation; + return &(iter->second); } } // namespace Utils diff --git a/src/envoy/utils/authn.h b/src/envoy/utils/authn.h index 395eb88a817..dad2300041f 100644 --- a/src/envoy/utils/authn.h +++ b/src/envoy/utils/authn.h @@ -14,7 +14,8 @@ */ #include "common/common/logger.h" -#include "envoy/http/header_map.h" +#include "common/protobuf/protobuf.h" +#include "envoy/api/v2/core/base.pb.h" #include "google/protobuf/struct.pb.h" #include "src/istio/authn/context.pb.h" @@ -23,36 +24,16 @@ namespace Utils { class Authentication : public Logger::Loggable { public: - // Saves (authentication) result to header with proper encoding (base64). The - // location is internal implementation detail, and is chosen to avoid possible - // collision. If header already contains data in that location, function will - // returns false and data is *not* overwritten. - static bool SaveResultToHeader(const istio::authn::Result& result, - Http::HeaderMap* headers); - // Save authentication attributes into the data Struct. static void SaveAuthAttributesToStruct(const istio::authn::Result& result, ::google::protobuf::Struct& data); - // Looks up authentication result data in the header. If data is available, - // decodes and output result proto. Returns false if data is not available, or - // in bad format. - static bool FetchResultFromHeader(const Http::HeaderMap& headers, - istio::authn::Result* result); - - // Clears authentication result in header, if exist. - static void ClearResultInHeader(Http::HeaderMap* headers); - - // Returns true if there is header entry at thelocation that is used to store - // authentication result. (function does not check for validity of the data - // though). - static bool HasResultInHeader(const Http::HeaderMap& headers); - - private: - // Return the header location key. For testing purpose only. - static const Http::LowerCaseString& GetHeaderLocation(); - - friend class AuthenticationTest; + // Returns a pointer to the authentication result from metadata. Typically, + // the input metadata is the request info's dynamic metadata. Authentication + // result, if available, is stored under authentication filter metdata. + // Returns nullptr if there is no data for that filter. + static const ProtobufWkt::Struct* GetResultFromMetadata( + const envoy::api::v2::core::Metadata& metadata); }; } // namespace Utils diff --git a/src/envoy/utils/authn_test.cc b/src/envoy/utils/authn_test.cc index 4e75067d615..cc97855cda9 100644 --- a/src/envoy/utils/authn_test.cc +++ b/src/envoy/utils/authn_test.cc @@ -15,7 +15,6 @@ #include "src/envoy/utils/authn.h" #include "common/protobuf/protobuf.h" -#include "envoy/http/header_map.h" #include "include/istio/utils/attribute_names.h" #include "src/istio/authn/context.pb.h" #include "test/test_common/utility.h" @@ -31,31 +30,9 @@ class AuthenticationTest : public testing::Test { test_result_.set_principal("foo"); test_result_.set_peer_user("bar"); } - - const Http::LowerCaseString& GetHeaderLocation() { - return Authentication::GetHeaderLocation(); - } - Http::TestHeaderMapImpl request_headers_{}; Result test_result_; }; -TEST_F(AuthenticationTest, SaveEmptyResult) { - EXPECT_FALSE(Authentication::HasResultInHeader(request_headers_)); - EXPECT_TRUE(Authentication::SaveResultToHeader(Result{}, &request_headers_)); - EXPECT_TRUE(Authentication::HasResultInHeader(request_headers_)); - const auto entry = request_headers_.get(GetHeaderLocation()); - EXPECT_TRUE(entry != nullptr); - EXPECT_EQ("", entry->value().getStringView()); -} - -TEST_F(AuthenticationTest, SaveSomeResult) { - EXPECT_TRUE( - Authentication::SaveResultToHeader(test_result_, &request_headers_)); - const auto entry = request_headers_.get(GetHeaderLocation()); - EXPECT_TRUE(entry != nullptr); - EXPECT_EQ("CgNmb28SA2Jhcg==", entry->value().getStringView()); -} - TEST_F(AuthenticationTest, SaveAuthAttributesToStruct) { istio::authn::Result result; ::google::protobuf::Struct data; @@ -69,6 +46,8 @@ TEST_F(AuthenticationTest, SaveAuthAttributesToStruct) { origin->add_audiences("audiences0"); origin->add_audiences("audiences1"); origin->set_presenter("presenter"); + origin->add_groups("group1"); + origin->add_groups("group2"); auto claim = origin->mutable_claims(); (*claim)["key1"] = "value1"; (*claim)["key2"] = "value2"; @@ -92,6 +71,18 @@ TEST_F(AuthenticationTest, SaveAuthAttributesToStruct) { .at(istio::utils::AttributeName::kRequestAuthAudiences) .string_value(), "audiences0"); + EXPECT_EQ(data.fields() + .at(istio::utils::AttributeName::kRequestAuthGroups) + .list_value() + .values(0) + .string_value(), + "group1"); + EXPECT_EQ(data.fields() + .at(istio::utils::AttributeName::kRequestAuthGroups) + .list_value() + .values(1) + .string_value(), + "group2"); EXPECT_EQ(data.fields() .at(istio::utils::AttributeName::kRequestAuthPresenter) .string_value(), @@ -110,38 +101,5 @@ TEST_F(AuthenticationTest, SaveAuthAttributesToStruct) { "rawclaim"); } -TEST_F(AuthenticationTest, ResultAlreadyExist) { - request_headers_.addCopy(GetHeaderLocation(), "somedata"); - EXPECT_TRUE(Authentication::HasResultInHeader(request_headers_)); - EXPECT_FALSE(Authentication::SaveResultToHeader(Result{}, &request_headers_)); - EXPECT_TRUE(Authentication::HasResultInHeader(request_headers_)); - const auto entry = request_headers_.get(GetHeaderLocation()); - EXPECT_TRUE(entry != nullptr); - EXPECT_EQ("somedata", entry->value().getStringView()); -} - -TEST_F(AuthenticationTest, FetchResultNotExit) { - Result result; - EXPECT_FALSE( - Authentication::FetchResultFromHeader(request_headers_, &result)); -} - -TEST_F(AuthenticationTest, FetchResultBadFormat) { - request_headers_.addCopy(GetHeaderLocation(), "somedata"); - EXPECT_TRUE(Authentication::HasResultInHeader(request_headers_)); - Result result; - EXPECT_FALSE( - Authentication::FetchResultFromHeader(request_headers_, &result)); -} - -TEST_F(AuthenticationTest, FetchResult) { - EXPECT_TRUE( - Authentication::SaveResultToHeader(test_result_, &request_headers_)); - Result fetch_result; - EXPECT_TRUE( - Authentication::FetchResultFromHeader(request_headers_, &fetch_result)); - EXPECT_TRUE(TestUtility::protoEqual(test_result_, fetch_result)); -} - } // namespace Utils } // namespace Envoy diff --git a/src/envoy/utils/filter_names.cc b/src/envoy/utils/filter_names.cc new file mode 100644 index 00000000000..2626766b5c2 --- /dev/null +++ b/src/envoy/utils/filter_names.cc @@ -0,0 +1,26 @@ +/* Copyright 2018 Istio Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "src/envoy/utils/filter_names.h" + +namespace Envoy { +namespace Utils { + +// TODO: using more standard naming, e.g istio.jwt, istio.authn +const char IstioFilterName::kJwt[] = "jwt-auth"; +const char IstioFilterName::kAuthentication[] = "istio_authn"; + +} // namespace Utils +} // namespace Envoy diff --git a/src/envoy/utils/filter_names.h b/src/envoy/utils/filter_names.h new file mode 100644 index 00000000000..64b79dfff2c --- /dev/null +++ b/src/envoy/utils/filter_names.h @@ -0,0 +1,32 @@ +/* Copyright 2018 Istio Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +namespace Envoy { +namespace Utils { + +// These are name of (Istio) filters that currently output data to +// dynamicMetadata (by convention, under the the entry using filter name itself +// as key). Define them here for easy access. +struct IstioFilterName { + static const char kJwt[]; + static const char kAuthentication[]; +}; + +} // namespace Utils +} // namespace Envoy \ No newline at end of file diff --git a/src/envoy/utils/utils.cc b/src/envoy/utils/utils.cc index c85764b359f..6197cacedf5 100644 --- a/src/envoy/utils/utils.cc +++ b/src/envoy/utils/utils.cc @@ -90,20 +90,27 @@ bool GetDestinationUID(const envoy::api::v2::core::Metadata& metadata, return true; } -bool GetSourceUser(const Network::Connection* connection, std::string* user) { +bool GetPrincipal(const Network::Connection* connection, bool peer, + std::string* principal) { if (connection) { Ssl::Connection* ssl = const_cast(connection->ssl()); if (ssl != nullptr) { - std::string result = ssl->uriSanPeerCertificate(); - if (result.empty()) { // empty source user is not allowed + std::string result; + if (peer) { + result = ssl->uriSanPeerCertificate(); + } else { + result = ssl->uriSanLocalCertificate(); + } + + if (result.empty()) { // empty result is not allowed return false; } if (result.length() >= kSPIFFEPrefix.length() && result.compare(0, kSPIFFEPrefix.length(), kSPIFFEPrefix) == 0) { // Strip out the prefix "spiffe://" in the identity. - *user = result.substr(kSPIFFEPrefix.size()); + *principal = result.substr(kSPIFFEPrefix.size()); } else { - *user = result; + *principal = result; } return true; } @@ -116,6 +123,16 @@ bool IsMutualTLS(const Network::Connection* connection) { connection->ssl()->peerCertificatePresented(); } +bool GetRequestedServerName(const Network::Connection* connection, + std::string* name) { + if (connection && !connection->requestedServerName().empty()) { + *name = std::string(connection->requestedServerName()); + return true; + } + + return false; +} + Status ParseJsonMessage(const std::string& json, Message* output) { ::google::protobuf::util::JsonParseOptions options; options.ignore_unknown_fields = true; diff --git a/src/envoy/utils/utils.h b/src/envoy/utils/utils.h index 339de3a3fa7..6ffde52c3c9 100644 --- a/src/envoy/utils/utils.h +++ b/src/envoy/utils/utils.h @@ -37,12 +37,17 @@ bool GetIpPort(const Network::Address::Ip* ip, std::string* str_ip, int* port); bool GetDestinationUID(const envoy::api::v2::core::Metadata& metadata, std::string* uid); -// Get user id from ssl. -bool GetSourceUser(const Network::Connection* connection, std::string* user); +// Get peer or local principal URI. +bool GetPrincipal(const Network::Connection* connection, bool peer, + std::string* principal); // Returns true if connection is mutual TLS enabled. bool IsMutualTLS(const Network::Connection* connection); +// Get requested server name, SNI in case of TLS +bool GetRequestedServerName(const Network::Connection* connection, + std::string* name); + // Parse JSON string into message. ::google::protobuf::util::Status ParseJsonMessage( const std::string& json, ::google::protobuf::Message* output); diff --git a/src/istio/api_spec/http_api_spec_parser_test.cc b/src/istio/api_spec/http_api_spec_parser_test.cc index ff7a409a066..db46b2acbef 100644 --- a/src/istio/api_spec/http_api_spec_parser_test.cc +++ b/src/istio/api_spec/http_api_spec_parser_test.cc @@ -27,8 +27,8 @@ using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::config::client::HTTPAPISpec; using ::istio::utils::AttributesBuilder; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace api_spec { diff --git a/src/istio/authn/context.proto b/src/istio/authn/context.proto index 5ea3fdb67d5..b9537ee8490 100644 --- a/src/istio/authn/context.proto +++ b/src/istio/authn/context.proto @@ -40,6 +40,10 @@ message JwtPayload { // object (map) to access other claims that not cover with the string claims // map above. string raw_claims = 6; + + // The groups claim in the JWT. + // Example: [‘group1’, ‘group2’] + repeated string groups = 7; } // Container to hold authenticated attributes from X509 (mTLS). diff --git a/src/istio/control/http/attributes_builder.cc b/src/istio/control/http/attributes_builder.cc index 7acdff379cd..91835c7df67 100644 --- a/src/istio/control/http/attributes_builder.cc +++ b/src/istio/control/http/attributes_builder.cc @@ -15,6 +15,8 @@ #include "src/istio/control/http/attributes_builder.h" +#include + #include "include/istio/utils/attribute_names.h" #include "include/istio/utils/attributes_builder.h" #include "include/istio/utils/status.h" @@ -64,77 +66,63 @@ void AttributesBuilder::ExtractRequestHeaderAttributes(CheckData *check_data) { builder.AddString(it.name, it.default_value); } } + + std::string query_path; + if (check_data->GetUrlPath(&query_path)) { + builder.AddString(utils::AttributeName::kRequestUrlPath, query_path); + } + + std::map query_map; + if (check_data->GetRequestQueryParams(&query_map) && query_map.size() > 0) { + builder.AddStringMap(utils::AttributeName::kRequestQueryParams, query_map); + } } void AttributesBuilder::ExtractAuthAttributes(CheckData *check_data) { - istio::authn::Result authn_result; - if (check_data->GetAuthenticationResult(&authn_result)) { - utils::AttributesBuilder builder(&request_->attributes); - if (!authn_result.principal().empty()) { - builder.AddString(utils::AttributeName::kRequestAuthPrincipal, - authn_result.principal()); - } - if (!authn_result.peer_user().empty()) { - // TODO(diemtvu): remove kSourceUser once migration to source.principal is - // over. https://github.com/istio/istio/issues/4689 - builder.AddString(utils::AttributeName::kSourceUser, - authn_result.peer_user()); - builder.AddString(utils::AttributeName::kSourcePrincipal, - authn_result.peer_user()); - } - if (authn_result.has_origin()) { - const auto &origin = authn_result.origin(); - if (!origin.audiences().empty()) { - // TODO(diemtvu): this should be send as repeated field once mixer - // support string_list (https://github.com/istio/istio/issues/2802) For - // now, just use the first value. - builder.AddString(utils::AttributeName::kRequestAuthAudiences, - origin.audiences(0)); - } - if (!origin.presenter().empty()) { - builder.AddString(utils::AttributeName::kRequestAuthPresenter, - origin.presenter()); - } - if (!origin.claims().empty()) { - builder.AddProtobufStringMap(utils::AttributeName::kRequestAuthClaims, - origin.claims()); - } - if (!origin.raw_claims().empty()) { - builder.AddString(utils::AttributeName::kRequestAuthRawClaims, - origin.raw_claims()); + utils::AttributesBuilder builder(&request_->attributes); + + std::string destination_principal; + if (check_data->GetPrincipal(false, &destination_principal)) { + builder.AddString(utils::AttributeName::kDestinationPrincipal, + destination_principal); + } + static const std::set kAuthenticationStringAttributes = { + utils::AttributeName::kRequestAuthPrincipal, + utils::AttributeName::kSourceUser, + utils::AttributeName::kSourcePrincipal, + utils::AttributeName::kRequestAuthAudiences, + utils::AttributeName::kRequestAuthPresenter, + utils::AttributeName::kRequestAuthRawClaims, + }; + const auto *authn_result = check_data->GetAuthenticationResult(); + if (authn_result != nullptr) { + // Not all data in authentication results need to be sent to mixer (e.g + // groups), so we need to iterate on pre-approved attributes only. + for (const auto &attribute : kAuthenticationStringAttributes) { + const auto &iter = authn_result->fields().find(attribute); + if (iter != authn_result->fields().end() && + !iter->second.string_value().empty()) { + builder.AddString(attribute, iter->second.string_value()); } } - return; - } - // Fallback to extract from jwt filter directly. This can be removed once - // authn filter is in place. - std::map payload; - utils::AttributesBuilder builder(&request_->attributes); - if (check_data->GetJWTPayload(&payload) && !payload.empty()) { - // Populate auth attributes. - if (payload.count("iss") > 0 && payload.count("sub") > 0) { - builder.AddString(utils::AttributeName::kRequestAuthPrincipal, - payload["iss"] + "/" + payload["sub"]); - } - if (payload.count("aud") > 0) { - builder.AddString(utils::AttributeName::kRequestAuthAudiences, - payload["aud"]); + // Add string-map attribute (kRequestAuthClaims) + const auto &claims = + authn_result->fields().find(utils::AttributeName::kRequestAuthClaims); + if (claims != authn_result->fields().end()) { + builder.AddProtoStructStringMap(utils::AttributeName::kRequestAuthClaims, + claims->second.struct_value()); } - if (payload.count("azp") > 0) { - builder.AddString(utils::AttributeName::kRequestAuthPresenter, - payload["azp"]); - } - builder.AddStringMap(utils::AttributeName::kRequestAuthClaims, payload); + return; } + + // Fallback to source.principal extracted from mTLS if no authentication + // filter is installed std::string source_user; - if (check_data->GetSourceUser(&source_user)) { - // TODO(diemtvu): remove kSourceUser once migration to source.principal is - // over. https://github.com/istio/istio/issues/4689 - builder.AddString(utils::AttributeName::kSourceUser, source_user); + if (check_data->GetPrincipal(true, &source_user)) { builder.AddString(utils::AttributeName::kSourcePrincipal, source_user); } -} // namespace http +} void AttributesBuilder::ExtractForwardedAttributes(CheckData *check_data) { std::string forwarded_data; @@ -154,9 +142,22 @@ void AttributesBuilder::ExtractCheckAttributes(CheckData *check_data) { utils::AttributesBuilder builder(&request_->attributes); + // connection remote IP is always reported as origin IP + std::string source_ip; + int source_port; + if (check_data->GetSourceIpPort(&source_ip, &source_port)) { + builder.AddBytes(utils::AttributeName::kOriginIp, source_ip); + } + builder.AddBool(utils::AttributeName::kConnectionMtls, check_data->IsMutualTLS()); + std::string requested_server_name; + if (check_data->GetRequestedServerName(&requested_server_name)) { + builder.AddString(utils::AttributeName::kConnectionRequestedServerName, + requested_server_name); + } + builder.AddTimestamp(utils::AttributeName::kRequestTime, std::chrono::system_clock::now()); @@ -235,6 +236,21 @@ void AttributesBuilder::ExtractReportAttributes(ReportData *report_data) { builder.AddString(utils::AttributeName::kResponseGrpcMessage, grpc_status.message); } + + builder.AddString(utils::AttributeName::kContextProxyErrorCode, + info.response_flags); + + ReportData::RbacReportInfo rbac_info; + if (report_data->GetRbacReportInfo(&rbac_info)) { + if (!rbac_info.permissive_resp_code.empty()) { + builder.AddString(utils::AttributeName::kRbacPermissiveResponseCode, + rbac_info.permissive_resp_code); + } + if (!rbac_info.permissive_policy_id.empty()) { + builder.AddString(utils::AttributeName::kRbacPermissivePolicyId, + rbac_info.permissive_policy_id); + } + } } } // namespace http diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index d1475f734b9..305fac3b44b 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -15,6 +15,7 @@ #include "src/istio/control/http/attributes_builder.h" +#include "gmock/gmock.h" #include "google/protobuf/text_format.h" #include "google/protobuf/util/message_differencer.h" #include "gtest/gtest.h" @@ -28,14 +29,121 @@ using ::google::protobuf::util::MessageDifferencer; using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::Attributes_StringMap; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace control { namespace http { namespace { +MATCHER_P(EqualsAttribute, expected, "") { + const auto matched = MessageDifferencer::Equals(arg, expected); + if (!matched) { + GOOGLE_LOG(INFO) << arg.DebugString() << " vs " << expected.DebugString(); + } + return matched; +} +const char kCheckAttributesWithoutAuthnFilter[] = R"( +attributes { + key: "connection.mtls" + value { + bool_value: true + } +} +attributes { + key: "connection.requested_server_name" + value { + string_value: "www.google.com" + } +} +attributes { + key: "context.protocol" + value { + string_value: "http" + } +} +attributes { + key: "destination.principal" + value { + string_value: "destination_user" + } +} +attributes { + key: "origin.ip" + value { + bytes_value: "1.2.3.4" + } +} +attributes { + key: "request.headers" + value { + string_map_value { + entries { + key: "host" + value: "localhost" + } + entries { + key: "path" + value: "/books?a=b&c=d" + } + } + } +} +attributes { + key: "request.host" + value { + string_value: "localhost" + } +} +attributes { + key: "request.path" + value { + string_value: "/books?a=b&c=d" + } +} +attributes { + key: "request.query_params" + value { + string_map_value { + entries { + key: "a" + value: "b" + } + entries { + key: "c" + value: "d" + } + } + } +} +attributes { + key: "request.scheme" + value { + string_value: "http" + } +} +attributes { + key: "request.time" + value { + timestamp_value { + } + } +} +attributes { + key: "request.url_path" + value { + string_value: "/books" + } +} +attributes { + key: "source.principal" + value { + string_value: "test_user" + } +} +)"; + const char kCheckAttributes[] = R"( attributes { key: "context.protocol" @@ -53,7 +161,7 @@ attributes { } entries { key: "path" - value: "/books" + value: "/books?a=b&c=d" } } } @@ -66,10 +174,31 @@ attributes { } attributes { key: "request.path" + value { + string_value: "/books?a=b&c=d" + } +} +attributes { + key: "request.url_path" value { string_value: "/books" } } +attributes { + key: "request.query_params" + value { + string_map_value { + entries { + key: "a" + value: "b" + } + entries { + key: "c" + value: "d" + } + } + } +} attributes { key: "request.scheme" value { @@ -89,6 +218,12 @@ attributes { bool_value: true } } +attributes { + key: "connection.requested_server_name" + value { + string_value: "www.google.com" + } +} attributes { key: "source.principal" value { @@ -101,6 +236,18 @@ attributes { string_value: "test_user" } } +attributes { + key: "origin.ip" + value { + bytes_value: "1.2.3.4" + } +} +attributes { + key: "destination.principal" + value { + string_value: "destination_user" + } +} attributes { key: "request.auth.audiences" value { @@ -154,6 +301,12 @@ attributes { string_value: "thisisiss/thisissub" } } +attributes { + key: "request.auth.raw_claims" + value { + string_value: "test_raw_claims" + } +} )"; const char kReportAttributes[] = R"( @@ -229,6 +382,125 @@ attributes { } } } +attributes { + key: "context.proxy_error_code" + value { + string_value: "NR" + } +} +attributes { + key: "rbac.permissive.response_code" + value { + string_value: "403" + } +} +attributes { + key: "rbac.permissive.effective_policy_id" + value { + string_value: "policy-foo" + } +} +)"; + +constexpr char kAuthenticationResultStruct[] = R"( +fields { + key: "request.auth.audiences" + value { + string_value: "thisisaud" + } +} +fields { + key: "request.auth.claims" + value { + struct_value { + fields { + key: "iss" + value { + string_value: "thisisiss" + } + } + fields { + key: "sub" + value { + string_value: "thisissub" + } + } + fields { + key: "aud" + value { + string_value: "thisisaud" + } + } + fields { + key: "azp" + value { + string_value: "thisisazp" + } + } + fields { + key: "email" + value { + string_value: "thisisemail@email.com" + } + } + fields { + key: "iat" + value { + string_value: "1512754205" + } + } + fields { + key: "exp" + value { + string_value: "5112754205" + } + } + } + } +} +fields { + key: "request.auth.groups" + value { + list_value { + values { + string_value: "group1" + } + values { + string_value: "group2" + } + } + } +} +fields { + key: "request.auth.presenter" + value { + string_value: "thisisazp" + } +} +fields { + key: "request.auth.principal" + value { + string_value: "thisisiss/thisissub" + } +} +fields { + key: "request.auth.raw_claims" + value { + string_value: "test_raw_claims" + } +} +fields { + key: "source.principal" + value { + string_value: "test_user" + } +} +fields { + key: "source.user" + value { + string_value: "test_user" + } +} )"; void ClearContextTime(const std::string &name, RequestContext *request) { @@ -247,7 +519,7 @@ TEST(AttributesBuilderTest, TestExtractForwardedAttributes) { Attributes attr; (*attr.mutable_attributes())["test_key"].set_string_value("test_value"); - ::testing::NiceMock mock_data; + ::testing::StrictMock mock_data; EXPECT_CALL(mock_data, ExtractIstioAttributes(_)) .WillOnce(Invoke([&attr](std::string *data) -> bool { attr.SerializeToString(data); @@ -257,12 +529,12 @@ TEST(AttributesBuilderTest, TestExtractForwardedAttributes) { RequestContext request; AttributesBuilder builder(&request); builder.ExtractForwardedAttributes(&mock_data); - EXPECT_TRUE(MessageDifferencer::Equals(request.attributes, attr)); + EXPECT_THAT(request.attributes, EqualsAttribute(attr)); } TEST(AttributesBuilderTest, TestForwardAttributes) { Attributes forwarded_attr; - ::testing::NiceMock mock_header; + ::testing::StrictMock mock_header; EXPECT_CALL(mock_header, AddIstioAttributes(_)) .WillOnce(Invoke([&forwarded_attr](const std::string &data) { EXPECT_TRUE(forwarded_attr.ParseFromString(data)); @@ -273,23 +545,41 @@ TEST(AttributesBuilderTest, TestForwardAttributes) { "test_value"); AttributesBuilder::ForwardAttributes(origin_attr, &mock_header); - EXPECT_TRUE(MessageDifferencer::Equals(origin_attr, forwarded_attr)); + EXPECT_THAT(forwarded_attr, EqualsAttribute(origin_attr)); } -TEST(AttributesBuilderTest, TestCheckAttributes) { - ::testing::NiceMock mock_data; - EXPECT_CALL(mock_data, GetSourceUser(_)) - .WillOnce(Invoke([](std::string *user) -> bool { - *user = "test_user"; +TEST(AttributesBuilderTest, TestCheckAttributesWithoutAuthnFilter) { + // In production, it is expected that authn filter always available whenver + // mTLS or JWT is in used. This test case merely for completness to illustrate + // what attributes are populated if authn filter is missing. + ::testing::StrictMock mock_data; + EXPECT_CALL(mock_data, GetPrincipal(_, _)) + .WillRepeatedly(Invoke([](bool peer, std::string *user) -> bool { + if (peer) { + *user = "test_user"; + } else { + *user = "destination_user"; + } return true; })); EXPECT_CALL(mock_data, IsMutualTLS()).WillOnce(Invoke([]() -> bool { return true; })); + EXPECT_CALL(mock_data, GetRequestedServerName(_)) + .WillOnce(Invoke([](std::string *name) -> bool { + *name = "www.google.com"; + return true; + })); + EXPECT_CALL(mock_data, GetSourceIpPort(_, _)) + .WillOnce(Invoke([](std::string *ip, int *port) -> bool { + *ip = "1.2.3.4"; + *port = 8080; + return true; + })); EXPECT_CALL(mock_data, GetRequestHeaders()) .WillOnce(Invoke([]() -> std::map { std::map map; - map["path"] = "/books"; + map["path"] = "/books?a=b&c=d"; map["host"] = "localhost"; return map; })); @@ -297,7 +587,7 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { .WillRepeatedly(Invoke( [](CheckData::HeaderType header_type, std::string *value) -> bool { if (header_type == CheckData::HEADER_PATH) { - *value = "/books"; + *value = "/books?a=b&c=d"; return true; } else if (header_type == CheckData::HEADER_HOST) { *value = "localhost"; @@ -305,17 +595,18 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { } return false; })); - EXPECT_CALL(mock_data, GetAuthenticationResult(_)) - .WillOnce(testing::Return(false)); - EXPECT_CALL(mock_data, GetJWTPayload(_)) - .WillOnce(Invoke([](std::map *payload) -> bool { - (*payload)["iss"] = "thisisiss"; - (*payload)["sub"] = "thisissub"; - (*payload)["aud"] = "thisisaud"; - (*payload)["azp"] = "thisisazp"; - (*payload)["email"] = "thisisemail@email.com"; - (*payload)["iat"] = "1512754205"; - (*payload)["exp"] = "5112754205"; + EXPECT_CALL(mock_data, GetAuthenticationResult()) + .WillOnce(testing::Return(nullptr)); + + EXPECT_CALL(mock_data, GetUrlPath(_)) + .WillOnce(Invoke([](std::string *path) -> bool { + *path = "/books"; + return true; + })); + EXPECT_CALL(mock_data, GetRequestQueryParams(_)) + .WillOnce(Invoke([](std::map *map) -> bool { + (*map)["a"] = "b"; + (*map)["c"] = "d"; return true; })); @@ -325,26 +616,41 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { ClearContextTime(utils::AttributeName::kRequestTime, &request); - std::string out_str; - TextFormat::PrintToString(request.attributes, &out_str); - GOOGLE_LOG(INFO) << "===" << out_str << "==="; - Attributes expected_attributes; - ASSERT_TRUE( - TextFormat::ParseFromString(kCheckAttributes, &expected_attributes)); - EXPECT_TRUE( - MessageDifferencer::Equals(request.attributes, expected_attributes)); + ASSERT_TRUE(TextFormat::ParseFromString(kCheckAttributesWithoutAuthnFilter, + &expected_attributes)); + EXPECT_THAT(request.attributes, EqualsAttribute(expected_attributes)); } -TEST(AttributesBuilderTest, TestCheckAttributesWithAuthNResult) { - ::testing::NiceMock mock_data; +TEST(AttributesBuilderTest, TestCheckAttributes) { + ::testing::StrictMock mock_data; EXPECT_CALL(mock_data, IsMutualTLS()).WillOnce(Invoke([]() -> bool { return true; })); + EXPECT_CALL(mock_data, GetPrincipal(_, _)) + .WillRepeatedly(Invoke([](bool peer, std::string *user) -> bool { + if (peer) { + *user = "test_user"; + } else { + *user = "destination_user"; + } + return true; + })); + EXPECT_CALL(mock_data, GetRequestedServerName(_)) + .WillOnce(Invoke([](std::string *name) -> bool { + *name = "www.google.com"; + return true; + })); + EXPECT_CALL(mock_data, GetSourceIpPort(_, _)) + .WillOnce(Invoke([](std::string *ip, int *port) -> bool { + *ip = "1.2.3.4"; + *port = 8080; + return true; + })); EXPECT_CALL(mock_data, GetRequestHeaders()) .WillOnce(Invoke([]() -> std::map { std::map map; - map["path"] = "/books"; + map["path"] = "/books?a=b&c=d"; map["host"] = "localhost"; return map; })); @@ -352,7 +658,7 @@ TEST(AttributesBuilderTest, TestCheckAttributesWithAuthNResult) { .WillRepeatedly(Invoke( [](CheckData::HeaderType header_type, std::string *value) -> bool { if (header_type == CheckData::HEADER_PATH) { - *value = "/books"; + *value = "/books?a=b&c=d"; return true; } else if (header_type == CheckData::HEADER_HOST) { *value = "localhost"; @@ -360,21 +666,21 @@ TEST(AttributesBuilderTest, TestCheckAttributesWithAuthNResult) { } return false; })); - EXPECT_CALL(mock_data, GetAuthenticationResult(_)) - .WillOnce(Invoke([](istio::authn::Result *result) -> bool { - result->set_principal("thisisiss/thisissub"); - result->set_peer_user("test_user"); - result->mutable_origin()->add_audiences("thisisaud"); - result->mutable_origin()->set_presenter("thisisazp"); - (*result->mutable_origin()->mutable_claims())["iss"] = "thisisiss"; - (*result->mutable_origin()->mutable_claims())["sub"] = "thisissub"; - (*result->mutable_origin()->mutable_claims())["aud"] = "thisisaud"; - (*result->mutable_origin()->mutable_claims())["azp"] = "thisisazp"; - (*result->mutable_origin()->mutable_claims())["email"] = - "thisisemail@email.com"; - (*result->mutable_origin()->mutable_claims())["iat"] = "1512754205"; - (*result->mutable_origin()->mutable_claims())["exp"] = "5112754205"; - result->mutable_origin()->set_raw_claims("test_raw_claims"); + google::protobuf::Struct authn_result; + ASSERT_TRUE( + TextFormat::ParseFromString(kAuthenticationResultStruct, &authn_result)); + + EXPECT_CALL(mock_data, GetAuthenticationResult()) + .WillOnce(testing::Return(&authn_result)); + EXPECT_CALL(mock_data, GetUrlPath(_)) + .WillOnce(Invoke([](std::string *path) -> bool { + *path = "/books"; + return true; + })); + EXPECT_CALL(mock_data, GetRequestQueryParams(_)) + .WillOnce(Invoke([](std::map *map) -> bool { + (*map)["a"] = "b"; + (*map)["c"] = "d"; return true; })); @@ -384,27 +690,14 @@ TEST(AttributesBuilderTest, TestCheckAttributesWithAuthNResult) { ClearContextTime(utils::AttributeName::kRequestTime, &request); - std::string out_str; - TextFormat::PrintToString(request.attributes, &out_str); - GOOGLE_LOG(INFO) << "===" << out_str << "==="; - Attributes expected_attributes; ASSERT_TRUE( TextFormat::ParseFromString(kCheckAttributes, &expected_attributes)); - // kCheckAttributes is also used in TestCheckAttributes, which is a deprecated - // way to construct mixer attribute (it was a fallback when authn filter is - // not available, which can be removed after 0.8). For now, modifying expected - // data manually for this test. - (*expected_attributes - .mutable_attributes())[utils::AttributeName::kRequestAuthRawClaims] - .set_string_value("test_raw_claims"); - - EXPECT_TRUE( - MessageDifferencer::Equals(request.attributes, expected_attributes)); + EXPECT_THAT(request.attributes, EqualsAttribute(expected_attributes)); } TEST(AttributesBuilderTest, TestReportAttributes) { - ::testing::NiceMock mock_data; + ::testing::StrictMock mock_data; EXPECT_CALL(mock_data, GetDestinationIpPort(_, _)) .WillOnce(Invoke([](std::string *ip, int *port) -> bool { *ip = "1.2.3.4"; @@ -431,6 +724,7 @@ TEST(AttributesBuilderTest, TestReportAttributes) { info->request_total_size = 240; info->duration = std::chrono::nanoseconds(1); info->response_code = 404; + info->response_flags = "NR"; })); EXPECT_CALL(mock_data, GetGrpcStatus(_)) .WillOnce(Invoke([](ReportData::GrpcStatus *status) -> bool { @@ -438,6 +732,12 @@ TEST(AttributesBuilderTest, TestReportAttributes) { status->message = "grpc-message"; return true; })); + EXPECT_CALL(mock_data, GetRbacReportInfo(_)) + .WillOnce(Invoke([](ReportData::RbacReportInfo *report_info) -> bool { + report_info->permissive_resp_code = "403"; + report_info->permissive_policy_id = "policy-foo"; + return true; + })); RequestContext request; AttributesBuilder builder(&request); @@ -445,10 +745,6 @@ TEST(AttributesBuilderTest, TestReportAttributes) { ClearContextTime(utils::AttributeName::kResponseTime, &request); - std::string out_str; - TextFormat::PrintToString(request.attributes, &out_str); - GOOGLE_LOG(INFO) << "===" << out_str << "==="; - Attributes expected_attributes; ASSERT_TRUE( TextFormat::ParseFromString(kReportAttributes, &expected_attributes)); @@ -461,18 +757,18 @@ TEST(AttributesBuilderTest, TestReportAttributes) { (*expected_attributes .mutable_attributes())[utils::AttributeName::kResponseGrpcMessage] .set_string_value("grpc-message"); - EXPECT_TRUE( - MessageDifferencer::Equals(request.attributes, expected_attributes)); + EXPECT_THAT(request.attributes, EqualsAttribute(expected_attributes)); } TEST(AttributesBuilderTest, TestReportAttributesWithDestIP) { - ::testing::NiceMock mock_data; + ::testing::StrictMock mock_data; EXPECT_CALL(mock_data, GetDestinationIpPort(_, _)) .WillOnce(Invoke([](std::string *ip, int *port) -> bool { *ip = "2.3.4.5"; *port = 8080; return true; })); + EXPECT_CALL(mock_data, GetDestinationUID(_)).WillOnce(testing::Return(false)); EXPECT_CALL(mock_data, GetResponseHeaders()) .WillOnce(Invoke([]() -> std::map { std::map map; @@ -488,6 +784,14 @@ TEST(AttributesBuilderTest, TestReportAttributesWithDestIP) { info->request_total_size = 240; info->duration = std::chrono::nanoseconds(1); info->response_code = 404; + info->response_flags = "NR"; + })); + EXPECT_CALL(mock_data, GetGrpcStatus(_)).WillOnce(testing::Return(false)); + EXPECT_CALL(mock_data, GetRbacReportInfo(_)) + .WillOnce(Invoke([](ReportData::RbacReportInfo *report_info) -> bool { + report_info->permissive_resp_code = "403"; + report_info->permissive_policy_id = "policy-foo"; + return true; })); RequestContext request; @@ -497,15 +801,10 @@ TEST(AttributesBuilderTest, TestReportAttributesWithDestIP) { ClearContextTime(utils::AttributeName::kResponseTime, &request); - std::string out_str; - TextFormat::PrintToString(request.attributes, &out_str); - GOOGLE_LOG(INFO) << "===" << out_str << "==="; - Attributes expected_attributes; ASSERT_TRUE( TextFormat::ParseFromString(kReportAttributes, &expected_attributes)); - EXPECT_TRUE( - MessageDifferencer::Equals(request.attributes, expected_attributes)); + EXPECT_THAT(request.attributes, EqualsAttribute(expected_attributes)); } } // namespace diff --git a/src/istio/control/http/mock_check_data.h b/src/istio/control/http/mock_check_data.h index 9fc44f20d71..106ac78f28e 100644 --- a/src/istio/control/http/mock_check_data.h +++ b/src/istio/control/http/mock_check_data.h @@ -17,6 +17,7 @@ #define ISTIO_CONTROL_HTTP_MOCK_CHECK_DATA_H #include "gmock/gmock.h" +#include "google/protobuf/struct.pb.h" #include "include/istio/control/http/check_data.h" namespace istio { @@ -29,7 +30,7 @@ class MockCheckData : public CheckData { MOCK_CONST_METHOD1(ExtractIstioAttributes, bool(std::string *data)); MOCK_CONST_METHOD2(GetSourceIpPort, bool(std::string *ip, int *port)); - MOCK_CONST_METHOD1(GetSourceUser, bool(std::string *user)); + MOCK_CONST_METHOD2(GetPrincipal, bool(bool peer, std::string *user)); MOCK_CONST_METHOD0(GetRequestHeaders, std::map()); MOCK_CONST_METHOD2(FindHeaderByType, bool(HeaderType header_type, std::string *value)); @@ -41,9 +42,13 @@ class MockCheckData : public CheckData { bool(const std::string &name, std::string *value)); MOCK_CONST_METHOD1(GetJWTPayload, bool(std::map *payload)); - MOCK_CONST_METHOD1(GetAuthenticationResult, - bool(istio::authn::Result *result)); + MOCK_CONST_METHOD0(GetAuthenticationResult, + const ::google::protobuf::Struct *()); MOCK_CONST_METHOD0(IsMutualTLS, bool()); + MOCK_CONST_METHOD1(GetRequestedServerName, bool(std::string *name)); + MOCK_CONST_METHOD1(GetUrlPath, bool(std::string *)); + MOCK_CONST_METHOD1(GetRequestQueryParams, + bool(std::map *)); }; // The mock object for HeaderUpdate interface. diff --git a/src/istio/control/http/mock_report_data.h b/src/istio/control/http/mock_report_data.h index 26ca3830133..423a1b6ef4f 100644 --- a/src/istio/control/http/mock_report_data.h +++ b/src/istio/control/http/mock_report_data.h @@ -31,6 +31,7 @@ class MockReportData : public ReportData { MOCK_CONST_METHOD2(GetDestinationIpPort, bool(std::string* ip, int* port)); MOCK_CONST_METHOD1(GetDestinationUID, bool(std::string* ip)); MOCK_CONST_METHOD1(GetGrpcStatus, bool(GrpcStatus* status)); + MOCK_CONST_METHOD1(GetRbacReportInfo, bool(RbacReportInfo* info)); }; } // namespace http diff --git a/src/istio/control/http/request_handler_impl_test.cc b/src/istio/control/http/request_handler_impl_test.cc index ddd9d8581e0..dcbbaf3d49f 100644 --- a/src/istio/control/http/request_handler_impl_test.cc +++ b/src/istio/control/http/request_handler_impl_test.cc @@ -35,8 +35,8 @@ using ::istio::mixerclient::MixerClient; using ::istio::mixerclient::TransportCheckFunc; using ::istio::quota_config::Requirement; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace control { @@ -150,7 +150,7 @@ TEST_F(RequestHandlerImplTest, TestHandlerDisabledCheckReport) { ::testing::NiceMock mock_header; // Not to extract attributes since both Check and Report are disabled. EXPECT_CALL(mock_data, GetSourceIpPort(_, _)).Times(0); - EXPECT_CALL(mock_data, GetSourceUser(_)).Times(0); + EXPECT_CALL(mock_data, GetPrincipal(_, _)).Times(0); // Check should NOT be called. EXPECT_CALL(*mock_client_, Check(_, _, _, _)).Times(0); @@ -172,8 +172,8 @@ TEST_F(RequestHandlerImplTest, TestHandlerDisabledCheck) { ::testing::NiceMock mock_data; ::testing::NiceMock mock_header; // Report is enabled so Attributes are extracted. - EXPECT_CALL(mock_data, GetSourceIpPort(_, _)).Times(0); - EXPECT_CALL(mock_data, GetSourceUser(_)).Times(1); + EXPECT_CALL(mock_data, GetSourceIpPort(_, _)).Times(1); + EXPECT_CALL(mock_data, GetPrincipal(_, _)).Times(2); // Check should NOT be called. EXPECT_CALL(*mock_client_, Check(_, _, _, _)).Times(0); @@ -193,8 +193,8 @@ TEST_F(RequestHandlerImplTest, TestHandlerDisabledCheck) { TEST_F(RequestHandlerImplTest, TestPerRouteAttributes) { ::testing::NiceMock mock_data; ::testing::NiceMock mock_header; - EXPECT_CALL(mock_data, GetSourceIpPort(_, _)).Times(0); - EXPECT_CALL(mock_data, GetSourceUser(_)).Times(1); + EXPECT_CALL(mock_data, GetSourceIpPort(_, _)).Times(1); + EXPECT_CALL(mock_data, GetPrincipal(_, _)).Times(2); // Check should be called. EXPECT_CALL(*mock_client_, Check(_, _, _, _)) @@ -221,8 +221,8 @@ TEST_F(RequestHandlerImplTest, TestPerRouteAttributes) { TEST_F(RequestHandlerImplTest, TestDefaultRouteAttributes) { ::testing::NiceMock mock_data; ::testing::NiceMock mock_header; - EXPECT_CALL(mock_data, GetSourceIpPort(_, _)).Times(0); - EXPECT_CALL(mock_data, GetSourceUser(_)).Times(1); + EXPECT_CALL(mock_data, GetSourceIpPort(_, _)).Times(1); + EXPECT_CALL(mock_data, GetPrincipal(_, _)).Times(2); // Check should be called. EXPECT_CALL(*mock_client_, Check(_, _, _, _)) @@ -254,8 +254,8 @@ TEST_F(RequestHandlerImplTest, TestDefaultRouteAttributes) { TEST_F(RequestHandlerImplTest, TestRouteAttributes) { ::testing::NiceMock mock_data; ::testing::NiceMock mock_header; - EXPECT_CALL(mock_data, GetSourceIpPort(_, _)).Times(0); - EXPECT_CALL(mock_data, GetSourceUser(_)).Times(1); + EXPECT_CALL(mock_data, GetSourceIpPort(_, _)).Times(1); + EXPECT_CALL(mock_data, GetPrincipal(_, _)).Times(2); ServiceConfig route_config; auto map3 = route_config.mutable_mixer_attributes()->mutable_attributes(); @@ -369,8 +369,8 @@ TEST_F(RequestHandlerImplTest, TestPerRouteApiSpec) { TEST_F(RequestHandlerImplTest, TestHandlerCheck) { ::testing::NiceMock mock_data; ::testing::NiceMock mock_header; - EXPECT_CALL(mock_data, GetSourceIpPort(_, _)).Times(0); - EXPECT_CALL(mock_data, GetSourceUser(_)).Times(1); + EXPECT_CALL(mock_data, GetSourceIpPort(_, _)).Times(1); + EXPECT_CALL(mock_data, GetPrincipal(_, _)).Times(2); // Check should be called. EXPECT_CALL(*mock_client_, Check(_, _, _, _)).Times(1); @@ -454,7 +454,7 @@ TEST_F(RequestHandlerImplTest, TestEmptyConfig) { ::testing::NiceMock mock_header; // Not to extract attributes since both Check and Report are disabled. EXPECT_CALL(mock_check, GetSourceIpPort(_, _)).Times(0); - EXPECT_CALL(mock_check, GetSourceUser(_)).Times(0); + EXPECT_CALL(mock_check, GetPrincipal(_, _)).Times(0); // Attributes is forwarded. EXPECT_CALL(mock_header, AddIstioAttributes(_)) diff --git a/src/istio/control/tcp/attributes_builder.cc b/src/istio/control/tcp/attributes_builder.cc index 64fa59ed473..e61bc0e3917 100644 --- a/src/istio/control/tcp/attributes_builder.cc +++ b/src/istio/control/tcp/attributes_builder.cc @@ -37,24 +37,38 @@ void AttributesBuilder::ExtractCheckAttributes(CheckData* check_data) { // it if (check_data->GetSourceIpPort(&source_ip, &source_port)) { builder.AddBytes(utils::AttributeName::kSourceIp, source_ip); + // connection remote IP is always reported as origin IP + builder.AddBytes(utils::AttributeName::kOriginIp, source_ip); } // TODO(diemtvu): add TCP authn filter similar to http case, and use authn // result output here instead. std::string source_user; - if (check_data->GetSourceUser(&source_user)) { + if (check_data->GetPrincipal(true, &source_user)) { // TODO(diemtvu): remove kSourceUser once migration to source.principal is // over. https://github.com/istio/istio/issues/4689 builder.AddString(utils::AttributeName::kSourceUser, source_user); builder.AddString(utils::AttributeName::kSourcePrincipal, source_user); } + + std::string destination_principal; + if (check_data->GetPrincipal(false, &destination_principal)) { + builder.AddString(utils::AttributeName::kDestinationPrincipal, + destination_principal); + } + builder.AddBool(utils::AttributeName::kConnectionMtls, check_data->IsMutualTLS()); + std::string requested_server_name; + if (check_data->GetRequestedServerName(&requested_server_name)) { + builder.AddString(utils::AttributeName::kConnectionRequestedServerName, + requested_server_name); + } + builder.AddTimestamp(utils::AttributeName::kContextTime, std::chrono::system_clock::now()); builder.AddString(utils::AttributeName::kContextProtocol, "tcp"); - builder.AddString(utils::AttributeName::kConnectionEvent, kConnectionOpen); // Get unique downstream connection ID, which is -. std::string connection_id = check_data->GetConnectionId(); @@ -62,7 +76,7 @@ void AttributesBuilder::ExtractCheckAttributes(CheckData* check_data) { } void AttributesBuilder::ExtractReportAttributes( - ReportData* report_data, bool is_final_report, + ReportData* report_data, ReportData::ConnectionEvent event, ReportData::ReportInfo* last_report_info) { utils::AttributesBuilder builder(&request_->attributes); @@ -77,7 +91,7 @@ void AttributesBuilder::ExtractReportAttributes( builder.AddInt64(utils::AttributeName::kConnectionSendTotalBytes, info.send_bytes); - if (is_final_report) { + if (event == ReportData::ConnectionEvent::CLOSE) { builder.AddDuration(utils::AttributeName::kConnectionDuration, info.duration); if (!request_->check_status.ok()) { @@ -87,6 +101,8 @@ void AttributesBuilder::ExtractReportAttributes( request_->check_status.ToString()); } builder.AddString(utils::AttributeName::kConnectionEvent, kConnectionClose); + } else if (event == ReportData::ConnectionEvent::OPEN) { + builder.AddString(utils::AttributeName::kConnectionEvent, kConnectionOpen); } else { last_report_info->received_bytes = info.received_bytes; last_report_info->send_bytes = info.send_bytes; diff --git a/src/istio/control/tcp/attributes_builder.h b/src/istio/control/tcp/attributes_builder.h index 1423f99d311..9b0f8004230 100644 --- a/src/istio/control/tcp/attributes_builder.h +++ b/src/istio/control/tcp/attributes_builder.h @@ -32,7 +32,8 @@ class AttributesBuilder { // Extract attributes for Check. void ExtractCheckAttributes(CheckData* check_data); // Extract attributes for Report. - void ExtractReportAttributes(ReportData* report_data, bool is_final_report, + void ExtractReportAttributes(ReportData* report_data, + ReportData::ConnectionEvent event, ReportData::ReportInfo* last_report_info); private: diff --git a/src/istio/control/tcp/attributes_builder_test.cc b/src/istio/control/tcp/attributes_builder_test.cc index a9d14c9b3b1..52ee3a984a2 100644 --- a/src/istio/control/tcp/attributes_builder_test.cc +++ b/src/istio/control/tcp/attributes_builder_test.cc @@ -26,9 +26,9 @@ using ::google::protobuf::TextFormat; using ::google::protobuf::util::MessageDifferencer; +using ::testing::_; using ::testing::Invoke; using ::testing::Return; -using ::testing::_; namespace istio { namespace control { @@ -36,12 +36,6 @@ namespace tcp { namespace { const char kCheckAttributes[] = R"( -attributes { - key: "connection.event" - value { - string_value: "open" - } -} attributes { key: "context.protocol" value { @@ -61,12 +55,24 @@ attributes { bytes_value: "1.2.3.4" } } +attributes { + key: "origin.ip" + value { + bytes_value: "1.2.3.4" + } +} attributes { key: "connection.mtls" value { bool_value: true } } +attributes { + key: "connection.requested_server_name" + value { + string_value: "www.google.com" + } +} attributes { key: "source.principal" value { @@ -79,6 +85,12 @@ attributes { string_value: "test_user" } } +attributes { + key: "destination.principal" + value { + string_value: "destination_user" + } +} attributes { key: "connection.id" value { @@ -87,6 +99,64 @@ attributes { } )"; +const char kFirstReportAttributes[] = R"( +attributes { + key: "connection.event" + value { + string_value: "open" + } +} +attributes { + key: "connection.received.bytes" + value { + int64_value: 0 + } +} +attributes { + key: "connection.received.bytes_total" + value { + int64_value: 0 + } +} +attributes { + key: "connection.sent.bytes" + value { + int64_value: 0 + } +} +attributes { + key: "connection.sent.bytes_total" + value { + int64_value: 0 + } +} +attributes { + key: "context.time" + value { + timestamp_value { + } + } +} +attributes { + key: "destination.ip" + value { + bytes_value: "1.2.3.4" + } +} +attributes { + key: "destination.port" + value { + int64_value: 8080 + } +} +attributes { + key: "destination.uid" + value { + string_value: "pod1.ns2" + } +} +)"; + const char kReportAttributes[] = R"( attributes { key: "connection.event" @@ -110,7 +180,7 @@ attributes { key: "connection.duration" value { duration_value { - nanos: 3 + nanos: 4 } } } @@ -299,13 +369,21 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { EXPECT_CALL(mock_data, IsMutualTLS()).WillOnce(Invoke([]() -> bool { return true; })); - EXPECT_CALL(mock_data, GetSourceUser(_)) - .WillOnce(Invoke([](std::string* user) -> bool { - *user = "test_user"; + EXPECT_CALL(mock_data, GetPrincipal(_, _)) + .WillRepeatedly(Invoke([](bool peer, std::string* user) -> bool { + if (peer) { + *user = "test_user"; + } else { + *user = "destination_user"; + } return true; })); EXPECT_CALL(mock_data, GetConnectionId()).WillOnce(Return("1234-5")); - + EXPECT_CALL(mock_data, GetRequestedServerName(_)) + .WillOnce(Invoke([](std::string* name) -> bool { + *name = "www.google.com"; + return true; + })); RequestContext request; AttributesBuilder builder(&request); builder.ExtractCheckAttributes(&mock_data); @@ -326,34 +404,39 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { TEST(AttributesBuilderTest, TestReportAttributes) { ::testing::NiceMock mock_data; EXPECT_CALL(mock_data, GetDestinationIpPort(_, _)) - .Times(3) + .Times(4) .WillRepeatedly(Invoke([](std::string* ip, int* port) -> bool { *ip = "1.2.3.4"; *port = 8080; return true; })); EXPECT_CALL(mock_data, GetDestinationUID(_)) - .Times(3) + .Times(4) .WillRepeatedly(Invoke([](std::string* uid) -> bool { *uid = "pod1.ns2"; return true; })); EXPECT_CALL(mock_data, GetReportInfo(_)) - .Times(3) + .Times(4) + .WillOnce(Invoke([](ReportData::ReportInfo* info) { + info->received_bytes = 0; + info->send_bytes = 0; + info->duration = std::chrono::nanoseconds(1); + })) .WillOnce(Invoke([](ReportData::ReportInfo* info) { info->received_bytes = 100; info->send_bytes = 200; - info->duration = std::chrono::nanoseconds(1); + info->duration = std::chrono::nanoseconds(2); })) .WillOnce(Invoke([](ReportData::ReportInfo* info) { info->received_bytes = 201; info->send_bytes = 404; - info->duration = std::chrono::nanoseconds(2); + info->duration = std::chrono::nanoseconds(3); })) .WillOnce(Invoke([](ReportData::ReportInfo* info) { info->received_bytes = 345; info->send_bytes = 678; - info->duration = std::chrono::nanoseconds(3); + info->duration = std::chrono::nanoseconds(4); })); RequestContext request; @@ -363,8 +446,8 @@ TEST(AttributesBuilderTest, TestReportAttributes) { ReportData::ReportInfo last_report_info{0ULL, 0ULL, std::chrono::nanoseconds::zero()}; - // Verify delta one report - builder.ExtractReportAttributes(&mock_data, /* is_final_report */ false, + // Verify first open report + builder.ExtractReportAttributes(&mock_data, ReportData::ConnectionEvent::OPEN, &last_report_info); ClearContextTime(&request); @@ -372,6 +455,22 @@ TEST(AttributesBuilderTest, TestReportAttributes) { TextFormat::PrintToString(request.attributes, &out_str); GOOGLE_LOG(INFO) << "===" << out_str << "==="; + ::istio::mixer::v1::Attributes expected_open_attributes; + ASSERT_TRUE(TextFormat::ParseFromString(kFirstReportAttributes, + &expected_open_attributes)); + EXPECT_TRUE( + MessageDifferencer::Equals(request.attributes, expected_open_attributes)); + EXPECT_EQ(0, last_report_info.received_bytes); + EXPECT_EQ(0, last_report_info.send_bytes); + + // Verify delta one report + builder.ExtractReportAttributes( + &mock_data, ReportData::ConnectionEvent::CONTINUE, &last_report_info); + ClearContextTime(&request); + + TextFormat::PrintToString(request.attributes, &out_str); + GOOGLE_LOG(INFO) << "===" << out_str << "==="; + ::istio::mixer::v1::Attributes expected_delta_attributes; ASSERT_TRUE(TextFormat::ParseFromString(kDeltaOneReportAttributes, &expected_delta_attributes)); @@ -381,8 +480,8 @@ TEST(AttributesBuilderTest, TestReportAttributes) { EXPECT_EQ(200, last_report_info.send_bytes); // Verify delta two report - builder.ExtractReportAttributes(&mock_data, /* is_final_report */ false, - &last_report_info); + builder.ExtractReportAttributes( + &mock_data, ReportData::ConnectionEvent::CONTINUE, &last_report_info); ClearContextTime(&request); out_str.clear(); @@ -398,8 +497,8 @@ TEST(AttributesBuilderTest, TestReportAttributes) { EXPECT_EQ(404, last_report_info.send_bytes); // Verify final report - builder.ExtractReportAttributes(&mock_data, /* is_final_report */ true, - &last_report_info); + builder.ExtractReportAttributes( + &mock_data, ReportData::ConnectionEvent::CLOSE, &last_report_info); ClearContextTime(&request); out_str.clear(); diff --git a/src/istio/control/tcp/mock_check_data.h b/src/istio/control/tcp/mock_check_data.h index 170e766b500..16cb7752553 100644 --- a/src/istio/control/tcp/mock_check_data.h +++ b/src/istio/control/tcp/mock_check_data.h @@ -27,8 +27,9 @@ namespace tcp { class MockCheckData : public CheckData { public: MOCK_CONST_METHOD2(GetSourceIpPort, bool(std::string* ip, int* port)); - MOCK_CONST_METHOD1(GetSourceUser, bool(std::string* user)); + MOCK_CONST_METHOD2(GetPrincipal, bool(bool peer, std::string* user)); MOCK_CONST_METHOD0(IsMutualTLS, bool()); + MOCK_CONST_METHOD1(GetRequestedServerName, bool(std::string* name)); MOCK_CONST_METHOD0(GetConnectionId, std::string()); }; diff --git a/src/istio/control/tcp/request_handler_impl.cc b/src/istio/control/tcp/request_handler_impl.cc index 168f0ca1919..f9b7b25d59c 100644 --- a/src/istio/control/tcp/request_handler_impl.cc +++ b/src/istio/control/tcp/request_handler_impl.cc @@ -53,19 +53,14 @@ CancelFunc RequestHandlerImpl::Check(CheckData* check_data, return client_context_->SendCheck(nullptr, on_done, &request_context_); } -// Make remote report call. -void RequestHandlerImpl::Report(ReportData* report_data) { - Report(report_data, /* is_final_report */ true); -} - -void RequestHandlerImpl::Report(ReportData* report_data, bool is_final_report) { +void RequestHandlerImpl::Report(ReportData* report_data, + ReportData::ConnectionEvent event) { if (!client_context_->enable_mixer_report()) { return; } AttributesBuilder builder(&request_context_); - builder.ExtractReportAttributes(report_data, is_final_report, - &last_report_info_); + builder.ExtractReportAttributes(report_data, event, &last_report_info_); client_context_->SendReport(request_context_); } diff --git a/src/istio/control/tcp/request_handler_impl.h b/src/istio/control/tcp/request_handler_impl.h index 3d4fa6ee693..ba7f890bd2c 100644 --- a/src/istio/control/tcp/request_handler_impl.h +++ b/src/istio/control/tcp/request_handler_impl.h @@ -35,14 +35,8 @@ class RequestHandlerImpl : public RequestHandler { ::istio::mixerclient::CheckDoneFunc on_done) override; // Make a Report call. - // TODO(JimmyCYJ): Let TCP filter use - // void Report(ReportData* report_data, bool is_final_report), and deprecate - // this method. - void Report(ReportData* report_data) override; - - // Make a Report call. If is_final_report is true, then report all attributes, - // otherwise, report delta attributes. - void Report(ReportData* report_data, bool is_final_report) override; + void Report(ReportData* report_data, + ReportData::ConnectionEvent event) override; private: // The request context object. diff --git a/src/istio/control/tcp/request_handler_impl_test.cc b/src/istio/control/tcp/request_handler_impl_test.cc index e6fe3b3425c..259fd06d7ce 100644 --- a/src/istio/control/tcp/request_handler_impl_test.cc +++ b/src/istio/control/tcp/request_handler_impl_test.cc @@ -31,8 +31,8 @@ using ::istio::mixerclient::MixerClient; using ::istio::mixerclient::TransportCheckFunc; using ::istio::quota_config::Requirement; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace control { @@ -66,7 +66,7 @@ class RequestHandlerImplTest : public ::testing::Test { TEST_F(RequestHandlerImplTest, TestHandlerDisabledCheck) { ::testing::NiceMock mock_data; EXPECT_CALL(mock_data, GetSourceIpPort(_, _)).Times(1); - EXPECT_CALL(mock_data, GetSourceUser(_)).Times(1); + EXPECT_CALL(mock_data, GetPrincipal(_, _)).Times(2); // Check should not be called. EXPECT_CALL(*mock_client_, Check(_, _, _, _)).Times(0); @@ -81,7 +81,7 @@ TEST_F(RequestHandlerImplTest, TestHandlerDisabledCheck) { TEST_F(RequestHandlerImplTest, TestHandlerCheck) { ::testing::NiceMock mock_data; EXPECT_CALL(mock_data, GetSourceIpPort(_, _)).Times(1); - EXPECT_CALL(mock_data, GetSourceUser(_)).Times(1); + EXPECT_CALL(mock_data, GetPrincipal(_, _)).Times(2); // Check should be called. EXPECT_CALL(*mock_client_, Check(_, _, _, _)) @@ -111,7 +111,7 @@ TEST_F(RequestHandlerImplTest, TestHandlerReport) { EXPECT_CALL(*mock_client_, Report(_)).Times(1); auto handler = controller_->CreateRequestHandler(); - handler->Report(&mock_data); + handler->Report(&mock_data, ReportData::CONTINUE); } } // namespace tcp diff --git a/src/istio/mixerclient/check_cache_test.cc b/src/istio/mixerclient/check_cache_test.cc index 0b43309d63e..e5636ccb8b0 100644 --- a/src/istio/mixerclient/check_cache_test.cc +++ b/src/istio/mixerclient/check_cache_test.cc @@ -323,5 +323,71 @@ TEST_F(CheckCacheTest, TestTwoReferenced) { EXPECT_TRUE(result3.IsCacheHit()); } +TEST_F(CheckCacheTest, TestTwoRequestHeaderMaps) { + CheckResponse denied_response; + denied_response.mutable_precondition()->set_valid_use_count(1000); + denied_response.mutable_precondition()->mutable_status()->set_code( + Code::PERMISSION_DENIED); + auto match = denied_response.mutable_precondition() + ->mutable_referenced_attributes() + ->add_attribute_matches(); + match->set_condition(ReferencedAttributes::EXACT); + match->set_name(15); // request.headers is used. + match->set_map_key(2); // sub map key is "source.name" + + Attributes attributes1; + utils::AttributesBuilder(&attributes1) + .AddStringMap("request.headers", + {{"source.ip", "foo"}, {"source.name", "baz"}}); + + CheckCache::CheckResult result; + cache_->Check(attributes1, &result); + EXPECT_FALSE(result.IsCacheHit()); + + result.SetResponse(Status::OK, attributes1, denied_response); + EXPECT_ERROR_CODE(Code::PERMISSION_DENIED, result.status()); + + // Cached response is used. + CheckCache::CheckResult result1; + cache_->Check(attributes1, &result1); + EXPECT_TRUE(result1.IsCacheHit()); + EXPECT_ERROR_CODE(Code::PERMISSION_DENIED, result1.status()); + + Attributes attributes2; + utils::AttributesBuilder(&attributes2) + .AddStringMap("request.headers", + {{"source.ip", "foo"}, {"source.name", "bar"}}); + + // Not in the cache since it has different value + CheckCache::CheckResult result2; + cache_->Check(attributes2, &result2); + EXPECT_FALSE(result2.IsCacheHit()); + + CheckResponse ok_response; + ok_response.mutable_precondition()->set_valid_use_count(1000); + auto match2 = ok_response.mutable_precondition() + ->mutable_referenced_attributes() + ->add_attribute_matches(); + match2->set_condition(ReferencedAttributes::EXACT); + match2->set_name(15); // request.headers is used. + match2->set_map_key(2); // sub map key is "source.name" + + // Store the response to the cache + result2.SetResponse(Status::OK, attributes2, ok_response); + EXPECT_OK(result2.status()); + + // Now it should be in the cache. + CheckCache::CheckResult result3; + cache_->Check(attributes2, &result3); + EXPECT_TRUE(result3.IsCacheHit()); + EXPECT_OK(result3.status()); + + // Also make sure key1 still in the cache + CheckCache::CheckResult result4; + cache_->Check(attributes1, &result4); + EXPECT_TRUE(result4.IsCacheHit()); + EXPECT_ERROR_CODE(Code::PERMISSION_DENIED, result4.status()); +} + } // namespace mixerclient } // namespace istio diff --git a/src/istio/mixerclient/client_impl_test.cc b/src/istio/mixerclient/client_impl_test.cc index f15022239ce..ee38af82753 100644 --- a/src/istio/mixerclient/client_impl_test.cc +++ b/src/istio/mixerclient/client_impl_test.cc @@ -27,8 +27,8 @@ using ::istio::mixer::v1::CheckRequest; using ::istio::mixer::v1::CheckResponse; using ::istio::mixerclient::CheckResponseInfo; using ::istio::quota_config::Requirement; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace mixerclient { diff --git a/src/istio/mixerclient/create_global_dictionary.py b/src/istio/mixerclient/create_global_dictionary.py index 3bfabdf2f08..c42e64b965c 100755 --- a/src/istio/mixerclient/create_global_dictionary.py +++ b/src/istio/mixerclient/create_global_dictionary.py @@ -62,7 +62,7 @@ with open(sys.argv[1]) as src_file: for line in src_file: if line.startswith("-"): - all_words += " \"" + line[1:].strip() + "\",\n" + all_words += " \"" + line[1:].strip().replace("\"", "\\\"") + "\",\n" print (TOP + all_words + BOTTOM) diff --git a/src/istio/mixerclient/quota_cache_test.cc b/src/istio/mixerclient/quota_cache_test.cc index 9d613db8874..9f691a3dd19 100644 --- a/src/istio/mixerclient/quota_cache_test.cc +++ b/src/istio/mixerclient/quota_cache_test.cc @@ -27,8 +27,8 @@ using ::istio::mixer::v1::CheckRequest; using ::istio::mixer::v1::CheckResponse; using ::istio::mixer::v1::ReferencedAttributes; using ::istio::quota_config::Requirement; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace mixerclient { diff --git a/src/istio/mixerclient/report_batch_test.cc b/src/istio/mixerclient/report_batch_test.cc index aec33ab251b..8893d8e4535 100644 --- a/src/istio/mixerclient/report_batch_test.cc +++ b/src/istio/mixerclient/report_batch_test.cc @@ -23,8 +23,8 @@ using ::google::protobuf::util::error::Code; using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::ReportRequest; using ::istio::mixer::v1::ReportResponse; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace mixerclient { diff --git a/src/istio/utils/attribute_names.cc b/src/istio/utils/attribute_names.cc index 5b50d411c44..d42e0666b38 100644 --- a/src/istio/utils/attribute_names.cc +++ b/src/istio/utils/attribute_names.cc @@ -21,6 +21,7 @@ namespace utils { // Define attribute names const char AttributeName::kSourceUser[] = "source.user"; const char AttributeName::kSourcePrincipal[] = "source.principal"; +const char AttributeName::kDestinationPrincipal[] = "destination.principal"; const char AttributeName::kRequestHeaders[] = "request.headers"; const char AttributeName::kRequestHost[] = "request.host"; @@ -28,6 +29,8 @@ const char AttributeName::kRequestMethod[] = "request.method"; const char AttributeName::kRequestPath[] = "request.path"; const char AttributeName::kRequestReferer[] = "request.referer"; const char AttributeName::kRequestScheme[] = "request.scheme"; +const char AttributeName::kRequestUrlPath[] = "request.url_path"; +const char AttributeName::kRequestQueryParams[] = "request.query_params"; const char AttributeName::kRequestBodySize[] = "request.size"; const char AttributeName::kRequestTotalSize[] = "request.total_size"; const char AttributeName::kRequestTime[] = "request.time"; @@ -49,6 +52,7 @@ const char AttributeName::kSourcePort[] = "source.port"; const char AttributeName::kDestinationIp[] = "destination.ip"; const char AttributeName::kDestinationPort[] = "destination.port"; const char AttributeName::kDestinationUID[] = "destination.uid"; +const char AttributeName::kOriginIp[] = "origin.ip"; const char AttributeName::kConnectionReceviedBytes[] = "connection.received.bytes"; const char AttributeName::kConnectionReceviedTotalBytes[] = @@ -58,6 +62,9 @@ const char AttributeName::kConnectionSendTotalBytes[] = "connection.sent.bytes_total"; const char AttributeName::kConnectionDuration[] = "connection.duration"; const char AttributeName::kConnectionMtls[] = "connection.mtls"; +const char AttributeName::kConnectionRequestedServerName[] = + "connection.requested_server_name"; + // Downstream TCP connection id. const char AttributeName::kConnectionId[] = "connection.id"; const char AttributeName::kConnectionEvent[] = "connection.event"; @@ -65,6 +72,7 @@ const char AttributeName::kConnectionEvent[] = "connection.event"; // Context attributes const char AttributeName::kContextProtocol[] = "context.protocol"; const char AttributeName::kContextTime[] = "context.time"; +const char AttributeName::kContextProxyErrorCode[] = "context.proxy_error_code"; // Check error code and message. const char AttributeName::kCheckErrorCode[] = "check.error_code"; @@ -77,6 +85,7 @@ const char AttributeName::kQuotaCacheHit[] = "quota.cache_hit"; // Authentication attributes const char AttributeName::kRequestAuthPrincipal[] = "request.auth.principal"; const char AttributeName::kRequestAuthAudiences[] = "request.auth.audiences"; +const char AttributeName::kRequestAuthGroups[] = "request.auth.groups"; const char AttributeName::kRequestAuthPresenter[] = "request.auth.presenter"; const char AttributeName::kRequestAuthClaims[] = "request.auth.claims"; const char AttributeName::kRequestAuthRawClaims[] = "request.auth.raw_claims"; @@ -84,5 +93,11 @@ const char AttributeName::kRequestAuthRawClaims[] = "request.auth.raw_claims"; const char AttributeName::kResponseGrpcStatus[] = "response.grpc_status"; const char AttributeName::kResponseGrpcMessage[] = "response.grpc_message"; +// Rbac attributes +const char AttributeName::kRbacPermissiveResponseCode[] = + "rbac.permissive.response_code"; +const char AttributeName::kRbacPermissivePolicyId[] = + "rbac.permissive.effective_policy_id"; + } // namespace utils } // namespace istio diff --git a/test/integration/BUILD b/test/integration/BUILD new file mode 100644 index 00000000000..6d4d98562b2 --- /dev/null +++ b/test/integration/BUILD @@ -0,0 +1,38 @@ +# Copyright 2018 Istio Authors. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +################################################################################ + +package(default_visibility = ["//visibility:public"]) + +load( + "@envoy//bazel:envoy_build_system.bzl", + "envoy_cc_test", +) + +envoy_cc_test( + name = "istio_http_integration_test", + srcs = ["istio_http_integration_test.cc"], + repository = "@envoy", + deps = [ + "@envoy//source/common/common:utility_lib", + "@envoy//test/integration:http_protocol_integration_lib", + "//include/istio/utils:attribute_names_header", + "//src/envoy/http/authn:filter_lib", + "//src/envoy/http/jwt_auth:http_filter_factory", + "//src/envoy/http/jwt_auth:jwt_lib", + "//src/envoy/utils:filter_names_lib", + "//src/envoy/http/mixer:filter_lib", + ], +) \ No newline at end of file diff --git a/test/integration/istio_http_integration_test.cc b/test/integration/istio_http_integration_test.cc new file mode 100644 index 00000000000..059ada91897 --- /dev/null +++ b/test/integration/istio_http_integration_test.cc @@ -0,0 +1,424 @@ +/* Copyright 2018 Istio Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// This test suite verifies the end-to-end behaviour of the HTTP filter chain +// with JWT + AuthN + Mixer. That chain is used in Istio, when authentication is +// active. Filters exchanges data between each other using request info (dynamic +// metadata) and that information can only be observed at the end (i.e from +// request to mixer backends). + +#include "fmt/printf.h" +#include "gmock/gmock.h" +#include "include/istio/utils/attribute_names.h" +#include "mixer/v1/check.pb.h" +#include "mixer/v1/report.pb.h" +#include "src/envoy/utils/filter_names.h" +#include "test/integration/http_protocol_integration.h" + +using ::google::protobuf::util::error::Code; +using ::testing::Contains; +using ::testing::Not; + +namespace Envoy { +namespace { + +// From +// https://github.com/istio/istio/blob/master/security/tools/jwt/samples/demo.jwt +constexpr char kGoodToken[] = + "eyJhbGciOiJSUzI1NiIsImtpZCI6IkRIRmJwb0lVcXJZOHQyenBBMnFYZkNtcjVWTzVaRXI0Un" + "pIVV8tZW52dlEiLC" + "J0eXAiOiJKV1QifQ." + "eyJleHAiOjQ2ODU5ODk3MDAsImZvbyI6ImJhciIsImlhdCI6MTUzMjM4OTcwMCwiaXNzIjoidG" + "VzdGluZ0BzZWN1cmUuaXN0aW8uaW8iLCJzdWIiOiJ0ZXN0aW5nQHNlY3VyZS5pc3Rpby5pbyJ9" + ".CfNnxWP2tcnR9q0v" + "xyxweaF3ovQYHYZl82hAUsn21bwQd9zP7c-LS9qd_vpdLG4Tn1A15NxfCjp5f7QNBUo-" + "KC9PJqYpgGbaXhaGx7bEdFW" + "jcwv3nZzvc7M__" + "ZpaCERdwU7igUmJqYGBYQ51vr2njU9ZimyKkfDe3axcyiBZde7G6dabliUosJvvKOPcKIWPccC" + "gef" + "Sj_GNfwIip3-SsFdlR7BtbVUcqR-yv-" + "XOxJ3Uc1MI0tz3uMiiZcyPV7sNCU4KRnemRIMHVOfuvHsU60_GhGbiSFzgPT" + "Aa9WTltbnarTbxudb_YEOx12JiwYToeX0DCPb43W1tzIBxgm8NxUg"; + +// Generated by gen-jwt.py as described in +// https://github.com/istio/istio/blob/master/security/tools/jwt/samples/README.md. +// `security/tools/jwt/samples/gen-jwt.py security/tools/jwt/samples/key.pem +// --expire=3153600000 --claims=rbac:rbac --iss "testing-rbac@secure.istio.io"` +constexpr char kRbacGoodToken[] = + "eyJhbGciOiJSUzI1NiIsImtpZCI6IkRIRmJwb0lVcXJZOHQyenBBMnFYZkNtcjVWTzVaRXI0Un" + "pIVV8tZW52dlEiLCJ0eXAiOiJKV1QifQ.eyJleHAiOjQ2ODc3ODQwODEsImlhdCI6MTUzNDE4N" + "DA4MSwiaXNzIjoidGVzdGluZy1yYmFjQHNlY3VyZS5pc3Rpby5pbyIsInJiYWMiOiJyYmFjIiw" + "ic3ViIjoidGVzdGluZy1yYmFjQHNlY3VyZS5pc3Rpby5pbyJ9.Cn4PADSzZ249_DMCFWF_JokR" + "bVgY-yoGkVqpW-aYHTYDShuLxfAdF1AAq5TLAi72A0UWBxwcZMIGcAudRdyM8-6ppXlj3P3Xg1" + "87d25-4EWR0SgVnW8DT2LCpeX9amPsKkKdo0L_ICfHzATsiqIN2GGvrIZWYHHrD1gNGwLBMSVU" + "tQxxkaw3k_yzAdzaitxJyMRGjTmTdl4ovdIBsxB9898wExet2etLz3ngfiM7EG5cpsd01Fxf_9" + "6LiXF8D4aM3k_cSQPrj3vGwRW4jSM27x0iGNaZIKNdoIZ861sfguiq6mMb1sVDbGhIW857M7z3" + "2R75bzlngKzeSEbBHXTF8g"; + +// Generate by gen-jwt.py as described in +// https://github.com/istio/istio/blob/master/security/tools/jwt/samples/README.md +// to generate token with invalid issuer. +// `security/tools/jwt/samples/gen-jwt.py security/tools/jwt/samples/key.pem +// --expire=3153600000 --iss "wrong-issuer@secure.istio.io"` +constexpr char kBadToken[] = + "eyJhbGciOiJSUzI1NiIsImtpZCI6IkRIRmJwb0lVcXJZOHQyenBBMnFYZkNtcjVWTzVaRXI0Un" + "pIVV8tZW52dlEiLCJ" + "0eXAiOiJKV1QifQ." + "eyJleHAiOjQ2ODcxODkyNTEsImlhdCI6MTUzMzU4OTI1MSwiaXNzIjoid3JvbmctaXNzdWVyQH" + "N" + "lY3VyZS5pc3Rpby5pbyIsInN1YiI6Indyb25nLWlzc3VlckBzZWN1cmUuaXN0aW8uaW8ifQ." + "Ye7RKrEgr3mUxRE1OF5" + "sCaaH6kg_OT-" + "mAM1HI3tTUp0ljVuxZLCcTXPvvEAjyeiNUm8fjeeER0fsXv7y8wTaA4FFw9x8NT9xS8pyLi6Rs" + "Twdjkq" + "0-Plu93VQk1R98BdbEVT-T5vVz7uACES4LQBqsvvTcLBbBNUvKs_" + "eJyZG71WJuymkkbL5Ki7CB73sQUMl2T3eORC7DJt" + "yn_C9Dxy2cwCzHrLZnnGz839_bX_yi29dI4veYCNBgU-" + "9ZwehqfgSCJWYUoBTrdM06N3jEemlWB83ZY4OXoW0pNx-ecu" + "3asJVbwyxV2_HT6_aUsdHwTYwHv2hXBjdKEfwZxSsBxbKpA"; + +constexpr char kExpectedPrincipal[] = + "testing@secure.istio.io/testing@secure.istio.io"; +constexpr char kRbacPrincipal[] = + "testing-rbac@secure.istio.io/testing-rbac@secure.istio.io"; +constexpr char kExpectedRawClaims[] = + "{\"exp\":4685989700,\"foo\":\"bar\",\"iat\":1532389700,\"iss\":\"testing@" + "secure.istio.io\"," + "\"sub\":\"testing@secure.istio.io\"}"; +constexpr char kDestinationUID[] = "dest.pod.123"; +constexpr char kSourceUID[] = "src.pod.xyz"; +constexpr char kTelemetryBackend[] = "telemetry-backend"; +constexpr char kPolicyBackend[] = "policy-backend"; + +// Generates basic test request header. +Http::TestHeaderMapImpl BaseRequestHeaders() { + return Http::TestHeaderMapImpl{{":method", "GET"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-forwarded-for", "10.0.0.1"}}; +} + +// Generates test request header with given token. +Http::TestHeaderMapImpl HeadersWithToken(const std::string& token) { + auto headers = BaseRequestHeaders(); + headers.addCopy("Authorization", "Bearer " + token); + return headers; +} + +std::string MakeJwtFilterConfig() { + constexpr char kJwtFilterTemplate[] = R"( + name: %s + config: + rules: + - issuer: "testing@secure.istio.io" + local_jwks: + inline_string: "%s" + - issuer: "testing-rbac@secure.istio.io" + local_jwks: + inline_string: "%s" + allow_missing_or_failed: true + )"; + // From + // https://github.com/istio/istio/blob/master/security/tools/jwt/samples/jwks.json + constexpr char kJwksInline[] = + "{ \"keys\":[ " + "{\"e\":\"AQAB\",\"kid\":\"DHFbpoIUqrY8t2zpA2qXfCmr5VO5ZEr4RzHU_-envvQ\"," + "\"kty\":\"RSA\",\"n\":\"xAE7eB6qugXyCAG3yhh7pkDkT65pHymX-" + "P7KfIupjf59vsdo91bSP9C8H07pSAGQO1MV" + "_xFj9VswgsCg4R6otmg5PV2He95lZdHtOcU5DXIg_" + "pbhLdKXbi66GlVeK6ABZOUW3WYtnNHD-91gVuoeJT_" + "DwtGGcp4ignkgXfkiEm4sw-4sfb4qdt5oLbyVpmW6x9cfa7vs2WTfURiCrBoUqgBo_-" + "4WTiULmmHSGZHOjzwa8WtrtOQGsAFjIbno85jp6MnGGGZPYZbDAa_b3y5u-" + "YpW7ypZrvD8BgtKVjgtQgZhLAGezMt0ua3DRrWnKqTZ0BJ_EyxOGuHJrLsn00fnMQ\"}]}"; + + return fmt::sprintf(kJwtFilterTemplate, Utils::IstioFilterName::kJwt, + StringUtil::escape(kJwksInline), + StringUtil::escape(kJwksInline)); +} + +std::string MakeAuthFilterConfig() { + constexpr char kAuthnFilterWithJwtTemplate[] = R"( + name: %s + config: + policy: + origins: + - jwt: + issuer: testing@secure.istio.io + jwks_uri: http://localhost:8081/ + - jwt: + issuer: testing-rbac@secure.istio.io + jwks_uri: http://localhost:8081/ + principalBinding: USE_ORIGIN +)"; + return fmt::sprintf(kAuthnFilterWithJwtTemplate, + Utils::IstioFilterName::kAuthentication); +} + +std::string MakeRbacFilterConfig() { + constexpr char kRbacFilterTemplate[] = R"( + name: envoy.filters.http.rbac + config: + rules: + policies: + "foo": + permissions: + - any: true + principals: + - metadata: + filter: %s + path: + - key: %s + value: + string_match: + exact: %s +)"; + return fmt::sprintf( + kRbacFilterTemplate, Utils::IstioFilterName::kAuthentication, + istio::utils::AttributeName::kRequestAuthPrincipal, kExpectedPrincipal); +} + +std::string MakeMixerFilterConfig() { + constexpr char kMixerFilterTemplate[] = R"( + name: mixer + config: + defaultDestinationService: "default" + mixerAttributes: + attributes: { + "destination.uid": { + stringValue: %s + } + } + serviceConfigs: { + "default": {} + } + transport: + attributes_for_mixer_proxy: + attributes: { + "source.uid": { + string_value: %s + } + } + report_cluster: %s + check_cluster: %s + )"; + return fmt::sprintf(kMixerFilterTemplate, kDestinationUID, kSourceUID, + kTelemetryBackend, kPolicyBackend); +} + +class IstioHttpIntegrationTest : public HttpProtocolIntegrationTest { + public: + void createUpstreams() override { + HttpProtocolIntegrationTest::createUpstreams(); + fake_upstreams_.emplace_back( + new FakeUpstream(0, FakeHttpConnection::Type::HTTP2, version_)); + telemetry_upstream_ = fake_upstreams_.back().get(); + + fake_upstreams_.emplace_back( + new FakeUpstream(0, FakeHttpConnection::Type::HTTP2, version_)); + policy_upstream_ = fake_upstreams_.back().get(); + } + + void SetUp() override { + config_helper_.addFilter(MakeMixerFilterConfig()); + config_helper_.addFilter(MakeRbacFilterConfig()); + config_helper_.addFilter(MakeAuthFilterConfig()); + config_helper_.addFilter(MakeJwtFilterConfig()); + + config_helper_.addConfigModifier(addCluster(kTelemetryBackend)); + config_helper_.addConfigModifier(addCluster(kPolicyBackend)); + + HttpProtocolIntegrationTest::initialize(); + } + + void TearDown() override { + cleanupConnection(fake_upstream_connection_); + cleanupConnection(telemetry_connection_); + cleanupConnection(policy_connection_); + } + + ConfigHelper::ConfigModifierFunction addCluster(const std::string& name) { + return [name](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + auto* cluster = bootstrap.mutable_static_resources()->add_clusters(); + cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); + cluster->mutable_http2_protocol_options(); + cluster->set_name(name); + }; + } + + void waitForTelemetryRequest(::istio::mixer::v1::ReportRequest* request) { + AssertionResult result = telemetry_upstream_->waitForHttpConnection( + *dispatcher_, telemetry_connection_); + RELEASE_ASSERT(result, result.message()); + result = telemetry_connection_->waitForNewStream(*dispatcher_, + telemetry_request_); + RELEASE_ASSERT(result, result.message()); + + result = telemetry_request_->waitForGrpcMessage(*dispatcher_, *request); + RELEASE_ASSERT(result, result.message()); + } + + // Must be called after waitForTelemetryRequest + void sendTelemetryResponse() { + telemetry_request_->startGrpcStream(); + telemetry_request_->sendGrpcMessage(::istio::mixer::v1::ReportResponse{}); + telemetry_request_->finishGrpcStream(Grpc::Status::Ok); + } + + void waitForPolicyRequest(::istio::mixer::v1::CheckRequest* request) { + AssertionResult result = policy_upstream_->waitForHttpConnection( + *dispatcher_, policy_connection_); + RELEASE_ASSERT(result, result.message()); + result = + policy_connection_->waitForNewStream(*dispatcher_, policy_request_); + RELEASE_ASSERT(result, result.message()); + + result = policy_request_->waitForGrpcMessage(*dispatcher_, *request); + RELEASE_ASSERT(result, result.message()); + } + + // Must be called after waitForPolicyRequest + void sendPolicyResponse() { + policy_request_->startGrpcStream(); + ::istio::mixer::v1::CheckResponse response; + response.mutable_precondition()->mutable_status()->set_code(Code::OK); + policy_request_->sendGrpcMessage(response); + policy_request_->finishGrpcStream(Grpc::Status::Ok); + } + + void cleanupConnection(FakeHttpConnectionPtr& connection) { + if (connection != nullptr) { + AssertionResult result = connection->close(); + RELEASE_ASSERT(result, result.message()); + result = connection->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); + } + } + + FakeUpstream* telemetry_upstream_{}; + FakeHttpConnectionPtr telemetry_connection_{}; + FakeStreamPtr telemetry_request_{}; + + FakeUpstream* policy_upstream_{}; + FakeHttpConnectionPtr policy_connection_{}; + FakeStreamPtr policy_request_{}; +}; + +INSTANTIATE_TEST_CASE_P( + Protocols, IstioHttpIntegrationTest, + testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams()), + HttpProtocolIntegrationTest::protocolTestParamsToString); + +TEST_P(IstioHttpIntegrationTest, NoJwt) { + // initialize(); + codec_client_ = + makeHttpConnection(makeClientConnection((lookupPort("http")))); + auto response = codec_client_->makeHeaderOnlyRequest(BaseRequestHeaders()); + + ::istio::mixer::v1::ReportRequest report_request; + waitForTelemetryRequest(&report_request); + // As authentication fail, report should not have 'word' that might come + // authN. + EXPECT_THAT(report_request.default_words(), + ::testing::AllOf(Contains(kDestinationUID), Contains("10.0.0.1"), + Not(Contains(kExpectedPrincipal)))); + sendTelemetryResponse(); + + response->waitForEndStream(); + EXPECT_TRUE(response->complete()); + EXPECT_STREQ("401", response->headers().Status()->value().c_str()); +} + +TEST_P(IstioHttpIntegrationTest, BadJwt) { + codec_client_ = + makeHttpConnection(makeClientConnection((lookupPort("http")))); + auto response = + codec_client_->makeHeaderOnlyRequest(HeadersWithToken(kBadToken)); + + ::istio::mixer::v1::ReportRequest report_request; + waitForTelemetryRequest(&report_request); + EXPECT_THAT(report_request.default_words(), + ::testing::AllOf(Contains(kDestinationUID), Contains("10.0.0.1"), + Not(Contains(kExpectedPrincipal)))); + sendTelemetryResponse(); + + response->waitForEndStream(); + EXPECT_TRUE(response->complete()); + EXPECT_STREQ("401", response->headers().Status()->value().c_str()); +} + +TEST_P(IstioHttpIntegrationTest, RbacDeny) { + codec_client_ = + makeHttpConnection(makeClientConnection((lookupPort("http")))); + auto response = + codec_client_->makeHeaderOnlyRequest(HeadersWithToken(kRbacGoodToken)); + + ::istio::mixer::v1::ReportRequest report_request; + waitForTelemetryRequest(&report_request); + // As authentication succeeded, report should have 'word' that comes from + // authN. + EXPECT_THAT(report_request.default_words(), + ::testing::AllOf(Contains(kDestinationUID), Contains("10.0.0.1"), + Contains(kRbacPrincipal))); + sendTelemetryResponse(); + + response->waitForEndStream(); + EXPECT_TRUE(response->complete()); + + // Expecting error code 403 for RBAC deny. + EXPECT_STREQ("403", response->headers().Status()->value().c_str()); +} + +TEST_P(IstioHttpIntegrationTest, GoodJwt) { + codec_client_ = + makeHttpConnection(makeClientConnection((lookupPort("http")))); + auto response = + codec_client_->makeHeaderOnlyRequest(HeadersWithToken(kGoodToken)); + + ::istio::mixer::v1::CheckRequest check_request; + waitForPolicyRequest(&check_request); + // Check request should see authn attributes. + EXPECT_THAT(check_request.attributes().words(), + ::testing::AllOf( + Contains(kDestinationUID), Contains("10.0.0.1"), + Contains(kExpectedPrincipal), Contains(kExpectedRawClaims), + Contains("testing@secure.istio.io"), Contains("sub"), + Contains("iss"), Contains("foo"), Contains("bar"))); + sendPolicyResponse(); + + waitForNextUpstreamRequest(0); + // Send backend response. + upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, + true); + response->waitForEndStream(); + + // Report (log) is sent after backen response. + ::istio::mixer::v1::ReportRequest report_request; + waitForTelemetryRequest(&report_request); + // Report request should also see the same authn attributes. + EXPECT_THAT(report_request.default_words(), + ::testing::AllOf( + Contains(kDestinationUID), Contains("10.0.0.1"), + Contains(kExpectedPrincipal), Contains(kExpectedRawClaims), + Contains("testing@secure.istio.io"), Contains("sub"), + Contains("iss"), Contains("foo"), Contains("bar"))); + sendTelemetryResponse(); + + EXPECT_TRUE(response->complete()); + EXPECT_STREQ("200", response->headers().Status()->value().c_str()); +} + +} // namespace +} // namespace Envoy diff --git a/tools/bazel.rc b/tools/bazel.rc index 942b60e4fea..75a93323f7c 100644 --- a/tools/bazel.rc +++ b/tools/bazel.rc @@ -1,7 +1,7 @@ # Copied from https://github.com/envoyproxy/envoy/blob/master/tools/bazel.rc # Envoy specific Bazel build/test options. -#build --workspace_status_command=bazel/get_workspace_status +build --workspace_status_command=tools/bazel_get_workspace_status # Basic ASAN/UBSAN that works for gcc build:asan --define ENVOY_CONFIG_ASAN=1 diff --git a/tools/bazel.rc.ci b/tools/bazel.rc.ci index 664f7b13380..cb9e599b9ca 100644 --- a/tools/bazel.rc.ci +++ b/tools/bazel.rc.ci @@ -1,7 +1,8 @@ +build --workspace_status_command=tools/bazel_get_workspace_status + # This is from Bazel's former travis setup, to avoid blowing up the RAM usage. startup --host_jvm_args=-Xmx8192m startup --host_jvm_args=-Xms8192m -startup --batch # This is so we understand failures better build --verbose_failures diff --git a/tools/bazel.rc.cloudbuilder b/tools/bazel.rc.cloudbuilder index 664f7b13380..cb9e599b9ca 100644 --- a/tools/bazel.rc.cloudbuilder +++ b/tools/bazel.rc.cloudbuilder @@ -1,7 +1,8 @@ +build --workspace_status_command=tools/bazel_get_workspace_status + # This is from Bazel's former travis setup, to avoid blowing up the RAM usage. startup --host_jvm_args=-Xmx8192m startup --host_jvm_args=-Xms8192m -startup --batch # This is so we understand failures better build --verbose_failures diff --git a/tools/bazel_get_workspace_status b/tools/bazel_get_workspace_status new file mode 100755 index 00000000000..b3928f9a1d9 --- /dev/null +++ b/tools/bazel_get_workspace_status @@ -0,0 +1,29 @@ +#!/bin/bash +# +# Copyright 2016 Istio Authors. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +################################################################################ + +if git rev-parse --verify --quiet HEAD >/dev/null; then + echo "BUILD_SCM_REVISION $(git rev-parse --verify HEAD)" +else + exit 1 +fi + +if git diff-index --quiet HEAD; then + echo "BUILD_SCM_STATUS Clean" +else + echo "BUILD_SCM_STATUS Modified" +fi