Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core] Add TLS/SSL support to gRPC channels #18631

Merged
merged 80 commits into from
Oct 21, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
80 commits
Select commit Hold shift + click to select a range
efe18dd
Add use_tls_ member to GrpcServer
oscarknagg Sep 8, 2021
d38af35
Hacky TLS
oscarknagg Sep 8, 2021
3b5f210
Create secure gRPC channels in Python code
oscarknagg Sep 8, 2021
01c5cd9
Remove unecessary std::cout
oscarknagg Sep 8, 2021
2769675
More TLS
oscarknagg Sep 8, 2021
c6ad485
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Sep 8, 2021
2962be3
Linting
oscarknagg Sep 8, 2021
64be21a
Add secure grpc in tests
oscarknagg Sep 8, 2021
d38e2b0
Fix secure grpc server initialisation
oscarknagg Sep 8, 2021
7aaa8ac
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Sep 8, 2021
1668ecc
Use single environment variable as feature flag
oscarknagg Sep 9, 2021
621cfc7
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Sep 9, 2021
a2c49d6
Pass environment in test_client_builder.py
oscarknagg Sep 9, 2021
0b73c38
Read RAY_USE_TLS in client worker
oscarknagg Sep 9, 2021
ddc8749
Unify init_grpc_channel and init_aiogrpc_channel functions
oscarknagg Sep 10, 2021
966fc49
Make function to add port to grpc server
oscarknagg Sep 10, 2021
b173b78
Upgrade to mTLS
oscarknagg Sep 10, 2021
bc39b8f
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Sep 10, 2021
65361a2
Function to load certs from env variables
oscarknagg Sep 10, 2021
2dcff3a
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Sep 13, 2021
f19e7a7
Add example cluster yaml which generates self-signed keys
oscarknagg Sep 13, 2021
b57c2e2
Add TLS auth test
oscarknagg Sep 14, 2021
a4cc458
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Sep 14, 2021
65f0080
Add some fixtures to run test_basic.py with TLS auth
oscarknagg Sep 15, 2021
da45c78
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Sep 15, 2021
b4dc0ca
Fix test_tls_auth.py
oscarknagg Sep 15, 2021
16c0cb3
Remove duplicated ReadFile function
oscarknagg Sep 15, 2021
30bebae
Formatting
oscarknagg Sep 15, 2021
c551c30
Remove EKS cluster YAML
oscarknagg Sep 15, 2021
1fa0fbf
Don't assume TLS env vars are set
oscarknagg Sep 15, 2021
2b0bc68
Add cryptography requirement to generate testing certs
oscarknagg Sep 15, 2021
ef5025a
Linting
oscarknagg Sep 15, 2021
d79fdd7
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Sep 16, 2021
de36d6a
Fix new_dashboard->dashboard merge
oscarknagg Sep 16, 2021
92627a8
Remove possibility of nullptr from RAY_USE_TLS
oscarknagg Sep 16, 2021
d3b47dc
clang-format 7.0.0 linting
oscarknagg Sep 16, 2021
08fc4b0
Linting
oscarknagg Sep 16, 2021
a70a355
Fix failing test_grpc_credentials test
oscarknagg Sep 16, 2021
cd613df
Make dashboard head classes use async grpc again
oscarknagg Sep 16, 2021
b296a8a
Add test_tls_auth to BUILD
oscarknagg Sep 16, 2021
1cc7744
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Sep 17, 2021
5528b51
Relax cryptography requirement
oscarknagg Sep 20, 2021
69f0618
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Sep 20, 2021
c77d97a
Lint
oscarknagg Sep 20, 2021
3cf6271
Worker._secure looks at env var
oscarknagg Sep 20, 2021
b84dbe6
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Sep 23, 2021
b82c932
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Sep 27, 2021
ddfa148
Apply changes from ci/travis/lint.sh
oscarknagg Sep 27, 2021
32acd64
Skip TLS tests on MacOS
oscarknagg Sep 27, 2021
d04fe6d
format.sh changes
oscarknagg Sep 27, 2021
53896b3
Address comments
oscarknagg Sep 27, 2021
09884ad
Merge branch 'tls' of github.com:oscarknagg/ray into tls
oscarknagg Oct 12, 2021
aea3e4e
Revert "Address comments"
oscarknagg Oct 12, 2021
8f02386
Merge master
oscarknagg Oct 12, 2021
6b0bced
Merge branch 'master' into tls
oscarknagg Oct 12, 2021
f57a61e
Merge remote-tracking branch 'upstream/master' into tls
oscarknagg Oct 12, 2021
78bbb34
Squashed commit of the following:
oscarknagg Oct 12, 2021
7639a65
Replace getenv with RayConfig
oscarknagg Oct 12, 2021
d95419a
Remove lingering errors from earlier merge
oscarknagg Oct 12, 2021
1c92af2
Address comments pt2
oscarknagg Oct 14, 2021
f2e1e55
Merge remote-tracking branch 'upstream/master' into tls
oscarknagg Oct 14, 2021
7c3f7b2
Tidy up
oscarknagg Oct 14, 2021
8d204c5
Hopefully fix lint
oscarknagg Oct 15, 2021
6954178
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Oct 15, 2021
94a52ae
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Oct 18, 2021
74d1652
Lint
oscarknagg Oct 18, 2021
c96043d
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Oct 18, 2021
50c2da2
Remove unecessary logic in ray_config_def.h
oscarknagg Oct 19, 2021
8599854
Actually check for ConnectionError in test_client_connect_to_tls_server
oscarknagg Oct 19, 2021
60355a2
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Oct 19, 2021
9dfd106
Remove unused ReadFile declaration
oscarknagg Oct 19, 2021
4feae45
Lint
oscarknagg Oct 19, 2021
5b57d7d
Replace grpc.insercure_channel with ray._private.utils.init_grpc_chan…
oscarknagg Oct 19, 2021
a600eaf
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Oct 19, 2021
67d32b7
Trigger retest
ericl Oct 19, 2021
6fa08dc
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Oct 20, 2021
f4032f1
Attempt to fix windows build
oscarknagg Oct 20, 2021
7fb64f0
Merge branch 'master' of https://github.com/ray-project/ray into tls
oscarknagg Oct 20, 2021
f4c8ae7
Merge branch 'master' into tls
ericl Oct 21, 2021
e74d707
Update worker.py
ericl Oct 21, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Remove duplicated ReadFile function
  • Loading branch information
