diff --git a/api/envoy/extensions/filters/network/postgres_proxy/v3alpha/postgres_proxy.proto b/api/envoy/extensions/filters/network/postgres_proxy/v3alpha/postgres_proxy.proto index aa8e0f5941bf..496599d1715f 100644 --- a/api/envoy/extensions/filters/network/postgres_proxy/v3alpha/postgres_proxy.proto +++ b/api/envoy/extensions/filters/network/postgres_proxy/v3alpha/postgres_proxy.proto @@ -29,4 +29,15 @@ message PostgresProxy { // are parsed. Parsing is required to produce Postgres proxy filter // metadata. Defaults to true. google.protobuf.BoolValue enable_sql_parsing = 2; + + // Controls whether to terminate SSL session initiated by a client. + // If the value is false, the Postgres proxy filter will not try to + // terminate SSL session, but will pass all the packets to the upstream server. + // If the value is true, the Postgres proxy filter will try to terminate SSL + // session. In order to do that, the filter chain must use :ref:`starttls transport socket + // `. + // If the filter does not manage to terminate the SSL session, it will close the connection from the client. + // Refer to official documentation for details + // `SSL Session Encryption Message Flow `_. + bool terminate_ssl = 3; } diff --git a/docs/root/configuration/listeners/network_filters/postgres_proxy_filter.rst b/docs/root/configuration/listeners/network_filters/postgres_proxy_filter.rst index eb9ffb93c79d..5ba819dc0e20 100644 --- a/docs/root/configuration/listeners/network_filters/postgres_proxy_filter.rst +++ b/docs/root/configuration/listeners/network_filters/postgres_proxy_filter.rst @@ -72,7 +72,8 @@ Every configured Postgres proxy filter has statistics rooted at postgres.`. Deprecated ---------- diff --git a/generated_api_shadow/envoy/extensions/filters/network/postgres_proxy/v3alpha/postgres_proxy.proto b/generated_api_shadow/envoy/extensions/filters/network/postgres_proxy/v3alpha/postgres_proxy.proto index aa8e0f5941bf..496599d1715f 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/postgres_proxy/v3alpha/postgres_proxy.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/postgres_proxy/v3alpha/postgres_proxy.proto @@ -29,4 +29,15 @@ message PostgresProxy { // are parsed. Parsing is required to produce Postgres proxy filter // metadata. Defaults to true. google.protobuf.BoolValue enable_sql_parsing = 2; + + // Controls whether to terminate SSL session initiated by a client. + // If the value is false, the Postgres proxy filter will not try to + // terminate SSL session, but will pass all the packets to the upstream server. + // If the value is true, the Postgres proxy filter will try to terminate SSL + // session. In order to do that, the filter chain must use :ref:`starttls transport socket + // `. + // If the filter does not manage to terminate the SSL session, it will close the connection from the client. + // Refer to official documentation for details + // `SSL Session Encryption Message Flow `_. + bool terminate_ssl = 3; } diff --git a/source/extensions/filters/network/postgres_proxy/config.cc b/source/extensions/filters/network/postgres_proxy/config.cc index 14180bc201b1..4e0eeb8c05b1 100644 --- a/source/extensions/filters/network/postgres_proxy/config.cc +++ b/source/extensions/filters/network/postgres_proxy/config.cc @@ -14,11 +14,14 @@ NetworkFilters::PostgresProxy::PostgresConfigFactory::createFilterFactoryFromPro Server::Configuration::FactoryContext& context) { ASSERT(!proto_config.stat_prefix().empty()); - const std::string stat_prefix = fmt::format("postgres.{}", proto_config.stat_prefix()); - const bool enable_sql = PROTOBUF_GET_WRAPPED_OR_DEFAULT(proto_config, enable_sql_parsing, true); + PostgresFilterConfig::PostgresFilterConfigOptions config_options; + config_options.stats_prefix_ = fmt::format("postgres.{}", proto_config.stat_prefix()); + config_options.enable_sql_parsing_ = + PROTOBUF_GET_WRAPPED_OR_DEFAULT(proto_config, enable_sql_parsing, true); + config_options.terminate_ssl_ = proto_config.terminate_ssl(); PostgresFilterConfigSharedPtr filter_config( - std::make_shared(stat_prefix, enable_sql, context.scope())); + std::make_shared(config_options, context.scope())); return [filter_config](Network::FilterManager& filter_manager) -> void { filter_manager.addFilter(std::make_shared(filter_config)); }; diff --git a/source/extensions/filters/network/postgres_proxy/postgres_decoder.cc b/source/extensions/filters/network/postgres_proxy/postgres_decoder.cc index 5fad2bcedd4a..f5d144dc4883 100644 --- a/source/extensions/filters/network/postgres_proxy/postgres_decoder.cc +++ b/source/extensions/filters/network/postgres_proxy/postgres_decoder.cc @@ -176,13 +176,13 @@ void DecoderImpl::initialize() { }; } -bool DecoderImpl::parseHeader(Buffer::Instance& data) { +Decoder::Result DecoderImpl::parseHeader(Buffer::Instance& data) { ENVOY_LOG(trace, "postgres_proxy: parsing message, len {}", data.length()); // The minimum size of the message sufficient for parsing is 5 bytes. if (data.length() < 5) { // not enough data in the buffer. - return false; + return Decoder::NeedMoreData; } if (!startup_) { @@ -198,7 +198,7 @@ bool DecoderImpl::parseHeader(Buffer::Instance& data) { ENVOY_LOG(trace, "postgres_proxy: cannot parse message. Need {} bytes in buffer", message_len_ + (startup_ ? 0 : 1)); // Not enough data in the buffer. - return false; + return Decoder::NeedMoreData; } if (startup_) { @@ -206,36 +206,56 @@ bool DecoderImpl::parseHeader(Buffer::Instance& data) { // Startup message with 1234 in the most significant 16 bits // indicate request to encrypt. if (code >= 0x04d20000) { - ENVOY_LOG(trace, "postgres_proxy: detected encrypted traffic."); encrypted_ = true; - startup_ = false; - incSessionsEncrypted(); + // Handler for SSLRequest (Int32(80877103) = 0x04d2162f) + // See details in https://www.postgresql.org/docs/current/protocol-message-formats.html. + if (code == 0x04d2162f) { + // Notify the filter that `SSLRequest` message was decoded. + // If the filter returns true, it means to pass the message upstream + // to the server. If it returns false it means, that filter will try + // to terminate SSL session and SSLRequest should not be passed to the + // server. + encrypted_ = callbacks_->onSSLRequest(); + } + + // Count it as recognized frontend message. + callbacks_->incMessagesFrontend(); + if (encrypted_) { + ENVOY_LOG(trace, "postgres_proxy: detected encrypted traffic."); + incSessionsEncrypted(); + startup_ = false; + } data.drain(data.length()); - return false; + return encrypted_ ? Decoder::ReadyForNext : Decoder::Stopped; } else { ENVOY_LOG(debug, "Detected version {}.{} of Postgres", code >> 16, code & 0x0000FFFF); - // 4 bytes of length and 4 bytes of version code. } } data.drain(startup_ ? 4 : 5); // Length plus optional 1st byte. ENVOY_LOG(trace, "postgres_proxy: msg parsed"); - return true; + return Decoder::ReadyForNext; } -bool DecoderImpl::onData(Buffer::Instance& data, bool frontend) { +Decoder::Result DecoderImpl::onData(Buffer::Instance& data, bool frontend) { // If encrypted, just drain the traffic. if (encrypted_) { ENVOY_LOG(trace, "postgres_proxy: ignoring {} bytes of encrypted data", data.length()); data.drain(data.length()); - return true; + return Decoder::ReadyForNext; + } + + if (!frontend && startup_) { + data.drain(data.length()); + return Decoder::ReadyForNext; } ENVOY_LOG(trace, "postgres_proxy: decoding {} bytes", data.length()); - if (!parseHeader(data)) { - return false; + const Decoder::Result result = parseHeader(data); + if (result != Decoder::ReadyForNext || encrypted_) { + return result; } MsgGroup& msg_processor = std::ref(frontend ? FE_messages_ : BE_messages_); @@ -283,7 +303,7 @@ bool DecoderImpl::onData(Buffer::Instance& data, bool frontend) { data.drain(bytes_to_read); ENVOY_LOG(trace, "postgres_proxy: {} bytes remaining in buffer", data.length()); - return true; + return Decoder::ReadyForNext; } // Method is called when C (CommandComplete) message has been diff --git a/source/extensions/filters/network/postgres_proxy/postgres_decoder.h b/source/extensions/filters/network/postgres_proxy/postgres_decoder.h index 409cdbba659c..b8326aaf8bc6 100644 --- a/source/extensions/filters/network/postgres_proxy/postgres_decoder.h +++ b/source/extensions/filters/network/postgres_proxy/postgres_decoder.h @@ -43,6 +43,8 @@ class DecoderCallbacks { virtual void incErrors(ErrorType) PURE; virtual void processQuery(const std::string&) PURE; + + virtual bool onSSLRequest() PURE; }; // Postgres message decoder. @@ -50,7 +52,16 @@ class Decoder { public: virtual ~Decoder() = default; - virtual bool onData(Buffer::Instance& data, bool frontend) PURE; + // The following values are returned by the decoder, when filter + // passes bytes of data via onData method: + enum Result { + ReadyForNext, // Decoder processed previous message and is ready for the next message. + NeedMoreData, // Decoder needs more data to reconstruct the message. + Stopped // Received and processed message disrupts the current flow. Decoder stopped accepting + // data. This happens when decoder wants filter to perform some action, for example to + // call starttls transport socket to enable TLS. + }; + virtual Result onData(Buffer::Instance& data, bool frontend) PURE; virtual PostgresSession& getSession() PURE; const Extensions::Common::SQLUtils::SQLUtils::DecoderAttributes& getAttributes() const { @@ -69,7 +80,7 @@ class DecoderImpl : public Decoder, Logger::Loggable { public: DecoderImpl(DecoderCallbacks* callbacks) : callbacks_(callbacks) { initialize(); } - bool onData(Buffer::Instance& data, bool frontend) override; + Result onData(Buffer::Instance& data, bool frontend) override; PostgresSession& getSession() override { return session_; } std::string getMessage() { return message_; } @@ -121,7 +132,7 @@ class DecoderImpl : public Decoder, Logger::Loggable { MsgAction unknown_; }; - bool parseHeader(Buffer::Instance& data); + Result parseHeader(Buffer::Instance& data); void decode(Buffer::Instance& data); void decodeAuthentication(); void decodeBackendStatements(); diff --git a/source/extensions/filters/network/postgres_proxy/postgres_filter.cc b/source/extensions/filters/network/postgres_proxy/postgres_filter.cc index bc20b37ba338..accc59117c48 100644 --- a/source/extensions/filters/network/postgres_proxy/postgres_filter.cc +++ b/source/extensions/filters/network/postgres_proxy/postgres_filter.cc @@ -11,10 +11,11 @@ namespace Extensions { namespace NetworkFilters { namespace PostgresProxy { -PostgresFilterConfig::PostgresFilterConfig(const std::string& stat_prefix, bool enable_sql_parsing, +PostgresFilterConfig::PostgresFilterConfig(const PostgresFilterConfigOptions& config_options, Stats::Scope& scope) - : enable_sql_parsing_(enable_sql_parsing), scope_{scope}, stats_{generateStats(stat_prefix, - scope)} {} + : enable_sql_parsing_(config_options.enable_sql_parsing_), + terminate_ssl_(config_options.terminate_ssl_), scope_{scope}, + stats_{generateStats(config_options.stats_prefix_, scope)} {} PostgresFilter::PostgresFilter(PostgresFilterConfigSharedPtr config) : config_{config} { if (!decoder_) { @@ -29,9 +30,12 @@ Network::FilterStatus PostgresFilter::onData(Buffer::Instance& data, bool) { // Frontend Buffer frontend_buffer_.add(data); - doDecode(frontend_buffer_, true); - - return Network::FilterStatus::Continue; + Network::FilterStatus result = doDecode(frontend_buffer_, true); + if (result == Network::FilterStatus::StopIteration) { + ASSERT(frontend_buffer_.length() == 0); + data.drain(data.length()); + } + return result; } Network::FilterStatus PostgresFilter::onNewConnection() { return Network::FilterStatus::Continue; } @@ -45,9 +49,7 @@ Network::FilterStatus PostgresFilter::onWrite(Buffer::Instance& data, bool) { // Backend Buffer backend_buffer_.add(data); - doDecode(backend_buffer_, false); - - return Network::FilterStatus::Continue; + return doDecode(backend_buffer_, false); } DecoderPtr PostgresFilter::createDecoder(DecoderCallbacks* callbacks) { @@ -186,12 +188,58 @@ void PostgresFilter::processQuery(const std::string& sql) { } } -void PostgresFilter::doDecode(Buffer::Instance& data, bool frontend) { +bool PostgresFilter::onSSLRequest() { + if (!config_->terminate_ssl_) { + // Signal to the decoder to continue. + return true; + } + // Send single bytes 'S' to indicate switch to TLS. + // Refer to official documentation for protocol details: + // https://www.postgresql.org/docs/current/protocol-flow.html + Buffer::OwnedImpl buf; + buf.add("S"); + // Add callback to be notified when the reply message has been + // transmitted. + read_callbacks_->connection().addBytesSentCallback([=](uint64_t bytes) -> bool { + // Wait until 'S' has been transmitted. + if (bytes >= 1) { + if (!read_callbacks_->connection().startSecureTransport()) { + ENVOY_CONN_LOG(info, "postgres_proxy: cannot enable secure transport. Check configuration.", + read_callbacks_->connection()); + read_callbacks_->connection().close(Network::ConnectionCloseType::NoFlush); + } else { + // Unsubscribe the callback. + config_->stats_.sessions_terminated_ssl_.inc(); + ENVOY_CONN_LOG(trace, "postgres_proxy: enabled SSL termination.", + read_callbacks_->connection()); + // Switch to TLS has been completed. + // Signal to the decoder to stop processing the current message (SSLRequest). + // Because Envoy terminates SSL, the message was consumed and should not be + // passed to other filters in the chain. + return false; + } + } + return true; + }); + read_callbacks_->connection().write(buf, false); + + return false; +} + +Network::FilterStatus PostgresFilter::doDecode(Buffer::Instance& data, bool frontend) { // Keep processing data until buffer is empty or decoder says // that it cannot process data in the buffer. - while ((0 < data.length()) && (decoder_->onData(data, frontend))) { - ; + while (0 < data.length()) { + switch (decoder_->onData(data, frontend)) { + case Decoder::NeedMoreData: + return Network::FilterStatus::Continue; + case Decoder::ReadyForNext: + continue; + case Decoder::Stopped: + return Network::FilterStatus::StopIteration; + } } + return Network::FilterStatus::Continue; } } // namespace PostgresProxy diff --git a/source/extensions/filters/network/postgres_proxy/postgres_filter.h b/source/extensions/filters/network/postgres_proxy/postgres_filter.h index f3ef83a6abce..f6740b3536ab 100644 --- a/source/extensions/filters/network/postgres_proxy/postgres_filter.h +++ b/source/extensions/filters/network/postgres_proxy/postgres_filter.h @@ -30,6 +30,7 @@ namespace PostgresProxy { COUNTER(messages_unknown) \ COUNTER(sessions) \ COUNTER(sessions_encrypted) \ + COUNTER(sessions_terminated_ssl) \ COUNTER(sessions_unencrypted) \ COUNTER(statements) \ COUNTER(statements_insert) \ @@ -62,10 +63,15 @@ struct PostgresProxyStats { */ class PostgresFilterConfig { public: - PostgresFilterConfig(const std::string& stat_prefix, bool enable_sql_parsing, - Stats::Scope& scope); + struct PostgresFilterConfigOptions { + std::string stats_prefix_; + bool enable_sql_parsing_; + bool terminate_ssl_; + }; + PostgresFilterConfig(const PostgresFilterConfigOptions& config_options, Stats::Scope& scope); bool enable_sql_parsing_{true}; + bool terminate_ssl_{false}; Stats::Scope& scope_; PostgresProxyStats stats_; @@ -105,8 +111,9 @@ class PostgresFilter : public Network::Filter, void incTransactionsCommit() override; void incTransactionsRollback() override; void processQuery(const std::string&) override; + bool onSSLRequest() override; - void doDecode(Buffer::Instance& data, bool); + Network::FilterStatus doDecode(Buffer::Instance& data, bool); DecoderPtr createDecoder(DecoderCallbacks* callbacks); void setDecoder(std::unique_ptr decoder) { decoder_ = std::move(decoder); } Decoder* getDecoder() const { return decoder_.get(); } diff --git a/test/extensions/filters/network/postgres_proxy/BUILD b/test/extensions/filters/network/postgres_proxy/BUILD index e95a2cffe158..f121e6b178e2 100644 --- a/test/extensions/filters/network/postgres_proxy/BUILD +++ b/test/extensions/filters/network/postgres_proxy/BUILD @@ -67,6 +67,7 @@ envoy_extension_cc_test( ], data = [ "postgres_test_config.yaml", + "//test/config/integration/certs", ], extension_name = "envoy.filters.network.postgres_proxy", deps = [ @@ -74,6 +75,8 @@ envoy_extension_cc_test( "//source/extensions/filters/network/postgres_proxy:config", "//source/extensions/filters/network/postgres_proxy:filter", "//source/extensions/filters/network/tcp_proxy:config", + "//source/extensions/transport_sockets/starttls:config", "//test/integration:integration_lib", + "@envoy_api//envoy/extensions/filters/network/postgres_proxy/v3alpha:pkg_cc_proto", ], ) diff --git a/test/extensions/filters/network/postgres_proxy/postgres_decoder_test.cc b/test/extensions/filters/network/postgres_proxy/postgres_decoder_test.cc index b2c81d77de1f..26a444644b24 100644 --- a/test/extensions/filters/network/postgres_proxy/postgres_decoder_test.cc +++ b/test/extensions/filters/network/postgres_proxy/postgres_decoder_test.cc @@ -24,6 +24,7 @@ class DecoderCallbacksMock : public DecoderCallbacks { MOCK_METHOD(void, incNotices, (NoticeType), (override)); MOCK_METHOD(void, incErrors, (ErrorType), (override)); MOCK_METHOD(void, processQuery, (const std::string&), (override)); + MOCK_METHOD(bool, onSSLRequest, (), (override)); }; // Define fixture class with decoder and mock callbacks. @@ -482,6 +483,9 @@ TEST_P(PostgresProxyFrontendEncrDecoderTest, EncyptedTraffic) { // Initial state is no-encryption. ASSERT_FALSE(decoder_->encrypted()); + // Indicate that decoder should continue with processing the message. + ON_CALL(callbacks_, onSSLRequest).WillByDefault(testing::Return(true)); + // Create SSLRequest. EXPECT_CALL(callbacks_, incSessionsEncrypted()); // Add length. @@ -489,7 +493,7 @@ TEST_P(PostgresProxyFrontendEncrDecoderTest, EncyptedTraffic) { // 1234 in the most significant 16 bits, and some code in the least significant 16 bits. // Add 4 bytes long code data_.writeBEInt(GetParam()); - decoder_->onData(data_, false); + decoder_->onData(data_, true); ASSERT_TRUE(decoder_->encrypted()); // Decoder should drain data. ASSERT_THAT(data_.length(), 0); @@ -510,6 +514,26 @@ TEST_P(PostgresProxyFrontendEncrDecoderTest, EncyptedTraffic) { INSTANTIATE_TEST_SUITE_P(FrontendEncryptedMessagesTests, PostgresProxyFrontendEncrDecoderTest, ::testing::Values(80877103, 80877104)); +// Test onSSLRequest callback. +TEST_F(PostgresProxyDecoderTest, TerminateSSL) { + // Set decoder to wait for initial message. + decoder_->setStartup(true); + + // Indicate that decoder should not continue with processing the message + // because filter will try to terminate SSL session. + EXPECT_CALL(callbacks_, onSSLRequest).WillOnce(testing::Return(false)); + + // Send initial message requesting SSL. + data_.writeBEInt(8); + // 1234 in the most significant 16 bits, and some code in the least significant 16 bits. + // Add 4 bytes long code + data_.writeBEInt(80877103); + decoder_->onData(data_, true); + + // Decoder should interpret the session as encrypted stream. + ASSERT_FALSE(decoder_->encrypted()); +} + class FakeBuffer : public Buffer::Instance { public: MOCK_METHOD(void, addDrainTracker, (std::function), (override)); diff --git a/test/extensions/filters/network/postgres_proxy/postgres_filter_test.cc b/test/extensions/filters/network/postgres_proxy/postgres_filter_test.cc index 63c9794ccee5..0b09e50bea60 100644 --- a/test/extensions/filters/network/postgres_proxy/postgres_filter_test.cc +++ b/test/extensions/filters/network/postgres_proxy/postgres_filter_test.cc @@ -20,7 +20,7 @@ using ::testing::WithArgs; // Decoder mock. class MockDecoderTest : public Decoder { public: - MOCK_METHOD(bool, onData, (Buffer::Instance&, bool), (override)); + MOCK_METHOD(Decoder::Result, onData, (Buffer::Instance&, bool), (override)); MOCK_METHOD(PostgresSession&, getSession, (), (override)); }; @@ -31,7 +31,10 @@ class PostgresFilterTest std::function>> { public: PostgresFilterTest() { - config_ = std::make_shared(stat_prefix_, true, scope_); + + PostgresFilterConfig::PostgresFilterConfigOptions config_options{stat_prefix_, true, false}; + + config_ = std::make_shared(config_options, scope_); filter_ = std::make_unique(config_); filter_->initializeReadFilterCallbacks(filter_callbacks_); @@ -79,22 +82,22 @@ TEST_P(PostgresFilterTest, ReadData) { // Simulate reading entire buffer. EXPECT_CALL(*decoderPtr, onData) - .WillOnce(WithArgs<0, 1>(Invoke([](Buffer::Instance& data, bool) -> bool { + .WillOnce(WithArgs<0, 1>(Invoke([](Buffer::Instance& data, bool) -> Decoder::Result { data.drain(data.length()); - return true; + return Decoder::ReadyForNext; }))); std::get<0>(GetParam())(filter_.get(), data_, false); ASSERT_THAT(std::get<1>(GetParam())(filter_.get()), 0); // Simulate reading entire data in two steps. EXPECT_CALL(*decoderPtr, onData) - .WillOnce(WithArgs<0, 1>(Invoke([](Buffer::Instance& data, bool) -> bool { + .WillOnce(WithArgs<0, 1>(Invoke([](Buffer::Instance& data, bool) -> Decoder::Result { data.drain(100); - return true; + return Decoder::ReadyForNext; }))) - .WillOnce(WithArgs<0, 1>(Invoke([](Buffer::Instance& data, bool) -> bool { + .WillOnce(WithArgs<0, 1>(Invoke([](Buffer::Instance& data, bool) -> Decoder::Result { data.drain(156); - return true; + return Decoder::ReadyForNext; }))); std::get<0>(GetParam())(filter_.get(), data_, false); ASSERT_THAT(std::get<1>(GetParam())(filter_.get()), 0); @@ -103,17 +106,17 @@ TEST_P(PostgresFilterTest, ReadData) { // for the third one there was not enough data. There should be 56 bytes // of unprocessed data. EXPECT_CALL(*decoderPtr, onData) - .WillOnce(WithArgs<0, 1>(Invoke([](Buffer::Instance& data, bool) -> bool { + .WillOnce(WithArgs<0, 1>(Invoke([](Buffer::Instance& data, bool) -> Decoder::Result { data.drain(100); - return true; + return Decoder::ReadyForNext; }))) - .WillOnce(WithArgs<0, 1>(Invoke([](Buffer::Instance& data, bool) -> bool { + .WillOnce(WithArgs<0, 1>(Invoke([](Buffer::Instance& data, bool) -> Decoder::Result { data.drain(100); - return true; + return Decoder::ReadyForNext; }))) - .WillOnce(WithArgs<0, 1>(Invoke([](Buffer::Instance& data, bool) -> bool { + .WillOnce(WithArgs<0, 1>(Invoke([](Buffer::Instance& data, bool) -> Decoder::Result { data.drain(0); - return false; + return Decoder::NeedMoreData; }))); std::get<0>(GetParam())(filter_.get(), data_, false); ASSERT_THAT(std::get<1>(GetParam())(filter_.get()), 56); @@ -291,8 +294,8 @@ TEST_F(PostgresFilterTest, NoticeMsgsStats) { TEST_F(PostgresFilterTest, EncryptedSessionStats) { data_.writeBEInt(8); // 1234 in the most significant 16 bits and some code in the least significant 16 bits. - data_.writeBEInt(80877103); // SSL code. - filter_->onData(data_, false); + data_.writeBEInt(80877104); // SSL code. + ASSERT_THAT(Network::FilterStatus::Continue, filter_->onData(data_, true)); ASSERT_THAT(filter_->getStats().sessions_.value(), 1); ASSERT_THAT(filter_->getStats().sessions_encrypted_.value(), 1); } @@ -305,7 +308,7 @@ TEST_F(PostgresFilterTest, MetadataIncorrectSQL) { setMetadata(); createPostgresMsg(data_, "Q", "BLAH blah blah"); - filter_->onData(data_, false); + ASSERT_THAT(Network::FilterStatus::Continue, filter_->onData(data_, true)); // SQL statement was wrong. No metadata should have been created. ASSERT_THAT(filter_->connection().streamInfo().dynamicMetadata().filter_metadata().contains( @@ -325,7 +328,7 @@ TEST_F(PostgresFilterTest, QueryMessageMetadata) { // Disable creating parsing SQL and creating metadata. filter_->getConfig()->enable_sql_parsing_ = false; createPostgresMsg(data_, "Q", "SELECT * FROM whatever"); - filter_->onData(data_, false); + ASSERT_THAT(Network::FilterStatus::Continue, filter_->onData(data_, false)); ASSERT_THAT(filter_->connection().streamInfo().dynamicMetadata().filter_metadata().contains( NetworkFilterNames::get().PostgresProxy), @@ -335,7 +338,7 @@ TEST_F(PostgresFilterTest, QueryMessageMetadata) { // Now enable SQL parsing and creating metadata. filter_->getConfig()->enable_sql_parsing_ = true; - filter_->onData(data_, false); + ASSERT_THAT(Network::FilterStatus::Continue, filter_->onData(data_, false)); auto& filter_meta = filter_->connection().streamInfo().dynamicMetadata().filter_metadata().at( NetworkFilterNames::get().PostgresProxy); @@ -351,6 +354,44 @@ TEST_F(PostgresFilterTest, QueryMessageMetadata) { ASSERT_THAT(filter_->getStats().statements_parsed_.value(), 1); } +// Test verifies that filter reacts to RequestSSL message. +// It should reply with "S" message and OnData should return +// Decoder::Stopped. +TEST_F(PostgresFilterTest, TerminateSSL) { + filter_->getConfig()->terminate_ssl_ = true; + EXPECT_CALL(filter_callbacks_, connection()).WillRepeatedly(ReturnRef(connection_)); + Network::Connection::BytesSentCb cb; + EXPECT_CALL(connection_, addBytesSentCallback(_)).WillOnce(testing::SaveArg<0>(&cb)); + Buffer::OwnedImpl buf; + EXPECT_CALL(connection_, write(_, false)).WillOnce(testing::SaveArg<0>(&buf)); + data_.writeBEInt(8); + // 1234 in the most significant 16 bits and some code in the least significant 16 bits. + data_.writeBEInt(80877103); // SSL code. + ASSERT_THAT(Network::FilterStatus::StopIteration, filter_->onData(data_, true)); + ASSERT_THAT('S', buf.peekBEInt(0)); + ASSERT_THAT(filter_->getStats().messages_.value(), 1); + ASSERT_THAT(filter_->getStats().messages_frontend_.value(), 1); + + // Now indicate through the callback that 1 bytes has been sent. + // Filter should call startSecureTransport and should not close the connection. + EXPECT_CALL(connection_, startSecureTransport()).WillOnce(testing::Return(true)); + EXPECT_CALL(connection_, close(_)).Times(0); + cb(1); + // Verify stats. This should not count as encrypted or unencrypted session. + ASSERT_THAT(filter_->getStats().sessions_terminated_ssl_.value(), 1); + ASSERT_THAT(filter_->getStats().sessions_encrypted_.value(), 0); + ASSERT_THAT(filter_->getStats().sessions_unencrypted_.value(), 0); + + // Call callback again, but this time indicate that startSecureTransport failed. + // Filter should close the connection. + EXPECT_CALL(connection_, startSecureTransport()).WillOnce(testing::Return(false)); + EXPECT_CALL(connection_, close(_)); + cb(1); + ASSERT_THAT(filter_->getStats().sessions_terminated_ssl_.value(), 1); + ASSERT_THAT(filter_->getStats().sessions_encrypted_.value(), 0); + ASSERT_THAT(filter_->getStats().sessions_unencrypted_.value(), 0); +} + } // namespace PostgresProxy } // namespace NetworkFilters } // namespace Extensions diff --git a/test/extensions/filters/network/postgres_proxy/postgres_integration_test.cc b/test/extensions/filters/network/postgres_proxy/postgres_integration_test.cc index 7df485bbb120..0820bb4fc5d1 100644 --- a/test/extensions/filters/network/postgres_proxy/postgres_integration_test.cc +++ b/test/extensions/filters/network/postgres_proxy/postgres_integration_test.cc @@ -1,3 +1,6 @@ +#include "envoy/extensions/filters/network/postgres_proxy/v3alpha/postgres_proxy.pb.h" +#include "envoy/extensions/filters/network/postgres_proxy/v3alpha/postgres_proxy.pb.validate.h" + #include "test/integration/fake_upstream.h" #include "test/integration/integration.h" #include "test/integration/utility.h" @@ -12,29 +15,58 @@ namespace Extensions { namespace NetworkFilters { namespace PostgresProxy { -class PostgresIntegrationTest : public testing::TestWithParam, - public BaseIntegrationTest { +class PostgresBaseIntegrationTest : public testing::TestWithParam, + public BaseIntegrationTest { - std::string postgresConfig() { - return fmt::format( + std::string postgresConfig(bool terminate_ssl, bool add_start_tls_transport_socket) { + std::string main_config = fmt::format( TestEnvironment::readFileToStringForTest(TestEnvironment::runfilesPath( "test/extensions/filters/network/postgres_proxy/postgres_test_config.yaml")), Platform::null_device_path, Network::Test::getLoopbackAddressString(GetParam()), Network::Test::getLoopbackAddressString(GetParam()), - Network::Test::getAnyAddressString(GetParam())); + Network::Test::getAnyAddressString(GetParam()), terminate_ssl ? "true" : "false"); + + if (add_start_tls_transport_socket) { + main_config += + fmt::format(R"EOF( + transport_socket: + name: "starttls" + typed_config: + "@type": type.googleapis.com/envoy.extensions.transport_sockets.starttls.v3.StartTlsConfig + cleartext_socket_config: + tls_socket_config: + common_tls_context: + tls_certificates: + certificate_chain: + filename: {} + private_key: + filename: {} + )EOF", + TestEnvironment::runfilesPath("test/config/integration/certs/servercert.pem"), + TestEnvironment::runfilesPath("test/config/integration/certs/serverkey.pem")); + } + + return main_config; } public: - PostgresIntegrationTest() : BaseIntegrationTest(GetParam(), postgresConfig()){}; + PostgresBaseIntegrationTest(bool terminate_ssl, bool add_starttls_transport_socket) + : BaseIntegrationTest(GetParam(), + postgresConfig(terminate_ssl, add_starttls_transport_socket)){}; void SetUp() override { BaseIntegrationTest::initialize(); } }; -INSTANTIATE_TEST_SUITE_P(IpVersions, PostgresIntegrationTest, - testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); + +// Base class for tests with `terminate_ssl` disabled and without +// `starttls` transport socket. +class BasicPostgresIntegrationTest : public PostgresBaseIntegrationTest { +public: + BasicPostgresIntegrationTest() : PostgresBaseIntegrationTest(false, false) {} +}; // Test that the filter is properly chained and reacts to successful login // message. -TEST_P(PostgresIntegrationTest, Login) { +TEST_P(BasicPostgresIntegrationTest, Login) { std::string str; std::string recv; @@ -75,6 +107,86 @@ TEST_P(PostgresIntegrationTest, Login) { test_server_->waitForCounterEq("postgres.postgres_stats.sessions", 1); } +INSTANTIATE_TEST_SUITE_P(IpVersions, BasicPostgresIntegrationTest, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); + +// Base class for tests with `terminate_ssl` enabled and `starttls` transport socket added. +class SSLPostgresIntegrationTest : public PostgresBaseIntegrationTest { +public: + SSLPostgresIntegrationTest() : PostgresBaseIntegrationTest(true, true) {} +}; + +// Test verifies that Postgres filter replies with correct code upon +// receiving request to terminate SSL. +TEST_P(SSLPostgresIntegrationTest, TerminateSSL) { + Buffer::OwnedImpl data; + + IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("listener_0")); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); + + // Send the startup message requesting SSL. + // Message is 8 bytes long. The first 4 bytes contain length (8) + // The next 8 bytes contain message code (RequestSSL=80877103) + data.writeBEInt(8); + data.writeBEInt(80877103); + + // Message will be processed by Postgres filter which + // is configured to accept SSL termination and confirm it + // by returning single byte 'S'. + ASSERT_TRUE(tcp_client->write(data.toString())); + data.drain(data.length()); + + tcp_client->waitForData("S", true); + + tcp_client->close(); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); + + // Make sure that the successful login bumped up the number of sessions. + test_server_->waitForCounterEq("postgres.postgres_stats.sessions_terminated_ssl", 1); +} + +INSTANTIATE_TEST_SUITE_P(IpVersions, SSLPostgresIntegrationTest, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); + +class SSLWrongConfigPostgresIntegrationTest : public PostgresBaseIntegrationTest { +public: + SSLWrongConfigPostgresIntegrationTest() : PostgresBaseIntegrationTest(true, false) {} +}; + +// Test verifies that Postgres filter closes connection when it is configured to +// terminate SSL, but underlying transport socket does not allow for such operation. +TEST_P(SSLWrongConfigPostgresIntegrationTest, TerminateSSLNoStartTlsTransportSocket) { + Buffer::OwnedImpl data; + + IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("listener_0")); + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); + + // Send the startup message requesting SSL. + // Message is 8 bytes long. The first 4 bytes contain length (8) + // The next 8 bytes contain message code (RequestSSL=80877103) + data.writeBEInt(8); + data.writeBEInt(80877103); + + // Message will be processed by Postgres filter which + // is configured to accept SSL termination and confirm it + // by returning single byte 'S'. + ASSERT_TRUE(tcp_client->write(data.toString())); + data.drain(data.length()); + + tcp_client->waitForData("S", true); + + tcp_client->waitForDisconnect(); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); + + // Make sure that the successful login bumped up the number of sessions. + test_server_->waitForCounterEq("postgres.postgres_stats.sessions_terminated_ssl", 0); +} + +INSTANTIATE_TEST_SUITE_P(IpVersions, SSLWrongConfigPostgresIntegrationTest, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); + } // namespace PostgresProxy } // namespace NetworkFilters } // namespace Extensions diff --git a/test/extensions/filters/network/postgres_proxy/postgres_test_config.yaml b/test/extensions/filters/network/postgres_proxy/postgres_test_config.yaml index 6c2877ebc56c..ccb1cac8621c 100644 --- a/test/extensions/filters/network/postgres_proxy/postgres_test_config.yaml +++ b/test/extensions/filters/network/postgres_proxy/postgres_test_config.yaml @@ -25,12 +25,13 @@ static_resources: port_value: 0 filter_chains: - filters: - - name: postgres - typed_config: - "@type": type.googleapis.com/envoy.extensions.filters.network.postgres_proxy.v3alpha.PostgresProxy - stat_prefix: postgres_stats - - name: tcp - typed_config: - "@type": type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy - stat_prefix: tcp_stats - cluster: cluster_0 + - name: postgres + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.network.postgres_proxy.v3alpha.PostgresProxy + stat_prefix: postgres_stats + terminate_ssl: {} + - name: tcp + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy + stat_prefix: tcp_stats + cluster: cluster_0 diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index 0819e488b28f..cdb1b0230b06 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -1064,6 +1064,7 @@ src ssize stackdriver stacktrace +starttls startup stateful statsd