Changeset 239852 in webkit


Ignore:
Timestamp:
Jan 10, 2019 5:42:34 PM (5 years ago)
Author:
jiewen_tan@apple.com
Message:

[WebAuthN] Change the nonce in the CTAP kInit command to weak random values
​https://bugs.webkit.org/show_bug.cgi?id=192061
<rdar://problem/46471091>

Reviewed by Chris Dumez.

Change the nonce in the CTAP kInit command to weak random values as the nonce is mainly
for being a probabilistically unique global identifier for hand shakes, instead of
preventing replay attacks. Otherwise, it might exhaust system entropy unnecessarily.

The patch also removes all logging when debugging the test case flakiness.

  • UIProcess/WebAuthentication/AuthenticatorManager.cpp:

(WebKit::AuthenticatorManager::respondReceived):
(WebKit::AuthenticatorManager::initTimeOutTimer):
(WebKit::AuthenticatorManager::timeOutTimerFired):

  • UIProcess/WebAuthentication/Cocoa/HidService.mm:

(WebKit::HidService::deviceAdded):

  • UIProcess/WebAuthentication/Mock/MockAuthenticatorManager.cpp:

(WebKit::MockAuthenticatorManager::respondReceivedInternal):

  • UIProcess/WebAuthentication/Mock/MockHidConnection.cpp:

(WebKit::MockHidConnection::send):

  • UIProcess/WebAuthentication/fido/CtapHidAuthenticator.cpp:

(WebKit::CtapHidAuthenticator::makeCredential):
(WebKit::CtapHidAuthenticator::getAssertion):

  • UIProcess/WebAuthentication/fido/CtapHidDriver.cpp:

(WebKit::CtapHidDriver::Worker::write):
(WebKit::CtapHidDriver::Worker::read):
(WebKit::CtapHidDriver::Worker::returnMessage):
(WebKit::CtapHidDriver::transact):
(WebKit::CtapHidDriver::continueAfterChannelAllocated):
(WebKit::CtapHidDriver::continueAfterResponseReceived):

