Bug 1900132 - streamline PVerifySSLServerCert protocol r=jschanck

Previously the PVerifySSLServerCert protocol consisted of two functions: one to
call when certificate verification succeeded, and another to call upon failure.
This was unnecessary, as the code before and after this protocol didn't have
the same split. This patch unifies the protocol to better match the surrounding
code. It also takes the opportunity to make use of some IPC helpers to
serialize enums rather than manually casting to and from basic integer types.

Differential Revision: https://phabricator.services.mozilla.com/D212594
This commit is contained in:
Dana Keeler 2024-06-05 23:58:02 +00:00
Родитель c73ce972ef
Коммит 777066906a
7 изменённых файлов: 66 добавлений и 57 удалений

Просмотреть файл

@ -5,6 +5,7 @@
#ifndef mozilla_ipc_TransportSecurityInfoUtils_h
#define mozilla_ipc_TransportSecurityInfoUtils_h
#include "CertVerifier.h"
#include "ipc/EnumSerializer.h"
#include "mozilla/RefPtr.h"
#include "nsITransportSecurityInfo.h"
@ -38,6 +39,12 @@ struct ParamTraits<nsITransportSecurityInfo::OverridableErrorCategory>
nsITransportSecurityInfo::OverridableErrorCategory::ERROR_UNSET,
nsITransportSecurityInfo::OverridableErrorCategory::ERROR_TIME> {};
template <>
struct ParamTraits<mozilla::psm::EVStatus>
: public ContiguousEnumSerializerInclusive<mozilla::psm::EVStatus,
mozilla::psm::EVStatus::NotEV,
mozilla::psm::EVStatus::EV> {};
} // namespace IPC
#endif // mozilla_ipc_TransportSecurityInfoUtils_h

Просмотреть файл

