Bug 1244960 - FIDO u2f NSSToken (Part 3): Review updates r?keeler
MozReview-Commit-ID: FkPHy9GGarU
--- a/dom/crypto/CryptoBuffer.cpp
+++ b/dom/crypto/CryptoBuffer.cpp
@@ -187,26 +187,30 @@ CryptoBuffer::ToSECItem(PLArenaPool *aAr
JSObject*
CryptoBuffer::ToUint8Array(JSContext* aCx) const
{
return Uint8Array::Create(aCx, Length(), Elements());
}
bool
-CryptoBuffer::ToNewUnsignedBuffer(uint8_t** buf, uint32_t* bufLen) const
+CryptoBuffer::ToNewUnsignedBuffer(uint8_t** aBuf, uint32_t* aBufLen) const
{
- uint8_t* tmp = reinterpret_cast<uint8_t*>(moz_xmalloc(Length()));
- if (!tmp) {
+ MOZ_ASSERT(aBuf);
+ MOZ_ASSERT(aBufLen);
+
+ uint32_t dataLen = Length();
+ uint8_t* tmp = reinterpret_cast<uint8_t*>(moz_xmalloc(dataLen));
+ if (NS_WARN_IF(!tmp)) {
return false;
}
- memcpy(tmp, Elements(), Length());
- *buf = tmp;
- *bufLen = Length();
+ memcpy(tmp, Elements(), dataLen);
+ *aBuf = tmp;
+ *aBufLen = dataLen;
return true;
}
// "BigInt" comes from the WebCrypto spec
// ("unsigned long" isn't very "big", of course)
// Likewise, the spec calls for big-endian ints
bool
CryptoBuffer::GetBigIntValue(unsigned long& aRetVal)
--- a/dom/crypto/CryptoBuffer.h
+++ b/dom/crypto/CryptoBuffer.h
@@ -42,17 +42,17 @@ public:
aArray.ComputeLengthAndData();
return Assign(aArray.Data(), aArray.Length());
}
nsresult FromJwkBase64(const nsString& aBase64);
nsresult ToJwkBase64(nsString& aBase64);
bool ToSECItem(PLArenaPool* aArena, SECItem* aItem) const;
JSObject* ToUint8Array(JSContext* aCx) const;
- bool ToNewUnsignedBuffer(uint8_t** buf, uint32_t* bufLen) const;
+ bool ToNewUnsignedBuffer(uint8_t** aBuf, uint32_t* aBufLen) const;
bool GetBigIntValue(unsigned long& aRetVal);
};
} // namespace dom
} // namespace mozilla
#endif // mozilla_dom_CryptoBuffer_h
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -4243,86 +4243,96 @@ ContentParent::RecvSetURITitle(const URI
nsCOMPtr<IHistory> history = services::GetHistoryService();
if (history) {
history->SetURITitle(ourURI, title);
}
return true;
}
bool
-ContentParent::RecvNSSU2FTokenIsCompatibleVersion(const nsString& version,
- bool* isCompatible)
-{
+ContentParent::RecvNSSU2FTokenIsCompatibleVersion(const nsString& aVersion,
+ bool* aIsCompatible)
+{
+ MOZ_ASSERT(aIsCompatible);
+
nsCOMPtr<nsINSSU2FToken> nssToken(do_GetService(NS_NSSU2FTOKEN_CONTRACTID));
if (NS_WARN_IF(!nssToken)) {
return false;
}
- nsresult rv = nssToken->IsCompatibleVersion(version, isCompatible);
+ nsresult rv = nssToken->IsCompatibleVersion(aVersion, aIsCompatible);
return NS_SUCCEEDED(rv);
}
bool
-ContentParent::RecvNSSU2FTokenIsRegistered(nsTArray<uint8_t>&& keyHandle,
- bool* isValidKeyHandle)
-{
+ContentParent::RecvNSSU2FTokenIsRegistered(nsTArray<uint8_t>&& aKeyHandle,
+ bool* aIsValidKeyHandle)
+{
+ MOZ_ASSERT(aIsValidKeyHandle);
+
nsCOMPtr<nsINSSU2FToken> nssToken(do_GetService(NS_NSSU2FTOKEN_CONTRACTID));
if (NS_WARN_IF(!nssToken)) {
return false;
}
- nsresult rv = nssToken->IsRegistered(keyHandle.Elements(), keyHandle.Length(),
- isValidKeyHandle);
+ nsresult rv = nssToken->IsRegistered(aKeyHandle.Elements(), aKeyHandle.Length(),
+ aIsValidKeyHandle);
return NS_SUCCEEDED(rv);
}
bool
-ContentParent::RecvNSSU2FTokenRegister(nsTArray<uint8_t>&& application,
- nsTArray<uint8_t>&& challenge,
- nsTArray<uint8_t>* registration)
-{
+ContentParent::RecvNSSU2FTokenRegister(nsTArray<uint8_t>&& aApplication,
+ nsTArray<uint8_t>&& aChallenge,
+ nsTArray<uint8_t>* aRegistration)
+{
+ MOZ_ASSERT(aRegistration);
+
nsCOMPtr<nsINSSU2FToken> nssToken(do_GetService(NS_NSSU2FTOKEN_CONTRACTID));
if (NS_WARN_IF(!nssToken)) {
return false;
}
uint8_t* buffer;
uint32_t bufferlen;
- nsresult rv = nssToken->Register(application.Elements(), application.Length(),
- challenge.Elements(), challenge.Length(),
+ nsresult rv = nssToken->Register(aApplication.Elements(), aApplication.Length(),
+ aChallenge.Elements(), aChallenge.Length(),
&buffer, &bufferlen);
if (NS_WARN_IF(NS_FAILED(rv))) {
return false;
}
- registration->ReplaceElementsAt(0, registration->Length(), buffer, bufferlen);
+ MOZ_ASSERT(buffer);
+ aRegistration->ReplaceElementsAt(0, aRegistration->Length(), buffer, bufferlen);
free(buffer);
return NS_SUCCEEDED(rv);
}
bool
-ContentParent::RecvNSSU2FTokenSign(nsTArray<uint8_t>&& application,
- nsTArray<uint8_t>&& challenge,
- nsTArray<uint8_t>&& keyHandle,
- nsTArray<uint8_t>* signature)
-{
+ContentParent::RecvNSSU2FTokenSign(nsTArray<uint8_t>&& aApplication,
+ nsTArray<uint8_t>&& aChallenge,
+ nsTArray<uint8_t>&& aKeyHandle,
+ nsTArray<uint8_t>* aSignature)
+{
+ MOZ_ASSERT(aSignature);
+
nsCOMPtr<nsINSSU2FToken> nssToken(do_GetService(NS_NSSU2FTOKEN_CONTRACTID));
if (NS_WARN_IF(!nssToken)) {
return false;
}
uint8_t* buffer;
uint32_t bufferlen;
- nsresult rv = nssToken->Sign(application.Elements(), application.Length(),
- challenge.Elements(), challenge.Length(),
- keyHandle.Elements(), keyHandle.Length(),
+ nsresult rv = nssToken->Sign(aApplication.Elements(), aApplication.Length(),
+ aChallenge.Elements(), aChallenge.Length(),
+ aKeyHandle.Elements(), aKeyHandle.Length(),
&buffer, &bufferlen);
if (NS_WARN_IF(NS_FAILED(rv))) {
return false;
}
- signature->ReplaceElementsAt(0, signature->Length(), buffer, bufferlen);
+ MOZ_ASSERT(buffer);
+ aSignature->ReplaceElementsAt(0, aSignature->Length(), buffer, bufferlen);
free(buffer);
return NS_SUCCEEDED(rv);
}
bool
ContentParent::RecvGetSystemMemory(const uint64_t& aGetterId)
{
uint32_t memoryTotal = 0;
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@@ -744,30 +744,30 @@ private:
virtual bool
RecvPBlobConstructor(PBlobParent* aActor,
const BlobConstructorParams& params) override;
virtual bool
DeallocPCrashReporterParent(PCrashReporterParent* crashreporter) override;
- virtual bool RecvNSSU2FTokenIsCompatibleVersion(const nsString& version,
- bool* isCompatible) override;
+ virtual bool RecvNSSU2FTokenIsCompatibleVersion(const nsString& aVersion,
+ bool* aIsCompatible) override;
- virtual bool RecvNSSU2FTokenIsRegistered(nsTArray<uint8_t>&& keyHandle,
- bool* isValidKeyHandle) override;
+ virtual bool RecvNSSU2FTokenIsRegistered(nsTArray<uint8_t>&& aKeyHandle,
+ bool* aIsValidKeyHandle) override;
- virtual bool RecvNSSU2FTokenRegister(nsTArray<uint8_t>&& application,
- nsTArray<uint8_t>&& challenge,
- nsTArray<uint8_t>* registration) override;
+ virtual bool RecvNSSU2FTokenRegister(nsTArray<uint8_t>&& aApplication,
+ nsTArray<uint8_t>&& aChallenge,
+ nsTArray<uint8_t>* aRegistration) override;
- virtual bool RecvNSSU2FTokenSign(nsTArray<uint8_t>&& application,
- nsTArray<uint8_t>&& challenge,
- nsTArray<uint8_t>&& keyHandle,
- nsTArray<uint8_t>* signature) override;
+ virtual bool RecvNSSU2FTokenSign(nsTArray<uint8_t>&& aApplication,
+ nsTArray<uint8_t>&& aChallenge,
+ nsTArray<uint8_t>&& aKeyHandle,
+ nsTArray<uint8_t>* aSignature) override;
virtual bool RecvIsSecureURI(const uint32_t& aType, const URIParams& aURI,
const uint32_t& aFlags, bool* aIsSecureURI) override;
virtual bool RecvAccumulateMixedContentHSTS(const URIParams& aURI,
const bool& aActive) override;
virtual bool DeallocPHalParent(PHalParent*) override;
--- a/dom/u2f/U2F.cpp
+++ b/dom/u2f/U2F.cpp
@@ -91,128 +91,136 @@ U2F::Init(nsPIDOMWindowInner* aParent, E
}
if (NS_WARN_IF(mOrigin.IsEmpty())) {
return;
}
if (!EnsureNSSInitializedChromeOrContent()) {
MOZ_LOG(gU2FLog, LogLevel::Debug, ("Failed to get NSS context for U2F"));
+ aRv.Throw(NS_ERROR_FAILURE);
return;
}
if (XRE_IsParentProcess()) {
mNSSToken = do_GetService(NS_NSSU2FTOKEN_CONTRACTID);
if (NS_WARN_IF(!mNSSToken)) {
+ aRv.Throw(NS_ERROR_FAILURE);
return;
}
}
aRv = mUSBToken.Init();
if (NS_WARN_IF(aRv.Failed())) {
return;
}
}
nsresult
-U2F::NSSTokenIsCompatible(const nsString& versionString, bool* isCompatible)
+U2F::NSSTokenIsCompatible(const nsString& aVersionString, bool* aIsCompatible)
{
+ MOZ_ASSERT(aIsCompatible);
+
if (XRE_IsParentProcess()) {
MOZ_ASSERT(mNSSToken);
- return mNSSToken->IsCompatibleVersion(versionString, isCompatible);
+ return mNSSToken->IsCompatibleVersion(aVersionString, aIsCompatible);
}
ContentChild* cc = ContentChild::GetSingleton();
MOZ_ASSERT(cc);
- if (!cc->SendNSSU2FTokenIsCompatibleVersion(versionString, isCompatible)) {
+ if (!cc->SendNSSU2FTokenIsCompatibleVersion(aVersionString, aIsCompatible)) {
return NS_ERROR_FAILURE;
}
return NS_OK;
}
nsresult
-U2F::NSSTokenIsRegistered(CryptoBuffer& keyHandle, bool* isRegistered)
+U2F::NSSTokenIsRegistered(CryptoBuffer& aKeyHandle, bool* aIsRegistered)
{
+ MOZ_ASSERT(aIsRegistered);
+
if (XRE_IsParentProcess()) {
MOZ_ASSERT(mNSSToken);
- return mNSSToken->IsRegistered(keyHandle.Elements(), keyHandle.Length(),
- isRegistered);
+ return mNSSToken->IsRegistered(aKeyHandle.Elements(), aKeyHandle.Length(),
+ aIsRegistered);
}
ContentChild* cc = ContentChild::GetSingleton();
MOZ_ASSERT(cc);
- if (!cc->SendNSSU2FTokenIsRegistered(keyHandle, isRegistered)) {
+ if (!cc->SendNSSU2FTokenIsRegistered(aKeyHandle, aIsRegistered)) {
return NS_ERROR_FAILURE;
}
return NS_OK;
}
nsresult
-U2F::NSSTokenRegister(CryptoBuffer& application, CryptoBuffer& challenge,
- CryptoBuffer& registrationData)
+U2F::NSSTokenRegister(CryptoBuffer& aApplication, CryptoBuffer& aChallenge,
+ CryptoBuffer& aRegistrationData)
{
if (XRE_IsParentProcess()) {
MOZ_ASSERT(mNSSToken);
uint8_t* buffer;
uint32_t bufferlen;
nsresult rv;
- rv = mNSSToken->Register(application.Elements(), application.Length(),
- challenge.Elements(), challenge.Length(),
+ rv = mNSSToken->Register(aApplication.Elements(), aApplication.Length(),
+ aChallenge.Elements(), aChallenge.Length(),
&buffer, &bufferlen);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
- registrationData.Assign(buffer, bufferlen);
+ MOZ_ASSERT(buffer);
+ aRegistrationData.Assign(buffer, bufferlen);
free(buffer);
return NS_OK;
}
nsTArray<uint8_t> registrationBuffer;
ContentChild* cc = ContentChild::GetSingleton();
MOZ_ASSERT(cc);
- if (!cc->SendNSSU2FTokenRegister(application, challenge,
+ if (!cc->SendNSSU2FTokenRegister(aApplication, aChallenge,
®istrationBuffer)) {
return NS_ERROR_FAILURE;
}
- registrationData.Assign(registrationBuffer);
+ aRegistrationData.Assign(registrationBuffer);
return NS_OK;
}
nsresult
-U2F::NSSTokenSign(CryptoBuffer& keyHandle, CryptoBuffer& application,
- CryptoBuffer& challenge, CryptoBuffer& signatureData)
+U2F::NSSTokenSign(CryptoBuffer& aKeyHandle, CryptoBuffer& aApplication,
+ CryptoBuffer& aChallenge, CryptoBuffer& aSignatureData)
{
if (XRE_IsParentProcess()) {
MOZ_ASSERT(mNSSToken);
uint8_t* buffer;
uint32_t bufferlen;
- nsresult rv = mNSSToken->Sign(application.Elements(), application.Length(),
- challenge.Elements(), challenge.Length(),
- keyHandle.Elements(), keyHandle.Length(),
+ nsresult rv = mNSSToken->Sign(aApplication.Elements(), aApplication.Length(),
+ aChallenge.Elements(), aChallenge.Length(),
+ aKeyHandle.Elements(), aKeyHandle.Length(),
&buffer, &bufferlen);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
- signatureData.Assign(buffer, bufferlen);
+ MOZ_ASSERT(buffer);
+ aSignatureData.Assign(buffer, bufferlen);
free(buffer);
return NS_OK;
}
nsTArray<uint8_t> signatureBuffer;
ContentChild* cc = ContentChild::GetSingleton();
MOZ_ASSERT(cc);
- if (!cc->SendNSSU2FTokenSign(application, challenge, keyHandle,
+ if (!cc->SendNSSU2FTokenSign(aApplication, aChallenge, aKeyHandle,
&signatureBuffer)) {
return NS_ERROR_FAILURE;
}
- signatureData.Assign(signatureBuffer);
+ aSignatureData.Assign(signatureBuffer);
return NS_OK;
}
nsresult
U2F::AssembleClientData(const nsAString& aTyp,
const nsAString& aChallenge,
CryptoBuffer& aClientData) const
{
@@ -415,17 +423,17 @@ U2F::Register(const nsAString& aAppId,
if (NS_FAILED(rv)) {
SendError<U2FRegisterCallback, RegisterResponse>(aCallback,
ErrorCode::OTHER_ERROR);
return;
}
if (isCompatible && isRegistered) {
SendError<U2FRegisterCallback, RegisterResponse>(aCallback,
- ErrorCode::DEVICE_INELIGIBLE);
+ ErrorCode::DEVICE_INELIGIBLE);
return;
}
}
}
// Search the requests in order for the first some token can fulfill
for (size_t i = 0; i < aRegisterRequests.Length(); ++i) {
RegisterRequest request(aRegisterRequests[i]);