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

security: Do not let grpc reload ssh cert if not changed (#9068) (#9069) #9080

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .github/licenserc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,6 @@ header:
- '.devcontainer/'
- '**/OWNERS'
- 'OWNERS_ALIASES'
- 'dbms/src/Common/tests/tls/'

comment: on-failure
6 changes: 4 additions & 2 deletions dbms/src/Common/TiFlashSecurity.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,13 @@ class TiFlashSecurityConfig : public ConfigObject
return allowed_common_names.count(cert.commonName()) > 0;
}

grpc::SslCredentialsOptions readAndCacheSslCredentialOptions()
std::optional<grpc::SslCredentialsOptions> readAndCacheSslCredentialOptions()
{
std::unique_lock lock(mu);
// if ssl_cerd_options_cached = true, it means the same options has already been returned to grpc-core
// don't need to return it again if ssl cert is not actually changed
if (ssl_cerd_options_cached)
return options;
return {};
options.pem_root_certs = readFile(ca_path);
options.pem_cert_chain = readFile(cert_path);
options.pem_private_key = readFile(key_path);
Expand Down
83 changes: 83 additions & 0 deletions dbms/src/Common/tests/gtest_tiflash_security.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
// limitations under the License.

#include <Common/TiFlashSecurity.h>
#include <Poco/File.h>
#include <Poco/FileStream.h>
#include <TestUtils/ConfigTestUtils.h>
#include <gtest/gtest.h>

Expand Down Expand Up @@ -188,5 +190,86 @@ redact_info_log=false
ASSERT_TRUE(tiflash_config_2.allowedCommonNames().empty());
ASSERT_FALSE(tiflash_config_2.hasTlsConfig());
}

String createConfigString(String & ca_path, String & cert_path, String & key_path)
{
String ret =
R"(
[security]
ca_path=")";
ret += ca_path;
ret += R"("
cert_path=")";
ret += cert_path;
ret += R"("
key_path=")";
ret += key_path;
ret += R"("
)";
return ret;
}

