Changeset 238246 in webkit


Ignore:
Timestamp:
Nov 15, 2018 1:24:51 PM (5 years ago)
Author:
jiewen_tan@apple.com
Message:

[WebAuthN] Use a real nonce for CTAPHID_INIT
​https://bugs.webkit.org/show_bug.cgi?id=191533
<rdar://problem/46103502>

Reviewed by Brent Fulgham.

Source/WebCore:

New tests are added into existing test files.

  • Modules/webauthn/fido/FidoConstants.h:

Source/WebKit:

Use a real nonce for CTAPHID_INIT request according to:
​https://fidoalliance.org/specs/fido-v2.0-ps-20170927/fido-client-to-authenticator-protocol-v2.0-ps-20170927.html#ctaphid_init-0x06.
The challenge here is the new transaction needs to start in the next runloop otherwise a dead lock will form:
wrong nonce -> new transaction -> new nonce -> write init request -> read init response from last run as it
piped in the run loop -> wrong nonce of course -> ...
To break the above dead lock, we have to start the new transaction in the next run. However, that isn't
sufficient as the arrived init response will be piped in HidConnection::m_inputReports, which is designed
on purpose to store any data packets within (initialized, terminated) time interval to prevent data loss in
the case when HidConnection::registerDataReceivedCallback is called after the first data packet's arrival.
In order to break the dead lock completely, HidConnection::invalidateCache will bnnne called prior to every
send to delete any potential init response from last run. HidConnection::invalidateCache is not necessary
for other protocols though. The above scenario is more or less a design flaw in CTAP HID.

Of course, all above scenarios are covered in our mock tests.

  • UIProcess/API/C/WKWebsiteDataStoreRef.cpp:

(WKWebsiteDataStoreSetWebAuthenticationMockConfiguration):

  • UIProcess/WebAuthentication/Cocoa/HidConnection.h:

(WebKit::HidConnection::invalidateCache):

  • UIProcess/WebAuthentication/Mock/MockHidConnection.cpp:

(WebKit::MockHidConnection::send):
(WebKit::MockHidConnection::parseRequest):
(WebKit::MockHidConnection::feedReports):

  • UIProcess/WebAuthentication/Mock/MockHidConnection.h:
  • UIProcess/WebAuthentication/Mock/MockWebAuthenticationConfiguration.h:
  • UIProcess/WebAuthentication/fido/CtapHidDriver.cpp:

(WebKit::CtapHidDriver::Worker::transact):
(WebKit::CtapHidDriver::CtapHidDriver):
(WebKit::CtapHidDriver::transact):
(WebKit::CtapHidDriver::continueAfterChannelAllocated):
(WebKit::CtapHidDriver::returnResponse):

  • UIProcess/WebAuthentication/fido/CtapHidDriver.h:

LayoutTests:

  • http/wpt/webauthn/ctap-hid-failure.https-expected.txt:
  • http/wpt/webauthn/ctap-hid-failure.https.html:
  • http/wpt/webauthn/ctap-hid-success.https-expected.txt:
  • http/wpt/webauthn/ctap-hid-success.https.html:
