Skip to content

Commit

Permalink
[native] Add JWT processing when using http(s) comms
Browse files Browse the repository at this point in the history
Adding creation and verification of a JWT signed using HMAC SHA-256.
The JWT is used for authentication of internal communication.
On the Java side PR #19706 added authentication for internal
communication using a JWT token signed with HMAC SHA-256.

Adding this comment enables Prestissimo to to be configured
to verify internal communication requests, e.g from a Java
coordinator.

New system configuration options:
 - internal-communication.jwt.enabled=[true/false]
 - internal-communication.shared-secret=<shared-secret-value>
 - internal-communication.jwt.expiration-seconds=<value in seconds>

A new external dependency jet-cpp from the https://github.com/Thalhammer/jwt-cpp
repo is added to a new setup-adapter.sh script for Prestissimo.
In the dependency directory run
"<path to presto-native-execution>/scripts/setup-adapter.sh jwt" to install it
The jwt-cpp project handles the creation/parsing/verification of the JWT.

A new build options are added to the build environment:
PRESTO_ENABLE_JWT - default off, if on adds jet creation and verification capability,
 turned on during the pipeline build.
  • Loading branch information
czentgr authored and majetideepak committed Oct 4, 2023
1 parent 3736f69 commit ed5727c
Show file tree
Hide file tree
Showing 20 changed files with 981 additions and 303 deletions.
13 changes: 12 additions & 1 deletion .circleci/continue_config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,14 @@ jobs:
command: |
cd presto-native-execution
make velox-submodule
- run:
name: "Install JWT adapter dependency"
command: |
mkdir -p ${HOME}/adapter-deps/install
source /opt/rh/gcc-toolset-9/enable
set -xu
cd presto-native-execution
DEPENDENCY_DIR=${HOME}/adapter-deps PROMPT_ALWAYS_RESPOND=n ./scripts/setup-adapters.sh jwt
- run:
name: "Calculate merge-base date for CCache"
command: git show -s --format=%cd --date="format:%Y%m%d" $(git merge-base origin/master HEAD) | tee merge-base-date
Expand All @@ -102,6 +110,7 @@ jobs:
-DCMAKE_BUILD_TYPE=Debug \
-DPRESTO_ENABLE_PARQUET=ON \
-DPRESTO_ENABLE_REMOTE_FUNCTIONS=ON \
-DPRESTO_ENABLE_JWT=ON \
-DCMAKE_PREFIX_PATH=/usr/local \
-DCMAKE_CXX_COMPILER_LAUNCHER=ccache
ninja -C _build/debug -j 8
Expand Down Expand Up @@ -210,13 +219,14 @@ jobs:
cd presto-native-execution
make velox-submodule
- run:
name: "Install S3 adapter dependencies"
name: "Install all adapter dependencies"
command: |
mkdir -p ${HOME}/adapter-deps/install
source /opt/rh/gcc-toolset-9/enable
set -xu
cd presto-native-execution
DEPENDENCY_DIR=${HOME}/adapter-deps PROMPT_ALWAYS_RESPOND=n ./velox/scripts/setup-adapters.sh
DEPENDENCY_DIR=${HOME}/adapter-deps PROMPT_ALWAYS_RESPOND=n ./scripts/setup-adapters.sh jwt
- run:
name: "Build All"
command: |
Expand All @@ -232,6 +242,7 @@ jobs:
-DPRESTO_ENABLE_PARQUET=ON \
-DPRESTO_ENABLE_S3=ON \
-DPRESTO_ENABLE_REMOTE_FUNCTIONS=ON \
-DPRESTO_ENABLE_JWT=ON \
-DCMAKE_PREFIX_PATH=/usr/local \
-DCMAKE_CXX_COMPILER_LAUNCHER=ccache
ninja -C _build/release -j 8
Expand Down
25 changes: 23 additions & 2 deletions presto-docs/src/main/sphinx/develop/presto-native.rst
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ HTTP endpoints related to tasks are registered to Proxygen in