oscarknagg committed Sep 15, 2021
commit 16c0cb3a30e3b0fb1d638f547d019994964312c2
31 changes: 31 additions & 0 deletions src/ray/rpc/common.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2017 The Ray Authors.
//
// 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:https://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 <fstream>
#include <sstream>

#include "ray/rpc/common.h"

namespace ray {
namespace rpc {

std::string ReadCert(std::string cert_filepath) {
std::ifstream t(cert_filepath);
std::stringstream buffer;
buffer << t.rdbuf();
return buffer.str();
};

} // namespace rpc
} // namespace ray
22 changes: 22 additions & 0 deletions src/ray/rpc/common.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright 2017 The Ray Authors.
//
// 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:https://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.

namespace ray {
namespace rpc {

// Utility to read cert file from a particular location
std::string ReadCert(std::string cert_filepath);

} // namespace rpc
} // namespace ray
16 changes: 4 additions & 12 deletions src/ray/rpc/grpc_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,12 @@
#include <grpcpp/grpcpp.h>

#include <boost/asio.hpp>
#include <fstream>
#include <sstream>

#include "ray/common/grpc_util.h"
#include "ray/common/ray_config.h"
#include "ray/common/status.h"
#include "ray/rpc/client_call.h"
#include "ray/rpc/common.h"

namespace ray {
namespace rpc {
Expand Down Expand Up @@ -108,13 +107,6 @@ class GrpcClient {
/// Whether to use TLS.
bool use_tls_;

std::string ReadFile(std::string filename) {
std::ifstream t(filename);
std::stringstream buffer;
buffer << t.rdbuf();
return buffer.str();
};

std::shared_ptr<grpc::Channel> BuildChannel(
grpc::ChannelArguments argument,
std::string address,
Expand All @@ -124,9 +116,9 @@ class GrpcClient {
std::string server_cert_file = std::string(std::getenv("RAY_TLS_SERVER_CERT"));
std::string server_key_file = std::string(std::getenv("RAY_TLS_SERVER_KEY"));
std::string root_cert_file = std::string(std::getenv("RAY_TLS_CA_CERT"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we document how to configure TLS in Ray? Could be a README or added to .rst

std::string server_cert_chain = ReadFile(server_cert_file);
std::string private_key = ReadFile(server_key_file);
std::string cacert = ReadFile(root_cert_file);
std::string server_cert_chain = ReadCert(server_cert_file);
std::string private_key = ReadCert(server_key_file);
std::string cacert = ReadCert(root_cert_file);

grpc::SslCredentialsOptions ssl_opts;
ssl_opts.pem_root_certs=cacert;
Expand Down
16 changes: 4 additions & 12 deletions src/ray/rpc/grpc_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@
#include <grpcpp/impl/service_type.h>

#include <boost/asio/detail/socket_holder.hpp>
#include <fstream>
#include <sstream>

#include "ray/common/ray_config.h"
#include "ray/rpc/grpc_server.h"
#include "ray/rpc/common.h"
#include "ray/stats/metric.h"
#include "ray/util/util.h"

Expand All @@ -48,13 +47,6 @@ GrpcServer::GrpcServer(std::string name, const uint32_t port, int num_threads, b
cqs_.resize(num_threads_);
}

std::string GrpcServer::ReadFile(std::string filename) {
std::ifstream t(filename);
std::stringstream buffer;
buffer << t.rdbuf();
return buffer.str();
};

void GrpcServer::Run() {
uint32_t specified_port = port_;
std::string server_address("0.0.0.0:" + std::to_string(port_));
Expand All @@ -80,9 +72,9 @@ void GrpcServer::Run() {
std::string root_cert_file = std::string(std::getenv("RAY_TLS_CA_CERT"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally don't use std::getenv except at process startup since it's not thread-safe; you can instead define these in ray_internal_flag_def.h and access then via RayConfig::instance().RAY_TLS_CA_CERT etc.

Copy link
Contributor Author

@oscarknagg oscarknagg Oct 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read the comments in ray_internal_flag_def.h and it specifies that it should contain internal flags that should not be set by users. As these environment variables will be set by users should I instead define these flags in ray_config_def.h?

In fact I could change the Python code to access these via ray._config too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to move it to ray_config_def.h then!


// Create credentials from hardcoded location
std::string rootcert = ReadFile(root_cert_file);
std::string servercert = ReadFile(server_cert_file);
std::string serverkey = ReadFile(server_key_file);
std::string rootcert = ReadCert(root_cert_file);
std::string servercert = ReadCert(server_cert_file);
std::string serverkey = ReadCert(server_key_file);
grpc::SslServerCredentialsOptions::PemKeyCertPair pkcp = {serverkey.c_str(),
servercert.c_str()};
// grpc::SslServerCredentialsOptions ssl_opts;
Expand Down
2 changes: 0 additions & 2 deletions src/ray/rpc/grpc_server.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
#include <boost/asio.hpp>
#include <thread>
#include <utility>
#include <fstream>
#include <sstream>

#include "ray/common/asio/instrumented_io_context.h"
#include "ray/common/status.h"
Expand Down