Bug 1244960 - FIDO u2f NSSToken (Part 3): Review updates. r=keeler

MozReview-Commit-ID: FkPHy9GGarU

--HG--
extra : transplant_source : %8Dd0%BA%FD%E93%8E%A7%C5%01Cn%02%0F%0C%3E%5D%9C%96

dom/crypto/CryptoBuffer.cpp

@@ -192,16 +192,20 @@ CryptoBuffer::ToUint8Array(JSContext* aCx) const
 }
 
 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;
 }
 

dom/crypto/CryptoBuffer.h

@@ -47,7 +47,7 @@ public:
   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);
 };

dom/ipc/ContentParent.cpp

@@ -4248,76 +4248,86 @@ ContentParent::RecvSetURITitle(const URIParams& uri,
 }
 
 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);
 }

dom/ipc/ContentParent.h

@@ -749,20 +749,20 @@ private:
   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;

dom/u2f/U2F.cpp

@@ -96,12 +96,14 @@ U2F::Init(nsPIDOMWindowInner* aParent, ErrorResult& aRv)
 
   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;
     }
   }
@@ -113,55 +115,60 @@ U2F::Init(nsPIDOMWindowInner* aParent, ErrorResult& aRv)
 }
 
 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;
   }
@@ -169,32 +176,33 @@ U2F::NSSTokenRegister(CryptoBuffer& application, CryptoBuffer& challenge,
   nsTArray<uint8_t> registrationBuffer;
   ContentChild* cc = ContentChild::GetSingleton();
   MOZ_ASSERT(cc);
-  if (!cc->SendNSSU2FTokenRegister(application, challenge,
+  if (!cc->SendNSSU2FTokenRegister(aApplication, aChallenge,
                                    &registrationBuffer)) {
     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;
   }
@@ -202,12 +210,12 @@ U2F::NSSTokenSign(CryptoBuffer& keyHandle, CryptoBuffer& application,
   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;
 }
 
@@ -420,7 +428,7 @@ U2F::Register(const nsAString& aAppId,
 
       if (isCompatible && isRegistered) {
         SendError<U2FRegisterCallback, RegisterResponse>(aCallback,
-                                                  ErrorCode::DEVICE_INELIGIBLE);
+                                                         ErrorCode::DEVICE_INELIGIBLE);
         return;
       }
     }