Skip to content

GitLab

T
tangled
Commit aa1faf82 authored by Vadym Doroshenko's avatar Vadym Doroshenko Committed by Commit Bot
Browse files
Options

Revert "Give the desktop a public key in QR handshakes." 

This reverts commit fccf3a8f.

Reason for revert: 
This CL breaks on Linux MSan bot
FidoCableDiscoveryTest.TestDiscoveryFindsNewAppleDevice
FidoCableDiscoveryTest.TestDiscoveryFindsIncorrectDevice

First failure: 
https://ci.chromium.org/p/chromium/builders/ci/Linux%20MSan%20Tests/22909

Original change's description:
> Give the desktop a public key in QR handshakes.
> 
> This change switches the QR-based handshake from NNpsk0 to KNpsk0. I.e.
> it gives the desktop a public-key. That key is included in the QR code
> and means that an attacker who sees the QR code cannot impersonate the
> desktop and pair with the phone. (The other direction is still possible,
> however, and seems fundamental without the user entering a confirmation
> code on the desktop.)
> 
> BUG=1002262
> 
> Change-Id: I7254a8aa17df71d2bcabb4371a1c18914ed20641
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2109032
> Commit-Queue: Adam Langley <agl@chromium.org>
> Auto-Submit: Adam Langley <agl@chromium.org>
> Reviewed-by: Martin Kreichgauer <martinkr@google.com>
> Cr-Commit-Position: refs/heads/master@{#752196}

TBR=agl@chromium.org,martinkr@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1002262
Change-Id: Id75ad0bab2862b3dafe73d20cf762883cc24faf9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2113771Reviewed-by: default avatarVadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752403}
parent f738c9c7
Hide whitespace changes
Inline Side-by-side
Showing with 79 additions and 278 deletions
chrome/android/features/cablev2_authenticator/internal/native/cablev2_authenticator_android.cc
View file @ aa1faf82
......@@ -175,8 +175,6 @@ struct AuthenticatorState {
base::Optional<device::cablev2::NonceAndEID> qr_advert;
// qr_psk_gen_key contains the PSK generating key derived from the QR secret.
base::Optional<device::CablePskGeneratorKey> qr_psk_gen_key;
// peer_identity is the public-key of the desktop from the scanned QR code.
base::Optional<bssl::UniquePtr<EC_POINT>> qr_peer_identity;
};
// Client represents the state of a single BLE peer.
......@@ -275,16 +273,15 @@ class Client {
handshake_result = device::cablev2::RespondToHandshake(
auth_state_->pairing_data.v2->psk_gen_key,
auth_state_->pairing_advert, auth_state_->identity_key.get(),
/*peer_identity=*/nullptr, /*pairing_data=*/nullptr,
message->second, &response);
/*pairing_data=*/nullptr, message->second, &response);
} else if (auth_state_->qr_advert.has_value() &&
requested_eid == auth_state_->qr_advert->second) {
// TODO: QR handshakes currently always send pairing data, but it's
// optional in the protocol.
handshake_result = device::cablev2::RespondToHandshake(
*auth_state_->qr_psk_gen_key, *auth_state_->qr_advert,
/*identity=*/nullptr, auth_state_->qr_peer_identity->get(),
&auth_state_->pairing_data, message->second, &response);
/*identity=*/nullptr, &auth_state_->pairing_data, message->second,
&response);
} else {
FIDO_LOG(ERROR) << "Peer is connecting to unknown EID "
<< base::HexEncode(requested_eid);
......@@ -490,36 +487,21 @@ class CableInterface : public Client::Delegate {
base::StringPiece qr_url_base64(qr_url);
qr_url_base64 = qr_url_base64.substr(sizeof(kPrefix) - 1);
std::string qr_data_str;
std::string qr_secret_str;
if (!base::Base64UrlDecode(qr_url_base64,
base::Base64UrlDecodePolicy::DISALLOW_PADDING,
&qr_data_str) ||
qr_data_str.size() != device::kCableQRDataSize) {
&qr_secret_str) ||
qr_secret_str.size() != device::kCableQRSecretSize) {
FIDO_LOG(ERROR) << "QR decoding failed: " << qr_url;
return;
}
const base::Optional<device::CableDiscoveryData> discovery_data =
device::CableDiscoveryData::FromQRData(
base::span<const uint8_t, device::kCableQRDataSize>(
reinterpret_cast<const uint8_t*>(qr_data_str.data()),
qr_data_str.size()));
if (!discovery_data) {
FIDO_LOG(ERROR) << "Failed to decode QR data from: " << qr_url;
return;
}
auth_state_.qr_psk_gen_key.emplace(discovery_data->v2->psk_gen_key);
bssl::UniquePtr<EC_GROUP> p256(
EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1));
auth_state_.qr_peer_identity.emplace(EC_POINT_new(p256.get()));
CHECK(EC_POINT_oct2point(p256.get(), auth_state_.qr_peer_identity->get(),
discovery_data->v2->peer_identity->data(),
discovery_data->v2->peer_identity->size(),
/*ctx=*/nullptr));
uint8_t qr_secret[device::kCableQRSecretSize];
memcpy(qr_secret, qr_secret_str.data(), sizeof(qr_secret));
const device::CableDiscoveryData discovery_data(qr_secret);
auth_state_.qr_psk_gen_key.emplace(discovery_data.v2->psk_gen_key);
StartAdvertising(discovery_data->v2->eid_gen_key,
StartAdvertising(discovery_data.v2->eid_gen_key,
&auth_state_.qr_advert.emplace());
}
......
chrome/browser/ui/views/webauthn/authenticator_qr_sheet_view.cc
View file @ aa1faf82
......@@ -172,26 +172,27 @@ base::span<uint8_t> QRDataForCurrentTime(
uint8_t out_buf[QRCode::kInputBytes],
base::span<const uint8_t, 32> qr_generator_key) {
const int64_t current_tick = device::CableDiscoveryData::CurrentTimeTick();
const device::CableQRData qr_data =
device::CableDiscoveryData::DeriveQRData(qr_generator_key, current_tick);
auto qr_secret = device::CableDiscoveryData::DeriveQRSecret(qr_generator_key,
current_tick);
std::string base64_qr_data;
std::string base64_qr_secret;
base::Base64UrlEncode(
base::StringPiece(reinterpret_cast<const char*>(qr_data.data()),
sizeof(qr_data)),
base::Base64UrlEncodePolicy::OMIT_PADDING, &base64_qr_data);
static constexpr size_t kEncodedDataLength =
Base64EncodedSize(sizeof(qr_data));
DCHECK_EQ(kEncodedDataLength, base64_qr_data.size());
base::StringPiece(reinterpret_cast<const char*>(qr_secret.data()),
qr_secret.size()),
base::Base64UrlEncodePolicy::OMIT_PADDING, &base64_qr_secret);
static constexpr size_t kEncodedSecretLength =
Base64EncodedSize(sizeof(qr_secret));
DCHECK_EQ(kEncodedSecretLength, base64_qr_secret.size());
static constexpr char kPrefix[] = "fido://c1/";
static constexpr size_t kPrefixLength = sizeof(kPrefix) - 1;
static_assert(QRCode::kInputBytes >= kPrefixLength + kEncodedDataLength,
static_assert(QRCode::kInputBytes >= kPrefixLength + kEncodedSecretLength,
"unexpected QR input length");
memcpy(out_buf, kPrefix, kPrefixLength);
memcpy(&out_buf[kPrefixLength], base64_qr_data.data(), kEncodedDataLength);
return base::span<uint8_t>(out_buf, kPrefixLength + kEncodedDataLength);
memcpy(&out_buf[kPrefixLength], base64_qr_secret.data(),
kEncodedSecretLength);
return base::span<uint8_t>(out_buf, kPrefixLength + kEncodedSecretLength);
}
} // anonymous namespace
......
device/fido/cable/cable_discovery_data.h
View file @ aa1faf82
......@@ -17,11 +17,6 @@ constexpr size_t kCableEphemeralIdSize = 16;
constexpr size_t kCableSessionPreKeySize = 32;
constexpr size_t kCableQRSecretSize = 16;
constexpr size_t kCableNonceSize = 8;
constexpr size_t kCableIdentityKeySeedSize = 32;
constexpr size_t kCableCompressedPublicKeySize =
/* type byte */ 1 + /* field element */ (256 / 8);
constexpr size_t kCableQRDataSize =
kCableCompressedPublicKeySize + kCableQRSecretSize;
using CableEidArray = std::array<uint8_t, kCableEphemeralIdSize>;
using CableSessionPreKeyArray = std::array<uint8_t, kCableSessionPreKeySize>;
......@@ -40,8 +35,6 @@ using CablePskGeneratorKey = std::array<uint8_t, 32>;
// CableAuthenticatorIdentityKey is a P-256 public value used to authenticate a
// paired phone.
using CableAuthenticatorIdentityKey = std::array<uint8_t, 65>;
using CableIdentityKeySeed = std::array<uint8_t, kCableIdentityKeySeedSize>;
using CableQRData = std::array<uint8_t, kCableQRDataSize>;
// Encapsulates information required to discover Cable device per single
// credential. When multiple credentials are enrolled to a single account
......@@ -61,25 +54,14 @@ struct COMPONENT_EXPORT(DEVICE_FIDO) CableDiscoveryData {
const CableEidArray& client_eid,
const CableEidArray& authenticator_eid,
const CableSessionPreKeyArray& session_pre_key);
// Creates discovery data given a specific QR secret and identity key seed.
// This will be used on the QR-displaying-side of a QR handshake. See
// |DeriveQRSecret| and |DeriveIdentityKeySeed| for how to generate such
// secrets.
CableDiscoveryData(
base::span<const uint8_t, kCableQRSecretSize> qr_secret,
base::span<const uint8_t, kCableIdentityKeySeedSize> identity_key_seed);
// Creates discovery data given a specific QR secret. See |DeriveQRSecret| for
// how to generate such secrets.
explicit CableDiscoveryData(
base::span<const uint8_t, kCableQRSecretSize> qr_secret);
CableDiscoveryData();
CableDiscoveryData(const CableDiscoveryData& data);
~CableDiscoveryData();
// Creates discovery data given QR data, which contains a compressed public
// key and the QR secret. This will be used by the QR-scanning-side of a QR
// handshake. Returns |nullopt| if the embedded elliptic-curve point is
// invalid.
static base::Optional<CableDiscoveryData> FromQRData(
base::span<const uint8_t,
kCableCompressedPublicKeySize + kCableQRSecretSize> qr_data);
CableDiscoveryData& operator=(const CableDiscoveryData& other);
bool operator==(const CableDiscoveryData& other) const;
......@@ -100,19 +82,6 @@ struct COMPONENT_EXPORT(DEVICE_FIDO) CableDiscoveryData {
base::span<const uint8_t, 32> qr_generator_key,
const int64_t tick);
// DeriveIdentityKeySeed returns a seed that can be used to create a P-256
// identity key for a handshake using |EC_KEY_derive_from_secret|.
static CableIdentityKeySeed DeriveIdentityKeySeed(
base::span<const uint8_t, 32> qr_generator_key,
const int64_t tick);
// DeriveQRData returns the QR data, a combination of QR secret and public
// identity key. This is base64url-encoded and placed in a caBLE v2 QR code
// with a prefix prepended.
static CableQRData DeriveQRData(
base::span<const uint8_t, 32> qr_generator_key,
const int64_t tick);
// version indicates whether v1 or v2 data is contained in this object.
// |INVALID| is not a valid version but is set as the default to catch any
// cases where the version hasn't been set explicitly.
......@@ -133,7 +102,6 @@ struct COMPONENT_EXPORT(DEVICE_FIDO) CableDiscoveryData {
CableEidGeneratorKey eid_gen_key;
CablePskGeneratorKey psk_gen_key;
base::Optional<CableAuthenticatorIdentityKey> peer_identity;
base::Optional<CableIdentityKeySeed> local_identity_seed;
// peer_name is an authenticator-controlled, UTF8-valid string containing
// the self-reported, human-friendly name of a v2 authenticator. This need
// not be filled in when handshaking but an authenticator may provide it
......@@ -141,10 +109,6 @@ struct COMPONENT_EXPORT(DEVICE_FIDO) CableDiscoveryData {
base::Optional<std::string> peer_name;
};
base::Optional<V2Data> v2;
private:
void InitFromQRSecret(
base::span<const uint8_t, kCableQRSecretSize> qr_secret);
};
} // namespace device
......
device/fido/cable/fido_cable_discovery.cc
View file @ aa1faf82
......@@ -30,10 +30,8 @@
#include "device/fido/fido_parsing_utils.h"
#include "third_party/boringssl/src/include/openssl/aes.h"
#include "third_party/boringssl/src/include/openssl/digest.h"
#include "third_party/boringssl/src/include/openssl/ec.h"
#include "third_party/boringssl/src/include/openssl/hkdf.h"
#include "third_party/boringssl/src/include/openssl/mem.h"
#include "third_party/boringssl/src/include/openssl/obj.h"
namespace device {
......@@ -106,31 +104,6 @@ std::unique_ptr<BluetoothAdvertisement::Data> ConstructAdvertisementData(
return advertisement_data;
}
enum class QRValue : uint8_t {
QR_SECRET = 0,
IDENTITY_KEY_SEED = 1,
};
void DeriveQRValue(base::span<const uint8_t, 32> qr_generator_key,
const int64_t tick,
QRValue type,
base::span<uint8_t> out) {
// This function only needs to be consistent within a given running process.
// Thus it's fine if future compilers change the padding etc of this structure
// and thus change the HKDF output for a given key and tick count.
struct {
int64_t tick;
QRValue type;
} hkdf_input = {tick, type};
bool ok =
HKDF(out.data(), out.size(), EVP_sha256(), qr_generator_key.data(),
qr_generator_key.size(),
/*salt=*/nullptr, 0, reinterpret_cast<const uint8_t*>(&hkdf_input),
sizeof(hkdf_input));
DCHECK(ok);
}
} // namespace
// CableDiscoveryData -------------------------------------
......@@ -151,36 +124,22 @@ CableDiscoveryData::CableDiscoveryData(
}
CableDiscoveryData::CableDiscoveryData(
base::span<const uint8_t, kCableQRSecretSize> qr_secret,
base::span<const uint8_t, kCableIdentityKeySeedSize> identity_key_seed) {
InitFromQRSecret(qr_secret);
v2->local_identity_seed = fido_parsing_utils::Materialize(identity_key_seed);
}
base::span<const uint8_t, kCableQRSecretSize> qr_secret) {
version = Version::V2;
v2.emplace();
// static
base::Optional<CableDiscoveryData> CableDiscoveryData::FromQRData(
base::span<const uint8_t,
kCableCompressedPublicKeySize + kCableQRSecretSize> qr_data) {
auto qr_secret = qr_data.subspan(kCableCompressedPublicKeySize);
CableDiscoveryData discovery_data;
discovery_data.InitFromQRSecret(base::span<const uint8_t, kCableQRSecretSize>(
qr_secret.data(), qr_secret.size()));
bssl::UniquePtr<EC_GROUP> p256(
EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1));
bssl::UniquePtr<EC_POINT> point(EC_POINT_new(p256.get()));
if (!EC_POINT_oct2point(p256.get(), point.get(), qr_data.data(),
kCableCompressedPublicKeySize, /*ctx=*/nullptr)) {
return base::nullopt;
}
CableAuthenticatorIdentityKey& identity_key =
discovery_data.v2->peer_identity.emplace();
CHECK_EQ(identity_key.size(),
EC_POINT_point2oct(
p256.get(), point.get(), POINT_CONVERSION_UNCOMPRESSED,
identity_key.data(), identity_key.size(), /*ctx=*/nullptr));
static const char kEIDGen[] = "caBLE QR to EID generator key";
bool ok =
HKDF(v2->eid_gen_key.data(), v2->eid_gen_key.size(), EVP_sha256(),
qr_secret.data(), qr_secret.size(), /*salt=*/nullptr, 0,
reinterpret_cast<const uint8_t*>(kEIDGen), sizeof(kEIDGen) - 1);
DCHECK(ok);
return discovery_data;
static const char kPSKGen[] = "caBLE QR to PSK generator key";
ok = HKDF(v2->psk_gen_key.data(), v2->psk_gen_key.size(), EVP_sha256(),
qr_secret.data(), qr_secret.size(), /*salt=*/nullptr, 0,
reinterpret_cast<const uint8_t*>(kPSKGen), sizeof(kPSKGen) - 1);
DCHECK(ok);
}
CableDiscoveryData::CableDiscoveryData(const CableDiscoveryData& data) =
......@@ -281,69 +240,24 @@ int64_t CableDiscoveryData::CurrentTimeTick() {
std::array<uint8_t, kCableQRSecretSize> CableDiscoveryData::DeriveQRSecret(
base::span<const uint8_t, 32> qr_generator_key,
const int64_t tick) {
std::array<uint8_t, kCableQRSecretSize> ret;
DeriveQRValue(qr_generator_key, tick, QRValue::QR_SECRET, ret);
return ret;
}
union {
int64_t i;
uint8_t bytes[8];
} current_tick;
current_tick.i = tick;
// static
CableIdentityKeySeed CableDiscoveryData::DeriveIdentityKeySeed(
base::span<const uint8_t, 32> qr_generator_key,
const int64_t tick) {
std::array<uint8_t, kCableIdentityKeySeedSize> ret;
DeriveQRValue(qr_generator_key, tick, QRValue::IDENTITY_KEY_SEED, ret);
std::array<uint8_t, kCableQRSecretSize> ret;
bool ok = HKDF(ret.data(), ret.size(), EVP_sha256(), qr_generator_key.data(),
qr_generator_key.size(),
/*salt=*/nullptr, 0, current_tick.bytes, sizeof(current_tick));
DCHECK(ok);
return ret;
}
// static
CableQRData CableDiscoveryData::DeriveQRData(
base::span<const uint8_t, 32> qr_generator_key,
const int64_t tick) {
auto identity_key_seed = DeriveIdentityKeySeed(qr_generator_key, tick);
bssl::UniquePtr<EC_GROUP> p256(
EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1));
bssl::UniquePtr<EC_KEY> identity_key(EC_KEY_derive_from_secret(
p256.get(), identity_key_seed.data(), identity_key_seed.size()));
const EC_POINT* public_key = EC_KEY_get0_public_key(identity_key.get());
CableQRData qr_data;
static_assert(
qr_data.size() == kCableCompressedPublicKeySize + kCableQRSecretSize,
"this code needs to be updated");
CHECK_EQ(kCableCompressedPublicKeySize,
EC_POINT_point2oct(p256.get(), public_key,
POINT_CONVERSION_COMPRESSED, qr_data.data(),
kCableCompressedPublicKeySize, /*ctx=*/nullptr));
auto qr_secret = CableDiscoveryData::DeriveQRSecret(qr_generator_key, tick);
memcpy(&qr_data.data()[kCableCompressedPublicKeySize], qr_secret.data(),
qr_secret.size());
return qr_data;
}
CableDiscoveryData::V2Data::V2Data() = default;
CableDiscoveryData::V2Data::V2Data(const V2Data&) = default;
CableDiscoveryData::V2Data::~V2Data() = default;
void CableDiscoveryData::InitFromQRSecret(
base::span<const uint8_t, kCableQRSecretSize> qr_secret) {
version = Version::V2;
v2.emplace();
static const char kEIDGen[] = "caBLE QR to EID generator key";
bool ok =
HKDF(v2->eid_gen_key.data(), v2->eid_gen_key.size(), EVP_sha256(),
qr_secret.data(), qr_secret.size(), /*salt=*/nullptr, 0,
reinterpret_cast<const uint8_t*>(kEIDGen), sizeof(kEIDGen) - 1);
DCHECK(ok);
static const char kPSKGen[] = "caBLE QR to PSK generator key";
ok = HKDF(v2->psk_gen_key.data(), v2->psk_gen_key.size(), EVP_sha256(),
qr_secret.data(), qr_secret.size(), /*salt=*/nullptr, 0,
reinterpret_cast<const uint8_t*>(kPSKGen), sizeof(kPSKGen) - 1);
DCHECK(ok);
}
// FidoCableDiscovery::CableV1DiscoveryEvent ---------------------------------
// CableV1DiscoveryEvent enumerates several things that can occur during a caBLE
......@@ -459,8 +373,7 @@ FidoCableDiscovery::CreateHandshakeHandler(
handler.reset(new FidoCableV2HandshakeHandler(
device, discovery_data.v2->psk_gen_key, nonce, eid,
discovery_data.v2->peer_identity,
discovery_data.v2->local_identity_seed, *pairing_callback_));
discovery_data.v2->peer_identity, *pairing_callback_));
break;
}
......@@ -869,9 +782,7 @@ FidoCableDiscovery::GetCableDiscoveryDataFromAuthenticatorEid(
for (int i = 0; i < kNumPreviousTicks; i++) {
auto qr_secret = CableDiscoveryData::DeriveQRSecret(*qr_generator_key_,
current_tick - i);
auto identity_key_seed = CableDiscoveryData::DeriveIdentityKeySeed(
*qr_generator_key_, current_tick - i);
CableDiscoveryData candidate(qr_secret, identity_key_seed);
CableDiscoveryData candidate(qr_secret);
auto maybe_nonce = candidate.Match(authenticator_eid);
if (maybe_nonce) {
return Result(candidate, *maybe_nonce, authenticator_eid, i);
......@@ -879,11 +790,10 @@ FidoCableDiscovery::GetCableDiscoveryDataFromAuthenticatorEid(
}
if (base::Contains(noted_obsolete_eids_, authenticator_eid)) {
std::array<uint8_t, kCableIdentityKeySeedSize> dummy_seed;
for (int i = kNumPreviousTicks; i < 2 * kNumPreviousTicks; i++) {
auto qr_secret = CableDiscoveryData::DeriveQRSecret(*qr_generator_key_,
current_tick - i);
CableDiscoveryData candidate(qr_secret, dummy_seed);
CableDiscoveryData candidate(qr_secret);
if (candidate.Match(authenticator_eid)) {
noted_obsolete_eids_.insert(authenticator_eid);
FIDO_LOG(DEBUG)
......
device/fido/cable/fido_cable_handshake_handler.cc
View file @ aa1faf82
......@@ -187,13 +187,11 @@ FidoCableV2HandshakeHandler::FidoCableV2HandshakeHandler(
base::span<const uint8_t, 8> nonce,
base::span<const uint8_t, kCableEphemeralIdSize> eid,
base::Optional<base::span<const uint8_t, kP256PointSize>> peer_identity,
base::Optional<base::span<const uint8_t, kCableIdentityKeySeedSize>>
local_seed,
base::RepeatingCallback<void(std::unique_ptr<CableDiscoveryData>)>
pairing_callback)
: cable_device_(cable_device),
pairing_callback_(std::move(pairing_callback)),
handshake_(psk_gen_key, nonce, eid, peer_identity, local_seed) {}
handshake_(psk_gen_key, nonce, eid, peer_identity) {}
FidoCableV2HandshakeHandler::~FidoCableV2HandshakeHandler() = default;
......
device/fido/cable/fido_cable_handshake_handler.h
View file @ aa1faf82
......@@ -87,8 +87,6 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableV2HandshakeHandler
base::span<const uint8_t, 8> nonce,
base::span<const uint8_t, kCableEphemeralIdSize> eid,
base::Optional<base::span<const uint8_t, 65>> peer_identity,
base::Optional<base::span<const uint8_t, kCableIdentityKeySeedSize>>
local_seed,
base::RepeatingCallback<void(std::unique_ptr<CableDiscoveryData>)>
pairing_callback);
~FidoCableV2HandshakeHandler() override;
......
device/fido/cable/fido_cable_handshake_handler_v2_fuzzer.cc
View file @ aa1faf82
......@@ -36,11 +36,6 @@ constexpr std::array<uint8_t, 65> kTestPeerIdentity = {
0x9c, 0xa5, 0x5c, 0x51, 0x2f, 0x9e, 0x4a, 0x00, 0x12, 0x66,
};
constexpr char kTestDeviceAddress[] = "Fake_Address";
constexpr std::array<uint8_t, 32> kLocalSeed = {
0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05,
0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05,
0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05,
};
} // namespace
......@@ -60,7 +55,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* raw_data, size_t size) {
device::FidoCableV2HandshakeHandler handshake_handler_v2(
&test_cable_device, kTestPSKGeneratorKey, kTestNonce, kTestEphemeralID,
peer_identity, kLocalSeed, base::DoNothing());
peer_identity, base::DoNothing());
handshake_handler_v2.InitiateCableHandshake(base::DoNothing());
handshake_handler_v2.ValidateAuthenticatorHandshakeMessage(input);
return 0;
......
device/fido/cable/noise.cc
View file @ aa1faf82
......@@ -59,11 +59,11 @@ Noise::~Noise() = default;
void Noise::Init(Noise::HandshakeType type) {
// See https://www.noiseprotocol.org/noise.html#the-handshakestate-object
static const char kKNProtocolName[] = "Noise_KNpsk0_P256_AESGCM_SHA256";
static const char kNNProtocolName[] = "Noise_NNpsk0_P256_AESGCM_SHA256";
static const char kNKProtocolName[] = "Noise_NKpsk0_P256_AESGCM_SHA256";
static_assert(sizeof(kNKProtocolName) == sizeof(kKNProtocolName),
static_assert(sizeof(kNKProtocolName) == sizeof(kNNProtocolName),
"protocol names are different lengths");
static_assert(sizeof(kKNProtocolName) == crypto::kSHA256Length,
static_assert(sizeof(kNNProtocolName) == crypto::kSHA256Length,
"name may need padding if not HASHLEN bytes long");
static_assert(
std::tuple_size<decltype(chaining_key_)>::value == crypto::kSHA256Length,
......@@ -76,8 +76,8 @@ void Noise::Init(Noise::HandshakeType type) {
memcpy(chaining_key_.data(), kNKProtocolName, sizeof(kNKProtocolName));
break;
case HandshakeType::kKNpsk0:
memcpy(chaining_key_.data(), kKNProtocolName, sizeof(kKNProtocolName));
case HandshakeType::kNNpsk0:
memcpy(chaining_key_.data(), kNNProtocolName, sizeof(kNNProtocolName));
break;
}
......
device/fido/cable/noise.h
View file @ aa1faf82
......@@ -22,7 +22,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) Noise {
public:
// HandshakeType enumerates the supported handshake patterns.
enum class HandshakeType {
kKNpsk0, // https://noiseexplorer.com/patterns/KNpsk0/
kNNpsk0, // https://noiseexplorer.com/patterns/NNpsk0/
kNKpsk0, // https://noiseexplorer.com/patterns/NKpsk0/
};
......
device/fido/cable/v2_handshake.cc
View file @ aa1faf82
......@@ -171,20 +171,14 @@ HandshakeInitiator::HandshakeInitiator(
base::span<const uint8_t, 32> psk_gen_key,
base::span<const uint8_t, 8> nonce,
base::span<const uint8_t, kCableEphemeralIdSize> eid,
base::Optional<base::span<const uint8_t, kP256PointSize>> peer_identity,
base::Optional<base::span<const uint8_t, kCableIdentityKeySeedSize>>
local_seed)
base::Optional<base::span<const uint8_t, kP256PointSize>> peer_identity)
: eid_(fido_parsing_utils::Materialize(eid)) {
DCHECK(peer_identity.has_value() ^ local_seed.has_value());
HKDF(psk_.data(), psk_.size(), EVP_sha256(), psk_gen_key.data(),
psk_gen_key.size(), /*salt=*/nonce.data(), nonce.size(),
/*info=*/nullptr, 0);
if (peer_identity) {
peer_identity_ = fido_parsing_utils::Materialize(*peer_identity);
}
if (local_seed) {
local_seed_ = fido_parsing_utils::Materialize(*local_seed);
}
}
HandshakeInitiator::~HandshakeInitiator() = default;
......@@ -194,7 +188,7 @@ std::vector<uint8_t> HandshakeInitiator::BuildInitialMessage() {
noise_.Init(Noise::HandshakeType::kNKpsk0);
noise_.MixHash(kPairedPrologue);
} else {
noise_.Init(Noise::HandshakeType::kKNpsk0);
noise_.Init(Noise::HandshakeType::kNNpsk0);
noise_.MixHash(kQRPrologue);
}
......@@ -252,11 +246,11 @@ HandshakeInitiator::ProcessResponse(base::span<const uint8_t> response) {
bssl::UniquePtr<EC_POINT> peer_point(
EC_POINT_new(EC_KEY_get0_group(ephemeral_key_.get())));
uint8_t shared_key_ee[32];
uint8_t shared_key[32];
const EC_GROUP* group = EC_KEY_get0_group(ephemeral_key_.get());
if (!EC_POINT_oct2point(group, peer_point.get(), peer_point_bytes.data(),
peer_point_bytes.size(), /*ctx=*/nullptr) ||
!ECDH_compute_key(shared_key_ee, sizeof(shared_key_ee), peer_point.get(),
!ECDH_compute_key(shared_key, sizeof(shared_key), peer_point.get(),
ephemeral_key_.get(), /*kdf=*/nullptr)) {
FIDO_LOG(DEBUG) << "Peer's P-256 point not on curve.";
return base::nullopt;
......@@ -264,19 +258,7 @@ HandshakeInitiator::ProcessResponse(base::span<const uint8_t> response) {
noise_.MixHash(peer_point_bytes);
noise_.MixKey(peer_point_bytes);
noise_.MixKey(shared_key_ee);
if (local_seed_) {
uint8_t shared_key_se[32];
bssl::UniquePtr<EC_KEY> identity_key(EC_KEY_derive_from_secret(
group, local_seed_->data(), local_seed_->size()));
if (!ECDH_compute_key(shared_key_se, sizeof(shared_key_se),
peer_point.get(), identity_key.get(),
/*kdf=*/nullptr)) {
return base::nullopt;
}
noise_.MixKey(shared_key_se);
}
noise_.MixKey(shared_key);
auto plaintext = noise_.DecryptAndHash(ciphertext);
if (!plaintext || plaintext->empty() != peer_identity_.has_value()) {
......@@ -286,7 +268,7 @@ HandshakeInitiator::ProcessResponse(base::span<const uint8_t> response) {
base::Optional<std::unique_ptr<CableDiscoveryData>> discovery_data;
if (!peer_identity_) {
// Handshakes without a peer identity (i.e. KNpsk0 handshakes setup from a
// Handshakes without a peer identity (i.e. NNpsk0 handshakes setup from a
// QR code) send a padded message in the reply. This message can,
// optionally, contain CBOR-encoded, long-term pairing information.
const size_t padding_length = (*plaintext)[plaintext->size() - 1];
......@@ -340,12 +322,10 @@ base::Optional<std::unique_ptr<Crypter>> RespondToHandshake(
base::span<const uint8_t, 32> psk_gen_key,
const NonceAndEID& nonce_and_eid,
const EC_KEY* identity,
const EC_POINT* peer_identity,
const CableDiscoveryData* pairing_data,
base::span<const uint8_t> in,
std::vector<uint8_t>* out_response) {
DCHECK(identity == nullptr || pairing_data == nullptr);
DCHECK(identity == nullptr ^ peer_identity == nullptr);
CBS cbs = CBSFromSpan(in);
......@@ -368,7 +348,7 @@ base::Optional<std::unique_ptr<Crypter>> RespondToHandshake(
noise.Init(device::Noise::HandshakeType::kNKpsk0);
noise.MixHash(kPairedPrologue);
} else {
noise.Init(device::Noise::HandshakeType::kKNpsk0);
noise.Init(device::Noise::HandshakeType::kNNpsk0);
noise.MixHash(kQRPrologue);
}
......@@ -418,21 +398,12 @@ base::Optional<std::unique_ptr<Crypter>> RespondToHandshake(
noise.MixHash(ephemeral_key_public_bytes);
noise.MixKey(ephemeral_key_public_bytes);
uint8_t shared_key_ee[32];
if (!ECDH_compute_key(shared_key_ee, sizeof(shared_key_ee), peer_point.get(),
uint8_t shared_key[32];
if (!ECDH_compute_key(shared_key, sizeof(shared_key), peer_point.get(),
ephemeral_key.get(), /*kdf=*/nullptr)) {
return base::nullopt;
}
noise.MixKey(shared_key_ee);
if (peer_identity) {
uint8_t shared_key_se[32];
if (!ECDH_compute_key(shared_key_se, sizeof(shared_key_se), peer_identity,
ephemeral_key.get(), /*kdf=*/nullptr)) {
return base::nullopt;
}
noise.MixKey(shared_key_se);
}
noise.MixKey(shared_key);
std::vector<uint8_t> my_ciphertext;
if (!identity) {
......
device/fido/cable/v2_handshake.h
View file @ aa1faf82
......@@ -78,11 +78,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) HandshakeInitiator {
// peer_identity, if given, specifies that this is a paired handshake
// and then contains an X9.62, P-256 public key for the peer. Otherwise
// this is a QR-code handshake.
base::Optional<base::span<const uint8_t, kP256PointSize>> peer_identity,
// local_identity must be provided if |peer_identity| is not. It contains
// the seed for deriving the local identity key.
base::Optional<base::span<const uint8_t, kCableIdentityKeySeedSize>>
local_identity);
base::Optional<base::span<const uint8_t, kP256PointSize>> peer_identity);
~HandshakeInitiator();
......@@ -104,7 +100,6 @@ class COMPONENT_EXPORT(DEVICE_FIDO) HandshakeInitiator {
std::array<uint8_t, 32> psk_;
base::Optional<std::array<uint8_t, kP256PointSize>> peer_identity_;
base::Optional<std::array<uint8_t, kCableIdentityKeySeedSize>> local_seed_;
bssl::UniquePtr<EC_KEY> ephemeral_key_;
};
......@@ -117,9 +112,6 @@ base::Optional<std::unique_ptr<Crypter>> RespondToHandshake(
// identity, if not nullptr, specifies that this is a paired handshake and
// contains the long-term identity key for this authenticator.
const EC_KEY* identity,
// peer_identity, which must be non-nullptr iff |identity| is nullptr,
// contains the peer's public key as derived from the QR-code data.
const EC_POINT* peer_identity,
// pairing_data, if not nullptr, contains long-term pairing data that will
// be shared with the peer. This is mutually exclusive with |identity|.
const CableDiscoveryData* pairing_data,
......
device/fido/cable/v2_handshake_unittest.cc
View file @ aa1faf82
......@@ -19,7 +19,6 @@ class CableV2HandshakeTest : public ::testing::Test {
std::fill(psk_gen_key_.begin(), psk_gen_key_.end(), 0);
std::fill(nonce_and_eid_.first.begin(), nonce_and_eid_.first.end(), 1);
std::fill(nonce_and_eid_.second.begin(), nonce_and_eid_.second.end(), 2);
std::fill(local_seed_.begin(), local_seed_.end(), 3);
p256_key_.reset(EC_KEY_new_by_curve_name(NID_X9_62_prime256v1));
const EC_GROUP* group = EC_KEY_get0_group(p256_key_.get());
......@@ -29,19 +28,13 @@ class CableV2HandshakeTest : public ::testing::Test {
POINT_CONVERSION_UNCOMPRESSED,
p256_public_key_.data(),
p256_public_key_.size(), /*ctx=*/nullptr));
bssl::UniquePtr<EC_KEY> qr_identity(EC_KEY_derive_from_secret(
group, local_seed_.data(), local_seed_.size()));
qr_identity_.reset(
EC_POINT_dup(EC_KEY_get0_public_key(qr_identity.get()), group));
}
protected:
std::array<uint8_t, 32> psk_gen_key_;
NonceAndEID nonce_and_eid_;
bssl::UniquePtr<EC_KEY> p256_key_;
bssl::UniquePtr<EC_POINT> qr_identity_;
std::array<uint8_t, kP256PointSize> p256_public_key_;
std::array<uint8_t, kCableIdentityKeySeedSize> local_seed_;
};
TEST_F(CableV2HandshakeTest, MessageEncrytion) {
......@@ -80,13 +73,12 @@ TEST_F(CableV2HandshakeTest, OneTimeQRHandshake) {
HandshakeInitiator initiator(
use_correct_key ? psk_gen_key_ : wrong_psk_gen_key,
nonce_and_eid_.first, nonce_and_eid_.second,
/*peer_identity=*/base::nullopt, local_seed_);
/*peer_identity=*/base::nullopt);
std::vector<uint8_t> message = initiator.BuildInitialMessage();
std::vector<uint8_t> response;
base::Optional<std::unique_ptr<Crypter>> response_crypter(
RespondToHandshake(psk_gen_key_, nonce_and_eid_, /*identity=*/nullptr,
qr_identity_.get(), /*pairing_data=*/nullptr,
message, &response));
/*pairing_data=*/nullptr, message, &response));
ASSERT_EQ(response_crypter.has_value(), use_correct_key);
if (!use_correct_key) {
continue;
......@@ -113,12 +105,12 @@ TEST_F(CableV2HandshakeTest, PairingQRHandshake) {
HandshakeInitiator initiator(psk_gen_key_, nonce_and_eid_.first,
nonce_and_eid_.second,
/*peer_identity=*/base::nullopt, local_seed_);
/*peer_identity=*/base::nullopt);
std::vector<uint8_t> message = initiator.BuildInitialMessage();
std::vector<uint8_t> response;
base::Optional<std::unique_ptr<Crypter>> response_crypter(
RespondToHandshake(psk_gen_key_, nonce_and_eid_, /*identity=*/nullptr,
qr_identity_.get(), &pairing, message, &response));
&pairing, message, &response));
ASSERT_TRUE(response_crypter.has_value());
base::Optional<std::pair<std::unique_ptr<Crypter>,
base::Optional<std::unique_ptr<CableDiscoveryData>>>>
......@@ -146,14 +138,12 @@ TEST_F(CableV2HandshakeTest, PairedHandshake) {
SCOPED_TRACE(use_correct_key);
HandshakeInitiator initiator(psk_gen_key_, nonce_and_eid_.first,
nonce_and_eid_.second, p256_public_key_,
/*local_seed=*/base::nullopt);
nonce_and_eid_.second, p256_public_key_);
std::vector<uint8_t> message = initiator.BuildInitialMessage();
std::vector<uint8_t> response;
base::Optional<std::unique_ptr<Crypter>> response_crypter(
RespondToHandshake(psk_gen_key_, nonce_and_eid_,
use_correct_key ? p256_key_.get() : wrong_key.get(),
/*peer_identity=*/nullptr,
/*pairing=*/nullptr, message, &response));
ASSERT_EQ(response_crypter.has_value(), use_correct_key);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment