Reuse tls context #11
Conversation
| @@ -6,6 +6,8 @@ | |||
| #include <string> | |||
| #include <vector> | |||
|
|
|||
| #include <userver/crypto/ssl_ctx.hpp> | |||
| //#include <userver/crypto/ssl.hpp> | |||
| void* GetRawSslCtx() const noexcept; | ||
|
|
||
| private: | ||
| void AddCertAuthorities(const std::vector<Certificate>& cert_authorities); |
There was a problem hiding this comment.
If you use pimp idiom the private methods here seem unnecessary
|
|
||
| if (cert_chain.size() > 1) { | ||
| auto it = std::next(cert_chain.begin()); | ||
| for (; it != cert_chain.end(); ++it) { |
There was a problem hiding this comment.
if could be removed and everything else simplified
for (auto it == std::next(...); ...) {}
| SslCtx(const SslCtx&) = delete; | ||
| SslCtx& operator=(const SslCtx&) = delete; | ||
|
|
||
| void* GetRawSslCtx() const noexcept; |
There was a problem hiding this comment.
What's the point of removing the type information if it's casted to a specific type when this method is called?
| void ReadTlsSettings(const storages::secdist::SecdistConfig& secdist); | ||
| void InitSslCtx(); |
There was a problem hiding this comment.
Does it make sense to have a separate method though? If they need to be called together anyway
| throw KeyParseError(FormatSslError("Failed to get subject name from certificate")); | ||
| } | ||
|
|
||
| // Используем BIO для более гибкого форматирования |
There was a problem hiding this comment.
Why russian comments?
Was the code copied?
| throw KeyParseError(FormatSslError("Failed to create BIO")); | ||
| } | ||
|
|
||
| // Флаги форматирования можно настроить по вкусу |
| } | ||
| #if OPENSSL_VERSION_NUMBER >= 0x010100000L | ||
| if (1 != SSL_CTX_set_min_proto_version(ssl_ctx, TLS1_VERSION)) { | ||
| throw CryptoException(crypto::FormatSslError("Failed create an SSL context: SSL_CTX_set_min_proto_version")); |
There was a problem hiding this comment.
it seems it's going to be a memory leak of ssl_ctx if we throw here
|
|
||
| char* data = nullptr; | ||
| long len = BIO_get_mem_data(bio, &data); | ||
| std::string result(data, len); |
There was a problem hiding this comment.
The constructor may throw (although it's rare - it's still a possibility) which will leak bio
There was a problem hiding this comment.
It's better to use RAII as the reasoning about the exception safety is quite complex
Note: by creating a PR or an issue you automatically agree to the CLA. See CONTRIBUTING.md. Feel free to remove this note, the agreement holds.