From d9dd1182b99b107f7aa6fb2e5ab3a987e87a3373 Mon Sep 17 00:00:00 2001 From: Reece Wilson Date: Fri, 18 Nov 2022 04:15:05 +0000 Subject: [PATCH] [+] TLS pinning [*] ISocket::Shutdown(*bool bNow*), allowing for flush of the send channel when false [*] Fix StartRead and StartWrite after shutdown (NT) [*] Amended dead-lock --- Include/Aurora/IO/Net/ISocketBase.hpp | 2 +- Include/Aurora/IO/TLS/ICertificateChain.hpp | 3 +- Source/Crypto/X509/x509.cpp | 50 +++++++------- Source/Crypto/X509/x509.hpp | 9 +++ Source/IO/Net/AuNetSocket.NT.cpp | 21 +++++- Source/IO/Net/AuNetSocket.NT.hpp | 2 +- Source/IO/Net/AuNetSocket.hpp | 3 +- Source/IO/Net/AuNetSocketChannelOutput.cpp | 27 ++++++-- Source/IO/Net/AuNetSocketChannelOutput.hpp | 2 + Source/IO/Net/AuNetSocketServer.cpp | 5 +- Source/IO/Net/AuNetSocketServer.hpp | 2 +- Source/IO/Net/AuNetStream.NT.cpp | 20 ++++++ Source/IO/TLS/TLSCertificateChain.cpp | 75 ++++++++++++++++++++- Source/IO/TLS/TLSCertificateChain.hpp | 5 +- Source/IO/TLS/TLSContext.cpp | 29 ++++++-- Source/IO/TLS/TLSContext.hpp | 3 +- Source/IO/TLS/TLSProtocolRecv.cpp | 7 ++ 17 files changed, 217 insertions(+), 48 deletions(-) diff --git a/Include/Aurora/IO/Net/ISocketBase.hpp b/Include/Aurora/IO/Net/ISocketBase.hpp index cd5ba73c..0893a581 100644 --- a/Include/Aurora/IO/Net/ISocketBase.hpp +++ b/Include/Aurora/IO/Net/ISocketBase.hpp @@ -14,7 +14,7 @@ namespace Aurora::IO::Net virtual const NetError &GetError() = 0; virtual const NetEndpoint &GetLocalEndpoint() = 0; - virtual void Shutdown() = 0; + virtual void Shutdown(bool bNow = true) = 0; virtual void Destroy() = 0; virtual AuUInt ToPlatformHandle() = 0; diff --git a/Include/Aurora/IO/TLS/ICertificateChain.hpp b/Include/Aurora/IO/TLS/ICertificateChain.hpp index 5716e59a..1d44fc8c 100644 --- a/Include/Aurora/IO/TLS/ICertificateChain.hpp +++ b/Include/Aurora/IO/TLS/ICertificateChain.hpp @@ -12,7 +12,8 @@ namespace Aurora::IO::TLS struct ICertificateChain { virtual AuUInt32 GetCertificateCount() = 0; - virtual AuMemoryViewRead GetCertificate(AuUInt32 idx) = 0; + virtual AuSPtr GetCertificate(AuUInt32 idx) = 0; + virtual Crypto::X509::DecodedCertificate GetCertificateDetails(AuUInt32 idx) = 0; }; AUKN_SYM AuSPtr ChainFromOne(const AuMemoryViewRead &read); diff --git a/Source/Crypto/X509/x509.cpp b/Source/Crypto/X509/x509.cpp index c5a6c209..70965076 100644 --- a/Source/Crypto/X509/x509.cpp +++ b/Source/Crypto/X509/x509.cpp @@ -351,35 +351,37 @@ namespace Aurora::Crypto::X509 return AuTime::FromCivilTime(tm, true); } + void DecodeInternal(const mbedtls_x509_crt &crt, DecodedCertificate &out) + { + auto &issuer = crt.issuer; + auto &subject = crt.subject; + + FindCommonNames(issuer, out.issuer); + FindCommonNames(subject, out.subject); + + out.validity.issued = ConvertTime(crt.valid_from); + out.validity.expire = ConvertTime(crt.valid_to); + + x509_get_ca_id((mbedtls_x509_crt *)&crt, out.issuer.id); + x509_get_subject_id((mbedtls_x509_crt *)&crt, out.subject.id); + + out.serialNumber.resize(crt.serial.len); + memcpy(out.serialNumber.data(), crt.serial.p, out.serialNumber.size()); + + out.algorithmOid.resize(crt.sig_oid.len); + memcpy(out.algorithmOid.data(), crt.sig_oid.p, out.algorithmOid.size()); + + AuList oscp; + x509_get_aia((mbedtls_x509_crt *)&crt, oscp, out.AIAs); + } + AUKN_SYM bool Decode(const Certificate &der, DecodedCertificate &out) { - bool failed = false; - return ParseCert(der, [&](mbedtls_x509_crt &crt) { - auto &issuer = crt.issuer; - auto &subject = crt.subject; - - FindCommonNames(issuer, out.issuer); - FindCommonNames(subject, out.subject); - - out.validity.issued = ConvertTime(crt.valid_from); - out.validity.expire = ConvertTime(crt.valid_to); - - x509_get_ca_id(&crt, out.issuer.id); - x509_get_subject_id(&crt, out.subject.id); - - out.serialNumber.resize(crt.serial.len); - memcpy(out.serialNumber.data(), crt.serial.p, out.serialNumber.size()); - - out.algorithmOid.resize(crt.sig_oid.len); - memcpy(out.algorithmOid.data(), crt.sig_oid.p, out.algorithmOid.size()); - - AuList oscp; - x509_get_aia(&crt, oscp, out.AIAs); - }) && !failed; - + DecodeInternal(crt, out); + }); } static bool IsHighRiskStateIssuer(const mbedtls_x509_crt &ca) diff --git a/Source/Crypto/X509/x509.hpp b/Source/Crypto/X509/x509.hpp index a523854b..59e55931 100644 --- a/Source/Crypto/X509/x509.hpp +++ b/Source/Crypto/X509/x509.hpp @@ -7,3 +7,12 @@ ***/ #pragma once +#include "../Crypto.hpp" +#include + +namespace Aurora::Crypto::X509 +{ + struct DecodedCertificate; + + void DecodeInternal(const mbedtls_x509_crt &crt, DecodedCertificate &out); +} \ No newline at end of file diff --git a/Source/IO/Net/AuNetSocket.NT.cpp b/Source/IO/Net/AuNetSocket.NT.cpp index 2943998f..232632af 100644 --- a/Source/IO/Net/AuNetSocket.NT.cpp +++ b/Source/IO/Net/AuNetSocket.NT.cpp @@ -210,9 +210,24 @@ namespace Aurora::IO::Net return ::ioctlsocket(this->osHandle_, FIONBIO, &iMode) == 0; } - void Socket::Shutdown() + void Socket::Shutdown(bool bNow) { - this->SendEnd(); - ::shutdown(this->osHandle_, SD_BOTH); + if (bNow) + { + this->SendEnd(); + ::shutdown(this->osHandle_, SD_BOTH); + } + else + { + if (!this->socketChannel_.outputChannel.AsWritableByteBuffer()->RemainingBytes()) + { + this->Shutdown(true); + } + else + { + this->socketChannel_.outputChannel.bShutdownOnComplete = true; + this->socketChannel_.ScheduleOutOfFrameWrite(); + } + } } } \ No newline at end of file diff --git a/Source/IO/Net/AuNetSocket.NT.hpp b/Source/IO/Net/AuNetSocket.NT.hpp index 04748557..3d1df7a1 100644 --- a/Source/IO/Net/AuNetSocket.NT.hpp +++ b/Source/IO/Net/AuNetSocket.NT.hpp @@ -58,7 +58,7 @@ namespace Aurora::IO::Net virtual bool MakeNonblocking() override; - virtual void Shutdown() override; + virtual void Shutdown(bool bNow) override; virtual void CloseSocket() override; }; diff --git a/Source/IO/Net/AuNetSocket.hpp b/Source/IO/Net/AuNetSocket.hpp index 11294549..9d16bb32 100644 --- a/Source/IO/Net/AuNetSocket.hpp +++ b/Source/IO/Net/AuNetSocket.hpp @@ -106,6 +106,8 @@ namespace Aurora::IO::Net AuUInt endpointSize_ {}; bool bHasFinalized_ {}; + bool bHasEnded {}; + protected: AuUInt osHandle_; @@ -124,7 +126,6 @@ namespace Aurora::IO::Net NetError error_; - bool bHasEnded {}; bool bHasErrored_ {}; bool bHasConnected_ {}; }; diff --git a/Source/IO/Net/AuNetSocketChannelOutput.cpp b/Source/IO/Net/AuNetSocketChannelOutput.cpp index e63ffdfa..73e1ddb8 100644 --- a/Source/IO/Net/AuNetSocketChannelOutput.cpp +++ b/Source/IO/Net/AuNetSocketChannelOutput.cpp @@ -141,7 +141,20 @@ namespace Aurora::IO::Net void SocketChannelOutput::WriteTick() { - AU_LOCK_GUARD(this->lock_); + bool bShouldShutdown {}; + { + AU_LOCK_GUARD(this->lock_); + bShutdownOnComplete = WriteTickLocked(); + } + + if (bShouldShutdown) + { + this->pParent_->Shutdown(true); + } + } + + bool SocketChannelOutput::WriteTickLocked() + { #if defined(AURORA_IS_MODERNNT_DERIVED) auto pHackTransaction = @@ -156,7 +169,7 @@ namespace Aurora::IO::Net // IsComplete? if (!pHackTransaction->bLatch) { - return; + return false; } } @@ -168,7 +181,7 @@ namespace Aurora::IO::Net { SysPushErrorIO("Couldn't begin wait"); this->pParent_->SendErrorBeginShutdown({}); - return; + return false; } if (!this->pNetWriteTransaction_->StartWrite(0, pFrameToSend)) @@ -176,13 +189,19 @@ namespace Aurora::IO::Net this->pParent_->ToWorkerEx()->DecrementIOEventTaskCounter(); SysPushErrorIO("Couldn't dispatch the to-send frame"); this->pParent_->SendErrorBeginShutdown({}); - return; + return false; } } else { this->pNetWriteTransaction_->SetCallback({}); + if (this->bShutdownOnComplete) + { + return true; + } } + + return false; } void SocketChannelOutput::OnEndOfReadTick() diff --git a/Source/IO/Net/AuNetSocketChannelOutput.hpp b/Source/IO/Net/AuNetSocketChannelOutput.hpp index f0b0c46a..88cb99a5 100644 --- a/Source/IO/Net/AuNetSocketChannelOutput.hpp +++ b/Source/IO/Net/AuNetSocketChannelOutput.hpp @@ -27,11 +27,13 @@ namespace Aurora::IO::Net // Write outbound shit void SchedWriteTick(); void SendIfData(); + bool WriteTickLocked(); void WriteTick(); void OnEndOfReadTick(); void OnAsyncFileOpFinished(AuUInt64 offset, AuUInt32 length) override; + bool bShutdownOnComplete {}; private: SocketBase * pParent_; AuSPtr pNetWriteTransaction_; diff --git a/Source/IO/Net/AuNetSocketServer.cpp b/Source/IO/Net/AuNetSocketServer.cpp index b805c939..80763bec 100644 --- a/Source/IO/Net/AuNetSocketServer.cpp +++ b/Source/IO/Net/AuNetSocketServer.cpp @@ -174,13 +174,14 @@ namespace Aurora::IO::Net { } - void SocketServer::Shutdown() + void SocketServer::Shutdown(bool bNow) { + Socket::Shutdown(bNow); } void SocketServer::Destroy() { - + Socket::Destroy(); } void SocketServer::ScheduleAcceptTick() diff --git a/Source/IO/Net/AuNetSocketServer.hpp b/Source/IO/Net/AuNetSocketServer.hpp index a1b57db9..a1f673b9 100644 --- a/Source/IO/Net/AuNetSocketServer.hpp +++ b/Source/IO/Net/AuNetSocketServer.hpp @@ -35,7 +35,7 @@ namespace Aurora::IO::Net virtual void FinishConstructAsync() override; - virtual void Shutdown() override; + virtual void Shutdown(bool bNow) override; virtual void Destroy() override; void ScheduleAcceptTick(); diff --git a/Source/IO/Net/AuNetStream.NT.cpp b/Source/IO/Net/AuNetStream.NT.cpp index 07f79ec0..535dbbe3 100644 --- a/Source/IO/Net/AuNetStream.NT.cpp +++ b/Source/IO/Net/AuNetStream.NT.cpp @@ -59,6 +59,16 @@ namespace Aurora::IO::Net return false; } + if (!this->pSocket) + { + return false; + } + + if (this->pSocket->bHasEnded) + { + return false; + } + if (this->bIsIrredeemable) { SysPushErrorIO("Transaction was signaled to be destroyed to reset mid synchronizable operation. You can no longer use this stream object"); @@ -154,6 +164,16 @@ namespace Aurora::IO::Net return false; } + if (!this->pSocket) + { + return false; + } + + if (this->pSocket->bHasEnded) + { + return false; + } + if (!memoryView) { SysPushErrorArg(); diff --git a/Source/IO/TLS/TLSCertificateChain.cpp b/Source/IO/TLS/TLSCertificateChain.cpp index 6ecf5699..7cc89d61 100644 --- a/Source/IO/TLS/TLSCertificateChain.cpp +++ b/Source/IO/TLS/TLSCertificateChain.cpp @@ -7,6 +7,7 @@ ***/ #include "TLS.hpp" #include "TLSCertificateChain.hpp" +#include namespace Aurora::IO::TLS { @@ -22,11 +23,77 @@ namespace Aurora::IO::TLS AuUInt32 CertificateChain::GetCertificateCount() { - return 0; + AuUInt32 ret {}; + + auto pCert = this->pCertificate; + if (!pCert) + { + return {}; + } + do + { + auto index = ret++; + } + while (pCert = pCert->next); + + return ret; } - AuMemoryViewRead CertificateChain::GetCertificate(AuUInt32 idx) + AuSPtr CertificateChain::GetCertificate(AuUInt32 idx) { + AuUInt32 ret {}; + + auto pCert = this->pCertificate; + if (!pCert) + { + return {}; + } + do + { + auto index = ret++; + + if (index == idx) + { + struct View : AuMemoryViewRead + { + View(const AuMemoryViewRead &in, AuSPtr pin) : + AuMemoryViewRead(in), + pin(pin) + { } + + AuSPtr pin; + }; + + return AuMakeSharedThrow(AuMemoryViewRead { pCert->raw.p, pCert->raw.len}, AuSharedFromThis()); + } + } + while (pCert = pCert->next); + + return {}; + } + + Crypto::X509::DecodedCertificate CertificateChain::GetCertificateDetails(AuUInt32 idx) + { + AuUInt32 ret {}; + + auto pCert = this->pCertificate; + if (!pCert) + { + return {}; + } + do + { + auto index = ret++; + + if (index == idx) + { + Crypto::X509::DecodedCertificate cert; + AuCrypto::X509::DecodeInternal(*pCert, cert); + return cert; + } + } + while (pCert = pCert->next); + return {}; } @@ -44,6 +111,7 @@ namespace Aurora::IO::TLS cert.length); if (iRet != 0) { + this->pCertificate = nullptr; SysPushErrorCrypto("Failed to parse certificate chain: {}", iRet); return false; } @@ -66,6 +134,7 @@ namespace Aurora::IO::TLS cert.length); if (iRet != 0) { + this->pCertificate = nullptr; SysPushErrorCrypto("Failed to parse certificate chain: {}", iRet); return false; } @@ -95,7 +164,7 @@ namespace Aurora::IO::TLS bool CertificateChain::Init(const mbedtls_x509_crt *pCert) { - this->pCertificate = &this->ownCertificate; + this->pCertificate = (mbedtls_x509_crt *)pCert; return this->Precache(); } diff --git a/Source/IO/TLS/TLSCertificateChain.hpp b/Source/IO/TLS/TLSCertificateChain.hpp index 5becabd7..586b39e5 100644 --- a/Source/IO/TLS/TLSCertificateChain.hpp +++ b/Source/IO/TLS/TLSCertificateChain.hpp @@ -8,13 +8,14 @@ #pragma once namespace Aurora::IO::TLS { - struct CertificateChain : ICertificateChain + struct CertificateChain : ICertificateChain, AuEnableSharedFromThis { CertificateChain(); ~CertificateChain(); virtual AuUInt32 GetCertificateCount() override; - virtual AuMemoryViewRead GetCertificate(AuUInt32 idx) override; + virtual AuSPtr GetCertificate(AuUInt32 idx) override; + virtual Crypto::X509::DecodedCertificate GetCertificateDetails(AuUInt32 idx) override; bool Init(const AuList &certs); bool Init(const AuList &certs); diff --git a/Source/IO/TLS/TLSContext.cpp b/Source/IO/TLS/TLSContext.cpp index f61ab3ce..03dbdc2d 100644 --- a/Source/IO/TLS/TLSContext.cpp +++ b/Source/IO/TLS/TLSContext.cpp @@ -10,6 +10,8 @@ #include #include #include +#include +#include "TLSCertificateChain.hpp" namespace Aurora::IO::TLS { @@ -127,14 +129,31 @@ namespace Aurora::IO::TLS return uBytesRead; } - bool TLSContext::CheckCertificate(const AuMemoryViewRead &read) + bool TLSContext::CheckCertificate(mbedtls_x509_crt const *child, const AuMemoryViewRead &read) { if (!this->meta_.pCertPin) { return true; } - return this->meta_.pCertPin->CheckCertificate({}, read); + if (this->bPinLock_) + { + return true; + } + + auto pCertChain = AuMakeShared(); + if (!pCertChain) + { + SysPushErrorMemory(); + return false; + } + + pCertChain->Init(child); + + auto bRet = this->meta_.pCertPin->CheckCertificate(pCertChain, read); + this->bPinLock_ = true; + pCertChain->pCertificate = nullptr; + return bRet; } // @@ -204,7 +223,7 @@ namespace Aurora::IO::TLS mbedtls_x509_crt **candidate_cas) -> int { - return ((TLSContext *)p_ctx)->CheckCertificate({ child->raw.p, child->raw.len }) ? 0 : -1; + return ((TLSContext *)p_ctx)->CheckCertificate(child, { child->raw.p, child->raw.len }) ? 0 : -1; }, this); ::mbedtls_ssl_conf_rng(&this->conf, mbedtls_ctr_drbg_random, &gCtrDrbg); @@ -407,7 +426,7 @@ namespace Aurora::IO::TLS if (auto pSocket = this->wpSocket_.lock()) { - pSocket->Shutdown(); + pSocket->Shutdown(false); } AuResetMember(this->meta_); @@ -478,6 +497,8 @@ namespace Aurora::IO::TLS this->bIsFatal = false; this->iFatalError = 0; + this->bPinLock_ = false; + this->channelRecv_.HasCompletedHandshake() = false; if (::mbedtls_ssl_session_reset(&this->ssl) != 0) diff --git a/Source/IO/TLS/TLSContext.hpp b/Source/IO/TLS/TLSContext.hpp index 47574a61..c4d47b7b 100644 --- a/Source/IO/TLS/TLSContext.hpp +++ b/Source/IO/TLS/TLSContext.hpp @@ -60,6 +60,7 @@ namespace Aurora::IO::TLS bool bIsFatal {}; bool bIsAlive {}; + bool bPinLock_ {}; int iFatalError {}; mbedtls_ssl_context ssl {}; @@ -67,7 +68,7 @@ namespace Aurora::IO::TLS int Read(void *pOut, AuUInt length); int Write(const void *pIn, AuUInt length); - bool CheckCertificate(const AuMemoryViewRead &read); + bool CheckCertificate(mbedtls_x509_crt const *child, const AuMemoryViewRead &read); private: diff --git a/Source/IO/TLS/TLSProtocolRecv.cpp b/Source/IO/TLS/TLSProtocolRecv.cpp index 45013926..15a7ad7e 100644 --- a/Source/IO/TLS/TLSProtocolRecv.cpp +++ b/Source/IO/TLS/TLSProtocolRecv.cpp @@ -64,6 +64,7 @@ namespace Aurora::IO::TLS this->bHasCompletedHandshake_ = true; this->pParent_->bIsAlive = true; + this->pParent_->bPinLock_ = false; return true; } @@ -102,6 +103,12 @@ namespace Aurora::IO::TLS this->pParent_->OnClose(); return true; } + case MBEDTLS_ERR_X509_FATAL_ERROR: + { + this->bHasFailedOnce = true; + this->pParent_->OnClose(); + return false; + } case 0: { bComplete = true;