diff --git a/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs b/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs index 24eb83ea5..5c8453710 100644 --- a/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs +++ b/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs @@ -247,12 +247,15 @@ private async Task> ReceiveResponse( // Get initialization packet, handling keep-alive var initPacket = await _connection.ReceiveAsync(cancellationToken).ConfigureAwait(false); - while (GetPacketCommand(initPacket.Span) == CtapConstants.CtapHidKeepAlive) + while (IsKeepAlivePacket(initPacket.Span)) { + ValidateInitPacket(initPacket.Span, channelId); _logger.LogTrace("Received keep-alive, waiting for response"); initPacket = await _connection.ReceiveAsync(cancellationToken).ConfigureAwait(false); } + ValidateInitPacket(initPacket.Span, channelId); + var responseLength = GetPacketLength(initPacket.Span); if (responseLength > CtapConstants.MaxPayloadSize) throw new InvalidOperationException($"Response length {responseLength} exceeds max payload size"); @@ -261,32 +264,25 @@ private async Task> ReceiveResponse( var responseData = new byte[responseLength]; var initDataLength = Math.Min(responseLength, CtapConstants.InitDataSize); - // Ensure we don't try to read more data than the packet contains - var availableDataInPacket = Math.Min(initDataLength, initPacket.Length - CtapConstants.InitHeaderSize); - if (availableDataInPacket < 0) - availableDataInPacket = 0; - - initPacket.Span.Slice(CtapConstants.InitHeaderSize, availableDataInPacket) + initPacket.Span.Slice(CtapConstants.InitHeaderSize, initDataLength) .CopyTo(responseData); // Receive continuation packets if needed - var bytesReceived = availableDataInPacket; + var bytesReceived = initDataLength; + byte expectedSequence = 0; while (bytesReceived < responseLength) { var contPacket = await _connection.ReceiveAsync(cancellationToken).ConfigureAwait(false); + ValidateContinuationPacket(contPacket.Span, channelId, expectedSequence); var contDataLength = Math.Min( responseLength - bytesReceived, CtapConstants.ContinuationDataSize); - // Ensure we don't try to read more data than the packet contains - var availableContData = Math.Min(contDataLength, contPacket.Length - CtapConstants.ContinuationHeaderSize); - if (availableContData < 0) - availableContData = 0; - - contPacket.Span.Slice(CtapConstants.ContinuationHeaderSize, availableContData) + contPacket.Span.Slice(CtapConstants.ContinuationHeaderSize, contDataLength) .CopyTo(responseData.AsSpan(bytesReceived)); - bytesReceived += availableContData; + bytesReceived += contDataLength; + expectedSequence++; } _logger.LogTrace("Received {Length} bytes in response", responseLength); @@ -343,6 +339,44 @@ private static byte[] ConstructContinuationPacket(uint channelId, byte sequence, private static byte GetPacketCommand(ReadOnlySpan packet) => (byte)(packet[4] & ~CtapConstants.InitPacketMask); + private static bool IsKeepAlivePacket(ReadOnlySpan packet) => + packet.Length >= CtapConstants.InitHeaderSize && + (packet[4] & CtapConstants.InitPacketMask) != 0 && + GetPacketCommand(packet) == CtapConstants.CtapHidKeepAlive; + + private static void ValidateInitPacket(ReadOnlySpan packet, uint channelId) + { + if (packet.Length != CtapConstants.PacketSize) + throw new InvalidOperationException("CTAP HID init packet must be exactly 64 bytes"); + + var packetChannelId = BinaryPrimitives.ReadUInt32BigEndian(packet); + if (packetChannelId != channelId) + throw new InvalidOperationException("CTAP HID init packet channel mismatch"); + + if ((packet[4] & CtapConstants.InitPacketMask) == 0) + throw new InvalidOperationException("CTAP HID response packet is not an init packet"); + } + + private static void ValidateContinuationPacket(ReadOnlySpan packet, uint channelId, byte expectedSequence) + { + if (packet.Length != CtapConstants.PacketSize) + throw new InvalidOperationException("CTAP HID continuation packet must be exactly 64 bytes"); + + var packetChannelId = BinaryPrimitives.ReadUInt32BigEndian(packet); + if (packetChannelId != channelId) + throw new InvalidOperationException("CTAP HID continuation packet channel mismatch"); + + if ((packet[4] & CtapConstants.InitPacketMask) != 0) + throw new InvalidOperationException("CTAP HID continuation packet has init bit set"); + + var sequence = packet[4]; + if (!IsExpectedContinuationSequence(sequence, expectedSequence)) + throw new InvalidOperationException("CTAP HID continuation packet sequence mismatch"); + } + + internal static bool IsExpectedContinuationSequence(byte sequence, byte expectedSequence) => + (sequence & ~CtapConstants.InitPacketMask) == (expectedSequence & ~CtapConstants.InitPacketMask); + /// /// Extracts the payload length from an init packet. /// diff --git a/src/Core/tests/Yubico.YubiKit.Core.UnitTests/Protocols/Fido/Hid/FidoHidProtocolTests.cs b/src/Core/tests/Yubico.YubiKit.Core.UnitTests/Protocols/Fido/Hid/FidoHidProtocolTests.cs new file mode 100644 index 000000000..99255bf0c --- /dev/null +++ b/src/Core/tests/Yubico.YubiKit.Core.UnitTests/Protocols/Fido/Hid/FidoHidProtocolTests.cs @@ -0,0 +1,298 @@ +// Copyright 2026 Yubico AB +// +// Licensed under the Apache License, Version 2.0 (the "License"). +// You may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +using System.Buffers.Binary; +using Yubico.YubiKit.Core.Devices; +using Yubico.YubiKit.Core.Protocols.Fido.Hid; + +namespace Yubico.YubiKit.Core.UnitTests.Protocols.Fido.Hid; + +public class FidoHidProtocolTests +{ + [Theory] + [InlineData(0, 0, true)] + [InlineData(1, 1, true)] + [InlineData(0x7F, 0x7F, true)] + [InlineData(0, 0x80, true)] + [InlineData(1, 0x81, true)] + [InlineData(1, 0, false)] + public void IsExpectedContinuationSequence_MasksSequenceToSevenBits( + byte sequence, + byte expectedSequence, + bool expected) + { + Assert.Equal(expected, FidoHidProtocol.IsExpectedContinuationSequence(sequence, expectedSequence)); + } + + [Fact] + public async Task SendVendorCommandAsync_ResponseWithWrongContinuationSequence_ThrowsInvalidOperationException() + { + var connection = new FakeFidoHidConnection(); + var protocol = new FidoHidProtocol(connection); + + var responsePayload = Enumerable.Range(0, CtapConstants.InitDataSize + 1) + .Select(i => (byte)i) + .ToArray(); + connection.QueueResponsePackets( + CreateInitPacket(0x01020304, CtapConstants.CtapVendorFirst, responsePayload), + CreateContinuationPacket(0x01020304, sequence: 1, responsePayload.AsSpan(CtapConstants.InitDataSize))); + + await Assert.ThrowsAsync(() => + protocol.SendVendorCommandAsync(CtapConstants.CtapVendorFirst, ReadOnlyMemory.Empty, TestContext.Current.CancellationToken)); + } + + [Fact] + public async Task SendVendorCommandAsync_ResponseWithWrongContinuationChannel_ThrowsInvalidOperationException() + { + var connection = new FakeFidoHidConnection(); + var protocol = new FidoHidProtocol(connection); + + var responsePayload = Enumerable.Range(0, CtapConstants.InitDataSize + 1) + .Select(i => (byte)i) + .ToArray(); + connection.QueueResponsePackets( + CreateInitPacket(0x01020304, CtapConstants.CtapVendorFirst, responsePayload), + CreateContinuationPacket(0x05060708, sequence: 0, responsePayload.AsSpan(CtapConstants.InitDataSize))); + + var ex = await Assert.ThrowsAsync(() => + protocol.SendVendorCommandAsync(CtapConstants.CtapVendorFirst, ReadOnlyMemory.Empty, TestContext.Current.CancellationToken)); + + Assert.Contains("channel", ex.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task SendVendorCommandAsync_ResponseWithWrongInitChannel_ThrowsInvalidOperationException() + { + var connection = new FakeFidoHidConnection(); + var protocol = new FidoHidProtocol(connection); + + var responsePayload = new byte[] { 0xAA }; + connection.QueueResponsePackets( + CreateInitPacket(0x05060708, CtapConstants.CtapVendorFirst, responsePayload)); + + var ex = await Assert.ThrowsAsync(() => + protocol.SendVendorCommandAsync(CtapConstants.CtapVendorFirst, ReadOnlyMemory.Empty, TestContext.Current.CancellationToken)); + + Assert.Contains("channel", ex.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task SendVendorCommandAsync_ResponseWithShortInitPacket_ThrowsInvalidOperationException() + { + var connection = new FakeFidoHidConnection(); + var protocol = new FidoHidProtocol(connection); + + connection.QueueResponsePackets([0x01, 0x02, 0x03]); + + var ex = await Assert.ThrowsAsync(() => + protocol.SendVendorCommandAsync(CtapConstants.CtapVendorFirst, ReadOnlyMemory.Empty, TestContext.Current.CancellationToken)); + + Assert.Contains("exactly", ex.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task SendVendorCommandAsync_ResponseWithPartialInitPacket_ThrowsInvalidOperationException() + { + var connection = new FakeFidoHidConnection(); + var protocol = new FidoHidProtocol(connection); + + var packet = CreateInitPacket(0x01020304, CtapConstants.CtapVendorFirst, [0xAA]); + connection.QueueResponsePackets(packet[..CtapConstants.InitHeaderSize]); + + var ex = await Assert.ThrowsAsync(() => + protocol.SendVendorCommandAsync(CtapConstants.CtapVendorFirst, ReadOnlyMemory.Empty, TestContext.Current.CancellationToken)); + + Assert.Contains("exactly", ex.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task SendVendorCommandAsync_ResponseWithPartialContinuationPacket_ThrowsInvalidOperationException() + { + var connection = new FakeFidoHidConnection(); + var protocol = new FidoHidProtocol(connection); + + var responsePayload = Enumerable.Range(0, CtapConstants.InitDataSize + 1) + .Select(i => (byte)i) + .ToArray(); + var continuation = CreateContinuationPacket(0x01020304, sequence: 0, responsePayload.AsSpan(CtapConstants.InitDataSize)); + connection.QueueResponsePackets( + CreateInitPacket(0x01020304, CtapConstants.CtapVendorFirst, responsePayload), + continuation[..CtapConstants.ContinuationHeaderSize]); + + var ex = await Assert.ThrowsAsync(() => + protocol.SendVendorCommandAsync(CtapConstants.CtapVendorFirst, ReadOnlyMemory.Empty, TestContext.Current.CancellationToken)); + + Assert.Contains("exactly", ex.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task SendVendorCommandAsync_ResponseWithWrongChannelKeepAlive_ThrowsInvalidOperationException() + { + var connection = new FakeFidoHidConnection(); + var protocol = new FidoHidProtocol(connection); + + connection.QueueResponsePackets( + CreateInitPacket(0x05060708, CtapConstants.CtapHidKeepAlive, [0x01]), + CreateInitPacket(0x01020304, CtapConstants.CtapVendorFirst, [0xAA])); + + var ex = await Assert.ThrowsAsync(() => + protocol.SendVendorCommandAsync(CtapConstants.CtapVendorFirst, ReadOnlyMemory.Empty, TestContext.Current.CancellationToken)); + + Assert.Contains("channel", ex.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task SendVendorCommandAsync_ResponseWithInitPacketAsContinuation_ThrowsInvalidOperationException() + { + var connection = new FakeFidoHidConnection(); + var protocol = new FidoHidProtocol(connection); + + var responsePayload = Enumerable.Range(0, CtapConstants.InitDataSize + 1) + .Select(i => (byte)i) + .ToArray(); + connection.QueueResponsePackets( + CreateInitPacket(0x01020304, CtapConstants.CtapVendorFirst, responsePayload), + CreateInitPacket(0x01020304, CtapConstants.CtapHidPing, responsePayload.AsSpan(CtapConstants.InitDataSize))); + + var ex = await Assert.ThrowsAsync(() => + protocol.SendVendorCommandAsync(CtapConstants.CtapVendorFirst, ReadOnlyMemory.Empty, TestContext.Current.CancellationToken)); + + Assert.Contains("continuation", ex.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task SendVendorCommandAsync_ResponseWithExpectedContinuationSequence_Succeeds() + { + var connection = new FakeFidoHidConnection(); + var protocol = new FidoHidProtocol(connection); + + var responsePayload = Enumerable.Range(0, CtapConstants.InitDataSize + 1) + .Select(i => (byte)i) + .ToArray(); + connection.QueueResponsePackets( + CreateInitPacket(0x01020304, CtapConstants.CtapVendorFirst, responsePayload), + CreateContinuationPacket(0x01020304, sequence: 0, responsePayload.AsSpan(CtapConstants.InitDataSize))); + + var response = await protocol.SendVendorCommandAsync( + CtapConstants.CtapVendorFirst, + ReadOnlyMemory.Empty, + TestContext.Current.CancellationToken); + + Assert.Equal(responsePayload, response.ToArray()); + } + + [Fact] + public async Task SendVendorCommandAsync_ResponseContinuationInInitPositionMatchingKeepAliveCommand_ThrowsInvalidOperationException() + { + var connection = new FakeFidoHidConnection(); + var protocol = new FidoHidProtocol(connection); + + connection.QueueResponsePackets( + CreateContinuationPacket(0x01020304, CtapConstants.CtapHidKeepAlive, [0xAA]), + CreateInitPacket(0x01020304, CtapConstants.CtapVendorFirst, [0xBB])); + + var ex = await Assert.ThrowsAsync(() => protocol.SendVendorCommandAsync( + CtapConstants.CtapVendorFirst, + ReadOnlyMemory.Empty, + TestContext.Current.CancellationToken)); + + Assert.Contains("init", ex.Message, StringComparison.OrdinalIgnoreCase); + } + + private static byte[] CreateInitPacket(uint channelId, byte command, ReadOnlySpan payload) + { + var packet = new byte[CtapConstants.PacketSize]; + BinaryPrimitives.WriteUInt32BigEndian(packet, channelId); + packet[4] = (byte)(command | CtapConstants.InitPacketMask); + packet[5] = (byte)(payload.Length >> 8); + packet[6] = (byte)payload.Length; + payload[..Math.Min(payload.Length, CtapConstants.InitDataSize)] + .CopyTo(packet.AsSpan(CtapConstants.InitHeaderSize)); + return packet; + } + + private static byte[] CreateContinuationPacket(uint channelId, byte sequence, ReadOnlySpan payload) + { + var packet = new byte[CtapConstants.PacketSize]; + BinaryPrimitives.WriteUInt32BigEndian(packet, channelId); + packet[4] = sequence; + payload[..Math.Min(payload.Length, CtapConstants.ContinuationDataSize)] + .CopyTo(packet.AsSpan(CtapConstants.ContinuationHeaderSize)); + return packet; + } + + private sealed class FakeFidoHidConnection : IFidoHidConnection + { + private readonly Queue _responsePackets = new(); + private byte[]? _lastInitRequest; + private bool _initResponseSent; + + public int PacketSize => CtapConstants.PacketSize; + + public ConnectionType Type => ConnectionType.HidFido; + + public void QueueResponsePackets(params byte[][] packets) + { + foreach (var packet in packets) + { + _responsePackets.Enqueue(packet); + } + } + + public Task SendAsync(ReadOnlyMemory packet, CancellationToken cancellationToken = default) + { + cancellationToken.ThrowIfCancellationRequested(); + if ((packet.Span[4] & ~CtapConstants.InitPacketMask) == CtapConstants.CtapHidInit) + { + _lastInitRequest = packet.ToArray(); + } + + return Task.CompletedTask; + } + + public Task> ReceiveAsync(CancellationToken cancellationToken = default) + { + cancellationToken.ThrowIfCancellationRequested(); + if (!_initResponseSent) + { + _initResponseSent = true; + return Task.FromResult>(CreateInitResponse()); + } + + return Task.FromResult>(_responsePackets.Dequeue()); + } + + public void Dispose() + { + } + + public ValueTask DisposeAsync() => ValueTask.CompletedTask; + + private byte[] CreateInitResponse() + { + if (_lastInitRequest is null) + throw new InvalidOperationException("INIT request was not sent."); + + var payload = new byte[17]; + _lastInitRequest.AsSpan(CtapConstants.InitHeaderSize, CtapConstants.NonceSize).CopyTo(payload); + BinaryPrimitives.WriteUInt32BigEndian(payload.AsSpan(8), 0x01020304); + payload[12] = 2; + payload[13] = 5; + payload[14] = 8; + payload[15] = 0; + payload[16] = 0; + return CreateInitPacket(CtapConstants.BroadcastChannelId, CtapConstants.CtapHidInit, payload); + } + } +} \ No newline at end of file