PR feedback + constification

This commit is contained in:
comex 2023-06-25 19:23:23 -07:00
parent b9f9d0a642
commit d885dd5b64
8 changed files with 62 additions and 60 deletions

View file

@ -114,7 +114,7 @@ public:
~ISslConnection() {
shared_data_->connection_count--;
if (fd_to_close_.has_value()) {
s32 fd = *fd_to_close_;
const s32 fd = *fd_to_close_;
if (!do_not_close_socket_) {
LOG_ERROR(Service_SSL,
"do_not_close_socket was changed after setting socket; is this right?");
@ -123,7 +123,7 @@ public:
if (bsd) {
auto err = bsd->CloseImpl(fd);
if (err != Service::Sockets::Errno::SUCCESS) {
LOG_ERROR(Service_SSL, "failed to close duplicated socket: {}", err);
LOG_ERROR(Service_SSL, "Failed to close duplicated socket: {}", err);
}
}
}
@ -151,7 +151,7 @@ private:
if (do_not_close_socket_) {
auto res = bsd->DuplicateSocketImpl(fd);
if (!res.has_value()) {
LOG_ERROR(Service_SSL, "failed to duplicate socket");
LOG_ERROR(Service_SSL, "Failed to duplicate socket with fd {}", fd);
return ResultInvalidSocket;
}
fd = *res;
@ -171,7 +171,7 @@ private:
}
Result SetHostNameImpl(const std::string& hostname) {
LOG_DEBUG(Service_SSL, "SetHostNameImpl({})", hostname);
LOG_DEBUG(Service_SSL, "called. hostname={}", hostname);
ASSERT(!did_handshake_);
Result res = backend_->SetHostName(hostname);
if (res == ResultSuccess) {
@ -191,9 +191,9 @@ private:
ASSERT(mode == IoMode::Blocking || mode == IoMode::NonBlocking);
ASSERT_OR_EXECUTE(socket_, { return ResultNoSocket; });
bool non_block = mode == IoMode::NonBlocking;
Network::Errno e = socket_->SetNonBlock(non_block);
if (e != Network::Errno::SUCCESS) {
const bool non_block = mode == IoMode::NonBlocking;
const Network::Errno error = socket_->SetNonBlock(non_block);
if (error != Network::Errno::SUCCESS) {
LOG_ERROR(Service_SSL, "Failed to set native socket non-block flag to {}", non_block);
}
return ResultSuccess;
@ -307,13 +307,13 @@ private:
}
void DoHandshakeGetServerCert(HLERequestContext& ctx) {
Result res = DoHandshakeImpl();
const Result res = DoHandshakeImpl();
u32 certs_count = 0;
u32 certs_size = 0;
if (res == ResultSuccess) {
auto certs = backend_->GetServerCerts();
if (certs.Succeeded()) {
std::vector<u8> certs_buf = SerializeServerCerts(*certs);
const std::vector<u8> certs_buf = SerializeServerCerts(*certs);
ctx.WriteBuffer(certs_buf);
certs_count = static_cast<u32>(certs->size());
certs_size = static_cast<u32>(certs_buf.size());
@ -377,7 +377,7 @@ private:
get_server_cert_chain_ = static_cast<bool>(parameters.value);
break;
default:
LOG_WARNING(Service_SSL, "unrecognized option={}, value={}", parameters.option,
LOG_WARNING(Service_SSL, "Unknown option={}, value={}", parameters.option,
parameters.value);
}

View file

@ -23,11 +23,11 @@ constexpr Result ResultInvalidSocket{ErrorModule::SSLSrv, 106};
constexpr Result ResultTimeout{ErrorModule::SSLSrv, 205};
constexpr Result ResultInternalError{ErrorModule::SSLSrv, 999}; // made up
constexpr Result ResultWouldBlock{ErrorModule::SSLSrv, 204};
// ^ ResultWouldBlock is returned from Read and Write, and oddly, DoHandshake,
// ResultWouldBlock is returned from Read and Write, and oddly, DoHandshake,
// with no way in the latter case to distinguish whether the client should poll
// for read or write. The one official client I've seen handles this by always
// polling for read (with a timeout).
constexpr Result ResultWouldBlock{ErrorModule::SSLSrv, 204};
class SSLConnectionBackend {
public:

View file

@ -8,7 +8,8 @@
namespace Service::SSL {
ResultVal<std::unique_ptr<SSLConnectionBackend>> CreateSSLConnectionBackend() {
LOG_ERROR(Service_SSL, "No SSL backend on this platform");
LOG_ERROR(Service_SSL,
"Can't create SSL connection because no SSL backend is available on this platform");
return ResultInternalError;
}

View file

@ -90,15 +90,15 @@ public:
Result DoHandshake() override {
SSL_set_verify_result(ssl_, X509_V_OK);
int ret = SSL_do_handshake(ssl_);
long verify_result = SSL_get_verify_result(ssl_);
const int ret = SSL_do_handshake(ssl_);
const long verify_result = SSL_get_verify_result(ssl_);
if (verify_result != X509_V_OK) {
LOG_ERROR(Service_SSL, "SSL cert verification failed because: {}",
X509_verify_cert_error_string(verify_result));
return CheckOpenSSLErrors();
}
if (ret <= 0) {
int ssl_err = SSL_get_error(ssl_, ret);
const int ssl_err = SSL_get_error(ssl_, ret);
if (ssl_err == SSL_ERROR_ZERO_RETURN ||
(ssl_err == SSL_ERROR_SYSCALL && got_read_eof_)) {
LOG_ERROR(Service_SSL, "SSL handshake failed because server hung up");
@ -110,18 +110,18 @@ public:
ResultVal<size_t> Read(std::span<u8> data) override {
size_t actual;
int ret = SSL_read_ex(ssl_, data.data(), data.size(), &actual);
const int ret = SSL_read_ex(ssl_, data.data(), data.size(), &actual);
return HandleReturn("SSL_read_ex", actual, ret);
}
ResultVal<size_t> Write(std::span<const u8> data) override {
size_t actual;
int ret = SSL_write_ex(ssl_, data.data(), data.size(), &actual);
const int ret = SSL_write_ex(ssl_, data.data(), data.size(), &actual);
return HandleReturn("SSL_write_ex", actual, ret);
}
ResultVal<size_t> HandleReturn(const char* what, size_t actual, int ret) {
int ssl_err = SSL_get_error(ssl_, ret);
const int ssl_err = SSL_get_error(ssl_, ret);
CheckOpenSSLErrors();
switch (ssl_err) {
case SSL_ERROR_NONE:
@ -255,7 +255,7 @@ public:
ResultVal<std::unique_ptr<SSLConnectionBackend>> CreateSSLConnectionBackend() {
auto conn = std::make_unique<SSLConnectionBackendOpenSSL>();
Result res = conn->Init();
const Result res = conn->Init();
if (res.IsFailure()) {
return res;
}

View file

@ -38,7 +38,7 @@ static void OneTimeInit() {
// certificate, and presenting one to some arbitrary server
// might be a privacy concern? Who knows, though.
SECURITY_STATUS ret =
const SECURITY_STATUS ret =
AcquireCredentialsHandle(nullptr, const_cast<LPTSTR>(UNISP_NAME), SECPKG_CRED_OUTBOUND,
nullptr, &schannel_cred, nullptr, nullptr, &cred_handle, nullptr);
if (ret != SEC_E_OK) {
@ -121,14 +121,14 @@ public:
}
Result FillCiphertextReadBuf() {
size_t fill_size = read_buf_fill_size_ ? read_buf_fill_size_ : 4096;
const size_t fill_size = read_buf_fill_size_ ? read_buf_fill_size_ : 4096;
read_buf_fill_size_ = 0;
// This unnecessarily zeroes the buffer; oh well.
size_t offset = ciphertext_read_buf_.size();
const size_t offset = ciphertext_read_buf_.size();
ASSERT_OR_EXECUTE(offset + fill_size >= offset, { return ResultInternalError; });
ciphertext_read_buf_.resize(offset + fill_size, 0);
auto read_span = std::span(ciphertext_read_buf_).subspan(offset, fill_size);
auto [actual, err] = socket_->Recv(0, read_span);
const auto read_span = std::span(ciphertext_read_buf_).subspan(offset, fill_size);
const auto [actual, err] = socket_->Recv(0, read_span);
switch (err) {
case Network::Errno::SUCCESS:
ASSERT(static_cast<size_t>(actual) <= fill_size);
@ -147,7 +147,7 @@ public:
// Returns success if the write buffer has been completely emptied.
Result FlushCiphertextWriteBuf() {
while (!ciphertext_write_buf_.empty()) {
auto [actual, err] = socket_->Send(ciphertext_write_buf_, 0);
const auto [actual, err] = socket_->Send(ciphertext_write_buf_, 0);
switch (err) {
case Network::Errno::SUCCESS:
ASSERT(static_cast<size_t>(actual) <= ciphertext_write_buf_.size());
@ -165,9 +165,10 @@ public:
}
Result CallInitializeSecurityContext() {
unsigned long req = ISC_REQ_ALLOCATE_MEMORY | ISC_REQ_CONFIDENTIALITY | ISC_REQ_INTEGRITY |
ISC_REQ_REPLAY_DETECT | ISC_REQ_SEQUENCE_DETECT | ISC_REQ_STREAM |
ISC_REQ_USE_SUPPLIED_CREDS;
const unsigned long req = ISC_REQ_ALLOCATE_MEMORY | ISC_REQ_CONFIDENTIALITY |
ISC_REQ_INTEGRITY | ISC_REQ_REPLAY_DETECT |
ISC_REQ_SEQUENCE_DETECT | ISC_REQ_STREAM |
ISC_REQ_USE_SUPPLIED_CREDS;
unsigned long attr;
// https://learn.microsoft.com/en-us/windows/win32/secauthn/initializesecuritycontext--schannel
std::array<SecBuffer, 2> input_buffers{{
@ -219,7 +220,7 @@ public:
ciphertext_read_buf_.size());
}
SECURITY_STATUS ret =
const SECURITY_STATUS ret =
InitializeSecurityContextA(&cred_handle, initial_call_done ? &ctxt_ : nullptr,
// Caller ensured we have set a hostname:
const_cast<char*>(hostname_.value().c_str()), req,
@ -231,15 +232,15 @@ public:
nullptr); // ptsExpiry
if (output_buffers[0].pvBuffer) {
std::span span(static_cast<u8*>(output_buffers[0].pvBuffer),
output_buffers[0].cbBuffer);
const std::span span(static_cast<u8*>(output_buffers[0].pvBuffer),
output_buffers[0].cbBuffer);
ciphertext_write_buf_.insert(ciphertext_write_buf_.end(), span.begin(), span.end());
FreeContextBuffer(output_buffers[0].pvBuffer);
}
if (output_buffers[1].pvBuffer) {
std::span span(static_cast<u8*>(output_buffers[1].pvBuffer),
output_buffers[1].cbBuffer);
const std::span span(static_cast<u8*>(output_buffers[1].pvBuffer),
output_buffers[1].cbBuffer);
// The documentation doesn't explain what format this data is in.
LOG_DEBUG(Service_SSL, "Got a {}-byte alert buffer: {}", span.size(),
Common::HexToString(span));
@ -280,7 +281,7 @@ public:
}
Result GrabStreamSizes() {
SECURITY_STATUS ret =
const SECURITY_STATUS ret =
QueryContextAttributes(&ctxt_, SECPKG_ATTR_STREAM_SIZES, &stream_sizes_);
if (ret != SEC_E_OK) {
LOG_ERROR(Service_SSL, "QueryContextAttributes(SECPKG_ATTR_STREAM_SIZES) failed: {}",
@ -301,7 +302,7 @@ public:
}
while (1) {
if (!cleartext_read_buf_.empty()) {
size_t read_size = std::min(cleartext_read_buf_.size(), data.size());
const size_t read_size = std::min(cleartext_read_buf_.size(), data.size());
std::memcpy(data.data(), cleartext_read_buf_.data(), read_size);
cleartext_read_buf_.erase(cleartext_read_buf_.begin(),
cleartext_read_buf_.begin() + read_size);
@ -366,7 +367,7 @@ public:
return ResultInternalError;
}
}
Result r = FillCiphertextReadBuf();
const Result r = FillCiphertextReadBuf();
if (r != ResultSuccess) {
return r;
}
@ -430,7 +431,7 @@ public:
.pBuffers = buffers.data(),
};
SECURITY_STATUS ret = EncryptMessage(&ctxt_, /*fQOP*/ 0, &desc, /*MessageSeqNo*/ 0);
const SECURITY_STATUS ret = EncryptMessage(&ctxt_, /*fQOP*/ 0, &desc, /*MessageSeqNo*/ 0);
if (ret != SEC_E_OK) {
LOG_ERROR(Service_SSL, "EncryptMessage failed: {}", Common::NativeErrorToString(ret));
return ResultInternalError;
@ -445,19 +446,19 @@ public:
}
ResultVal<size_t> WriteAlreadyEncryptedData() {
Result r = FlushCiphertextWriteBuf();
const Result r = FlushCiphertextWriteBuf();
if (r != ResultSuccess) {
return r;
}
// write buf is empty
size_t cleartext_bytes_written = cleartext_write_buf_.size();
const size_t cleartext_bytes_written = cleartext_write_buf_.size();
cleartext_write_buf_.clear();
return cleartext_bytes_written;
}
ResultVal<std::vector<std::vector<u8>>> GetServerCerts() override {
PCCERT_CONTEXT returned_cert = nullptr;
SECURITY_STATUS ret =
const SECURITY_STATUS ret =
QueryContextAttributes(&ctxt_, SECPKG_ATTR_REMOTE_CERT_CONTEXT, &returned_cert);
if (ret != SEC_E_OK) {
LOG_ERROR(Service_SSL,
@ -527,7 +528,7 @@ public:
ResultVal<std::unique_ptr<SSLConnectionBackend>> CreateSSLConnectionBackend() {
auto conn = std::make_unique<SSLConnectionBackendSchannel>();
Result res = conn->Init();
const Result res = conn->Init();
if (res.IsFailure()) {
return res;
}