* POST: v1/task: This processes a `TaskUpdateRequest`
* GET: v1/task: This returns a serialized `TaskInfo` (used for comprehensive
metrics, may be reported less frequently)
metrics, may be reported less frequently)
* GET: v1/task/status: This returns
a serialized `TaskStatus` (used for query progress tracking, must be reported
frequently)
Expand Down Expand Up @@ -104,5 +104,26 @@ The following properties allow the configuration of remote function execution:

The UDS (unix domain socket) path to communicate with a local remote
function server. If specified, takes precedence over
``remote-function-server.thrift.address`` and
``remote-function-server.thrift.address`` and
``remote-function-server.thrift.port``.

JWT authentication support
--------------------------

Prestissimo supports JWT authentication for internal communication.
For details on the generally supported parameters visit `JWT <../security/internal-communication.html#jwt>`_.

There is also an additional parameter:

``internal-communication.jwt.expiration-seconds``
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

* **Type** ``integer``
* **Default value:** ``300``

There is a time period between creating the JWT on the client
and verification by the server.
If the time period is less than or equal to the parameter value, the request
is valid.
If the time period exceeds the parameter value, the request is rejected as
authentication failure (HTTP 401).
6 changes: 6 additions & 0 deletions presto-native-execution/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ option(PRESTO_ENABLE_REMOTE_FUNCTIONS "Enable remote function support" OFF)

option(PRESTO_ENABLE_TESTING "Enable tests" ON)

option(PRESTO_ENABLE_JWT "Enable JWT (JSON Web Token) authentication" OFF)

# Set all Velox options below
add_compile_definitions(FOLLY_HAVE_INT128_T=1)

Expand Down Expand Up @@ -201,4 +203,8 @@ else()
add_definitions(-DVELOX_DISABLE_GOOGLETEST)
endif()

if(PRESTO_ENABLE_JWT)
add_compile_definitions(PRESTO_ENABLE_JWT)
endif()

add_subdirectory(presto_cpp)
2 changes: 2 additions & 0 deletions presto-native-execution/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ PRESTO_ENABLE_PARQUET ?= "OFF"
PRESTO_ENABLE_S3 ?= "OFF"
PRESTO_ENABLE_HDFS ?= "OFF"
PRESTO_ENABLE_REMOTE_FUNCTIONS ?= "OFF"
PRESTO_ENABLE_JWT ?= "OFF"
EXTRA_CMAKE_FLAGS ?= ""

CMAKE_FLAGS := -DTREAT_WARNINGS_AS_ERRORS=${TREAT_WARNINGS_AS_ERRORS}
Expand All @@ -35,6 +36,7 @@ CMAKE_FLAGS += -DPRESTO_ENABLE_PARQUET=$(PRESTO_ENABLE_PARQUET)
CMAKE_FLAGS += -DPRESTO_ENABLE_S3=$(PRESTO_ENABLE_S3)
CMAKE_FLAGS += -DPRESTO_ENABLE_HDFS=$(PRESTO_ENABLE_HDFS)
CMAKE_FLAGS += -DPRESTO_ENABLE_REMOTE_FUNCTIONS=$(PRESTO_ENABLE_REMOTE_FUNCTIONS)
CMAKE_FLAGS += -DPRESTO_ENABLE_JWT=$(PRESTO_ENABLE_JWT)

SHELL := /bin/bash

Expand Down
9 changes: 9 additions & 0 deletions presto-native-execution/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,15 @@ This dependency can be installed by running the script below from the

`./velox/scripts/setup-adapters.sh aws`

To enable JWT authentication support, set `PRESTO_ENABLE_JWT = "ON"` in
the environment.

