Skip to content

Commit

Permalink
add option to skip verify peer cert during token endpoint request (#203)
Browse files Browse the repository at this point in the history
* add option to skip verify peer cert during token endpoint request

Signed-off-by: Shikugawa <Shikugawa@gmail.com>

* fix

Signed-off-by: Shikugawa <Shikugawa@gmail.com>

* fix

Signed-off-by: Shikugawa <Shikugawa@gmail.com>

* fix

Signed-off-by: Shikugawa <Shikugawa@gmail.com>

* fix

Signed-off-by: Shikugawa <Shikugawa@gmail.com>

* fix

Signed-off-by: Shikugawa <Shikugawa@gmail.com>
  • Loading branch information
Shikugawa authored Jan 21, 2022
1 parent 5d3a902 commit 68be5b6
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 32 deletions.
14 changes: 14 additions & 0 deletions config/oidc/config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,13 @@ message OIDCConfig {
// default to 1200 seconds, 20min.
// Optional.
uint32 periodic_fetch_interval_sec = 2;

// If set to true, the verification of the destination certificate will be skipped when
// making a request to the JWKs URI. This option is useful when you want to use a
// self-signed certificate for testing purposes, but basically should not be set to
// true in any other cases.
// Optional.
bool skip_verify_peer_cert = 3;
}

oneof jwks_config {
Expand Down Expand Up @@ -188,4 +195,11 @@ message OIDCConfig {
// When specified, the Authservice will use the configured Redis server to store session data.
// Optional.
RedisConfig redis_session_store_config = 16;

// If set to true, the verification of the destination certificate will be skipped when
// making a request to the Token Endpoint. This option is useful when you want to use a
// self-signed certificate for testing purposes, but basically should not be set to true
// in any other cases.
// Optional.
bool skip_verify_peer_cert = 18;
}
2 changes: 2 additions & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ The configuration of an OpenID Connect filter that can be used to retrieve ident
| JwksFetcherConfig | This message defines a setting to allow asynchronous retrieval and update of the JWK for JWT validation at regular intervals. | message |
| jwks_uri | Request URI that has the JWKs. Required. | string |
| periodic_fetch_interval_sec | Request interval to check whether new JWKs are available. If not specified, default to 1200 seconds, 20min. Optional. | uint32 |
| skip_verify_peer_cert | If set to true, the verification of the destination certificate will be skipped when making a request to the JWKs URI. This option is useful when you want to use a self-signed certificate for testing purposes, but basically should not be set to true in any other cases. Optional. | bool |
| jwks_config | | oneof |
| jwks | The JSON JWKS response from the OIDC provider’s `jwks_uri` URI which can be found in the OIDC provider's [configuration response](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationResponse). Note that this JSON value must be escaped when embedded in a json configmap (see [example](https://github.com/istio-ecosystem/authservice/blob/master/bookinfo-example/config/authservice-configmap-template.yaml)). Used during token verification. | string |
| jwks_fetcher | Configuration to allow JWKs to be retrieved and updated asynchronously at regular intervals. | JwksFetcherConfig |
Expand All @@ -104,6 +105,7 @@ The configuration of an OpenID Connect filter that can be used to retrieve ident
| trusted_certificate_authority | When specified, the Authservice will trust the specified Certificate Authority when performing HTTPS calls to the Token Endpoint of the OIDC Identity Provider. Optional. | string |
| proxy_uri | The Authservice makes two kinds of direct network connections directly to the OIDC Provider. Both are POST requests to the configured `token_uri` of the OIDC Provider. The first is to exchange the authorization code for tokens, and the other is to use the refresh token to obtain new tokens. Configure the `proxy_uri` when both of these requests should be made through a web proxy. The format of `proxy_uri` is `http://proxyserver.example.com:8080`, where `:<port_number>` is optional. Userinfo (usernames and passwords) in the `proxy_uri` setting are not yet supported. The `proxy_uri` should always start with `http://`. The Authservice will upgrade the connection to the OIDC provider to HTTPS using an HTTP CONNECT request to the proxy server. The proxy server will see the hostname and port number of the OIDC provider in plain text in the CONNECT request, but all other communication will occur over an encrypted HTTPS connection negotiated directly between the Authservice and the OIDC provider. See also the related `trusted_certificate_authority` configuration option. Optional. | string |
| redis_session_store_config | When specified, the Authservice will use the configured Redis server to store session data. Optional. | RedisConfig |
| skip_verify_peer_cert | If set to true, the verification of the destination certificate will be skipped when making a request to the Token Endpoint. This option is useful when you want to use a self-signed certificate for testing purposes, but basically should not be set to true in any other cases. Optional. | bool |



Expand Down
17 changes: 11 additions & 6 deletions src/filters/oidc/jwks_resolver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,23 @@ namespace oidc {

namespace {
constexpr uint32_t kJwksInitialFetchDelaySec = 3;
}
constexpr uint32_t kJwksPeriodicFetchIntervalSec = 1200;
} // namespace

DynamicJwksResolverImpl::JwksFetcher::JwksFetcher(
DynamicJwksResolverImpl* parent, common::http::ptr_t http_ptr,
const std::string& jwks_uri,
std::chrono::seconds periodic_fetch_interval_sec,
const config::oidc::OIDCConfig::JwksFetcherConfig& config,
boost::asio::io_context& io_context)
: parent_(parent),
jwks_uri_(jwks_uri),
jwks_uri_(config.jwks_uri()),
http_ptr_(http_ptr),
ioc_(io_context),
periodic_fetch_interval_sec_(periodic_fetch_interval_sec),
timer_(ioc_, periodic_fetch_interval_sec_) {
periodic_fetch_interval_sec_(
std::chrono::seconds(config.periodic_fetch_interval_sec() != 0
? config.periodic_fetch_interval_sec()
: kJwksPeriodicFetchIntervalSec)),
timer_(ioc_, periodic_fetch_interval_sec_),
verify_peer_cert_(!config.skip_verify_peer_cert()) {
// Extract initial JWKs.
// After timer callback sucessful, next timer invocation will be scheduled.
timer_.expires_at(std::chrono::steady_clock::now() +
Expand All @@ -33,6 +37,7 @@ void DynamicJwksResolverImpl::JwksFetcher::request(
const boost::system::error_code&) {
boost::asio::spawn(ioc_, [this](boost::asio::yield_context yield) {
common::http::TransportSocketOptions opt;
opt.verify_peer_ = verify_peer_cert_;
auto resp = http_ptr_->Get(jwks_uri_, {}, "", opt, "", ioc_, yield);
auto next_schedule_interval = periodic_fetch_interval_sec_;

Expand Down
27 changes: 7 additions & 20 deletions src/filters/oidc/jwks_resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
namespace authservice {
namespace filters {
namespace oidc {
namespace {
constexpr uint32_t kJwksPeriodicFetchIntervalSec = 1200;
}
class JwksResolver {
public:
virtual ~JwksResolver() = default;
Expand Down Expand Up @@ -68,7 +65,7 @@ class DynamicJwksResolverImpl : public JwksResolver {
class JwksFetcher {
public:
JwksFetcher(DynamicJwksResolverImpl* parent, common::http::ptr_t http_ptr,
const std::string& jwks_uri, std::chrono::seconds duration,
const config::oidc::OIDCConfig::JwksFetcherConfig& config,
boost::asio::io_context& ioc);

private:
Expand All @@ -80,16 +77,13 @@ class DynamicJwksResolverImpl : public JwksResolver {
boost::asio::io_context& ioc_;
std::chrono::seconds periodic_fetch_interval_sec_;
boost::asio::steady_timer timer_;
bool verify_peer_cert_ = false;
};

explicit DynamicJwksResolverImpl(const std::string& jwks_uri,
std::chrono::seconds duration,
common::http::ptr_t http_ptr,
boost::asio::io_context& ioc) {
if (duration != std::chrono::seconds(0)) {
jwks_fetcher_ = std::make_unique<JwksFetcher>(this, http_ptr, jwks_uri,
duration, ioc);
}
explicit DynamicJwksResolverImpl(
const config::oidc::OIDCConfig::JwksFetcherConfig& config,
common::http::ptr_t http_ptr, boost::asio::io_context& ioc) {
jwks_fetcher_ = std::make_unique<JwksFetcher>(this, http_ptr, config, ioc);
}

void updateJwks(const std::string& new_jwks) {
Expand Down Expand Up @@ -138,16 +132,9 @@ class JwksResolverCacheImpl final : public JwksResolverCache {
std::make_shared<oidc::StaticJwksResolverImpl>(config_.jwks());
break;
case config::oidc::OIDCConfig::kJwksFetcher: {
uint32_t periodic_fetch_interval_sec =
config_.jwks_fetcher().periodic_fetch_interval_sec();
if (periodic_fetch_interval_sec == 0) {
periodic_fetch_interval_sec = kJwksPeriodicFetchIntervalSec;
}

auto http_ptr = common::http::ptr_t(new common::http::HttpImpl);
resolver_ = std::make_shared<oidc::DynamicJwksResolverImpl>(
config_.jwks_fetcher().jwks_uri(),
std::chrono::seconds(periodic_fetch_interval_sec), http_ptr, ioc);
config_.jwks_fetcher(), http_ptr, ioc);
break;
}
default:
Expand Down
2 changes: 2 additions & 0 deletions src/filters/oidc/oidc_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,7 @@ std::shared_ptr<TokenResponse> OidcFilter::RefreshToken(
spdlog::info("{}: POSTing to refresh access token", __func__);
common::http::TransportSocketOptions opt;
opt.ca_cert_ = idp_config_.trusted_certificate_authority();
opt.verify_peer_ = !idp_config_.skip_verify_peer_cert();
auto retrieved_token_response =
http_ptr_->Post(idp_config_.token_uri(), headers,
common::http::Http::EncodeFormData(params), opt,
Expand Down Expand Up @@ -652,6 +653,7 @@ google::rpc::Code OidcFilter::RetrieveToken(

common::http::TransportSocketOptions opt;
opt.ca_cert_ = idp_config_.trusted_certificate_authority();
opt.verify_peer_ = !idp_config_.skip_verify_peer_cert();
auto retrieve_token_response =
http_ptr_->Post(idp_config_.token_uri(), headers,
common::http::Http::EncodeFormData(params), opt,
Expand Down
20 changes: 14 additions & 6 deletions test/filters/oidc/jwks_resolver_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,10 @@ void setExpectedRemoteJwks(common::http::HttpMock& mock_http,
TEST(JwksResolverTest, TestDynamicJwksResolver) {
boost::asio::io_context io_context;
auto mock_http = std::make_shared<common::http::HttpMock>();
DynamicJwksResolverImpl resolver("istio.io", std::chrono::seconds(1),
mock_http, io_context);
config::oidc::OIDCConfig::JwksFetcherConfig config;
config.set_jwks_uri("istio.io");
config.set_periodic_fetch_interval_sec(1);
DynamicJwksResolverImpl resolver(config, mock_http, io_context);

// First flight to extract invalid JWKs.
setExpectedRemoteJwks(*mock_http, invalid_jwt_public_key_);
Expand Down Expand Up @@ -140,8 +142,11 @@ TEST(JwksResolverTest, TestDynamicJwksResolver) {
TEST(JwksResolverTest, TestDynamicJwksResolverWithInvalidHttpStatus) {
boost::asio::io_context io_context;
auto mock_http = std::make_shared<common::http::HttpMock>();
DynamicJwksResolverImpl resolver("istio.io", std::chrono::seconds(1),
mock_http, io_context);
config::oidc::OIDCConfig::JwksFetcherConfig config;
config.set_jwks_uri("istio.io");
config.set_periodic_fetch_interval_sec(1);

DynamicJwksResolverImpl resolver(config, mock_http, io_context);

// Never initialized with invalid HTTP status.
EXPECT_CALL(*mock_http, Get(Eq("istio.io"), _, _, _, _, _, _))
Expand All @@ -168,8 +173,11 @@ TEST(JwksResolverTest,

// Configured request interval as 10000 sec. This value is enough to guarantee
// that second request will not be invoked for 3 sec evloop run.
DynamicJwksResolverImpl resolver("istio.io", std::chrono::seconds(10000),
mock_http, io_context);
config::oidc::OIDCConfig::JwksFetcherConfig config;
config.set_jwks_uri("istio.io");
config.set_periodic_fetch_interval_sec(10000);

DynamicJwksResolverImpl resolver(config, mock_http, io_context);

// Initially make mock server always return invalid HTTP response as 503.
EXPECT_CALL(*mock_http, Get(Eq("istio.io"), _, _, _, _, _, _))
Expand Down

0 comments on commit 68be5b6

Please sign in to comment.