Sync 1.0.0-enter code review

This commit is contained in:
ZPaC 2022-10-12 15:20:06 +08:00
parent c405880d62
commit d80d351ebc
6 changed files with 83 additions and 32 deletions

View File

@ -266,8 +266,8 @@ void SSLSocketOperation::Handshake(int fd, Connection *conn) const {
return;
}
auto err_msg = ERR_reason_error_string(static_cast<uint64_t>(err));
MS_LOG(ERROR) << "Failed to do the ssl handshake, retval: " << retval << ", errno: " << err
<< ", err info: " << err_msg;
MS_LOG(WARNING) << "Failed to do the ssl handshake, retval: " << retval << ", errno: " << err
<< ", err info: " << err_msg;
if (err == SSL_ERROR_WANT_WRITE) {
(void)conn->recv_event_loop->UpdateEpollEvent(fd, EPOLLOUT | EPOLLHUP | EPOLLERR | EPOLLRDHUP);
} else if (err == SSL_ERROR_WANT_READ) {

View File

@ -516,7 +516,7 @@ void CommUtil::verifyCertPipeline(const X509 *caCert, const X509 *subCert) {
}
if (!CommUtil::verifySingature(caCert, subCert)) {
MS_LOG(EXCEPTION) << "Verify Singature failed.";
MS_LOG(EXCEPTION) << "Verify Signature failed.";
}
if (!CommUtil::verifyExtendedAttributes(caCert)) {

View File

@ -59,8 +59,8 @@ void SSLClient::InitSSL() {
MS_LOG(INFO) << "client cert: " << client_cert;
// 2. Parse the client password.
std::string client_password = PSContext::instance()->client_password();
if (client_password.empty()) {
char *client_password = PSContext::instance()->client_password();
if (strlen(client_password) == 0) {
MS_LOG(EXCEPTION) << "The client password's value is empty.";
}
EVP_PKEY *pkey = nullptr;
@ -71,26 +71,29 @@ void SSLClient::InitSSL() {
MS_LOG(EXCEPTION) << "Read client cert file failed.";
}
PKCS12 *p12 = d2i_PKCS12_bio(bio, nullptr);
BIO_free_all(bio);
if (p12 == nullptr) {
MS_LOG(EXCEPTION) << "Create PKCS12 cert failed, please check whether the certificate is correct.";
}
BIO_free_all(bio);
if (!PKCS12_parse(p12, client_password.c_str(), &pkey, &cert, &ca_stack)) {
MS_LOG(EXCEPTION) << "PKCS12_parse failed.";
if (PKCS12_parse(p12, client_password, &pkey, &cert, &ca_stack) == 0) {
if (ERR_GET_REASON(ERR_peek_last_error()) == PKCS12_R_MAC_VERIFY_FAILURE) {
MS_LOG(EXCEPTION) << "The client password is invalid!";
}
MS_LOG(EXCEPTION) << "PKCS12_parse failed, the reason is " << ERR_reason_error_string(ERR_peek_last_error());
}
PSContext::instance()->ClearClientPassword();
PKCS12_free(p12);
if (cert == nullptr) {
MS_LOG(EXCEPTION) << "the cert is nullptr";
}
if (pkey == nullptr) {
MS_LOG(EXCEPTION) << "the key is nullptr";
MS_EXCEPTION_IF_NULL(cert);
MS_EXCEPTION_IF_NULL(pkey);
if (ca_stack != nullptr) {
MS_LOG(EXCEPTION) << "The cert is invalid: ca_stack should be empty.";
}
// 3. load ca cert.
std::string ca_path = CommUtil::ParseConfig(*config_, kCaCertPath);
if (!CommUtil::IsFileExists(ca_path)) {
MS_LOG(WARNING) << "The key:" << kCaCertPath << "'s value is not exist.";
MS_LOG(EXCEPTION) << "The key:" << kCaCertPath << "'s value is not exist.";
}
BIO *ca_bio = BIO_new_file(ca_path.c_str(), "r");
if (ca_bio == nullptr) {
@ -113,6 +116,8 @@ void SSLClient::InitSSL() {
StartCheckCertTime(*config_, cert);
EVP_PKEY_free(pkey);
X509_free(caCert);
X509_free(cert);
BIO_vfree(ca_bio);
if (crl != nullptr) {
X509_CRL_free(crl);

View File

@ -73,8 +73,8 @@ void SSLWrapper::InitSSL() {
MS_LOG(INFO) << "The server cert path:" << server_cert;
// 2. Parse the server password.
std::string server_password = PSContext::instance()->server_password();
if (server_password.empty()) {
char *server_password = PSContext::instance()->server_password();
if (strlen(server_password) == 0) {
MS_LOG(EXCEPTION) << "The client password's value is empty.";
}
@ -86,17 +86,28 @@ void SSLWrapper::InitSSL() {
MS_LOG(EXCEPTION) << "Read server cert file failed.";
}
PKCS12 *p12 = d2i_PKCS12_bio(bio, nullptr);
BIO_free_all(bio);
if (p12 == nullptr) {
MS_LOG(EXCEPTION) << "Create PKCS12 cert failed, please check whether the certificate is correct.";
}
BIO_free_all(bio);
if (!PKCS12_parse(p12, server_password.c_str(), &pkey, &cert, &ca_stack)) {
MS_LOG(EXCEPTION) << "PKCS12_parse failed.";
if (PKCS12_parse(p12, server_password, &pkey, &cert, &ca_stack) == 0) {
if (ERR_GET_REASON(ERR_peek_last_error()) == PKCS12_R_MAC_VERIFY_FAILURE) {
MS_LOG(EXCEPTION) << "The server password is invalid!";
}
MS_LOG(EXCEPTION) << "PKCS12_parse failed, the reason is " << ERR_reason_error_string(ERR_peek_last_error());
}
PSContext::instance()->ClearServerPassword();
PKCS12_free(p12);
MS_EXCEPTION_IF_NULL(cert);
MS_EXCEPTION_IF_NULL(pkey);
if (ca_stack != nullptr) {
MS_LOG(EXCEPTION) << "The cert is invalid: ca_stack should be empty.";
}
std::string ca_path = CommUtil::ParseConfig(*config_, kCaCertPath);
if (!CommUtil::IsFileExists(ca_path)) {
MS_LOG(WARNING) << "The key:" << kCaCertPath << "'s value is not exist.";
MS_LOG(EXCEPTION) << "The key:" << kCaCertPath << "'s value is not exist.";
}
BIO *ca_bio = BIO_new_file(ca_path.c_str(), "r");
if (ca_bio == nullptr) {
@ -124,6 +135,8 @@ void SSLWrapper::InitSSL() {
StartCheckCertTime(*config_, cert, ca_path);
EVP_PKEY_free(pkey);
X509_free(caCert);
X509_free(cert);
BIO_vfree(ca_bio);
if (crl != nullptr) {
X509_CRL_free(crl);

View File

@ -246,11 +246,41 @@ bool PSContext::enable_ssl() const { return enable_ssl_; }
void PSContext::set_enable_ssl(bool enabled) { enable_ssl_ = enabled; }
std::string PSContext::client_password() const { return client_password_; }
void PSContext::set_client_password(const std::string &password) { client_password_ = password; }
char *PSContext::client_password() { return client_password_; }
void PSContext::set_client_password(const char *password) {
if (strlen(password) >= kMaxPasswordLen) {
MS_LOG(EXCEPTION) << "Client password is longer than max password length " << kMaxPasswordLen;
}
int ret = memcpy_s(client_password_, kMaxPasswordLen, password, strlen(password));
if (ret != EOK) {
MS_LOG(EXCEPTION) << "memcpy_s client password failed, error: " << ret;
}
}
std::string PSContext::server_password() const { return server_password_; }
void PSContext::set_server_password(const std::string &password) { server_password_ = password; }
void PSContext::ClearClientPassword() {
int ret = memset_s(client_password_, kMaxPasswordLen, 0x00, kMaxPasswordLen);
if (ret != 0) {
MS_LOG(EXCEPTION) << "Clear client password failed, error: " << ret;
}
}
char *PSContext::server_password() { return server_password_; }
void PSContext::set_server_password(const char *password) {
if (strlen(password) >= kMaxPasswordLen) {
MS_LOG(EXCEPTION) << "Client password is longer than max password length " << kMaxPasswordLen;
}
int ret = memcpy_s(server_password_, kMaxPasswordLen, password, strlen(password));
if (ret != EOK) {
MS_LOG(EXCEPTION) << "memcpy_s server password failed, error: " << ret;
}
}
void PSContext::ClearServerPassword() {
int ret = memset_s(server_password_, kMaxPasswordLen, 0x00, kMaxPasswordLen);
if (ret != 0) {
MS_LOG(EXCEPTION) << "Clear client password failed, error: " << ret;
}
}
std::string PSContext::http_url_prefix() const { return http_url_prefix_; }

View File

@ -34,6 +34,7 @@ constexpr char kEnvRoleOfServer[] = "MS_SERVER";
constexpr char kEnvRoleOfWorker[] = "MS_WORKER";
constexpr char kEnvRoleOfScheduler[] = "MS_SCHED";
constexpr char kEnvRoleOfNotPS[] = "MS_NOT_PS";
constexpr size_t kMaxPasswordLen = 1024;
class BACKEND_EXPORT PSContext {
public:
@ -99,11 +100,13 @@ class BACKEND_EXPORT PSContext {
bool enable_ssl() const;
void set_enable_ssl(bool enabled);
std::string client_password() const;
void set_client_password(const std::string &password);
char *client_password();
void set_client_password(const char *password);
void ClearClientPassword();
std::string server_password() const;
void set_server_password(const std::string &password);
char *server_password();
void set_server_password(const char *password);
void ClearServerPassword();
std::string http_url_prefix() const;
@ -131,8 +134,8 @@ class BACKEND_EXPORT PSContext {
config_file_path_(""),
node_id_(""),
enable_ssl_(false),
client_password_(""),
server_password_(""),
client_password_(),
server_password_(),
http_url_prefix_(""),
instance_name_("") {}
// Use inline static instance pointer in case there are multiple static variables.
@ -169,9 +172,9 @@ class BACKEND_EXPORT PSContext {
// Whether to enable ssl for network communication.
bool enable_ssl_;
// Password used to decode p12 file.
std::string client_password_;
char client_password_[kMaxPasswordLen];
// Password used to decode p12 file.
std::string server_password_;
char server_password_[kMaxPasswordLen];
// http url prefix for http communication
std::string http_url_prefix_;
// The name of instance