JWT authentication support needs the [JWT CPP](https://github.com/Thalhammer/jwt-cpp) library.
This dependency can be installed by running the script below from the
`presto/presto-native-execution` directory.

`./scripts/setup-adapters.sh jwt`

* After installing the above dependencies, from the
`presto/presto-native-execution` directory, run `make`
* For development, use
Expand Down
15 changes: 15 additions & 0 deletions presto-native-execution/presto_cpp/main/PrestoServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "presto_cpp/main/http/HttpServer.h"
#include "presto_cpp/main/http/filters/AccessLogFilter.h"
#include "presto_cpp/main/http/filters/HttpEndpointLatencyFilter.h"
#include "presto_cpp/main/http/filters/InternalAuthenticationFilter.h"
#include "presto_cpp/main/http/filters/StatsFilter.h"
#include "presto_cpp/main/operators/BroadcastExchangeSource.h"
#include "presto_cpp/main/operators/BroadcastWrite.h"
Expand Down Expand Up @@ -163,6 +164,15 @@ void PrestoServer::run() {
clientCertAndKeyPath = optionalClientCertPath.value();
}

if (systemConfig->internalCommunicationJwtEnabled()) {
#ifndef PRESTO_ENABLE_JWT
VELOX_USER_FAIL("Internal JWT is enabled but not supported");
#endif
VELOX_USER_CHECK(
!(systemConfig->internalCommunicationSharedSecret().empty()),
"Internal JWT is enabled without a corresponding shared secret");
}

nodeVersion_ = systemConfig->prestoVersion();
httpExecThreads = systemConfig->httpExecThreads();
environment_ = nodeConfig->nodeEnvironment();
Expand Down Expand Up @@ -658,6 +668,11 @@ PrestoServer::getHttpServerFilters() {
httpServer_.get()));
}

// Always add the authentication filter to make sure the worker configuration
// is in line with the overall cluster configuration e.g. cannot have a worker
// without JWT enabled.
filters.push_back(
std::make_unique<http::filters::InternalAuthenticationFilterFactory>());
return filters;
}

