Bug 991993: Disable NSS for updater on OSX and enable native APIs. r=smichaud,rstrong
authorStephen Pohl <spohl.mozilla.bugs@gmail.com>
Wed, 22 Oct 2014 21:52:54 -0400 (2014-10-23)
changeset 237694 71c747f84d03a3f7e50e349f1848d9641b2029ae
parent 237693 7adb8fc053b5a2fe9ebf2d1b56890765567e35c3
child 237695 a0d5f4706bd24be060f97c3d88d72c7d842eb174
push id28546
push usernetzen@gmail.com
push dateMon, 06 Apr 2015 16:08:39 +0000 (2015-04-06)
treeherdermozilla-central@883e17fc475f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmichaud, rstrong
bugs991993
milestone40.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 991993: Disable NSS for updater on OSX and enable native APIs. r=smichaud,rstrong
modules/libmar/verify/MacVerifyCrypto.cpp
modules/libmar/verify/cryptox.h
toolkit/mozapps/update/updater/archivereader.cpp
toolkit/mozapps/update/updater/moz.build
toolkit/mozapps/update/updater/updater.cpp
--- a/modules/libmar/verify/MacVerifyCrypto.cpp
+++ b/modules/libmar/verify/MacVerifyCrypto.cpp
@@ -207,19 +207,20 @@ CryptoMac_VerifyUpdate(CryptoX_Signature
   }
 
   CFDataAppendBytes(inputData, (const uint8*)aBuf, aLen);
   return CryptoX_Success;
 }
 
 CryptoX_Result
 CryptoMac_LoadPublicKey(const unsigned char* aCertData,
+                        unsigned int aDataSize,
                         CryptoX_PublicKey* aPublicKey)
 {
-  if (!aCertData || !aPublicKey) {
+  if (!aCertData || aDataSize == 0 || !aPublicKey) {
     return CryptoX_Error;
   }
   *aPublicKey = NULL;
 
   if (!OnLionOrLater()) {
     if (!sCspHandle) {
       CSSM_RETURN rv;
       if (!sCssmInitialized) {
@@ -256,131 +257,40 @@ CryptoMac_LoadPublicKey(const unsigned c
                              0,
                              NULL,
                              &cspHandle);
       if (rv != CSSM_OK) {
         return CryptoX_Error;
       }
       sCspHandle = cspHandle;
     }
-
-    FILE* certFile = NULL;
-    long certFileSize = 0;
-    uint8* certBuffer = NULL;
-
-    certFile = fopen((char*)aCertData, "rb");
-    if (!certFile) {
-      return CryptoX_Error;
-    }
-    if (fseek(certFile, 0, SEEK_END)) {
-      fclose(certFile);
-      return CryptoX_Error;
-    }
-    certFileSize = ftell(certFile);
-    if (certFileSize < 0) {
-      fclose(certFile);
-      return CryptoX_Error;
-    }
-    certBuffer = (uint8*)malloc(certFileSize);
-    if (fseek(certFile, 0, SEEK_SET)) {
-      free(certBuffer);
-      fclose(certFile);
-      return CryptoX_Error;
-    }
-    uint readResult = fread(certBuffer, sizeof(uint8), certFileSize, certFile);
-    if (readResult != certFileSize) {
-      free(certBuffer);
-      fclose(certFile);
-      return CryptoX_Error;
-    }
-    fclose(certFile);
-
-    CFDataRef certData = CFDataCreate(kCFAllocatorDefault,
-                                      certBuffer,
-                                      certFileSize);
-    free(certBuffer);
-    if (!certData) {
-      return CryptoX_Error;
-    }
-
-    SecCertificateRef cert = SecCertificateCreateWithData(kCFAllocatorDefault,
-                                                          certData);
-    CFRelease(certData);
-    if (!cert) {
-      return CryptoX_Error;
-    }
-
-    SecKeyRef publicKey;
-    OSStatus status = SecCertificateCopyPublicKey(cert, (SecKeyRef*)&publicKey);
-    CFRelease(cert);
-    if (status) {
-      return CryptoX_Error;
-    }
-
-    *aPublicKey = (void*)publicKey;
-    return CryptoX_Success;
   }
 
-  CFURLRef url =
-    CFURLCreateFromFileSystemRepresentation(kCFAllocatorDefault,
-                                            aCertData,
-                                            strlen((char*)aCertData),
-                                            false);
-  if (!url) {
-    return CryptoX_Error;
-  }
-
-  CFReadStreamRef stream = CFReadStreamCreateWithFile(kCFAllocatorDefault, url);
-  if (!stream) {
-    CFRelease(url);
-    return CryptoX_Error;
-  }
-
-  SecTransformRef readTransform =
-    SecTransformCreateReadTransformWithReadStreamPtr(stream);
-  if (!readTransform) {
-    CFRelease(url);
-    CFRelease(stream);
-    return CryptoX_Error;
-  }
-
-  CFErrorRef error;
-  CFDataRef tempCertData = (CFDataRef)SecTransformExecutePtr(readTransform,
-                                                             &error);
-  if (!tempCertData || error) {
-    CFRelease(url);
-    CFRelease(stream);
-    CFRelease(readTransform);
+  CFDataRef certData = CFDataCreate(kCFAllocatorDefault,
+                                    aCertData,
+                                    aDataSize);
+  if (!certData) {
     return CryptoX_Error;
   }
 
   SecCertificateRef cert = SecCertificateCreateWithData(kCFAllocatorDefault,
-                                                        tempCertData);
+                                                        certData);
+  CFRelease(certData);
   if (!cert) {
-    CFRelease(url);
-    CFRelease(stream);
-    CFRelease(readTransform);
-    CFRelease(tempCertData);
     return CryptoX_Error;
   }
 
-  CryptoX_Result result = CryptoX_Error;
   OSStatus status = SecCertificateCopyPublicKey(cert,
                                                 (SecKeyRef*)aPublicKey);
-  if (status == 0) {
-    result = CryptoX_Success;
+  CFRelease(cert);
+  if (status != 0) {
+    return CryptoX_Error;
   }
 
-  CFRelease(url);
-  CFRelease(stream);
-  CFRelease(readTransform);
-  CFRelease(tempCertData);
-  CFRelease(cert);
-
-  return result;
+  return CryptoX_Success;
 }
 
 CryptoX_Result
 CryptoMac_VerifySignature(CryptoX_SignatureHandle* aInputData,
                           CryptoX_PublicKey* aPublicKey,
                           const unsigned char* aSignature,
                           unsigned int aSignatureLen)
 {
--- a/modules/libmar/verify/cryptox.h
+++ b/modules/libmar/verify/cryptox.h
@@ -69,16 +69,17 @@ CryptoX_Result NSS_VerifySignature(VFYCo
 #ifdef __cplusplus
 extern "C" {
 #endif
 CryptoX_Result CryptoMac_InitCryptoProvider();
 CryptoX_Result CryptoMac_VerifyBegin(CryptoX_SignatureHandle* aInputData);
 CryptoX_Result CryptoMac_VerifyUpdate(CryptoX_SignatureHandle* aInputData,
                                       void* aBuf, unsigned int aLen);
 CryptoX_Result CryptoMac_LoadPublicKey(const unsigned char* aCertData,
+                                       unsigned int aDataSize,
                                        CryptoX_PublicKey* aPublicKey);
 CryptoX_Result CryptoMac_VerifySignature(CryptoX_SignatureHandle* aInputData,
                                          CryptoX_PublicKey* aPublicKey,
                                          const unsigned char* aSignature,
                                          unsigned int aSignatureLen);
 void CryptoMac_FreeSignatureHandle(CryptoX_SignatureHandle* aInputData);
 void CryptoMac_FreePublicKey(CryptoX_PublicKey* aPublicKey);
 #ifdef __cplusplus
@@ -88,17 +89,17 @@ void CryptoMac_FreePublicKey(CryptoX_Pub
 #define CryptoX_InitCryptoProvider(aProviderHandle) \
   CryptoMac_InitCryptoProvider()
 #define CryptoX_VerifyBegin(aCryptoHandle, aInputData, aPublicKey) \
   CryptoMac_VerifyBegin(aInputData)
 #define CryptoX_VerifyUpdate(aInputData, aBuf, aLen) \
   CryptoMac_VerifyUpdate(aInputData, aBuf, aLen)
 #define CryptoX_LoadPublicKey(aProviderHandle, aCertData, aDataSize, \
                               aPublicKey) \
-  CryptoMac_LoadPublicKey(aCertData, aPublicKey)
+  CryptoMac_LoadPublicKey(aCertData, aDataSize, aPublicKey)
 #define CryptoX_VerifySignature(aInputData, aPublicKey, aSignature, \
                                 aSignatureLen) \
   CryptoMac_VerifySignature(aInputData, aPublicKey, aSignature, aSignatureLen)
 #define CryptoX_FreeSignatureHandle(aInputData) \
   CryptoMac_FreeSignatureHandle(aInputData)
 #define CryptoX_FreePublicKey(aPublicKey) \
   CryptoMac_FreePublicKey(aPublicKey)
 #define CryptoX_FreeCertificate(aCertificate)
--- a/toolkit/mozapps/update/updater/archivereader.cpp
+++ b/toolkit/mozapps/update/updater/archivereader.cpp
@@ -42,21 +42,26 @@ static char *outbuf = nullptr;
  * @param  archive   The MAR file to verify the signature on.
  * @param  certData  The certificate data.
  * @return OK on success, CERT_VERIFY_ERROR on failure.
 */
 template<uint32_t SIZE>
 int
 VerifyLoadedCert(MarFile *archive, const uint8_t (&certData)[SIZE])
 {
+  (void)archive;
+  (void)certData;
+
+#ifdef MOZ_VERIFY_MAR_SIGNATURE
   const uint32_t size = SIZE;
   const uint8_t* const data = &certData[0];
   if (mar_verify_signatures(archive, &data, &size, 1)) {
     return CERT_VERIFY_ERROR;
   }
+#endif
 
   return OK;
 }
 
 /**
  * Performs a verification on the opened MAR file.  Both the primary and backup 
  * keys stored are stored in the current process and at least the primary key 
  * will be tried.  Success will be returned as long as one of the two 
--- a/toolkit/mozapps/update/updater/moz.build
+++ b/toolkit/mozapps/update/updater/moz.build
@@ -8,16 +8,22 @@ Program('updater')
 
 SOURCES += [
     'archivereader.cpp',
     'bspatch.cpp',
     'updater.cpp',
 ]
 
 have_progressui = 0
+
+if CONFIG['MOZ_VERIFY_MAR_SIGNATURE']:
+    USE_LIBS += [
+        'verifymar',
+    ]
+
 if CONFIG['OS_ARCH'] == 'WINNT':
     have_progressui = 1
     SOURCES += [
         'loaddlls.cpp',
         'progressui_win.cpp',
         'win_dirent.cpp',
     ]
     RCINCLUDE = 'updater.rc'
@@ -27,35 +33,37 @@ if CONFIG['OS_ARCH'] == 'WINNT':
     USE_STATIC_LIBS = True
 
     # Pick up nsWindowsRestart.cpp
     LOCAL_INCLUDES += [
         '/toolkit/xre',
     ]
     USE_LIBS += [
         'updatecommon-standalone',
-        'verifymar',
     ]
     OS_LIBS += [
         'comctl32',
         'ws2_32',
         'shell32',
         'shlwapi',
         'crypt32',
         'advapi32',
     ]
-else:
+elif CONFIG['OS_ARCH'] == 'Linux':
     USE_LIBS += [
         'updatecommon',
         '/modules/libmar/sign/signmar',
-        '/modules/libmar/sign/verifymar',
         '/security/nss/lib/nss/nss3',
         '/security/nss/lib/util/nssutil3',
     ]
     OS_LIBS += CONFIG['NSPR_LIBS']
+else:
+    USE_LIBS += [
+        'updatecommon',
+    ]
 
 USE_LIBS += [
     'mar',
 ]
 
 if CONFIG['MOZ_NATIVE_BZ2']:
     OS_LIBS += CONFIG['MOZ_BZ2_LIBS']
 else:
--- a/toolkit/mozapps/update/updater/updater.cpp
+++ b/toolkit/mozapps/update/updater/updater.cpp
@@ -111,17 +111,17 @@ static int ioprio_set(int which, int who
 #endif
 
 # define MAYBE_USE_HARD_LINKS 1
 static bool sUseHardLinks = true;
 #else
 # define MAYBE_USE_HARD_LINKS 0
 #endif
 
-#if defined(MOZ_VERIFY_MAR_SIGNATURE) && !defined(XP_WIN)
+#if defined(MOZ_VERIFY_MAR_SIGNATURE) && !defined(XP_WIN) && !defined(XP_MACOSX)
 #include "nss.h"
 #include "prerror.h"
 #endif
 
 #ifdef XP_WIN
 #include "updatehelper.h"
 
 // Closes the handle if valid and if the updater is elevated returns with the
@@ -2280,19 +2280,19 @@ int NS_main(int argc, NS_tchar **argv)
     unsetenv("LD_PRELOAD");
     execv(argv[0], argv);
     __android_log_print(ANDROID_LOG_INFO, "updater",
                         "execve failed: errno: %d. Exiting...", errno);
     _exit(1);
   }
 #endif
 
-#if defined(MOZ_VERIFY_MAR_SIGNATURE) && !defined(XP_WIN)
-  // On Windows we rely on CyrptoAPI to do verifications so we don't need to
-  // initialize NSS at all there.
+#if defined(MOZ_VERIFY_MAR_SIGNATURE) && !defined(XP_WIN) && !defined(XP_MACOSX)
+  // On Windows and Mac we rely on native APIs to do verifications so we don't
+  // need to initialize NSS at all there.
   // Otherwise, minimize the amount of NSS we depend on by avoiding all the NSS
   // databases.
   if (NSS_NoDB_Init(NULL) != SECSuccess) {
    PRErrorCode error = PR_GetError();
    fprintf(stderr, "Could not initialize NSS: %s (%d)",
            PR_ErrorToName(error), (int) error);
     _exit(1);
   }