diff --git a/.github/workflows/commit.yaml b/.github/workflows/commit.yaml index 8d162304..b3c6d2fe 100644 --- a/.github/workflows/commit.yaml +++ b/.github/workflows/commit.yaml @@ -55,14 +55,13 @@ jobs: strategy: fail-fast: false # don't fail fast as sometimes failures are operating system specific. matrix: - os: - - "macos-11" - - "ubuntu-18.04" - mode: - - "default" - # On CI, by default, we use libc++. - - "clang" - - "clang-fips" + include: + - os: macos-11 + mode: default + - os: ubuntu-18.04 + mode: clang + - os: ubuntu-18.04 + mode: clang-fips steps: - name: Cancel when duplicated uses: styfle/cancel-workflow-action@0.4.1 @@ -100,7 +99,7 @@ jobs: # Prepare clang tooling and config when it is required. - name: Setup clang - if: matrix.mode == 'clang' || matrix.mode == 'clang-fips' + if: runner.os == 'Linux' && (matrix.mode == 'clang' || matrix.mode == 'clang-fips') # This downloads the required clang tooling when it is not downloaded yet. run: | make clang.bazelrc @@ -108,7 +107,7 @@ jobs: # Set BAZEL_FLAGS to FIPS mode only when it is required. - name: Setup FIPS mode - if: matrix.mode == 'clang-fips' + if: runner.os == 'Linux' && matrix.mode == 'clang-fips' run: echo "BAZEL_FLAGS=--config=libc++ --define=boringssl=fips" >> $GITHUB_ENV - name: Run all tests diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index a69883a5..9d52a3e4 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -17,14 +17,13 @@ jobs: strategy: fail-fast: false # don't fail fast as sometimes failures are operating system specific. matrix: - os: - - "macos-11" - - "ubuntu-18.04" - mode: - - "default" - # By default we use libc++. - - "clang" - - "clang-fips" + include: + - os: macos-11 + mode: default + - os: ubuntu-18.04 + mode: clang + - os: ubuntu-18.04 + mode: clang-fips steps: - name: Cancel when duplicated uses: styfle/cancel-workflow-action@0.4.1 @@ -62,20 +61,21 @@ jobs: # Prepare clang tooling and config when it is required. - name: Setup clang - if: matrix.mode == 'clang' || matrix.mode == 'clang-fips' + if: runner.os == 'Linux' && (matrix.mode == 'clang' || matrix.mode == 'clang-fips') # This downloads the required clang tooling when it is not downloaded yet. + # GITHUB_REF: refs/tags/0.5.1-rc1, resulted VERSION: "0.5.1". run: | make clang.bazelrc echo "BAZEL_FLAGS=--config=libc++" >> $GITHUB_ENV + echo "VERSION=${GITHUB_REF#refs/tags/}" >> $GITHUB_ENV # Set BAZEL_FLAGS to FIPS mode only when it is required. - name: Setup FIPS mode - if: matrix.mode == 'clang-fips' + if: runner.os == 'Linux' && matrix.mode == 'clang-fips' run: echo "BAZEL_FLAGS=--config=libc++ --define=boringssl=fips" >> $GITHUB_ENV - name: Create artifacts - # GITHUB_REF: refs/tags/0.5.1-rc1, resulted VERSION: "0.5.1". - run: VERSION=${GITHUB_REF#refs/tags/} MODE=${{ matrix.mode }} make dist + run: MODE=${{ matrix.mode }} make dist - name: Require static binary if: runner.os == 'Linux' && matrix.mode == 'clang' @@ -119,9 +119,6 @@ jobs: uses: softprops/action-gh-release@v1 with: files: | - dist-Linux-default/**/*.tar.gz dist-Linux-clang/**/*.tar.gz dist-Linux-clang-fips/**/*.tar.gz dist-macOS-default/**/*.tar.gz - dist-macOS-clang/**/*.tar.gz - dist-macOS-clang-fips/**/*.tar.gz diff --git a/Makefile b/Makefile index 45fd4c3c..22b095de 100644 --- a/Makefile +++ b/Makefile @@ -51,7 +51,7 @@ main_target := //src/main:$(binary_name) # Always use amd64 for bazelisk for build and test rules below, since we don't support for macOS # arm64 (with --host_javabase=@local_jdk//:jdk) yet (especially the protoc-gen-validate project: # "no matching toolchains found for types @io_bazel_rules_go//go:toolchain"). -bazel := GOARCH=amd64 $(go) run $(bazelisk@v) --output_user_root=$(bazel_cache_dir) +bazel := GOARCH=amd64 $(go) run $(bazelisk@v) $(if $(CI),--output_user_root=$(bazel_cache_dir),) buildifier := $(go_tools_dir)/buildifier envsubst := $(go_tools_dir)/envsubst protodoc := $(go_tools_dir)/protodoc @@ -90,7 +90,7 @@ build-%: dist: dist/$(binary_name)_$(goos)_amd64_$(MODE)_$(VERSION).tar.gz # Since we don't do cross-compilation yet (probably we can do it later via `zig cc`), we can only -# build artifact for the current `os` and `mode` pair (e.g. {os: 'macOS', mode: 'clang-fips'}). +# build artifact for the current `os` and `mode` pair (e.g. {os: 'linux', mode: 'clang-fips'}). dist/$(binary_name)_$(goos)_amd64_$(MODE)_$(VERSION).tar.gz: $(stripped_binary) ## Create build artifacts @$(eval DIST_DIR := $(shell mktemp -d)) @cp -f LICENSE $(DIST_DIR) diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 29f92f92..581937dc 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -60,6 +60,8 @@ def boost(): urls = [ "https://github.com/nelhage/rules_boost/archive/%s.tar.gz" % _RULES_BOOST_COMMIT, ], + patches = ["//bazel:rules_boost.patch"], + patch_args = ["-p1"], ) def com_github_redis_hiredis(): diff --git a/bazel/rules_boost.patch b/bazel/rules_boost.patch new file mode 100644 index 00000000..e66cd742 --- /dev/null +++ b/bazel/rules_boost.patch @@ -0,0 +1,14 @@ +# To make sure we use the provided @envoy//bazel:boringssl. +diff --git a/BUILD.boost b/BUILD.boost +index 4354973..1c11525 100644 +--- a/BUILD.boost ++++ b/BUILD.boost +@@ -481,7 +481,7 @@ cc_library( + visibility = ["//visibility:public"], + deps = [ + ":asio", +- "@openssl//:ssl", ++ "@envoy//bazel:boringssl", + ], + ) + diff --git a/bookinfo-example/authservice/templates/config.yaml b/bookinfo-example/authservice/templates/config.yaml index 247b6ca4..7bb94592 100644 --- a/bookinfo-example/authservice/templates/config.yaml +++ b/bookinfo-example/authservice/templates/config.yaml @@ -20,9 +20,14 @@ data: "listen_port": "10003", "log_level": "trace", "threads": 8, + "allow_unmatched_requests": "false", "chains": [ { "name": "idp_filter_chain", + "match": { + "header": ":authority", + "prefix": "localhost", + }, "filters": [ { "oidc": diff --git a/src/common/http/http.cc b/src/common/http/http.cc index 71683642..3d51c56e 100644 --- a/src/common/http/http.cc +++ b/src/common/http/http.cc @@ -424,7 +424,14 @@ response_t HttpImpl::Post( boost::asio::buffer(options.ca_cert_.data(), options.ca_cert_.size()), ca_ec); if (ca_ec) { - throw boost::system::system_error{ca_ec}; + // X509_R_CERT_ALREADY_IN_HASH_TABLE can be ignored. + // Reference: + // https://github.com/facebook/folly/blob/d3354e2282303402e70d829d19bfecce051a5850/folly/ssl/OpenSSLCertUtils.cpp#L367-L368. + if (ca_ec.category() != boost::asio::error::get_ssl_category() || + ERR_GET_REASON(ca_ec.value()) != + X509_R_CERT_ALREADY_IN_HASH_TABLE) { + throw boost::system::system_error{ca_ec}; + } } } @@ -543,7 +550,14 @@ response_t HttpImpl::Get( boost::asio::buffer(options.ca_cert_.data(), options.ca_cert_.size()), ca_ec); if (ca_ec) { - throw boost::system::system_error{ca_ec}; + // X509_R_CERT_ALREADY_IN_HASH_TABLE can be ignored. + // Reference: + // https://github.com/facebook/folly/blob/d3354e2282303402e70d829d19bfecce051a5850/folly/ssl/OpenSSLCertUtils.cpp#L367-L368. + if (ca_ec.category() != boost::asio::error::get_ssl_category() || + ERR_GET_REASON(ca_ec.value()) != + X509_R_CERT_ALREADY_IN_HASH_TABLE) { + throw boost::system::system_error{ca_ec}; + } } } diff --git a/src/service/async_service_impl.cc b/src/service/async_service_impl.cc index 9849e8d9..561c7316 100644 --- a/src/service/async_service_impl.cc +++ b/src/service/async_service_impl.cc @@ -18,6 +18,33 @@ namespace { constexpr uint16_t kHealthCheckServerPort = 10004; } +::grpc::Status convertGrpcStatus(const google::rpc::Code status) { + // See src/filters/filter.h:filter::Process for a description of how + // status codes should be handled + switch (status) { + case google::rpc::Code::OK: // The request was successful + case google::rpc::Code::UNAUTHENTICATED: // A filter indicated the + // request had no + // authentication but was + // processed correctly. + case google::rpc::Code::PERMISSION_DENIED: // A filter indicated + // insufficient permissions + // for the authenticated + // requester but was processed + // correctly. + return ::grpc::Status::OK; + case google::rpc::Code::INVALID_ARGUMENT: // The request was not well + // formed. Indicate a + // processing error to the + // caller. + return ::grpc::Status(::grpc::StatusCode::INVALID_ARGUMENT, + "invalid request"); + default: // All other errors are treated as internal processing + // failures. + return ::grpc::Status(::grpc::StatusCode::INTERNAL, "internal error"); + } +} + ProcessingStateV2::ProcessingStateV2( ProcessingStateFactory &parent, envoy::service::auth::v2::Authorization::AsyncService &service) diff --git a/src/service/async_service_impl.h b/src/service/async_service_impl.h index 1e4652d5..eca989a1 100644 --- a/src/service/async_service_impl.h +++ b/src/service/async_service_impl.h @@ -22,6 +22,8 @@ namespace authservice { namespace service { +::grpc::Status convertGrpcStatus(const google::rpc::Code status); + template ::grpc::Status Check( const RequestType &request, ResponseType &response, @@ -59,14 +61,6 @@ ::grpc::Status Check( return ::grpc::Status::OK; } - // TODO(incfly): Clean up trigger rule after checking the current Istio - // ExtAuthz API is sufficient. - const auto default_response_code = - allow_unmatched_requests - ? grpc::Status::OK - : grpc::Status(grpc::StatusCode::PERMISSION_DENIED, - "permission denied"); - // Find a configured processing chain. for (auto &chain : chains) { if (chain->Matches(&request_v3)) { @@ -80,7 +74,6 @@ ::grpc::Status Check( // Create a new instance of a processor. auto processor = chain->New(); auto status = processor->Process(&request_v3, &response_v3, ioc, yield); - // response v2/v3 conversion layer if constexpr (std::is_same_v< ResponseType, @@ -97,42 +90,44 @@ ::grpc::Status Check( ::envoy::service::auth::v3::CheckResponse>) { response = response_v3; } - - // See src/filters/filter.h:filter::Process for a description of how - // status codes should be handled - switch (status) { - case google::rpc::Code::OK: // The request was successful - case google::rpc::Code::UNAUTHENTICATED: // A filter indicated the - // request had no - // authentication but was - // processed correctly. - case google::rpc::Code::PERMISSION_DENIED: // A filter indicated - // insufficient permissions - // for the authenticated - // requester but was processed - // correctly. - return ::grpc::Status::OK; - case google::rpc::Code::INVALID_ARGUMENT: // The request was not well - // formed. Indicate a - // processing error to the - // caller. - return ::grpc::Status(::grpc::StatusCode::INVALID_ARGUMENT, - "invalid request"); - default: // All other errors are treated as internal processing - // failures. - return ::grpc::Status(::grpc::StatusCode::INTERNAL, - "internal error"); - } + return convertGrpcStatus(status); } } + // No matching filter chain found. + + // TODO(incfly): Clean up trigger rule after checking the current Istio + // ExtAuthz API is sufficient. + auto default_response_code = + grpc::Status(grpc::StatusCode::PERMISSION_DENIED, "permission denied"); + google::rpc::Code code = google::rpc::Code::PERMISSION_DENIED; + if (allow_unmatched_requests) { + default_response_code = grpc::Status::OK; + code = google::rpc::Code::OK; + } + envoy::service::auth::v2::CheckResponse response_v2; + envoy::service::auth::v3::CheckResponse response_v3; + response_v2.mutable_status()->set_code(code); + response_v3.mutable_status()->set_code(code); + if constexpr (std::is_same_v) { + response = response_v2; + } else if (std::is_same_v) { + response = response_v3; + } + + if constexpr (std::is_same_v) { + response = response_v3; + } - // No matching filter chain found. Allow request to continue. spdlog::debug( - "{}: no matching filter chain for request to {}://{}{}, respond with: " + "{}: no matching filter chain for request to " + "{}://{}{}, allow_unmatched_requests {}, respond with: " "{}", __func__, request.attributes().request().http().scheme(), request.attributes().request().http().host(), - request.attributes().request().http().path(), + request.attributes().request().http().path(), allow_unmatched_requests, default_response_code.error_code()); return default_response_code; } catch (const std::exception &exception) {