Expand Down
18 changes: 18 additions & 0 deletions presto-native-execution/presto_cpp/main/common/Configs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,9 @@ SystemConfig::SystemConfig() {
NUM_PROP(kTaskRunTimeSliceMicros, 50'000),
BOOL_PROP(kIncludeNodeInSpillPath, false),
NUM_PROP(kOldTaskCleanUpMs, 60'000),
STR_PROP(kInternalCommunicationJwtEnabled, "false"),
STR_PROP(kInternalCommunicationSharedSecret, ""),
NUM_PROP(kInternalCommunicationJwtExpirationSeconds, 300),
};
}

Expand Down Expand Up @@ -558,6 +561,21 @@ int32_t SystemConfig::oldTaskCleanUpMs() const {
return optionalProperty<int32_t>(kOldTaskCleanUpMs).value();
}

// The next three toggles govern the use of JWT for authentication
// for communication between the cluster nodes.
bool SystemConfig::internalCommunicationJwtEnabled() const {
return optionalProperty<bool>(kInternalCommunicationJwtEnabled).value();
}

std::string SystemConfig::internalCommunicationSharedSecret() const {
return optionalProperty(kInternalCommunicationSharedSecret).value();
}

int32_t SystemConfig::internalCommunicationJwtExpirationSeconds() const {
return optionalProperty<int32_t>(kInternalCommunicationJwtExpirationSeconds)
.value();
}

NodeConfig::NodeConfig() {
registeredProps_ =
std::unordered_map<std::string, folly::Optional<std::string>>{
Expand Down
14 changes: 14 additions & 0 deletions presto-native-execution/presto_cpp/main/common/Configs.h
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,14 @@ class SystemConfig : public ConfigBase {
static constexpr std::string_view kRemoteFunctionServerCatalogName{
"remote-function-server.catalog-name"};

/// Options to configure the internal (in-cluster) JWT authentication.
static constexpr std::string_view kInternalCommunicationJwtEnabled{
"internal-communication.jwt.enabled"};
static constexpr std::string_view kInternalCommunicationSharedSecret{
"internal-communication.shared-secret"};
static constexpr std::string_view kInternalCommunicationJwtExpirationSeconds{
"internal-communication.jwt.expiration-seconds"};

SystemConfig();

static SystemConfig* instance();
Expand Down Expand Up @@ -459,6 +467,12 @@ class SystemConfig : public ConfigBase {
bool includeNodeInSpillPath() const;

int32_t oldTaskCleanUpMs() const;

bool internalCommunicationJwtEnabled() const;

std::string internalCommunicationSharedSecret() const;

int32_t internalCommunicationJwtExpirationSeconds() const;
};

/// Provides access to node properties defined in node.properties file.
Expand Down
6 changes: 6 additions & 0 deletions presto-native-execution/presto_cpp/main/http/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@
# limitations under the License.
add_library(presto_http HttpClient.cpp HttpServer.cpp)

if(PRESTO_ENABLE_JWT)
add_compile_definitions(JWT_DISABLE_PICOJSON)
target_include_directories(
presto_http PRIVATE ${CMAKE_SOURCE_DIR}/presto_cpp/external/json)
endif()

add_subdirectory(filters)

target_link_libraries(
Expand Down
34 changes: 33 additions & 1 deletion presto-native-execution/presto_cpp/main/http/HttpClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#ifdef PRESTO_ENABLE_JWT
#include <folly/ssl/OpenSSLHash.h> // @manual
#include <jwt-cpp/jwt.h> // @manual
#include <jwt-cpp/traits/nlohmann-json/traits.h> //@manual
#endif // PRESTO_ENABLE_JWT
#include <velox/common/base/Exceptions.h>

#include "presto_cpp/main/common/Configs.h"
#include "presto_cpp/main/http/HttpClient.h"

Expand Down Expand Up @@ -346,4 +350,32 @@ folly::SemiFuture<std::unique_ptr<HttpResponse>> HttpClient::sendRequest(
return future;
}

void RequestBuilder::addJwtIfConfigured() {
#ifdef PRESTO_ENABLE_JWT
if (SystemConfig::instance()->internalCommunicationJwtEnabled()) {
// If JWT was enabled the secret cannot be empty.
auto secretHash = std::vector<uint8_t>(SHA256_DIGEST_LENGTH);
folly::ssl::OpenSSLHash::sha256(
folly::range(secretHash),
folly::ByteRange(folly::StringPiece(
SystemConfig::instance()->internalCommunicationSharedSecret())));

const auto time = std::chrono::system_clock::now();
const auto token =
jwt::create<jwt::traits::nlohmann_json>()
.set_subject(NodeConfig::instance()->nodeId())
.set_issued_at(time)
.set_expires_at(
time +
std::chrono::seconds{
SystemConfig::instance()
->internalCommunicationJwtExpirationSeconds()})
.sign(jwt::algorithm::hs256{std::string(
reinterpret_cast<char*>(secretHash.data()),
secretHash.size())});
header(kPrestoInternalBearer, token);
}
#endif // PRESTO_ENABLE_JWT
}

} // namespace facebook::presto::http
3 changes: 3 additions & 0 deletions presto-native-execution/presto_cpp/main/http/HttpClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,15 @@ class RequestBuilder {

folly::SemiFuture<std::unique_ptr<HttpResponse>>
send(HttpClient* client, const std::string& body = "", int64_t delayMs = 0) {
addJwtIfConfigured();
header(proxygen::HTTP_HEADER_CONTENT_LENGTH, std::to_string(body.size()));
headers_.ensureHostHeader();
return client->sendRequest(headers_, body, delayMs);
}

private:
void addJwtIfConfigured();

proxygen::HTTPMessage headers_;
};

Expand Down
2 changes: 2 additions & 0 deletions presto-native-execution/presto_cpp/main/http/HttpConstants.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ namespace facebook::presto::http {
const uint16_t kHttpOk = 200;
const uint16_t kHttpAccepted = 202;
const uint16_t kHttpNoContent = 204;
const uint16_t kHttpUnauthorized = 401;
const uint16_t kHttpNotFound = 404;
const uint16_t kHttpInternalServerError = 500;

const char kMimeTypeApplicationJson[] = "application/json";
const char kMimeTypeApplicationThrift[] = "application/x-thrift+binary";
static const char kPrestoInternalBearer[] = "X-Presto-Internal-Bearer";
} // namespace facebook::presto::http
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@
# limitations under the License.

add_library(http_filters AccessLogFilter.cpp HttpEndpointLatencyFilter.cpp
StatsFilter.cpp)
InternalAuthenticationFilter.cpp StatsFilter.cpp)

if(PRESTO_ENABLE_JWT)
target_include_directories(
http_filters PRIVATE ${CMAKE_SOURCE_DIR}/presto_cpp/external/json)
endif()

target_link_libraries(http_filters presto_common ${PROXYGEN_LIBRARIES})

Expand Down
Loading

0 comments on commit ed5727c

Please sign in to comment.