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:
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238246
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2018-11-15 Jiewen Tan <jiewen_tan@apple.com>
+
+ [WebAuthN] Use a real nonce for CTAPHID_INIT
+ https://bugs.webkit.org/show_bug.cgi?id=191533
+ <rdar://problem/46103502>
+
+ Reviewed by Brent Fulgham.
+
+ * 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:
+
2018-11-15 Justin Fan <justin_fan@apple.com>
[WebGPU] WebGPUCommandBuffer prototype
PASS CTAP HID with init sub stage data not sent error in a mock hid authenticator.
PASS CTAP HID with init sub stage empty report error in a mock hid authenticator.
PASS CTAP HID with init sub stage wrong channel id error in a mock hid authenticator.
+PASS CTAP HID with init sub stage wrong nonce error in a mock hid authenticator.
PASS CTAP HID with msg sub stage data not sent error in a mock hid authenticator.
PASS CTAP HID with msg sub stage empty report error in a mock hid authenticator.
PASS CTAP HID with msg sub stage wrong channel id error in a mock hid authenticator.
promise_test(function(t) {
if (window.testRunner)
+ testRunner.setWebAuthenticationMockConfiguration({ hid: { stage: "info", subStage: "init", error: "wrong-nonce" } });
+ return promiseRejects(t, "NotAllowedError", navigator.credentials.create(defaultOptions), "Operation timed out.");
+ }, "CTAP HID with init sub stage wrong nonce error in a mock hid authenticator.");
+
+ promise_test(function(t) {
+ if (window.testRunner)
testRunner.setWebAuthenticationMockConfiguration({ hid: { stage: "info", subStage: "msg", error: "data-not-sent" } });
return promiseRejects(t, "NotAllowedError", navigator.credentials.create(defaultOptions), "Operation timed out.");
}, "CTAP HID with msg sub stage data not sent error in a mock hid authenticator.");
PASS CTAP HID with fast data arrival in a mock hid authenticator.
PASS CTAP HID with continue after empty report in a mock hid authenticator.
PASS CTAP HID with continue after wrong channel id in a mock hid authenticator.
+PASS CTAP HID with continue after wrong nonce error in a mock hid authenticator.
assert_not_equals(credential, null);
});
}, "CTAP HID with continue after wrong channel id in a mock hid authenticator.");
+
+ promise_test(function(t) {
+ if (window.testRunner)
+ testRunner.setWebAuthenticationMockConfiguration({ hid: { stage: "info", subStage: "init", error: "wrong-nonce", payloadBase64: testCreationMessageBase64, continueAfterErrorData: true } });
+ return navigator.credentials.create(defaultOptions).then(credential => {
+ assert_not_equals(credential, undefined);
+ assert_not_equals(credential, null);
+ });
+ }, "CTAP HID with continue after wrong nonce error in a mock hid authenticator.");
</script>
+2018-11-15 Jiewen Tan <jiewen_tan@apple.com>
+
+ [WebAuthN] Use a real nonce for CTAPHID_INIT
+ https://bugs.webkit.org/show_bug.cgi?id=191533
+ <rdar://problem/46103502>
+
+ Reviewed by Brent Fulgham.
+
+ New tests are added into existing test files.
+
+ * Modules/webauthn/fido/FidoConstants.h:
+
2018-11-15 Justin Fan <justin_fan@apple.com>
[WebGPU] WebGPUCommandBuffer prototype
const size_t kHidInitPacketDataSize = kHidMaxPacketSize - kHidInitPacketHeaderSize;
const size_t kHidContinuationPacketDataSize = kHidMaxPacketSize - kHidContinuationPacketHeader;
const size_t kHidInitResponseSize = 17;
+const size_t kHidInitNonceLength = 8;
const uint8_t kHidMaxLockSeconds = 10;
+2018-11-15 Jiewen Tan <jiewen_tan@apple.com>
+
+ [WebAuthN] Use a real nonce for CTAPHID_INIT
+ https://bugs.webkit.org/show_bug.cgi?id=191533
+ <rdar://problem/46103502>
+
+ Reviewed by Brent Fulgham.
+
+ 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:
+
2018-11-15 Oriol Brufau <obrufau@igalia.com>
[css-logical] Implement flow-relative margin, padding and border shorthands
hid.error = MockWebAuthenticationConfiguration::Hid::Error::MaliciousPayload;
if (error == "unsupported-options")
hid.error = MockWebAuthenticationConfiguration::Hid::Error::UnsupportedOptions;
+ if (error == "wrong-nonce")
+ hid.error = MockWebAuthenticationConfiguration::Hid::Error::WrongNonce;
if (auto payloadBase64 = static_cast<WKStringRef>(WKDictionaryGetItemForKey(hidRef, adoptWK(WKStringCreateWithUTF8CString("PayloadBase64")).get())))
hid.payloadBase64 = WebKit::toImpl(payloadBase64)->string();
virtual void send(Vector<uint8_t>&& data, DataSentCallback&&);
void registerDataReceivedCallback(DataReceivedCallback&&);
void unregisterDataReceivedCallback();
+ void invalidateCache() { m_inputReports.clear(); }
void receiveReport(Vector<uint8_t>&&);
void MockHidConnection::send(Vector<uint8_t>&& data, DataSentCallback&& callback)
{
ASSERT(m_initialized);
- assembleRequest(WTFMove(data));
+ auto task = BlockPtr<void()>::fromCallable([weakThis = makeWeakPtr(*this), data = WTFMove(data), callback = WTFMove(callback)]() mutable {
+ ASSERT(!RunLoop::isMain());
+ RunLoop::main().dispatch([weakThis, data = WTFMove(data), callback = WTFMove(callback)]() mutable {
+ if (!weakThis)
+ return;
- auto sent = DataSent::Yes;
- if (stagesMatch() && m_configuration.hid->error == Mock::Error::DataNotSent)
- sent = DataSent::No;
+ weakThis->assembleRequest(WTFMove(data));
- auto task = BlockPtr<void()>::fromCallable([callback = WTFMove(callback), sent]() mutable {
- ASSERT(!RunLoop::isMain());
- RunLoop::main().dispatch([callback = WTFMove(callback), sent]() mutable {
+ auto sent = DataSent::Yes;
+ if (weakThis->stagesMatch() && weakThis->m_configuration.hid->error == Mock::Error::DataNotSent)
+ sent = DataSent::No;
callback(sent);
});
});
}
}
+ // Store nonce.
+ if (m_subStage == Mock::SubStage::Init) {
+ m_nonce = m_requestMessage->getMessagePayload();
+ ASSERT(m_nonce.size() == kHidInitNonceLength);
+ }
+
m_currentChannel = m_requestMessage->channelId();
m_requestMessage = std::nullopt;
if (m_configuration.hid->fastDataArrival)
if (m_subStage == Mock::SubStage::Init) {
Vector<uint8_t> payload;
payload.reserveInitialCapacity(kHidInitResponseSize);
- // FIXME(191533): Use a real nonce.
- payload.appendVector(Vector<uint8_t>({ 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07 }));
+ payload.appendVector(m_nonce);
+ if (stagesMatch() && m_configuration.hid->error == Mock::Error::WrongNonce)
+ payload[0]--;
payload.grow(kHidInitResponseSize);
cryptographicallyRandomValues(payload.data() + payload.size(), CtapChannelIdSize);
auto channel = kHidBroadcastChannel;
std::optional<FidoHidMessage> message;
if (m_stage == Mock::Stage::Info && m_subStage == Mock::SubStage::Msg) {
- auto infoData = encodeAsCBOR(AuthenticatorGetInfoResponse({ ProtocolVersion::kCtap }, Vector<uint8_t>(kAaguidLength)));
+ auto infoData = encodeAsCBOR(AuthenticatorGetInfoResponse({ ProtocolVersion::kCtap }, Vector<uint8_t>(kAaguidLength, 0u)));
infoData.insert(0, static_cast<uint8_t>(CtapDeviceResponseCode::kSuccess)); // Prepend status code.
if (stagesMatch() && m_configuration.hid->error == Mock::Error::WrongChannelId)
message = FidoHidMessage::create(m_currentChannel - 1, FidoHidDeviceCommand::kCbor, infoData);
uint32_t m_currentChannel { fido::kHidBroadcastChannel };
bool m_requireResidentKey { false };
bool m_requireUserVerification { false };
+ Vector<uint8_t> m_nonce;
};
} // namespace WebKit
EmptyReport,
WrongChannelId,
MaliciousPayload,
- UnsupportedOptions
+ UnsupportedOptions,
+ WrongNonce
};
String payloadBase64;
#if ENABLE(WEB_AUTHN) && PLATFORM(MAC)
#include <WebCore/FidoConstants.h>
+#include <wtf/CryptographicallyRandomNumber.h>
#include <wtf/RunLoop.h>
+#include <wtf/Vector.h>
#include <wtf/text/Base64.h>
namespace WebKit {
m_responseMessage.reset();
m_callback = WTFMove(callback);
+ // HidConnection could hold data from other applications, and thereofore invalidate it before each transaction.
+ m_connection->invalidateCache();
m_connection->send(m_requestMessage->popNextPacket(), [weakThis = makeWeakPtr(*this)](HidConnection::DataSent sent) mutable {
ASSERT(RunLoop::isMain());
if (!weakThis)
CtapHidDriver::CtapHidDriver(UniqueRef<HidConnection>&& connection)
: m_worker(makeUniqueRef<Worker>(WTFMove(connection)))
+ , m_nonce(kHidInitNonceLength)
{
}
{
ASSERT(m_state == State::Idle);
m_state = State::AllocateChannel;
+ m_channelId = kHidBroadcastChannel;
m_requestData = WTFMove(data);
m_responseCallback = WTFMove(callback);
// Allocate a channel.
- // FIXME(191533): Get a real nonce.
- auto initCommand = FidoHidMessage::create(kHidBroadcastChannel, FidoHidDeviceCommand::kInit, { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07 });
+ ASSERT(m_nonce.size() == kHidInitNonceLength);
+ cryptographicallyRandomValues(m_nonce.data(), m_nonce.size());
+ auto initCommand = FidoHidMessage::create(m_channelId, FidoHidDeviceCommand::kInit, m_nonce);
ASSERT(initCommand);
m_worker->transact(WTFMove(*initCommand), [weakThis = makeWeakPtr(*this)](std::optional<FidoHidMessage>&& response) mutable {
ASSERT(RunLoop::isMain());
returnResponse({ });
return;
}
- m_state = State::Ready;
ASSERT(message->channelId() == m_channelId);
- // FIXME(191534): Check Payload including nonce and everything
auto payload = message->getMessagePayload();
- size_t index = 8;
ASSERT(payload.size() == kHidInitResponseSize);
+ // Restart the transaction in the next run loop when nonce mismatches.
+ if (memcmp(payload.data(), m_nonce.data(), m_nonce.size())) {
+ m_state = State::Idle;
+ RunLoop::main().dispatch([weakThis = makeWeakPtr(*this), data = WTFMove(m_requestData), callback = WTFMove(m_responseCallback)]() mutable {
+ if (!weakThis)
+ return;
+ weakThis->transact(WTFMove(data), WTFMove(callback));
+ });
+ return;
+ }
+
+ m_state = State::Ready;
+ auto index = kHidInitNonceLength;
m_channelId = static_cast<uint32_t>(payload[index++]) << 24;
m_channelId |= static_cast<uint32_t>(payload[index++]) << 16;
m_channelId |= static_cast<uint32_t>(payload[index++]) << 8;
m_channelId |= static_cast<uint32_t>(payload[index]);
+ // FIXME(191534): Check the reset of the payload.
auto cmd = FidoHidMessage::create(m_channelId, FidoHidDeviceCommand::kCbor, m_requestData);
ASSERT(cmd);
m_worker->transact(WTFMove(*cmd), [weakThis = makeWeakPtr(*this)](std::optional<FidoHidMessage>&& response) mutable {
{
// Reset state before calling the response callback to avoid being deleted.
m_state = State::Idle;
- m_channelId = fido::kHidBroadcastChannel;
- m_requestData = { };
m_responseCallback(WTFMove(response));
}
private:
// Worker is the helper that maintains the transaction.
// https://fidoalliance.org/specs/fido-v2.0-ps-20170927/fido-client-to-authenticator-protocol-v2.0-ps-20170927.html#arbitration
- // FSM: Idle => Write => Read
+ // FSM: Idle => Write => Read.
class Worker : public CanMakeWeakPtr<Worker> {
WTF_MAKE_FAST_ALLOCATED;
WTF_MAKE_NONCOPYABLE(Worker);
// One request at a time.
Vector<uint8_t> m_requestData;
ResponseCallback m_responseCallback;
+ Vector<uint8_t> m_nonce;
};
} // namespace WebKit
git://git.webkit.org / WebKit-https.git / commitdiff