TEST(TiFlashSecurityTest, readAndCacheSslCredentialOptions)
{
const auto & test_info = testing::UnitTest::GetInstance()->current_test_info();
assert(test_info);
String file_name = test_info->file();
auto pos = file_name.find_last_of('/');
auto file_path = file_name.substr(0, pos);
// these certs are copied from https://github.com/grpc/grpc/tree/v1.64.0/src/python/grpcio_tests/tests/unit/credentials
auto ca_path = file_path + "/tls/ca.crt";
auto cert_path = file_path + "/tls/cert.crt";
auto key_path = file_path + "/tls/key.pem";
auto test = createConfigString(ca_path, cert_path, key_path);
auto config = loadConfigFromString(test);
const auto log = Logger::get();
TiFlashSecurityConfig tiflash_config(log);
tiflash_config.init(*config);
// first read will return a valid options
auto options = tiflash_config.readAndCacheSslCredentialOptions();
ASSERT_TRUE(options.has_value());
// not return valid options if cert is not changed
auto new_options = tiflash_config.readAndCacheSslCredentialOptions();
ASSERT_FALSE(new_options.has_value());
ca_path = file_path + "/tls/ca.crt.new";
cert_path = file_path + "/tls/cert.crt.new";
key_path = file_path + "/tls/key.pem.new";
{
Poco::FileOutputStream ca_fos(ca_path);
ca_fos << options.value().pem_root_certs;
Poco::FileOutputStream cert_fos(cert_path);
cert_fos << options.value().pem_cert_chain;
Poco::FileOutputStream key_fos(key_path);
key_fos << options.value().pem_private_key;
}
test = createConfigString(ca_path, cert_path, key_path);
config = loadConfigFromString(test);
ASSERT_TRUE(tiflash_config.update(*config));
// return a valid options if cert is changed
options = tiflash_config.readAndCacheSslCredentialOptions();
ASSERT_TRUE(options.has_value());
new_options = tiflash_config.readAndCacheSslCredentialOptions();
ASSERT_FALSE(new_options.has_value());
using namespace std::chrono_literals;
std::this_thread::sleep_for(1s);
{
// change the modified time
Poco::FileOutputStream fos(ca_path);
fos << options.value().pem_root_certs;
}
new_options = tiflash_config.readAndCacheSslCredentialOptions();
// no aware of the change since `update` is not called
ASSERT_FALSE(new_options.has_value());
ASSERT_TRUE(tiflash_config.update(*config));
// aware of the change and return a new options
new_options = tiflash_config.readAndCacheSslCredentialOptions();
ASSERT_TRUE(new_options.has_value());
Poco::File ca_file(ca_path);
ca_file.remove(false);
Poco::File cert_file(cert_path);
cert_file.remove(false);
Poco::File key_file(key_path);
key_file.remove(false);
}
} // namespace tests
} // namespace DB
20 changes: 20 additions & 0 deletions dbms/src/Common/tests/tls/ca.crt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
-----BEGIN CERTIFICATE-----
MIIDWjCCAkKgAwIBAgIUWrP0VvHcy+LP6UuYNtiL9gBhD5owDQYJKoZIhvcNAQEL
BQAwVjELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoM
GEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDEPMA0GA1UEAwwGdGVzdGNhMB4XDTIw
MDMxNzE4NTk1MVoXDTMwMDMxNTE4NTk1MVowVjELMAkGA1UEBhMCQVUxEzARBgNV
BAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMgUHR5IEx0
ZDEPMA0GA1UEAwwGdGVzdGNhMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKC
AQEAsGL0oXflF0LzoM+Bh+qUU9yhqzw2w8OOX5mu/iNCyUOBrqaHi7mGHx73GD01
diNzCzvlcQqdNIH6NQSL7DTpBjca66jYT9u73vZe2MDrr1nVbuLvfu9850cdxiUO
Inv5xf8+sTHG0C+a+VAvMhsLiRjsq+lXKRJyk5zkbbsETybqpxoJ+K7CoSy3yc/k
QIY3TipwEtwkKP4hzyo6KiGd/DPexie4nBUInN3bS1BUeNZ5zeaIC2eg3bkeeW7c
qT55b+Yen6CxY0TEkzBK6AKt/WUialKMgT0wbTxRZO7kUCH3Sq6e/wXeFdJ+HvdV
LPlAg5TnMaNpRdQih/8nRFpsdwIDAQABoyAwHjAMBgNVHRMEBTADAQH/MA4GA1Ud
DwEB/wQEAwICBDANBgkqhkiG9w0BAQsFAAOCAQEAkTrKZjBrJXHps/HrjNCFPb5a
THuGPCSsepe1wkKdSp1h4HGRpLoCgcLysCJ5hZhRpHkRihhef+rFHEe60UePQO3S
CVTtdJB4CYWpcNyXOdqefrbJW5QNljxgi6Fhvs7JJkBqdXIkWXtFk2eRgOIP2Eo9
/OHQHlYnwZFrk6sp4wPyR+A95S0toZBcyDVz7u+hOW0pGK3wviOe9lvRgj/H3Pwt
bewb0l+MhRig0/DVHamyVxrDRbqInU1/GTNCwcZkXKYFWSf92U+kIcTth24Q1gcw
eZiLl5FfrWokUNytFElXob0V0a5/kbhiLc3yWmvWqHTpqCALbVyF+rKJo2f5Kw==
-----END CERTIFICATE-----
22 changes: 22 additions & 0 deletions dbms/src/Common/tests/tls/cert.crt
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
-----BEGIN CERTIFICATE-----
MIIDtDCCApygAwIBAgIUbJfTREJ6k6/+oInWhV1O1j3ZT0IwDQYJKoZIhvcNAQEL
BQAwVjELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoM
GEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDEPMA0GA1UEAwwGdGVzdGNhMB4XDTIw
MDMxODAzMTA0MloXDTMwMDMxNjAzMTA0MlowZTELMAkGA1UEBhMCVVMxETAPBgNV
BAgMCElsbGlub2lzMRAwDgYDVQQHDAdDaGljYWdvMRUwEwYDVQQKDAxFeGFtcGxl
LCBDby4xGjAYBgNVBAMMESoudGVzdC5nb29nbGUuY29tMIIBIjANBgkqhkiG9w0B
AQEFAAOCAQ8AMIIBCgKCAQEA5xOONxJJ8b8Qauvob5/7dPYZfIcd+uhAWL2ZlTPz
Qvu4oF0QI4iYgP5iGgry9zEtCM+YQS8UhiAlPlqa6ANxgiBSEyMHH/xE8lo/+caY
GeACqy640Jpl/JocFGo3xd1L8DCawjlaj6eu7T7T/tpAV2qq13b5710eNRbCAfFe
8yALiGQemx0IYhlZXNbIGWLBNhBhvVjJh7UvOqpADk4xtl8o5j0xgMIRg6WJGK6c
6ffSIg4eP1XmovNYZ9LLEJG68tF0Q/yIN43B4dt1oq4jzSdCbG4F1EiykT2TmwPV
YDi8tml6DfOCDGnit8svnMEmBv/fcPd31GSbXjF8M+KGGQIDAQABo2swaTAJBgNV
HRMEAjAAMAsGA1UdDwQEAwIF4DBPBgNVHREESDBGghAqLnRlc3QuZ29vZ2xlLmZy
ghh3YXRlcnpvb2kudGVzdC5nb29nbGUuYmWCEioudGVzdC55b3V0dWJlLmNvbYcE
wKgBAzANBgkqhkiG9w0BAQsFAAOCAQEAS8hDQA8PSgipgAml7Q3/djwQ644ghWQv
C2Kb+r30RCY1EyKNhnQnIIh/OUbBZvh0M0iYsy6xqXgfDhCB93AA6j0i5cS8fkhH
Jl4RK0tSkGQ3YNY4NzXwQP/vmUgfkw8VBAZ4Y4GKxppdATjffIW+srbAmdDruIRM
wPeikgOoRrXf0LA1fi4TqxARzeRwenQpayNfGHTvVF9aJkl8HoaMunTAdG5pIVcr
9GKi/gEMpXUJbbVv3U5frX1Wo4CFo+rZWJ/LyCMeb0jciNLxSdMwj/E/ZuExlyeZ
gc9ctPjSMvgSyXEKv6Vwobleeg88V2ZgzenziORoWj4KszG/lbQZvg==
-----END CERTIFICATE-----
28 changes: 28 additions & 0 deletions dbms/src/Common/tests/tls/key.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
-----BEGIN PRIVATE KEY-----
MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQDnE443EknxvxBq
6+hvn/t09hl8hx366EBYvZmVM/NC+7igXRAjiJiA/mIaCvL3MS0Iz5hBLxSGICU+
WproA3GCIFITIwcf/ETyWj/5xpgZ4AKrLrjQmmX8mhwUajfF3UvwMJrCOVqPp67t
PtP+2kBXaqrXdvnvXR41FsIB8V7zIAuIZB6bHQhiGVlc1sgZYsE2EGG9WMmHtS86
qkAOTjG2XyjmPTGAwhGDpYkYrpzp99IiDh4/Veai81hn0ssQkbry0XRD/Ig3jcHh
23WiriPNJ0JsbgXUSLKRPZObA9VgOLy2aXoN84IMaeK3yy+cwSYG/99w93fUZJte
MXwz4oYZAgMBAAECggEBAIVn2Ncai+4xbH0OLWckabwgyJ4IM9rDc0LIU368O1kU
koais8qP9dujAWgfoh3sGh/YGgKn96VnsZjKHlyMgF+r4TaDJn3k2rlAOWcurGlj
1qaVlsV4HiEzp7pxiDmHhWvp4672Bb6iBG+bsjCUOEk/n9o9KhZzIBluRhtxCmw5
nw4Do7z00PTvN81260uPWSc04IrytvZUiAIx/5qxD72bij2xJ8t/I9GI8g4FtoVB
8pB6S/hJX1PZhh9VlU6Yk+TOfOVnbebG4W5138LkB835eqk3Zz0qsbc2euoi8Hxi
y1VGwQEmMQ63jXz4c6g+X55ifvUK9Jpn5E8pq+pMd7ECgYEA93lYq+Cr54K4ey5t
sWMa+ye5RqxjzgXj2Kqr55jb54VWG7wp2iGbg8FMlkQwzTJwebzDyCSatguEZLuB
gRGroRnsUOy9vBvhKPOch9bfKIl6qOgzMJB267fBVWx5ybnRbWN/I7RvMQf3k+9y
biCIVnxDLEEYyx7z85/5qxsXg/MCgYEA7wmWKtCTn032Hy9P8OL49T0X6Z8FlkDC
Rk42ygrc/MUbugq9RGUxcCxoImOG9JXUpEtUe31YDm2j+/nbvrjl6/bP2qWs0V7l
dTJl6dABP51pCw8+l4cWgBBX08Lkeen812AAFNrjmDCjX6rHjWHLJcpS18fnRRkP
V1d/AHWX7MMCgYEA6Gsw2guhp0Zf2GCcaNK5DlQab8OL4Hwrpttzo4kuTlwtqNKp
Q9H4al9qfF4Cr1TFya98+EVYf8yFRM3NLNjZpe3gwYf2EerlJj7VLcahw0KKzoN1
QBENfwgPLRk5sDkx9VhSmcfl/diLroZdpAwtv3vo4nEoxeuGFbKTGx3Qkf0CgYEA
xyR+dcb05Ygm3w4klHQTowQ10s1H80iaUcZBgQuR1ghEtDbUPZHsoR5t1xCB02ys
DgAwLv1bChIvxvH/L6KM8ovZ2LekBX4AviWxoBxJnfz/EVau98B0b1auRN6eSC83
FRuGldlSOW1z/nSh8ViizSYE5H5HX1qkXEippvFRE88CgYB3Bfu3YQY60ITWIShv
nNkdcbTT9eoP9suaRJjw92Ln+7ZpALYlQMKUZmJ/5uBmLs4RFwUTQruLOPL4yLTH
awADWUzs3IRr1fwn9E+zM8JVyKCnUEM3w4N5UZskGO2klashAd30hWO+knRv/y0r
uGIYs9Ek7YXlXIRVrzMwcsrt1w==
-----END PRIVATE KEY-----
14 changes: 9 additions & 5 deletions dbms/src/Server/FlashGrpcServerHolder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,15 @@ sslServerCertificateConfigCallback(
}
auto * context = static_cast<Context *>(arg);
auto options = context->getSecurityConfig()->readAndCacheSslCredentialOptions();
grpc_ssl_pem_key_cert_pair pem_key_cert_pair = {options.pem_private_key.c_str(), options.pem_cert_chain.c_str()};
*config = grpc_ssl_server_certificate_config_create(options.pem_root_certs.c_str(),
&pem_key_cert_pair,
1);
return GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW;
if (options.has_value())
{
grpc_ssl_pem_key_cert_pair pem_key_cert_pair = {options.value().pem_private_key.c_str(), options.value().pem_cert_chain.c_str()};
*config = grpc_ssl_server_certificate_config_create(options.value().pem_root_certs.c_str(),
&pem_key_cert_pair,
1);
return GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW;
}
return GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED;
}

grpc_server_credentials * grpcSslServerCredentialsCreateWithFetcher(
Expand Down