Location:
trunk
Files:
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r238245 r238246  
     12018-11-15  Jiewen Tan  <jiewen_tan@apple.com>
     2
     3        [WebAuthN] Use a real nonce for CTAPHID_INIT
     4        https://bugs.webkit.org/show_bug.cgi?id=191533
     5        <rdar://problem/46103502>
     6
     7        Reviewed by Brent Fulgham.
     8
     9        * http/wpt/webauthn/ctap-hid-failure.https-expected.txt:
     10        * http/wpt/webauthn/ctap-hid-failure.https.html:
     11        * http/wpt/webauthn/ctap-hid-success.https-expected.txt:
     12        * http/wpt/webauthn/ctap-hid-success.https.html:
     13
    1142018-11-15  Justin Fan  <justin_fan@apple.com>
    215

  • trunk/LayoutTests/http/wpt/webauthn/ctap-hid-failure.https-expected.txt

    r238166 r238246  
    33PASS CTAP HID with init sub stage empty report error in a mock hid authenticator.
    44PASS CTAP HID with init sub stage wrong channel id error in a mock hid authenticator.
     5PASS CTAP HID with init sub stage wrong nonce error in a mock hid authenticator.
    56PASS CTAP HID with msg sub stage data not sent error in a mock hid authenticator.
    67PASS CTAP HID with msg sub stage empty report error in a mock hid authenticator.

  • trunk/LayoutTests/http/wpt/webauthn/ctap-hid-failure.https.html

    r238166 r238246  
    4141    promise_test(function(t) {
    4242        if (window.testRunner)
     43            testRunner.setWebAuthenticationMockConfiguration({ hid: { stage: "info", subStage: "init", error: "wrong-nonce" } });
     44        return promiseRejects(t, "NotAllowedError", navigator.credentials.create(defaultOptions), "Operation timed out.");
     45    }, "CTAP HID with init sub stage wrong nonce error in a mock hid authenticator.");
     46
     47    promise_test(function(t) {
     48        if (window.testRunner)
    4349            testRunner.setWebAuthenticationMockConfiguration({ hid: { stage: "info", subStage: "msg", error: "data-not-sent" } });
    4450        return promiseRejects(t, "NotAllowedError", navigator.credentials.create(defaultOptions), "Operation timed out.");

  • trunk/LayoutTests/http/wpt/webauthn/ctap-hid-success.https-expected.txt

    r238166 r238246  
    44PASS CTAP HID with continue after empty report in a mock hid authenticator.
    55PASS CTAP HID with continue after wrong channel id in a mock hid authenticator.
     6PASS CTAP HID with continue after wrong nonce error in a mock hid authenticator.
    67

  • trunk/LayoutTests/http/wpt/webauthn/ctap-hid-success.https.html

    r238166 r238246  
    5555        });
    5656    }, "CTAP HID with continue after wrong channel id in a mock hid authenticator.");
     57
     58    promise_test(function(t) {
     59        if (window.testRunner)
     60            testRunner.setWebAuthenticationMockConfiguration({ hid: { stage: "info", subStage: "init", error: "wrong-nonce", payloadBase64: testCreationMessageBase64, continueAfterErrorData: true } });
     61        return navigator.credentials.create(defaultOptions).then(credential => {
     62            assert_not_equals(credential, undefined);
     63            assert_not_equals(credential, null);
     64        });
     65    }, "CTAP HID with continue after wrong nonce error in a mock hid authenticator.");
    5766</script>

  • trunk/Source/WebCore/ChangeLog

    r238245 r238246  
     12018-11-15  Jiewen Tan  <jiewen_tan@apple.com>
     2
     3        [WebAuthN] Use a real nonce for CTAPHID_INIT
     4        https://bugs.webkit.org/show_bug.cgi?id=191533
     5        <rdar://problem/46103502>
     6
     7        Reviewed by Brent Fulgham.
     8
     9        New tests are added into existing test files.
     10
     11        * Modules/webauthn/fido/FidoConstants.h:
     12
    1132018-11-15  Justin Fan  <justin_fan@apple.com>
    214

  • trunk/Source/WebCore/Modules/webauthn/fido/FidoConstants.h

    r238166 r238246  
    161161const size_t kHidContinuationPacketDataSize = kHidMaxPacketSize - kHidContinuationPacketHeader;
    162162const size_t kHidInitResponseSize = 17;
     163const size_t kHidInitNonceLength = 8;
    163164
    164165const uint8_t kHidMaxLockSeconds = 10;

  • trunk/Source/WebKit/ChangeLog

    r238244 r238246  
     12018-11-15  Jiewen Tan  <jiewen_tan@apple.com>
     2
     3        [WebAuthN] Use a real nonce for CTAPHID_INIT
     4        https://bugs.webkit.org/show_bug.cgi?id=191533
     5        <rdar://problem/46103502>
     6
     7        Reviewed by Brent Fulgham.
     8
     9        Use a real nonce for CTAPHID_INIT request according to:
     10        https://fidoalliance.org/specs/fido-v2.0-ps-20170927/fido-client-to-authenticator-protocol-v2.0-ps-20170927.html#ctaphid_init-0x06.
     11        The challenge here is the new transaction needs to start in the next runloop otherwise a dead lock will form:
     12        wrong nonce -> new transaction -> new nonce -> write init request -> read init response from last run as it
     13        piped in the run loop -> wrong nonce of course -> ...
     14        To break the above dead lock, we have to start the new transaction in the next run. However, that isn't
     15        sufficient as the arrived init response will be piped in HidConnection::m_inputReports, which is designed
     16        on purpose to store any data packets within (initialized, terminated) time interval to prevent data loss in
     17        the case when HidConnection::registerDataReceivedCallback is called after the first data packet's arrival.
     18        In order to break the dead lock completely, HidConnection::invalidateCache will bnnne called prior to every
     19        send to delete any potential init response from last run. HidConnection::invalidateCache is not necessary
     20        for other protocols though. The above scenario is more or less a design flaw in CTAP HID.
     21
     22        Of course, all above scenarios are covered in our mock tests.
     23
     24        * UIProcess/API/C/WKWebsiteDataStoreRef.cpp:
     25        (WKWebsiteDataStoreSetWebAuthenticationMockConfiguration):
     26        * UIProcess/WebAuthentication/Cocoa/HidConnection.h:
     27        (WebKit::HidConnection::invalidateCache):
     28        * UIProcess/WebAuthentication/Mock/MockHidConnection.cpp:
     29        (WebKit::MockHidConnection::send):
     30        (WebKit::MockHidConnection::parseRequest):
     31        (WebKit::MockHidConnection::feedReports):
     32        * UIProcess/WebAuthentication/Mock/MockHidConnection.h:
     33        * UIProcess/WebAuthentication/Mock/MockWebAuthenticationConfiguration.h:
     34        * UIProcess/WebAuthentication/fido/CtapHidDriver.cpp:
     35        (WebKit::CtapHidDriver::Worker::transact):
     36        (WebKit::CtapHidDriver::CtapHidDriver):
     37        (WebKit::CtapHidDriver::transact):
     38        (WebKit::CtapHidDriver::continueAfterChannelAllocated):
     39        (WebKit::CtapHidDriver::returnResponse):
     40        * UIProcess/WebAuthentication/fido/CtapHidDriver.h:
     41
    1422018-11-15  Oriol Brufau  <obrufau@igalia.com>
    243

  • trunk/Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp

    r238166 r238246  
    626626        if (error == "unsupported-options")
    627627            hid.error = MockWebAuthenticationConfiguration::Hid::Error::UnsupportedOptions;
     628        if (error == "wrong-nonce")
     629            hid.error = MockWebAuthenticationConfiguration::Hid::Error::WrongNonce;
    628630
    629631        if (auto payloadBase64 = static_cast<WKStringRef>(WKDictionaryGetItemForKey(hidRef, adoptWK(WKStringCreateWithUTF8CString("PayloadBase64")).get())))

  • trunk/Source/WebKit/UIProcess/WebAuthentication/Cocoa/HidConnection.h

    r238166 r238246  
    6060    void registerDataReceivedCallback(DataReceivedCallback&&);
    6161    void unregisterDataReceivedCallback();
     62    void invalidateCache() { m_inputReports.clear(); }
    6263
    6364    void receiveReport(Vector<uint8_t>&&);

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

    r238166 r238246  
    7171{
    7272    ASSERT(m_initialized);
    73     assembleRequest(WTFMove(data));
    74 
    75     auto sent = DataSent::Yes;
    76     if (stagesMatch() && m_configuration.hid->error == Mock::Error::DataNotSent)
    77         sent = DataSent::No;
    78 
    79     auto task = BlockPtr<void()>::fromCallable([callback = WTFMove(callback), sent]() mutable {
     73    auto task = BlockPtr<void()>::fromCallable([weakThis = makeWeakPtr(*this), data = WTFMove(data), callback = WTFMove(callback)]() mutable {
    8074        ASSERT(!RunLoop::isMain());
    81         RunLoop::main().dispatch([callback = WTFMove(callback), sent]() mutable {
     75        RunLoop::main().dispatch([weakThis, data = WTFMove(data), callback = WTFMove(callback)]() mutable {
     76            if (!weakThis)
     77                return;
     78
     79            weakThis->assembleRequest(WTFMove(data));
     80
     81            auto sent = DataSent::Yes;
     82            if (weakThis->stagesMatch() && weakThis->m_configuration.hid->error == Mock::Error::DataNotSent)
     83                sent = DataSent::No;
    8284            callback(sent);
    8385        });
    … …  
    164166    }
    165167
     168    // Store nonce.
     169    if (m_subStage == Mock::SubStage::Init) {
     170        m_nonce = m_requestMessage->getMessagePayload();
     171        ASSERT(m_nonce.size() == kHidInitNonceLength);
     172    }
     173
    166174    m_currentChannel = m_requestMessage->channelId();
    167175    m_requestMessage = std::nullopt;
    … …  
    177185        Vector<uint8_t> payload;
    178186        payload.reserveInitialCapacity(kHidInitResponseSize);
    179         // FIXME(191533): Use a real nonce.
    180         payload.appendVector(Vector<uint8_t>({ 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07 }));
     187        payload.appendVector(m_nonce);
     188        if (stagesMatch() && m_configuration.hid->error == Mock::Error::WrongNonce)
     189            payload[0]--;
    181190        payload.grow(kHidInitResponseSize);
    182191        cryptographicallyRandomValues(payload.data() + payload.size(), CtapChannelIdSize);
    … …  
    192201    std::optional<FidoHidMessage> message;
    193202    if (m_stage == Mock::Stage::Info && m_subStage == Mock::SubStage::Msg) {
    194         auto infoData = encodeAsCBOR(AuthenticatorGetInfoResponse({ ProtocolVersion::kCtap }, Vector<uint8_t>(kAaguidLength)));
     203        auto infoData = encodeAsCBOR(AuthenticatorGetInfoResponse({ ProtocolVersion::kCtap }, Vector<uint8_t>(kAaguidLength, 0u)));
    195204        infoData.insert(0, static_cast<uint8_t>(CtapDeviceResponseCode::kSuccess)); // Prepend status code.
    196205        if (stagesMatch() && m_configuration.hid->error == Mock::Error::WrongChannelId)

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

    r238166 r238246  
    6666    bool m_requireResidentKey { false };
    6767    bool m_requireUserVerification  { false };
     68    Vector<uint8_t> m_nonce;
    6869};
    6970

  • trunk/Source/WebKit/UIProcess/WebAuthentication/Mock/MockWebAuthenticationConfiguration.h

    r238166 r238246  
    5858            WrongChannelId,
    5959            MaliciousPayload,
    60             UnsupportedOptions
     60            UnsupportedOptions,
     61            WrongNonce
    6162        };
    6263

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

    r238166 r238246  
    3030
    3131#include <WebCore/FidoConstants.h>
     32#include <wtf/CryptographicallyRandomNumber.h>
    3233#include <wtf/RunLoop.h>
     34#include <wtf/Vector.h>
    3335#include <wtf/text/Base64.h>
    3436
    … …  
    5557    m_callback = WTFMove(callback);
    5658
     59    // HidConnection could hold data from other applications, and thereofore invalidate it before each transaction.
     60    m_connection->invalidateCache();
    5761    m_connection->send(m_requestMessage->popNextPacket(), [weakThis = makeWeakPtr(*this)](HidConnection::DataSent sent) mutable {
    5862        ASSERT(RunLoop::isMain());
    … …  
    130134CtapHidDriver::CtapHidDriver(UniqueRef<HidConnection>&& connection)
    131135    : m_worker(makeUniqueRef<Worker>(WTFMove(connection)))
     136    , m_nonce(kHidInitNonceLength)
    132137{
    133138}
    … …  
    137142    ASSERT(m_state == State::Idle);
    138143    m_state = State::AllocateChannel;
     144    m_channelId = kHidBroadcastChannel;
    139145    m_requestData = WTFMove(data);
    140146    m_responseCallback = WTFMove(callback);
    141147
    142148    // Allocate a channel.
    143     // FIXME(191533): Get a real nonce.
    144     auto initCommand = FidoHidMessage::create(kHidBroadcastChannel, FidoHidDeviceCommand::kInit, { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07 });
     149    ASSERT(m_nonce.size() == kHidInitNonceLength);
     150    cryptographicallyRandomValues(m_nonce.data(), m_nonce.size());
     151    auto initCommand = FidoHidMessage::create(m_channelId, FidoHidDeviceCommand::kInit, m_nonce);
    145152    ASSERT(initCommand);
    146153    m_worker->transact(WTFMove(*initCommand), [weakThis = makeWeakPtr(*this)](std::optional<FidoHidMessage>&& response) mutable {
    … …  
    159166        return;
    160167    }
     168    ASSERT(message->channelId() == m_channelId);
     169
     170    auto payload = message->getMessagePayload();
     171    ASSERT(payload.size() == kHidInitResponseSize);
     172    // Restart the transaction in the next run loop when nonce mismatches.
     173    if (memcmp(payload.data(), m_nonce.data(), m_nonce.size())) {
     174        m_state = State::Idle;
     175        RunLoop::main().dispatch([weakThis = makeWeakPtr(*this), data = WTFMove(m_requestData), callback = WTFMove(m_responseCallback)]() mutable {
     176            if (!weakThis)
     177                return;
     178            weakThis->transact(WTFMove(data), WTFMove(callback));
     179        });
     180        return;
     181    }
     182
    161183    m_state = State::Ready;
    162     ASSERT(message->channelId() == m_channelId);
    163 
    164     // FIXME(191534): Check Payload including nonce and everything
    165     auto payload = message->getMessagePayload();
    166     size_t index = 8;
    167     ASSERT(payload.size() == kHidInitResponseSize);
     184    auto index = kHidInitNonceLength;
    168185    m_channelId = static_cast<uint32_t>(payload[index++]) << 24;
    169186    m_channelId |= static_cast<uint32_t>(payload[index++]) << 16;
    170187    m_channelId |= static_cast<uint32_t>(payload[index++]) << 8;
    171188    m_channelId |= static_cast<uint32_t>(payload[index]);
     189    // FIXME(191534): Check the reset of the payload.
    172190    auto cmd = FidoHidMessage::create(m_channelId, FidoHidDeviceCommand::kCbor, m_requestData);
    173191    ASSERT(cmd);
    … …  
    191209    // Reset state before calling the response callback to avoid being deleted.
    192210    m_state = State::Idle;
    193     m_channelId = fido::kHidBroadcastChannel;
    194     m_requestData = { };
    195211    m_responseCallback(WTFMove(response));
    196212}

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

    r238166 r238246  
    6363    // Worker is the helper that maintains the transaction.
    6464    // https://fidoalliance.org/specs/fido-v2.0-ps-20170927/fido-client-to-authenticator-protocol-v2.0-ps-20170927.html#arbitration
    65     // FSM: Idle => Write => Read
     65    // FSM: Idle => Write => Read.
    6666    class Worker : public CanMakeWeakPtr<Worker> {
    6767        WTF_MAKE_FAST_ALLOCATED;
    … …  
    103103    Vector<uint8_t> m_requestData;
    104104    ResponseCallback m_responseCallback;
     105    Vector<uint8_t> m_nonce;
    105106};
    106107
Note: See TracChangeset for help on using the changeset viewer.