Changeset 214329 in webkit


Ignore:
Timestamp:
Mar 23, 2017, 6:36:06 PM (8 years ago)
Author:
Michael Catanzaro
Message:

window.crypto.getRandomValues() uses the insecure RC4 RNG
​https://bugs.webkit.org/show_bug.cgi?id=169623

Reviewed by Alex Christensen.

Source/WebCore:

  • PlatformMac.cmake:
  • WebCore.xcodeproj/project.pbxproj:
  • crypto/CryptoKey.cpp:

(WebCore::CryptoKey::randomData): Use this on Mac now.

  • crypto/mac/CryptoKeyMac.cpp: Removed.
  • page/Crypto.cpp:

(WebCore::Crypto::getRandomValues): Rollout r214188.

Source/WTF:

Remove the RC4 random generator in favor of using OS randomness for now. This is basically
a merge of ​https://codereview.chromium.org/1431233002 from Blink, original author "eroman".

  • wtf/CryptographicallyRandomNumber.cpp:

(WTF::cryptographicallyRandomNumber):
(WTF::cryptographicallyRandomValues):
(): Deleted.

Location:
trunk/Source
Files:
1 deleted
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WTF/ChangeLog

    r214299 r214329  
     12017-03-23  Michael Catanzaro  <mcatanzaro@igalia.com>
     2
     3        window.crypto.getRandomValues() uses the insecure RC4 RNG
     4        https://bugs.webkit.org/show_bug.cgi?id=169623
     5
     6        Reviewed by Alex Christensen.
     7
     8        Remove the RC4 random generator in favor of using OS randomness for now. This is basically
     9        a merge of https://codereview.chromium.org/1431233002 from Blink, original author "eroman".
     10
     11        * wtf/CryptographicallyRandomNumber.cpp:
     12        (WTF::cryptographicallyRandomNumber):
     13        (WTF::cryptographicallyRandomValues):
     14        (): Deleted.
     15
    1162017-03-23  Tomas Popela  <tpopela@redhat.com>
    217

  • trunk/Source/WTF/wtf/CryptographicallyRandomNumber.cpp

    r188642 r214329  
    11/*
    2  * Copyright (c) 1996, David Mazieres <dm@uun.org>
    3  * Copyright (c) 2008, Damien Miller <djm@openbsd.org>
     2 * Copyright (C) 2017 Igalia S.L.
    43 *
    5  * Permission to use, copy, modify, and distribute this software for any
    6  * purpose with or without fee is hereby granted, provided that the above
    7  * copyright notice and this permission notice appear in all copies.
     4 * Redistribution and use in source and binary forms, with or without
     5 * modification, are permitted provided that the following conditions
     6 * are met:
     7 * 1. Redistributions of source code must retain the above copyright
     8 *    notice, this list of conditions and the following disclaimer.
     9 * 2. Redistributions in binary form must reproduce the above copyright
     10 *    notice, this list of conditions and the following disclaimer in the
     11 *    documentation and/or other materials provided with the distribution.
    812 *
    9  * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
    10  * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
    11  * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
    12  * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
    13  * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
    14  * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
    15  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
    16  */
    17 
    18 /*
    19  * Arc4 random number generator for OpenBSD.
    20  *
    21  * This code is derived from section 17.1 of Applied Cryptography,
    22  * second edition, which describes a stream cipher allegedly
    23  * compatible with RSA Labs "RC4" cipher (the actual description of
    24  * which is a trade secret).  The same algorithm is used as a stream
    25  * cipher called "arcfour" in Tatu Ylonen's ssh package.
    26  *
    27  * RC4 is a registered trademark of RSA Laboratories.
     13 * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
     14 * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
     15 * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
     16 * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
     17 * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
     18 * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
     19 * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
     20 * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
     21 * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
     22 * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
     23 * THE POSSIBILITY OF SUCH DAMAGE.
    2824 */
    2925
    … …  
    3127#include "CryptographicallyRandomNumber.h"
    3228
    33 #include "NeverDestroyed.h"
    3429#include "OSRandomSource.h"
    35 #include <mutex>
    36 #include <wtf/Lock.h>
    3730
    3831namespace WTF {
    3932
    40 namespace {
    41 
    42 class ARC4Stream {
    43 public:
    44     ARC4Stream();
    45 
    46     uint8_t i;
    47     uint8_t j;
    48     uint8_t s[256];
    49 };
    50 
    51 class ARC4RandomNumberGenerator {
    52     WTF_MAKE_FAST_ALLOCATED;
    53 public:
    54     ARC4RandomNumberGenerator();
    55 
    56     uint32_t randomNumber();
    57     void randomValues(void* buffer, size_t length);
    58 
    59 private:
    60     inline void addRandomData(unsigned char *data, int length);
    61     void stir();
    62     void stirIfNeeded();
    63     inline uint8_t getByte();
    64     inline uint32_t getWord();
    65 
    66     ARC4Stream m_stream;
    67     int m_count;
    68     Lock m_mutex;
    69 };
    70 
    71 ARC4Stream::ARC4Stream()
     33uint32_t cryptographicallyRandomNumber()
    7234{
    73     for (int n = 0; n < 256; n++)
    74         s[n] = n;
    75     i = 0;
    76     j = 0;
     35    uint32_t result;
     36    cryptographicallyRandomValues(&result, sizeof(result));
     37    return result;
    7738}
    7839
    79 ARC4RandomNumberGenerator::ARC4RandomNumberGenerator()
    80     : m_count(0)
     40// FIXME: It is slow to always get the values directly from the OS.
     41void cryptographicallyRandomValues(void* buffer, size_t length)
    8142{
    82 }
    83 
    84 void ARC4RandomNumberGenerator::addRandomData(unsigned char* data, int length)
    85 {
    86     m_stream.i--;
    87     for (int n = 0; n < 256; n++) {
    88         m_stream.i++;
    89         uint8_t si = m_stream.s[m_stream.i];
    90         m_stream.j += si + data[n % length];
    91         m_stream.s[m_stream.i] = m_stream.s[m_stream.j];
    92         m_stream.s[m_stream.j] = si;
    93     }
    94     m_stream.j = m_stream.i;
    95 }
    96 
    97 void ARC4RandomNumberGenerator::stir()
    98 {
    99     unsigned char randomness[128];
    100     size_t length = sizeof(randomness);
    101     cryptographicallyRandomValuesFromOS(randomness, length);
    102     addRandomData(randomness, length);
    103 
    104     // Discard early keystream, as per recommendations in:
    105     // http://www.wisdom.weizmann.ac.il/~itsik/RC4/Papers/Rc4_ksa.ps
    106     for (int i = 0; i < 256; i++)
    107         getByte();
    108     m_count = 1600000;
    109 }
    110 
    111 void ARC4RandomNumberGenerator::stirIfNeeded()
    112 {
    113     if (m_count <= 0)
    114         stir();
    115 }
    116 
    117 uint8_t ARC4RandomNumberGenerator::getByte()
    118 {
    119     m_stream.i++;
    120     uint8_t si = m_stream.s[m_stream.i];
    121     m_stream.j += si;
    122     uint8_t sj = m_stream.s[m_stream.j];
    123     m_stream.s[m_stream.i] = sj;
    124     m_stream.s[m_stream.j] = si;
    125     return (m_stream.s[(si + sj) & 0xff]);
    126 }
    127 
    128 uint32_t ARC4RandomNumberGenerator::getWord()
    129 {
    130     uint32_t val;
    131     val = getByte() << 24;
    132     val |= getByte() << 16;
    133     val |= getByte() << 8;
    134     val |= getByte();
    135     return val;
    136 }
    137 
    138 uint32_t ARC4RandomNumberGenerator::randomNumber()
    139 {
    140     std::lock_guard<Lock> lock(m_mutex);
    141 
    142     m_count -= 4;
    143     stirIfNeeded();
    144     return getWord();
    145 }
    146 
    147 void ARC4RandomNumberGenerator::randomValues(void* buffer, size_t length)
    148 {
    149     std::lock_guard<Lock> lock(m_mutex);
    150 
    151     unsigned char* result = reinterpret_cast<unsigned char*>(buffer);
    152     stirIfNeeded();
    153     while (length--) {
    154         m_count--;
    155         stirIfNeeded();
    156         result[length] = getByte();
    157     }
    158 }
    159 
    160 ARC4RandomNumberGenerator& sharedRandomNumberGenerator()
    161 {
    162     static NeverDestroyed<ARC4RandomNumberGenerator> randomNumberGenerator;
    163 
    164     return randomNumberGenerator;
     43    cryptographicallyRandomValuesFromOS(static_cast<unsigned char*>(buffer), length);
    16544}
    16645
    16746}
    168 
    169 uint32_t cryptographicallyRandomNumber()
    170 {
    171     return sharedRandomNumberGenerator().randomNumber();
    172 }
    173 
    174 void cryptographicallyRandomValues(void* buffer, size_t length)
    175 {
    176     sharedRandomNumberGenerator().randomValues(buffer, length);
    177 }
    178 
    179 }

  • trunk/Source/WebCore/ChangeLog

    r214327 r214329  
     12017-03-23  Michael Catanzaro  <mcatanzaro@igalia.com>
     2
     3        window.crypto.getRandomValues() uses the insecure RC4 RNG
     4        https://bugs.webkit.org/show_bug.cgi?id=169623
     5
     6        Reviewed by Alex Christensen.
     7
     8        * PlatformMac.cmake:
     9        * WebCore.xcodeproj/project.pbxproj:
     10        * crypto/CryptoKey.cpp:
     11        (WebCore::CryptoKey::randomData): Use this on Mac now.
     12        * crypto/mac/CryptoKeyMac.cpp: Removed.
     13        * page/Crypto.cpp:
     14        (WebCore::Crypto::getRandomValues): Rollout r214188.
     15
    1162017-03-23  Chris Dumez  <cdumez@apple.com>
    217

  • trunk/Source/WebCore/PlatformMac.cmake

    r214279 r214329  
    219219    crypto/mac/CryptoAlgorithmRegistryMac.cpp
    220220    crypto/mac/CryptoKeyECMac.cpp
    221     crypto/mac/CryptoKeyMac.cpp
    222221    crypto/mac/CryptoKeyRSAMac.cpp
    223222    crypto/mac/SerializedCryptoKeyWrapMac.mm

  • trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj

    r214293 r214329  
    65116511                E19AC3F51824DC7900349426 /* CryptoAlgorithmSHA512.h in Headers */ = {isa = PBXBuildFile; fileRef = E19AC3ED1824DC7900349426 /* CryptoAlgorithmSHA512.h */; };
    65126512                E19AC3F71824E5D100349426 /* CryptoAlgorithmAesKeyGenParamsDeprecated.h in Headers */ = {isa = PBXBuildFile; fileRef = E19AC3F61824E5D100349426 /* CryptoAlgorithmAesKeyGenParamsDeprecated.h */; };
    6513                 E19AC3F9182566F700349426 /* CryptoKeyMac.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E19AC3F8182566F700349426 /* CryptoKeyMac.cpp */; };
    65146513                E19DA29C18189ADD00088BC8 /* CryptoAlgorithmHmacKeyParamsDeprecated.h in Headers */ = {isa = PBXBuildFile; fileRef = E19DA29B18189ADD00088BC8 /* CryptoAlgorithmHmacKeyParamsDeprecated.h */; };
    65156514                E1A1470811102B1500EEC0F3 /* ContainerNodeAlgorithms.h in Headers */ = {isa = PBXBuildFile; fileRef = E1A1470711102B1500EEC0F3 /* ContainerNodeAlgorithms.h */; };
    … …  
    1497414973                E19AC3ED1824DC7900349426 /* CryptoAlgorithmSHA512.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CryptoAlgorithmSHA512.h; sourceTree = "<group>"; };
    1497514974                E19AC3F61824E5D100349426 /* CryptoAlgorithmAesKeyGenParamsDeprecated.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CryptoAlgorithmAesKeyGenParamsDeprecated.h; sourceTree = "<group>"; };
    14976                 E19AC3F8182566F700349426 /* CryptoKeyMac.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = CryptoKeyMac.cpp; sourceTree = "<group>"; };
    1497714975                E19DA29B18189ADD00088BC8 /* CryptoAlgorithmHmacKeyParamsDeprecated.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CryptoAlgorithmHmacKeyParamsDeprecated.h; sourceTree = "<group>"; };
    1497814976                E1A1470711102B1500EEC0F3 /* ContainerNodeAlgorithms.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ContainerNodeAlgorithms.h; sourceTree = "<group>"; };
    … …  
    2423824236                                E1C266D618317AB4003F8B33 /* CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp */,
    2423924237                                5750A97A1E69161600705C4A /* CryptoKeyECMac.cpp */,
    24240                                 E19AC3F8182566F700349426 /* CryptoKeyMac.cpp */,
    2424124238                                E164FAA418315E1A00DB4E61 /* CryptoKeyRSAMac.cpp */,
    2424224239                                E18DF33618AAF14D00773E59 /* SerializedCryptoKeyWrapMac.mm */,
    … …  
    3056930566                                5750A97B1E69161600705C4A /* CryptoKeyECMac.cpp in Sources */,
    3057030567                                E125F8351822F18A00D84CD9 /* CryptoKeyHMAC.cpp in Sources */,
    30571                                 E19AC3F9182566F700349426 /* CryptoKeyMac.cpp in Sources */,
    3057230568                                57E657E01E71397800F941CA /* CryptoKeyRaw.cpp in Sources */,
    3057330569                                57E2336B1DCC262400F28D01 /* CryptoKeyRSA.cpp in Sources */,

  • trunk/Source/WebCore/crypto/CryptoKey.cpp

    r212484 r214329  
    6969}
    7070
    71 #if !OS(DARWIN) || PLATFORM(GTK)
    7271Vector<uint8_t> CryptoKey::randomData(size_t size)
    7372{
    … …  
    7675    return result;
    7776}
    78 #endif
    7977
    8078} // namespace WebCore

  • trunk/Source/WebCore/page/Crypto.cpp

    r214188 r214329  
    3232#include "Crypto.h"
    3333
    34 #if OS(DARWIN)
    35 #include "CommonCryptoUtilities.h"
    36 #endif
    3734#include "Document.h"
    3835#include "ExceptionCode.h"
    … …  
    6259    if (array.byteLength() > 65536)
    6360        return Exception { QUOTA_EXCEEDED_ERR };
    64 #if OS(DARWIN)
    65     int rc = CCRandomCopyBytes(kCCRandomDefault, array.baseAddress(), array.byteLength());
    66     RELEASE_ASSERT(rc == kCCSuccess);
    67 #else
    6861    cryptographicallyRandomValues(array.baseAddress(), array.byteLength());
    69 #endif
    7062    return { };
    7163}
Note: See TracChangeset for help on using the changeset viewer.