From d885dd5b642807d0587acad43668cfccfdf06d1e Mon Sep 17 00:00:00 2001 From: comex Date: Sun, 25 Jun 2023 19:23:23 -0700 Subject: PR feedback + constification --- src/core/hle/service/ssl/ssl.cpp | 20 +++++----- src/core/hle/service/ssl/ssl_backend.h | 4 +- src/core/hle/service/ssl/ssl_backend_none.cpp | 3 +- src/core/hle/service/ssl/ssl_backend_openssl.cpp | 14 +++---- src/core/hle/service/ssl/ssl_backend_schannel.cpp | 45 ++++++++++++----------- 5 files changed, 44 insertions(+), 42 deletions(-) (limited to 'src/core/hle/service/ssl') diff --git a/src/core/hle/service/ssl/ssl.cpp b/src/core/hle/service/ssl/ssl.cpp index a3b54c7f0..5638dd693 100644 --- a/src/core/hle/service/ssl/ssl.cpp +++ b/src/core/hle/service/ssl/ssl.cpp @@ -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 certs_buf = SerializeServerCerts(*certs); + const std::vector certs_buf = SerializeServerCerts(*certs); ctx.WriteBuffer(certs_buf); certs_count = static_cast(certs->size()); certs_size = static_cast(certs_buf.size()); @@ -377,7 +377,7 @@ private: get_server_cert_chain_ = static_cast(parameters.value); break; default: - LOG_WARNING(Service_SSL, "unrecognized option={}, value={}", parameters.option, + LOG_WARNING(Service_SSL, "Unknown option={}, value={}", parameters.option, parameters.value); } diff --git a/src/core/hle/service/ssl/ssl_backend.h b/src/core/hle/service/ssl/ssl_backend.h index 0dd8d9118..25c16bcc1 100644 --- a/src/core/hle/service/ssl/ssl_backend.h +++ b/src/core/hle/service/ssl/ssl_backend.h @@ -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: diff --git a/src/core/hle/service/ssl/ssl_backend_none.cpp b/src/core/hle/service/ssl/ssl_backend_none.cpp index eb01561e2..f2f0ef706 100644 --- a/src/core/hle/service/ssl/ssl_backend_none.cpp +++ b/src/core/hle/service/ssl/ssl_backend_none.cpp @@ -8,7 +8,8 @@ namespace Service::SSL { ResultVal> 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; } diff --git a/src/core/hle/service/ssl/ssl_backend_openssl.cpp b/src/core/hle/service/ssl/ssl_backend_openssl.cpp index bc797b76b..e7d5801fd 100644 --- a/src/core/hle/service/ssl/ssl_backend_openssl.cpp +++ b/src/core/hle/service/ssl/ssl_backend_openssl.cpp @@ -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 Read(std::span 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 Write(std::span 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 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> CreateSSLConnectionBackend() { auto conn = std::make_unique(); - Result res = conn->Init(); + const Result res = conn->Init(); if (res.IsFailure()) { return res; } diff --git a/src/core/hle/service/ssl/ssl_backend_schannel.cpp b/src/core/hle/service/ssl/ssl_backend_schannel.cpp index d293adcf7..775d5cc07 100644 --- a/src/core/hle/service/ssl/ssl_backend_schannel.cpp +++ b/src/core/hle/service/ssl/ssl_backend_schannel.cpp @@ -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(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(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(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 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(hostname_.value().c_str()), req, @@ -231,15 +232,15 @@ public: nullptr); // ptsExpiry if (output_buffers[0].pvBuffer) { - std::span span(static_cast(output_buffers[0].pvBuffer), - output_buffers[0].cbBuffer); + const std::span span(static_cast(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(output_buffers[1].pvBuffer), - output_buffers[1].cbBuffer); + const std::span span(static_cast(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 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>> 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> CreateSSLConnectionBackend() { auto conn = std::make_unique(); - Result res = conn->Init(); + const Result res = conn->Init(); if (res.IsFailure()) { return res; } -- cgit v1.2.3