Commit 1d08c092 authored by Adam Langley's avatar Adam Langley Committed by Commit Bot

device/fido: include a response length in commands.

Currently, no response length (L_e) is encoded in U2F commands. The U2F
spec[1] says “if the instruction is not expected to yield any response
bytes, L_e may be omitted” – i.e. we are specifying that no response is
allowed to our commands.

The VASCO SecureClick respects the maximum response length and thus
doesn't send a reply. Therefore Chromium's webauthn stack doesn't
currently work with these tokens.

This change causes us to always specify the maximum possible response
length for U2F register and sign commands, and fixes interop with the
VASCO token.

[1] https://fidoalliance.org/specs/fido-u2f-v1.2-ps-20170411/fido-u2f-raw-message-formats-v1.2-ps-20170411.html#extended-length-encoding

Change-Id: I5cc906eb6f167fb95c9a42a13ff46237d0e58b79
Reviewed-on: https://chromium-review.googlesource.com/979702
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546072}
parent bd6d9522
......@@ -119,8 +119,12 @@ std::vector<uint8_t> ApduCommand::GetEncodedCommand() const {
}
if (response_length_ > 0) {
encoded.push_back((response_length_ >> 8) & 0xff);
encoded.push_back(response_length_ & 0xff);
size_t response_length = response_length_;
if (response_length > kApduMaxResponseLength)
response_length = kApduMaxResponseLength;
// A zero value represents a response length of 65,536 bytes.
encoded.push_back((response_length >> 8) & 0xff);
encoded.push_back(response_length & 0xff);
}
return encoded;
}
......
......@@ -61,6 +61,8 @@ class COMPONENT_EXPORT(APDU) ApduCommand {
size_t response_length() const { return response_length_; }
const std::vector<uint8_t>& data() const { return data_; }
static constexpr size_t kApduMaxResponseLength = 65536;
private:
FRIEND_TEST_ALL_PREFIXES(ApduTest, TestDeserializeBasic);
FRIEND_TEST_ALL_PREFIXES(ApduTest, TestDeserializeComplex);
......@@ -76,7 +78,6 @@ class COMPONENT_EXPORT(APDU) ApduCommand {
// also limited to 16 bits in length with a value of 0x0000 corresponding to
// a length of 65536.
static constexpr size_t kApduMaxDataLength = 65535;
static constexpr size_t kApduMaxResponseLength = 65536;
static constexpr size_t kApduMaxLength =
kApduMaxDataLength + kApduMaxHeader + 2;
......
......@@ -343,7 +343,8 @@ TEST_F(U2fRegisterTest, TestCreateU2fRegisterCommand) {
0x07, 0x08, 0x09, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
0x09, 0x00, 0x01, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
0x09, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x00,
0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x00, 0x01,
0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x00, 0x01, 0x00,
0x00,
};
constexpr uint8_t kRegisterApduCommandWithAttestation[] = {
......@@ -359,6 +360,8 @@ TEST_F(U2fRegisterTest, TestCreateU2fRegisterCommand) {
0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x00, 0x01,
0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x00, 0x01, 0x02, 0x03,
0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x00, 0x01,
// Max response length
0x00, 0x00,
};
U2fRegister register_request(nullptr /* connector */, protocols_,
......
......@@ -97,6 +97,7 @@ base::Optional<std::vector<uint8_t>> U2fRequest::GetU2fSignApduCommand(
command.set_ins(base::strict_cast<uint8_t>(U2fApduInstruction::kSign));
command.set_p1(is_check_only_sign ? kP1CheckOnly : kP1TupRequiredConsumed);
command.set_data(data);
command.set_response_length(apdu::ApduCommand::kApduMaxResponseLength);
return command.GetEncodedCommand();
}
......@@ -114,6 +115,7 @@ base::Optional<std::vector<uint8_t>> U2fRequest::GetU2fRegisterApduCommand(
command.set_p1(kP1TupRequiredConsumed |
(is_individual_attestation ? kP1IndividualAttestation : 0));
command.set_data(data);
command.set_response_length(apdu::ApduCommand::kApduMaxResponseLength);
return command.GetEncodedCommand();
}
......
......@@ -179,7 +179,9 @@ TEST_F(U2fSignTest, TestCreateSignApduCommand) {
// Key Handle
0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x00,
0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x00, 0x01,
0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x00, 0x01
0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x00, 0x01,
// Max response length
0x00, 0x00,
// clang-format on
};
......@@ -209,7 +211,9 @@ TEST_F(U2fSignTest, TestCreateSignApduCommand) {
// Key Handle
0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x00,
0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x00, 0x01,
0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x00, 0x01
0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x00, 0x01,
// Max response length
0x00, 0x00,
// clang-format on
};
......
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