Location:
trunk/Source/WebKit
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r239851 r239852  
     12019-01-10  Jiewen Tan  <jiewen_tan@apple.com>
     2
     3        [WebAuthN] Change the nonce in the CTAP kInit command to weak random values
     4        https://bugs.webkit.org/show_bug.cgi?id=192061
     5        <rdar://problem/46471091>
     6
     7        Reviewed by Chris Dumez.
     8
     9        Change the nonce in the CTAP kInit command to weak random values as the nonce is mainly
     10        for being a probabilistically unique global identifier for hand shakes, instead of
     11        preventing replay attacks. Otherwise, it might exhaust system entropy unnecessarily.
     12
     13        The patch also removes all logging when debugging the test case flakiness.
     14
     15        * UIProcess/WebAuthentication/AuthenticatorManager.cpp:
     16        (WebKit::AuthenticatorManager::respondReceived):
     17        (WebKit::AuthenticatorManager::initTimeOutTimer):
     18        (WebKit::AuthenticatorManager::timeOutTimerFired):
     19        * UIProcess/WebAuthentication/Cocoa/HidService.mm:
     20        (WebKit::HidService::deviceAdded):
     21        * UIProcess/WebAuthentication/Mock/MockAuthenticatorManager.cpp:
     22        (WebKit::MockAuthenticatorManager::respondReceivedInternal):
     23        * UIProcess/WebAuthentication/Mock/MockHidConnection.cpp:
     24        (WebKit::MockHidConnection::send):
     25        * UIProcess/WebAuthentication/fido/CtapHidAuthenticator.cpp:
     26        (WebKit::CtapHidAuthenticator::makeCredential):
     27        (WebKit::CtapHidAuthenticator::getAssertion):
     28        * UIProcess/WebAuthentication/fido/CtapHidDriver.cpp:
     29        (WebKit::CtapHidDriver::Worker::write):
     30        (WebKit::CtapHidDriver::Worker::read):
     31        (WebKit::CtapHidDriver::Worker::returnMessage):
     32        (WebKit::CtapHidDriver::transact):
     33        (WebKit::CtapHidDriver::continueAfterChannelAllocated):
     34        (WebKit::CtapHidDriver::continueAfterResponseReceived):
     35
    1362019-01-10  Timothy Hatcher  <timothy@apple.com>
    237

  • trunk/Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp

    r239518 r239852  
    193193        m_pendingCompletionHandler(WTFMove(respond));
    194194        clearStateAsync();
    195         // FIXME(192061)
    196         LOG_ERROR("Stop timer.");
    197195        m_requestTimeOutTimer.stop();
    198196        return;
    … …  
    227225
    228226    unsigned timeOutInMsValue = std::min(maxTimeOutValue, timeOutInMs.valueOr(maxTimeOutValue));
    229     // FIXME(192061)
    230     LOG_ERROR("Start timer: %d. Current time: %f.", timeOutInMsValue, MonotonicTime::now().secondsSinceEpoch().value());
    231227    m_requestTimeOutTimer.startOneShot(Seconds::fromMilliseconds(timeOutInMsValue));
    232     LOG_ERROR("Seconds until fire: %f", m_requestTimeOutTimer.secondsUntilFire().value());
    233228}
    234229
    … …  
    236231{
    237232    ASSERT(m_requestTimeOutTimer.isActive());
    238     // FIXME(192061)
    239     LOG_ERROR("Timer fired: %f, Current time: %f", m_requestTimeOutTimer.secondsUntilFire().value(), MonotonicTime::now().secondsSinceEpoch().value());
    240233    m_pendingCompletionHandler((ExceptionData { NotAllowedError, "Operation timed out."_s }));
    241234    clearStateAsync();

  • trunk/Source/WebKit/UIProcess/WebAuthentication/Cocoa/HidService.mm

    r239752 r239852  
    9595    auto driver = std::make_unique<CtapHidDriver>(createHidConnection(device));
    9696    // Get authenticator info from the device.
    97     // FIXME(192061)
    98     LOG_ERROR("Start asking device info.");
    9997    driver->transact(encodeEmptyAuthenticatorRequest(CtapRequestCommand::kAuthenticatorGetInfo), [weakThis = makeWeakPtr(*this), ptr = driver.get()](Vector<uint8_t>&& response) {
    10098        ASSERT(RunLoop::isMain());

  • trunk/Source/WebKit/UIProcess/WebAuthentication/Mock/MockAuthenticatorManager.cpp

    r239323 r239852  
    4848    pendingCompletionHandler()(WTFMove(respond));
    4949    clearStateAsync();
    50     // FIXME(192061)
    51     LOG_ERROR("Stop timer.");
    5250    requestTimeOutTimer().stop();
    5351}

  • trunk/Source/WebKit/UIProcess/WebAuthentication/Mock/MockHidConnection.cpp

    r239757 r239852  
    7171void MockHidConnection::send(Vector<uint8_t>&& data, DataSentCallback&& callback)
    7272{
    73     // FIXME(192061): Remove all LOG_ERRORs.
    74     LOG_ERROR("Sending data: Phase 1. Current time: %f.", MonotonicTime::now().secondsSinceEpoch().value());
    7573    ASSERT(m_initialized);
    7674    auto task = makeBlockPtr([weakThis = makeWeakPtr(*this), data = WTFMove(data), callback = WTFMove(callback)]() mutable {
    7775        ASSERT(!RunLoop::isMain());
    78         LOG_ERROR("Sending data: Phase 2. Current time: %f.", MonotonicTime::now().secondsSinceEpoch().value());
    7976        RunLoop::main().dispatch([weakThis, data = WTFMove(data), callback = WTFMove(callback)]() mutable {
    80             LOG_ERROR("Sending data: Phase 3. Current time: %f.", MonotonicTime::now().secondsSinceEpoch().value());
    8177            if (!weakThis) {
    8278                callback(DataSent::No);
    … …  
    8480            }
    8581
    86             LOG_ERROR("Sending data: Phase 4. Current time: %f.", MonotonicTime::now().secondsSinceEpoch().value());
    8782            weakThis->assembleRequest(WTFMove(data));
    8883
    89             LOG_ERROR("Sending data: Phase 5. Current time: %f.", MonotonicTime::now().secondsSinceEpoch().value());
    9084            auto sent = DataSent::Yes;
    9185            if (weakThis->stagesMatch() && weakThis->m_configuration.hid->error == Mock::Error::DataNotSent)

  • trunk/Source/WebKit/UIProcess/WebAuthentication/fido/CtapHidAuthenticator.cpp

    r239235 r239852  
    5050void CtapHidAuthenticator::makeCredential()
    5151{
    52     // FIXME(192061)
    53     LOG_ERROR("Start making credentials.");
    5452    auto cborCmd = encodeMakeCredenitalRequestAsCBOR(requestData().hash, requestData().creationOptions, m_info.options().userVerificationAvailability());
    5553    m_driver->transact(WTFMove(cborCmd), [weakThis = makeWeakPtr(*this)](Vector<uint8_t>&& data) {
    … …  
    7371void CtapHidAuthenticator::getAssertion()
    7472{
    75     // FIXME(192061)
    76     LOG_ERROR("Start getting assertions.");
    7773    auto cborCmd = encodeGetAssertionRequestAsCBOR(requestData().hash, requestData().requestOptions, m_info.options().userVerificationAvailability());
    7874    m_driver->transact(WTFMove(cborCmd), [weakThis = makeWeakPtr(*this)](Vector<uint8_t>&& data) {

  • trunk/Source/WebKit/UIProcess/WebAuthentication/fido/CtapHidDriver.cpp

    r239752 r239852  
    3030
    3131#include <WebCore/FidoConstants.h>
    32 #include <wtf/CryptographicallyRandomNumber.h>
     32#include <wtf/RandomNumber.h>
    3333#include <wtf/RunLoop.h>
    3434#include <wtf/Vector.h>
    … …  
    7070{
    7171    ASSERT(m_state == State::Write);
    72     // FIXME(192061)
    73     LOG_ERROR("Start writing data.");
    7472    if (sent != HidConnection::DataSent::Yes) {
    7573        returnMessage(WTF::nullopt);
    … …  
    9997{
    10098    ASSERT(m_state == State::Read);
    101     // FIXME(192061)
    102     LOG_ERROR("Start reading data.");
    10399    if (!m_responseMessage) {
    104100        m_responseMessage = FidoHidMessage::createFromSerializedData(data);
    … …  
    131127void CtapHidDriver::Worker::returnMessage(Optional<fido::FidoHidMessage>&& message)
    132128{
    133     // FIXME(192061)
    134     LOG_ERROR("Start returning data.");
    135129    m_state = State::Idle;
    136130    m_connection->unregisterDataReceivedCallback();
    … …  
    153147
    154148    // Allocate a channel.
    155     // FIXME(192061)
    156     LOG_ERROR("Start allocating a channel.");
    157     ASSERT(m_nonce.size() == kHidInitNonceLength);
    158     cryptographicallyRandomValues(m_nonce.data(), m_nonce.size());
     149    // Use a pseudo random nonce instead of a cryptographically strong one as the nonce
     150    // is mainly for identifications.
     151    size_t steps = kHidInitNonceLength / sizeof(uint32_t);
     152    ASSERT(!(kHidInitNonceLength % sizeof(uint32_t)) && steps >= 1);
     153    for (size_t i = 0; i < steps; ++i) {
     154        uint32_t weakRandom = weakRandomUint32();
     155        memcpy(m_nonce.data() + i * sizeof(uint32_t), &weakRandom, sizeof(uint32_t));
     156    }
     157
    159158    auto initCommand = FidoHidMessage::create(m_channelId, FidoHidDeviceCommand::kInit, m_nonce);
    160159    ASSERT(initCommand);
    … …  
    196195    m_channelId |= static_cast<uint32_t>(payload[index]);
    197196    // FIXME(191534): Check the reset of the payload.
    198     // FIXME(192061)
    199     LOG_ERROR("Start sending the request.");
    200197    auto cmd = FidoHidMessage::create(m_channelId, m_protocol == ProtocolVersion::kCtap ? FidoHidDeviceCommand::kCbor : FidoHidDeviceCommand::kMsg, m_requestData);
    201198    ASSERT(cmd);
    … …  
    212209    ASSERT(m_state == State::Ready);
    213210    ASSERT(!message || message->channelId() == m_channelId);
    214     // FIXME(192061)
    215     LOG_ERROR("Start returning the response.");
    216211    returnResponse(message ? message->getMessagePayload() : Vector<uint8_t>());
    217212}
Note: See TracChangeset for help on using the changeset viewer.