@ -8,6 +8,8 @@
include PSMIPCTypes;
using mozilla::OriginAttributes from "mozilla/ipc/BackgroundUtils.h";
using mozilla::psm::EVStatus from "mozilla/ipc/TransportSecurityInfoUtils.h";
using nsITransportSecurityInfo::OverridableErrorCategory from "mozilla/ipc/TransportSecurityInfoUtils.h";
namespace mozilla {
namespace psm {
@ -16,13 +18,20 @@ namespace psm {
protocol PVerifySSLServerCert
{
child:
async OnVerifiedSSLServerCertSuccess(ByteArray[] aBuiltCertChain,
uint16_t aCertTransparencyStatus,
uint8_t aEVStatus,
bool isBuiltCertChainRootBuiltInRoot, bool aMadeOCSPRequests);
async OnVerifiedSSLServerCertFailure(int32_t aFinalError,
uint32_t aOverridableErrorCategory, bool aMadeOCSPRequests);
// This is not an incredibly intuitive order for these arguments, as
// `aFinalError` is probably the most salient argument (if it is 0, the
// connection will proceed, regardless of the value of, e.g. `aSucceeded`).
// However, this ordering matches the order of the related functions in
// SSLServerCertVerification.{h,cpp} (minus unnecessary arguments such as
// `aPeerCertChain`).
async OnVerifySSLServerCertFinished(ByteArray[] aBuiltCertChain,
uint16_t aCertTransparencyStatus,
EVStatus aEVStatus,
bool aSucceeded,
int32_t aFinalError,
OverridableErrorCategory aOverridableErrorCategory,
bool aIsBuiltCertChainRootBuiltInRoot,
bool aMadeOCSPRequests);
async __delete__();
};

Просмотреть файл

@ -1063,15 +1063,22 @@ void SSLServerCertVerificationResult::Dispatch(
mProviderFlags = aProviderFlags;
mMadeOCSPRequests = aMadeOCSPRequests;
if (mSucceeded && mBuiltChain.IsEmpty()) {
if (mSucceeded &&
(mBuiltChain.IsEmpty() || mFinalError != 0 ||
mOverridableErrorCategory !=
nsITransportSecurityInfo::OverridableErrorCategory::ERROR_UNSET)) {
MOZ_ASSERT_UNREACHABLE(
"if the handshake succeeded, the built chain shouldn't be empty");
"if certificate verification succeeded without overridden errors, the "
"built chain shouldn't be empty and any error bits should be unset");
mSucceeded = false;
mFinalError = SEC_ERROR_LIBRARY_FAILURE;
}
// Note that mSucceeded can be false while mFinalError is 0, in which case
// the connection will proceed.
if (!mSucceeded && mPeerCertChain.IsEmpty()) {
MOZ_ASSERT_UNREACHABLE(
"if the handshake failed, the peer chain shouldn't be empty");
"if certificate verification failed, the peer chain shouldn't be "
"empty");
mFinalError = SEC_ERROR_LIBRARY_FAILURE;
}

Просмотреть файл

@ -29,13 +29,16 @@ VerifySSLServerCertChild::VerifySSLServerCertChild(
mPeerCertChain(std::move(aPeerCertChain)),
mProviderFlags(aProviderFlags) {}
ipc::IPCResult VerifySSLServerCertChild::RecvOnVerifiedSSLServerCertSuccess(
ipc::IPCResult VerifySSLServerCertChild::RecvOnVerifySSLServerCertFinished(
nsTArray<ByteArray>&& aBuiltCertChain,
const uint16_t& aCertTransparencyStatus, const uint8_t& aEVStatus,
const uint16_t& aCertTransparencyStatus, const EVStatus& aEVStatus,
const bool& aSucceeded, int32_t aFinalError,
const nsITransportSecurityInfo::OverridableErrorCategory&
aOverridableErrorCategory,
const bool& aIsBuiltCertChainRootBuiltInRoot,
const bool& aMadeOCSPRequests) {
MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
("[%p] VerifySSLServerCertChild::RecvOnVerifiedSSLServerCertSuccess",
("[%p] VerifySSLServerCertChild::RecvOnVerifySSLServerCertFinished",
this));
nsTArray<nsTArray<uint8_t>> certBytesArray;
@ -43,24 +46,11 @@ ipc::IPCResult VerifySSLServerCertChild::RecvOnVerifiedSSLServerCertSuccess(
certBytesArray.AppendElement(std::move(cert.data()));
}
mResultTask->Dispatch(
std::move(certBytesArray), std::move(mPeerCertChain),
aCertTransparencyStatus, static_cast<EVStatus>(aEVStatus), true, 0,
nsITransportSecurityInfo::OverridableErrorCategory::ERROR_UNSET,
aIsBuiltCertChainRootBuiltInRoot, mProviderFlags, aMadeOCSPRequests);
return IPC_OK();
}
ipc::IPCResult VerifySSLServerCertChild::RecvOnVerifiedSSLServerCertFailure(
const int32_t& aFinalError, const uint32_t& aOverridableErrorCategory,
const bool& aMadeOCSPRequests) {
mResultTask->Dispatch(
nsTArray<nsTArray<uint8_t>>(), std::move(mPeerCertChain),
nsITransportSecurityInfo::CERTIFICATE_TRANSPARENCY_NOT_APPLICABLE,
EVStatus::NotEV, false, aFinalError,
static_cast<nsITransportSecurityInfo::OverridableErrorCategory>(
aOverridableErrorCategory),
false, mProviderFlags, aMadeOCSPRequests);
mResultTask->Dispatch(std::move(certBytesArray), std::move(mPeerCertChain),
aCertTransparencyStatus, aEVStatus, aSucceeded,
aFinalError, aOverridableErrorCategory,
aIsBuiltCertChainRootBuiltInRoot, mProviderFlags,
aMadeOCSPRequests);
return IPC_OK();
}

Просмотреть файл

@ -30,16 +30,15 @@ class VerifySSLServerCertChild : public PVerifySSLServerCertChild {
SSLServerCertVerificationResult* aResultTask,
nsTArray<nsTArray<uint8_t>>&& aPeerCertChain, uint32_t aProviderFlags);
ipc::IPCResult RecvOnVerifiedSSLServerCertSuccess(
ipc::IPCResult RecvOnVerifySSLServerCertFinished(
nsTArray<ByteArray>&& aBuiltCertChain,
const uint16_t& aCertTransparencyStatus, const uint8_t& aEVStatus,
const uint16_t& aCertTransparencyStatus, const EVStatus& aEVStatus,
const bool& aSucceeded, int32_t aFinalError,
const nsITransportSecurityInfo::OverridableErrorCategory&
aOverridableErrorCategory,
const bool& aIsBuiltCertChainRootBuiltInRoot,
const bool& aMadeOCSPRequests);
ipc::IPCResult RecvOnVerifiedSSLServerCertFailure(
const int32_t& aFinalError, const uint32_t& aOverridableErrorCategory,
const bool& aMadeOCSPRequests);
private:
~VerifySSLServerCertChild() = default;

Просмотреть файл

@ -28,21 +28,19 @@ VerifySSLServerCertParent::VerifySSLServerCertParent() {}
void VerifySSLServerCertParent::OnVerifiedSSLServerCert(
const nsTArray<ByteArray>& aBuiltCertChain,
uint16_t aCertificateTransparencyStatus, uint8_t aEVStatus, bool aSucceeded,
PRErrorCode aFinalError, uint32_t aOverridableErrorCategory,
uint16_t aCertificateTransparencyStatus, EVStatus aEVStatus,
bool aSucceeded, PRErrorCode aFinalError,
nsITransportSecurityInfo::OverridableErrorCategory
aOverridableErrorCategory,
bool aIsBuiltCertChainRootBuiltInRoot, bool aMadeOCSPRequests) {
if (!CanSend()) {
return;
}
if (aSucceeded) {
Unused << SendOnVerifiedSSLServerCertSuccess(
aBuiltCertChain, aCertificateTransparencyStatus, aEVStatus,
aIsBuiltCertChainRootBuiltInRoot, aMadeOCSPRequests);
} else {
Unused << SendOnVerifiedSSLServerCertFailure(
aFinalError, aOverridableErrorCategory, aMadeOCSPRequests);
}
Unused << SendOnVerifySSLServerCertFinished(
aBuiltCertChain, aCertificateTransparencyStatus, aEVStatus, aSucceeded,
aFinalError, aOverridableErrorCategory, aIsBuiltCertChainRootBuiltInRoot,
aMadeOCSPRequests);
Close();
}
@ -109,9 +107,8 @@ void IPCServerCertVerificationResult::Dispatch(
SaveIntermediateCerts(certBytesArray);
}
parent->OnVerifiedSSLServerCert(
builtCertChain, aCertificateTransparencyStatus,
static_cast<uint8_t>(aEVStatus), aSucceeded, aFinalError,
static_cast<uint32_t>(aOverridableErrorCategory),
builtCertChain, aCertificateTransparencyStatus, aEVStatus,
aSucceeded, aFinalError, aOverridableErrorCategory,
aIsBuiltCertChainRootBuiltInRoot, aMadeOCSPRequests);
}),
NS_DISPATCH_NORMAL);

Просмотреть файл

@ -37,13 +37,13 @@ class VerifySSLServerCertParent : public PVerifySSLServerCertParent {
const uint32_t& aProviderFlags,
const uint32_t& aCertVerifierFlags);
void OnVerifiedSSLServerCert(const nsTArray<ByteArray>& aBuiltCertChain,
uint16_t aCertificateTransparencyStatus,
uint8_t aEVStatus, bool aSucceeded,
PRErrorCode aFinalError,
uint32_t aOverridableErrorCategory,
bool aIsBuiltCertChainRootBuiltInRoot,
bool aMadeOCSPRequests);
void OnVerifiedSSLServerCert(
const nsTArray<ByteArray>& aBuiltCertChain,
uint16_t aCertificateTransparencyStatus, EVStatus aEVStatus,
bool aSucceeded, PRErrorCode aFinalError,
nsITransportSecurityInfo::OverridableErrorCategory
aOverridableErrorCategory,
bool aIsBuiltCertChainRootBuiltInRoot, bool aMadeOCSPRequests);
private:
virtual ~VerifySSLServerCertParent();