Bug 1364991 - Make U2FTokenManager use const where possible r=qdot
authorAxel Nennker <ignisvulpis@gmail.com>
Mon, 22 May 2017 16:40:29 -0700
changeset 360065 0300ea496f8b957061804a61662db34479aec6e9
parent 360064 8649b10a34c18f0e6c57106748750e0a67009d42
child 360066 1507721df493580f035695ad088890b5cbb78ba6
push id43205
push userihsiao@mozilla.com
push dateTue, 23 May 2017 07:37:31 +0000
treeherderautoland@0300ea496f8b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersqdot
bugs1364991
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1364991 - Make U2FTokenManager use const where possible r=qdot The U2F Soft Token, due to its usage of NSS, has to have const values be marked non-const - but no such limitation should exist for other implementations of U2F, so this patch moves the const_cast-ing from the U2FTokenManager-level down to the U2FSoftTokenManager, where it is actually necessary. Credit to Axel Nennker for this patch. MozReview-Commit-ID: Kw6zfTDI3GL
dom/webauthn/U2FSoftTokenManager.cpp
dom/webauthn/U2FSoftTokenManager.h
dom/webauthn/U2FTokenManager.cpp
dom/webauthn/U2FTokenManager.h
dom/webauthn/U2FTokenTransport.h
dom/webauthn/WebAuthnTransactionParent.cpp
--- a/dom/webauthn/U2FSoftTokenManager.cpp
+++ b/dom/webauthn/U2FSoftTokenManager.cpp
@@ -577,18 +577,18 @@ PrivateKeyFromKeyHandle(const UniquePK11
 bool
 U2FSoftTokenManager::IsCompatibleVersion(const nsAString& aVersion)
 {
   return mVersion == aVersion;
 }
 
 // IsRegistered determines if the provided key handle is usable by this token.
 nsresult
-U2FSoftTokenManager::IsRegistered(nsTArray<uint8_t>& aKeyHandle,
-                                  nsTArray<uint8_t>& aAppParam,
+U2FSoftTokenManager::IsRegistered(const nsTArray<uint8_t>& aKeyHandle,
+                                  const nsTArray<uint8_t>& aAppParam,
                                   bool& aResult)
 {
   nsNSSShutDownPreventionLock locker;
   if (NS_WARN_IF(isAlreadyShutDown())) {
     return NS_ERROR_FAILURE;
   }
 
   if (!mInitialized) {
@@ -598,19 +598,19 @@ U2FSoftTokenManager::IsRegistered(nsTArr
     }
   }
 
   UniquePK11SlotInfo slot(PK11_GetInternalSlot());
   MOZ_ASSERT(slot.get());
 
   // Decode the key handle
   UniqueSECKEYPrivateKey privKey = PrivateKeyFromKeyHandle(slot, mWrappingKey,
-                                                           aKeyHandle.Elements(),
+                                                           const_cast<uint8_t*>(aKeyHandle.Elements()),
                                                            aKeyHandle.Length(),
-                                                           aAppParam.Elements(),
+                                                           const_cast<uint8_t*>(aAppParam.Elements()),
                                                            aAppParam.Length(),
                                                            locker);
   aResult = privKey.get() != nullptr;
   return NS_OK;
 }
 
 // A U2F Register operation causes a new key pair to be generated by the token.
 // The token then returns the public key of the key pair, and a handle to the
@@ -627,18 +627,18 @@ U2FSoftTokenManager::IsRegistered(nsTArr
 // 1      0x05
 // 65     public key
 // 1      key handle length
 // *      key handle
 // ASN.1  attestation certificate
 // *      attestation signature
 //
 nsresult
-U2FSoftTokenManager::Register(nsTArray<uint8_t>& aApplication,
-                              nsTArray<uint8_t>& aChallenge,
+U2FSoftTokenManager::Register(const nsTArray<uint8_t>& aApplication,
+                              const nsTArray<uint8_t>& aChallenge,
                               /* out */ nsTArray<uint8_t>& aRegistration,
                               /* out */ nsTArray<uint8_t>& aSignature)
 {
   nsNSSShutDownPreventionLock locker;
   if (NS_WARN_IF(isAlreadyShutDown())) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
@@ -671,17 +671,17 @@ U2FSoftTokenManager::Register(nsTArray<u
   UniqueSECKEYPublicKey pubKey;
   rv = GenEcKeypair(slot, privKey, pubKey, locker);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return NS_ERROR_FAILURE;
   }
 
   // The key handle will be the result of keywrap(privKey, key=mWrappingKey)
   UniqueSECItem keyHandleItem = KeyHandleFromPrivateKey(slot, mWrappingKey,
-                                                        aApplication.Elements(),
+                                                        const_cast<uint8_t*>(aApplication.Elements()),
                                                         aApplication.Length(),
                                                         privKey, locker);
   if (NS_WARN_IF(!keyHandleItem.get())) {
     return NS_ERROR_FAILURE;
   }
 
   // Sign the challenge using the Attestation privkey (from attestCert)
   mozilla::dom::CryptoBuffer signedDataBuf;
@@ -739,19 +739,19 @@ U2FSoftTokenManager::Register(nsTArray<u
 //
 // The format of the signature data is as follows:
 //
 //  1     User presence
 //  4     Counter
 //  *     Signature
 //
 nsresult
-U2FSoftTokenManager::Sign(nsTArray<uint8_t>& aApplication,
-                          nsTArray<uint8_t>& aChallenge,
-                          nsTArray<uint8_t>& aKeyHandle,
+U2FSoftTokenManager::Sign(const nsTArray<uint8_t>& aApplication,
+                          const nsTArray<uint8_t>& aChallenge,
+                          const nsTArray<uint8_t>& aKeyHandle,
                           nsTArray<uint8_t>& aSignature)
 {
   nsNSSShutDownPreventionLock locker;
   if (NS_WARN_IF(isAlreadyShutDown())) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   MOZ_ASSERT(mInitialized);
@@ -772,19 +772,19 @@ U2FSoftTokenManager::Sign(nsTArray<uint8
             ("Parameter lengths are wrong! challenge=%d app=%d expected=%d",
              (uint32_t)aChallenge.Length(), (uint32_t)aApplication.Length(), kParamLen));
 
     return NS_ERROR_ILLEGAL_VALUE;
   }
 
   // Decode the key handle
   UniqueSECKEYPrivateKey privKey = PrivateKeyFromKeyHandle(slot, mWrappingKey,
-                                                           aKeyHandle.Elements(),
+                                                           const_cast<uint8_t*>(aKeyHandle.Elements()),
                                                            aKeyHandle.Length(),
-                                                           aApplication.Elements(),
+                                                           const_cast<uint8_t*>(aApplication.Elements()),
                                                            aApplication.Length(),
                                                            locker);
   if (NS_WARN_IF(!privKey.get())) {
     MOZ_LOG(gNSSTokenLog, LogLevel::Warning, ("Couldn't get the priv key!"));
     return NS_ERROR_FAILURE;
   }
 
   // Increment the counter and turn it into a SECItem
--- a/dom/webauthn/U2FSoftTokenManager.h
+++ b/dom/webauthn/U2FSoftTokenManager.h
@@ -19,26 +19,26 @@
 namespace mozilla {
 namespace dom {
 
 class U2FSoftTokenManager final : public U2FTokenTransport,
                                   public nsNSSShutDownObject
 {
 public:
   explicit U2FSoftTokenManager(uint32_t aCounter);
-  virtual nsresult Register(nsTArray<uint8_t>& aApplication,
-                            nsTArray<uint8_t>& aChallenge,
+  virtual nsresult Register(const nsTArray<uint8_t>& aApplication,
+                            const nsTArray<uint8_t>& aChallenge,
                             /* out */ nsTArray<uint8_t>& aRegistration,
                             /* out */ nsTArray<uint8_t>& aSignature) override;
-  virtual nsresult Sign(nsTArray<uint8_t>& aApplication,
-                        nsTArray<uint8_t>& aChallenge,
-                        nsTArray<uint8_t>& aKeyHandle,
+  virtual nsresult Sign(const nsTArray<uint8_t>& aApplication,
+                        const nsTArray<uint8_t>& aChallenge,
+                        const nsTArray<uint8_t>& aKeyHandle,
                         /* out */ nsTArray<uint8_t>& aSignature) override;
-  nsresult IsRegistered(nsTArray<uint8_t>& aKeyHandle,
-                        nsTArray<uint8_t>& aAppParam,
+  nsresult IsRegistered(const nsTArray<uint8_t>& aKeyHandle,
+                        const nsTArray<uint8_t>& aAppParam,
                         bool& aResult);
 
   // For nsNSSShutDownObject
   virtual void virtualDestroyNSSReference() override;
   void destructorSafeDestroyNSSReference();
 
 private:
   ~U2FSoftTokenManager();
--- a/dom/webauthn/U2FTokenManager.cpp
+++ b/dom/webauthn/U2FTokenManager.cpp
@@ -163,17 +163,17 @@ U2FTokenManager::Cancel(const nsresult& 
   if (mTransactionParent) {
     Unused << mTransactionParent->SendCancel(aError);
   }
   MaybeClearTransaction(mTransactionParent);
 }
 
 void
 U2FTokenManager::Register(WebAuthnTransactionParent* aTransactionParent,
-                          WebAuthnTransactionInfo& aTransactionInfo)
+                          const WebAuthnTransactionInfo& aTransactionInfo)
 {
   MOZ_LOG(gU2FTokenManagerLog, LogLevel::Debug, ("U2FAuthRegister"));
   MOZ_ASSERT(U2FPrefManager::Get());
   mTransactionParent = aTransactionParent;
 
   // Since we only have soft token available at the moment, use that if the pref
   // is on.
   //
@@ -224,17 +224,17 @@ U2FTokenManager::Register(WebAuthnTransa
     MaybeClearTransaction(mTransactionParent);
     return;
   }
   Cancel(NS_ERROR_DOM_NOT_ALLOWED_ERR);
 }
 
 void
 U2FTokenManager::Sign(WebAuthnTransactionParent* aTransactionParent,
-                      WebAuthnTransactionInfo& aTransactionInfo)
+                      const WebAuthnTransactionInfo& aTransactionInfo)
 {
   MOZ_LOG(gU2FTokenManagerLog, LogLevel::Debug, ("U2FAuthSign"));
   MOZ_ASSERT(U2FPrefManager::Get());
   mTransactionParent = aTransactionParent;
 
   // Since we only have soft token available at the moment, use that if the pref
   // is on.
   //
--- a/dom/webauthn/U2FTokenManager.h
+++ b/dom/webauthn/U2FTokenManager.h
@@ -40,19 +40,19 @@ public:
   {
     RegisterTransaction = 0,
     SignTransaction,
     NumTransactionTypes
   };
   NS_INLINE_DECL_REFCOUNTING(U2FTokenManager)
   static U2FTokenManager* Get();
   void Register(WebAuthnTransactionParent* aTransactionParent,
-                WebAuthnTransactionInfo& aTransactionInfo);
+                const WebAuthnTransactionInfo& aTransactionInfo);
   void Sign(WebAuthnTransactionParent* aTransactionParent,
-            WebAuthnTransactionInfo& aTransactionInfo);
+            const WebAuthnTransactionInfo& aTransactionInfo);
   void MaybeClearTransaction(WebAuthnTransactionParent* aParent);
   static void Initialize();
 private:
   U2FTokenManager();
   ~U2FTokenManager();
   void Cancel(const nsresult& aError);
   // Using a raw pointer here, as the lifetime of the IPC object is managed by
   // the PBackground protocol code. This means we cannot be left holding an
--- a/dom/webauthn/U2FTokenTransport.h
+++ b/dom/webauthn/U2FTokenTransport.h
@@ -16,23 +16,23 @@
 namespace mozilla {
 namespace dom {
 
 class U2FTokenTransport
 {
 public:
   NS_INLINE_DECL_REFCOUNTING(U2FTokenTransport);
   U2FTokenTransport() {}
-  virtual nsresult Register(nsTArray<uint8_t>& aApplication,
-                            nsTArray<uint8_t>& aChallenge,
+  virtual nsresult Register(const nsTArray<uint8_t>& aApplication,
+                            const nsTArray<uint8_t>& aChallenge,
                             /* out */ nsTArray<uint8_t>& aRegistration,
                             /* out */ nsTArray<uint8_t>& aSignature) = 0;
-  virtual nsresult Sign(nsTArray<uint8_t>& aApplication,
-                        nsTArray<uint8_t>& aChallenge,
-                        nsTArray<uint8_t>& aKeyHandle,
+  virtual nsresult Sign(const nsTArray<uint8_t>& aApplication,
+                        const nsTArray<uint8_t>& aChallenge,
+                        const nsTArray<uint8_t>& aKeyHandle,
                         /* out */ nsTArray<uint8_t>& aSignature) = 0;
 protected:
   virtual ~U2FTokenTransport() = default;
 };
 
 } // namespace dom
 } // namespace mozilla
 
--- a/dom/webauthn/WebAuthnTransactionParent.cpp
+++ b/dom/webauthn/WebAuthnTransactionParent.cpp
@@ -9,27 +9,25 @@
 
 namespace mozilla {
 namespace dom {
 
 mozilla::ipc::IPCResult
 WebAuthnTransactionParent::RecvRequestRegister(const WebAuthnTransactionInfo& aTransactionInfo)
 {
   U2FTokenManager* mgr = U2FTokenManager::Get();
-  // Cast away const here since NSS wants to be able to use non-const functions
-  mgr->Register(this, const_cast<WebAuthnTransactionInfo&>(aTransactionInfo));
+  mgr->Register(this, aTransactionInfo);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 WebAuthnTransactionParent::RecvRequestSign(const WebAuthnTransactionInfo& aTransactionInfo)
 {
   U2FTokenManager* mgr = U2FTokenManager::Get();
-  // Cast away const here since NSS wants to be able to use non-const functions
-  mgr->Sign(this, const_cast<WebAuthnTransactionInfo&>(aTransactionInfo));
+  mgr->Sign(this, aTransactionInfo);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 WebAuthnTransactionParent::RecvRequestCancel()
 {
   U2FTokenManager* mgr = U2FTokenManager::Get();
   mgr->MaybeClearTransaction(this);