From 83eee1d2266a1de374be0a8b2c0f2827f5e25bcf Mon Sep 17 00:00:00 2001 From: Liam Date: Sun, 16 Jul 2023 18:55:27 -0400 Subject: [PATCH] ssl: remove ResultVal use --- src/core/hle/service/sockets/nsd.cpp | 9 +- src/core/hle/service/ssl/ssl.cpp | 86 ++++++++++--------- src/core/hle/service/ssl/ssl_backend.h | 8 +- src/core/hle/service/ssl/ssl_backend_none.cpp | 2 +- .../hle/service/ssl/ssl_backend_openssl.cpp | 45 +++++----- .../hle/service/ssl/ssl_backend_schannel.cpp | 62 ++++++------- .../ssl/ssl_backend_securetransport.cpp | 39 ++++----- 7 files changed, 127 insertions(+), 124 deletions(-) diff --git a/src/core/hle/service/sockets/nsd.cpp b/src/core/hle/service/sockets/nsd.cpp index 5dfcaabb1..bac21752a 100644 --- a/src/core/hle/service/sockets/nsd.cpp +++ b/src/core/hle/service/sockets/nsd.cpp @@ -54,7 +54,7 @@ NSD::NSD(Core::System& system_, const char* name) : ServiceFramework{system_, na RegisterHandlers(functions); } -static ResultVal ResolveImpl(const std::string& fqdn_in) { +static std::string ResolveImpl(const std::string& fqdn_in) { // The real implementation makes various substitutions. // For now we just return the string as-is, which is good enough when not // connecting to real Nintendo servers. @@ -64,13 +64,10 @@ static ResultVal ResolveImpl(const std::string& fqdn_in) { static Result ResolveCommon(const std::string& fqdn_in, std::array& fqdn_out) { const auto res = ResolveImpl(fqdn_in); - if (res.Failed()) { - return res.Code(); - } - if (res->size() >= fqdn_out.size()) { + if (res.size() >= fqdn_out.size()) { return ResultOverflow; } - std::memcpy(fqdn_out.data(), res->c_str(), res->size() + 1); + std::memcpy(fqdn_out.data(), res.c_str(), res.size() + 1); return ResultSuccess; } diff --git a/src/core/hle/service/ssl/ssl.cpp b/src/core/hle/service/ssl/ssl.cpp index 9c96f9763..2cba9e5c9 100644 --- a/src/core/hle/service/ssl/ssl.cpp +++ b/src/core/hle/service/ssl/ssl.cpp @@ -4,6 +4,7 @@ #include "common/string_util.h" #include "core/core.h" +#include "core/hle/result.h" #include "core/hle/service/ipc_helpers.h" #include "core/hle/service/server_manager.h" #include "core/hle/service/service.h" @@ -141,12 +142,12 @@ private: bool did_set_host_name = false; bool did_handshake = false; - ResultVal SetSocketDescriptorImpl(s32 fd) { + Result SetSocketDescriptorImpl(s32* out_fd, s32 fd) { LOG_DEBUG(Service_SSL, "called, fd={}", fd); ASSERT(!did_handshake); auto bsd = system.ServiceManager().GetService("bsd:u"); ASSERT_OR_EXECUTE(bsd, { return ResultInternalError; }); - s32 ret_fd; + // Based on https://switchbrew.org/wiki/SSL_services#SetSocketDescriptor if (do_not_close_socket) { auto res = bsd->DuplicateSocketImpl(fd); @@ -156,9 +157,9 @@ private: } fd = *res; fd_to_close = fd; - ret_fd = fd; + *out_fd = fd; } else { - ret_fd = -1; + *out_fd = -1; } std::optional> sock = bsd->GetSocket(fd); if (!sock.has_value()) { @@ -167,7 +168,7 @@ private: } socket = std::move(*sock); backend->SetSocket(socket); - return ret_fd; + return ResultSuccess; } Result SetHostNameImpl(const std::string& hostname) { @@ -247,34 +248,36 @@ private: return ret; } - ResultVal> ReadImpl(size_t size) { + Result ReadImpl(std::vector* out_data, size_t size) { ASSERT_OR_EXECUTE(did_handshake, { return ResultInternalError; }); - std::vector res(size); - ResultVal actual = backend->Read(res); - if (actual.Failed()) { - return actual.Code(); + size_t actual_size{}; + Result res = backend->Read(&actual_size, *out_data); + if (res != ResultSuccess) { + return res; } - res.resize(*actual); + out_data->resize(actual_size); return res; } - ResultVal WriteImpl(std::span data) { + Result WriteImpl(size_t* out_size, std::span data) { ASSERT_OR_EXECUTE(did_handshake, { return ResultInternalError; }); - return backend->Write(data); + return backend->Write(out_size, data); } - ResultVal PendingImpl() { + Result PendingImpl(s32* out_pending) { LOG_WARNING(Service_SSL, "(STUBBED) called."); - return 0; + *out_pending = 0; + return ResultSuccess; } void SetSocketDescriptor(HLERequestContext& ctx) { IPC::RequestParser rp{ctx}; - const s32 fd = rp.Pop(); - const ResultVal res = SetSocketDescriptorImpl(fd); + const s32 in_fd = rp.Pop(); + s32 out_fd{-1}; + const Result res = SetSocketDescriptorImpl(&out_fd, in_fd); IPC::ResponseBuilder rb{ctx, 3}; - rb.Push(res.Code()); - rb.Push(res.ValueOr(-1)); + rb.Push(res); + rb.Push(out_fd); } void SetHostName(HLERequestContext& ctx) { @@ -313,14 +316,15 @@ private: }; static_assert(sizeof(OutputParameters) == 0x8); - const Result res = DoHandshakeImpl(); + Result res = DoHandshakeImpl(); OutputParameters out{}; if (res == ResultSuccess) { - auto certs = backend->GetServerCerts(); - if (certs.Succeeded()) { - const std::vector certs_buf = SerializeServerCerts(*certs); + std::vector> certs; + res = backend->GetServerCerts(&certs); + if (res == ResultSuccess) { + const std::vector certs_buf = SerializeServerCerts(certs); ctx.WriteBuffer(certs_buf); - out.certs_count = static_cast(certs->size()); + out.certs_count = static_cast(certs.size()); out.certs_size = static_cast(certs_buf.size()); } } @@ -330,29 +334,32 @@ private: } void Read(HLERequestContext& ctx) { - const ResultVal> res = ReadImpl(ctx.GetWriteBufferSize()); + std::vector output_bytes; + const Result res = ReadImpl(&output_bytes, ctx.GetWriteBufferSize()); IPC::ResponseBuilder rb{ctx, 3}; - rb.Push(res.Code()); - if (res.Succeeded()) { - rb.Push(static_cast(res->size())); - ctx.WriteBuffer(*res); + rb.Push(res); + if (res == ResultSuccess) { + rb.Push(static_cast(output_bytes.size())); + ctx.WriteBuffer(output_bytes); } else { rb.Push(static_cast(0)); } } void Write(HLERequestContext& ctx) { - const ResultVal res = WriteImpl(ctx.ReadBuffer()); + size_t write_size{0}; + const Result res = WriteImpl(&write_size, ctx.ReadBuffer()); IPC::ResponseBuilder rb{ctx, 3}; - rb.Push(res.Code()); - rb.Push(static_cast(res.ValueOr(0))); + rb.Push(res); + rb.Push(static_cast(write_size)); } void Pending(HLERequestContext& ctx) { - const ResultVal res = PendingImpl(); + s32 pending_size{0}; + const Result res = PendingImpl(&pending_size); IPC::ResponseBuilder rb{ctx, 3}; - rb.Push(res.Code()); - rb.Push(res.ValueOr(0)); + rb.Push(res); + rb.Push(pending_size); } void SetSessionCacheMode(HLERequestContext& ctx) { @@ -438,13 +445,14 @@ private: void CreateConnection(HLERequestContext& ctx) { LOG_WARNING(Service_SSL, "called"); - auto backend_res = CreateSSLConnectionBackend(); + std::unique_ptr backend; + const Result res = CreateSSLConnectionBackend(&backend); IPC::ResponseBuilder rb{ctx, 2, 0, 1}; - rb.Push(backend_res.Code()); - if (backend_res.Succeeded()) { + rb.Push(res); + if (res == ResultSuccess) { rb.PushIpcInterface(system, ssl_version, shared_data, - std::move(*backend_res)); + std::move(backend)); } } diff --git a/src/core/hle/service/ssl/ssl_backend.h b/src/core/hle/service/ssl/ssl_backend.h index 409f4367c..a2ec8e694 100644 --- a/src/core/hle/service/ssl/ssl_backend.h +++ b/src/core/hle/service/ssl/ssl_backend.h @@ -35,11 +35,11 @@ public: virtual void SetSocket(std::shared_ptr socket) = 0; virtual Result SetHostName(const std::string& hostname) = 0; virtual Result DoHandshake() = 0; - virtual ResultVal Read(std::span data) = 0; - virtual ResultVal Write(std::span data) = 0; - virtual ResultVal>> GetServerCerts() = 0; + virtual Result Read(size_t* out_size, std::span data) = 0; + virtual Result Write(size_t* out_size, std::span data) = 0; + virtual Result GetServerCerts(std::vector>* out_certs) = 0; }; -ResultVal> CreateSSLConnectionBackend(); +Result CreateSSLConnectionBackend(std::unique_ptr* out_backend); } // namespace Service::SSL diff --git a/src/core/hle/service/ssl/ssl_backend_none.cpp b/src/core/hle/service/ssl/ssl_backend_none.cpp index 2f4f23c42..a7fafd0a3 100644 --- a/src/core/hle/service/ssl/ssl_backend_none.cpp +++ b/src/core/hle/service/ssl/ssl_backend_none.cpp @@ -7,7 +7,7 @@ namespace Service::SSL { -ResultVal> CreateSSLConnectionBackend() { +Result CreateSSLConnectionBackend(std::unique_ptr* out_backend) { 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 6ca869dbf..b2dd37cd4 100644 --- a/src/core/hle/service/ssl/ssl_backend_openssl.cpp +++ b/src/core/hle/service/ssl/ssl_backend_openssl.cpp @@ -105,31 +105,30 @@ public: return ResultInternalError; } } - return HandleReturn("SSL_do_handshake", 0, ret).Code(); + return HandleReturn("SSL_do_handshake", 0, ret); } - ResultVal Read(std::span data) override { - size_t actual; - const int ret = SSL_read_ex(ssl, data.data(), data.size(), &actual); - return HandleReturn("SSL_read_ex", actual, ret); + Result Read(size_t* out_size, std::span data) override { + const int ret = SSL_read_ex(ssl, data.data(), data.size(), out_size); + return HandleReturn("SSL_read_ex", out_size, ret); } - ResultVal Write(std::span data) override { - size_t actual; - const int ret = SSL_write_ex(ssl, data.data(), data.size(), &actual); - return HandleReturn("SSL_write_ex", actual, ret); + Result Write(size_t* out_size, std::span data) override { + const int ret = SSL_write_ex(ssl, data.data(), data.size(), out_size); + return HandleReturn("SSL_write_ex", out_size, ret); } - ResultVal HandleReturn(const char* what, size_t actual, int ret) { + Result HandleReturn(const char* what, size_t* actual, int ret) { const int ssl_err = SSL_get_error(ssl, ret); CheckOpenSSLErrors(); switch (ssl_err) { case SSL_ERROR_NONE: - return actual; + return ResultSuccess; case SSL_ERROR_ZERO_RETURN: LOG_DEBUG(Service_SSL, "{} => SSL_ERROR_ZERO_RETURN", what); // DoHandshake special-cases this, but for Read and Write: - return size_t(0); + *actual = 0; + return ResultSuccess; case SSL_ERROR_WANT_READ: LOG_DEBUG(Service_SSL, "{} => SSL_ERROR_WANT_READ", what); return ResultWouldBlock; @@ -139,20 +138,20 @@ public: default: if (ssl_err == SSL_ERROR_SYSCALL && got_read_eof) { LOG_DEBUG(Service_SSL, "{} => SSL_ERROR_SYSCALL because server hung up", what); - return size_t(0); + *actual = 0; + return ResultSuccess; } LOG_ERROR(Service_SSL, "{} => other SSL_get_error return value {}", what, ssl_err); return ResultInternalError; } } - ResultVal>> GetServerCerts() override { + Result GetServerCerts(std::vector>* out_certs) override { STACK_OF(X509)* chain = SSL_get_peer_cert_chain(ssl); if (!chain) { LOG_ERROR(Service_SSL, "SSL_get_peer_cert_chain returned nullptr"); return ResultInternalError; } - std::vector> ret; int count = sk_X509_num(chain); ASSERT(count >= 0); for (int i = 0; i < count; i++) { @@ -161,10 +160,10 @@ public: unsigned char* buf = nullptr; int len = i2d_X509(x509, &buf); ASSERT_OR_EXECUTE(len >= 0 && buf, { continue; }); - ret.emplace_back(buf, buf + len); + out_certs->emplace_back(buf, buf + len); OPENSSL_free(buf); } - return ret; + return ResultSuccess; } ~SSLConnectionBackendOpenSSL() { @@ -253,13 +252,13 @@ public: std::shared_ptr socket; }; -ResultVal> CreateSSLConnectionBackend() { +Result CreateSSLConnectionBackend(std::unique_ptr* out_backend) { auto conn = std::make_unique(); - const Result res = conn->Init(); - if (res.IsFailure()) { - return res; - } - return conn; + + R_TRY(conn->Init()); + + *out_backend = std::move(conn); + return ResultSuccess; } namespace { diff --git a/src/core/hle/service/ssl/ssl_backend_schannel.cpp b/src/core/hle/service/ssl/ssl_backend_schannel.cpp index d8074339a..bda12b761 100644 --- a/src/core/hle/service/ssl/ssl_backend_schannel.cpp +++ b/src/core/hle/service/ssl/ssl_backend_schannel.cpp @@ -299,21 +299,22 @@ public: return ResultSuccess; } - ResultVal Read(std::span data) override { + Result Read(size_t* out_size, std::span data) override { + *out_size = 0; if (handshake_state != HandshakeState::Connected) { LOG_ERROR(Service_SSL, "Called Read but we did not successfully handshake"); return ResultInternalError; } if (data.size() == 0 || got_read_eof) { - return size_t(0); + return ResultSuccess; } while (1) { if (!cleartext_read_buf.empty()) { - const size_t read_size = std::min(cleartext_read_buf.size(), data.size()); - std::memcpy(data.data(), cleartext_read_buf.data(), read_size); + *out_size = std::min(cleartext_read_buf.size(), data.size()); + std::memcpy(data.data(), cleartext_read_buf.data(), *out_size); cleartext_read_buf.erase(cleartext_read_buf.begin(), - cleartext_read_buf.begin() + read_size); - return read_size; + cleartext_read_buf.begin() + *out_size); + return ResultSuccess; } if (!ciphertext_read_buf.empty()) { SecBuffer empty{ @@ -366,7 +367,8 @@ public: case SEC_I_CONTEXT_EXPIRED: // Server hung up by sending close_notify. got_read_eof = true; - return size_t(0); + *out_size = 0; + return ResultSuccess; default: LOG_ERROR(Service_SSL, "DecryptMessage failed: {}", Common::NativeErrorToString(ret)); @@ -379,18 +381,21 @@ public: } if (ciphertext_read_buf.empty()) { got_read_eof = true; - return size_t(0); + *out_size = 0; + return ResultSuccess; } } } - ResultVal Write(std::span data) override { + Result Write(size_t* out_size, std::span data) override { + *out_size = 0; + if (handshake_state != HandshakeState::Connected) { LOG_ERROR(Service_SSL, "Called Write but we did not successfully handshake"); return ResultInternalError; } if (data.size() == 0) { - return size_t(0); + return ResultSuccess; } data = data.subspan(0, std::min(data.size(), stream_sizes.cbMaximumMessage)); if (!cleartext_write_buf.empty()) { @@ -402,7 +407,7 @@ public: LOG_ERROR(Service_SSL, "Called Write but buffer does not match previous buffer"); return ResultInternalError; } - return WriteAlreadyEncryptedData(); + return WriteAlreadyEncryptedData(out_size); } else { cleartext_write_buf.assign(data.begin(), data.end()); } @@ -448,21 +453,21 @@ public: tmp_data_buf.end()); ciphertext_write_buf.insert(ciphertext_write_buf.end(), trailer_buf.begin(), trailer_buf.end()); - return WriteAlreadyEncryptedData(); + return WriteAlreadyEncryptedData(out_size); } - ResultVal WriteAlreadyEncryptedData() { + Result WriteAlreadyEncryptedData(size_t* out_size) { const Result r = FlushCiphertextWriteBuf(); if (r != ResultSuccess) { return r; } // write buf is empty - const size_t cleartext_bytes_written = cleartext_write_buf.size(); + *out_size = cleartext_write_buf.size(); cleartext_write_buf.clear(); - return cleartext_bytes_written; + return ResultSuccess; } - ResultVal>> GetServerCerts() override { + Result GetServerCerts(std::vector>* out_certs) override { PCCERT_CONTEXT returned_cert = nullptr; const SECURITY_STATUS ret = QueryContextAttributes(&ctxt, SECPKG_ATTR_REMOTE_CERT_CONTEXT, &returned_cert); @@ -473,16 +478,15 @@ public: return ResultInternalError; } PCCERT_CONTEXT some_cert = nullptr; - std::vector> certs; while ((some_cert = CertEnumCertificatesInStore(returned_cert->hCertStore, some_cert))) { - certs.emplace_back(static_cast(some_cert->pbCertEncoded), - static_cast(some_cert->pbCertEncoded) + - some_cert->cbCertEncoded); + out_certs->emplace_back(static_cast(some_cert->pbCertEncoded), + static_cast(some_cert->pbCertEncoded) + + some_cert->cbCertEncoded); } - std::reverse(certs.begin(), - certs.end()); // Windows returns certs in reverse order from what we want + std::reverse(out_certs->begin(), + out_certs->end()); // Windows returns certs in reverse order from what we want CertFreeCertificateContext(returned_cert); - return certs; + return ResultSuccess; } ~SSLConnectionBackendSchannel() { @@ -532,13 +536,13 @@ public: size_t read_buf_fill_size = 0; }; -ResultVal> CreateSSLConnectionBackend() { +Result CreateSSLConnectionBackend(std::unique_ptr* out_backend) { auto conn = std::make_unique(); - const Result res = conn->Init(); - if (res.IsFailure()) { - return res; - } - return conn; + + R_TRY(conn->Init()); + + *out_backend = std::move(conn); + return ResultSuccess; } } // namespace Service::SSL diff --git a/src/core/hle/service/ssl/ssl_backend_securetransport.cpp b/src/core/hle/service/ssl/ssl_backend_securetransport.cpp index b3083cbad..5f9e6bef7 100644 --- a/src/core/hle/service/ssl/ssl_backend_securetransport.cpp +++ b/src/core/hle/service/ssl/ssl_backend_securetransport.cpp @@ -103,24 +103,20 @@ public: return HandleReturn("SSLHandshake", 0, status).Code(); } - ResultVal Read(std::span data) override { - size_t actual; - OSStatus status = SSLRead(context, data.data(), data.size(), &actual); - ; - return HandleReturn("SSLRead", actual, status); + Result Read(size_t* out_size, std::span data) override { + OSStatus status = SSLRead(context, data.data(), data.size(), &out_size); + return HandleReturn("SSLRead", out_size, status); } - ResultVal Write(std::span data) override { - size_t actual; - OSStatus status = SSLWrite(context, data.data(), data.size(), &actual); - ; - return HandleReturn("SSLWrite", actual, status); + Result Write(size_t* out_size, std::span data) override { + OSStatus status = SSLWrite(context, data.data(), data.size(), &out_size); + return HandleReturn("SSLWrite", out_size, status); } - ResultVal HandleReturn(const char* what, size_t actual, OSStatus status) { + Result HandleReturn(const char* what, size_t* actual, OSStatus status) { switch (status) { case 0: - return actual; + return ResultSuccess; case errSSLWouldBlock: return ResultWouldBlock; default: { @@ -136,22 +132,21 @@ public: } } - ResultVal>> GetServerCerts() override { + Result GetServerCerts(std::vector>* out_certs) override { CFReleaser trust; OSStatus status = SSLCopyPeerTrust(context, &trust.ptr); if (status) { LOG_ERROR(Service_SSL, "SSLCopyPeerTrust failed: {}", OSStatusToString(status)); return ResultInternalError; } - std::vector> ret; for (CFIndex i = 0, count = SecTrustGetCertificateCount(trust); i < count; i++) { SecCertificateRef cert = SecTrustGetCertificateAtIndex(trust, i); CFReleaser data(SecCertificateCopyData(cert)); ASSERT_OR_EXECUTE(data, { return ResultInternalError; }); const u8* ptr = CFDataGetBytePtr(data); - ret.emplace_back(ptr, ptr + CFDataGetLength(data)); + out_certs->emplace_back(ptr, ptr + CFDataGetLength(data)); } - return ret; + return ResultSuccess; } static OSStatus ReadCallback(SSLConnectionRef connection, void* data, size_t* dataLength) { @@ -210,13 +205,13 @@ private: std::shared_ptr socket; }; -ResultVal> CreateSSLConnectionBackend() { +Result CreateSSLConnectionBackend(std::unique_ptr* out_backend) { auto conn = std::make_unique(); - const Result res = conn->Init(); - if (res.IsFailure()) { - return res; - } - return conn; + + R_TRY(conn->Init()); + + *out_backend = std::move(conn); + return ResultSuccess; } } // namespace Service::SSL