From e3dda1f2b7347a4005653ff1092ea23493d4fb03 Mon Sep 17 00:00:00 2001 From: Dennis Dyall Date: Tue, 23 Jun 2026 19:51:00 +0200 Subject: [PATCH 1/5] fix(core): validate CTAPHID continuation sequences --- .../src/Protocols/Fido/Hid/FidoHidProtocol.cs | 18 +- .../Fido/Hid/FidoHidProtocolTests.cs | 161 ++++++++++++++++++ 2 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 src/Core/tests/Yubico.YubiKit.Core.UnitTests/Protocols/Fido/Hid/FidoHidProtocolTests.cs diff --git a/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs b/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs index 24eb83ea5..3ff6b6fd0 100644 --- a/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs +++ b/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs @@ -271,9 +271,11 @@ private async Task> ReceiveResponse( // Receive continuation packets if needed var bytesReceived = availableDataInPacket; + byte expectedSequence = 0; while (bytesReceived < responseLength) { var contPacket = await _connection.ReceiveAsync(cancellationToken).ConfigureAwait(false); + ValidateContinuationPacket(contPacket.Span, expectedSequence); var contDataLength = Math.Min( responseLength - bytesReceived, CtapConstants.ContinuationDataSize); @@ -287,6 +289,7 @@ private async Task> ReceiveResponse( .CopyTo(responseData.AsSpan(bytesReceived)); bytesReceived += availableContData; + expectedSequence++; } _logger.LogTrace("Received {Length} bytes in response", responseLength); @@ -343,6 +346,19 @@ private static byte[] ConstructContinuationPacket(uint channelId, byte sequence, private static byte GetPacketCommand(ReadOnlySpan packet) => (byte)(packet[4] & ~CtapConstants.InitPacketMask); + private static void ValidateContinuationPacket(ReadOnlySpan packet, byte expectedSequence) + { + if (packet.Length < CtapConstants.ContinuationHeaderSize) + throw new InvalidOperationException("CTAP HID continuation packet is too short"); + + 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. /// @@ -397,4 +413,4 @@ public void Dispose() _connection.Dispose(); _disposed = true; } -} \ No newline at end of file +} 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..764ba8033 --- /dev/null +++ b/src/Core/tests/Yubico.YubiKit.Core.UnitTests/Protocols/Fido/Hid/FidoHidProtocolTests.cs @@ -0,0 +1,161 @@ +// 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_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()); + } + + 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); + } + } +} From 660567e5630dadbf023a9aeb97ad5017b91888dd Mon Sep 17 00:00:00 2001 From: Dennis Dyall Date: Wed, 24 Jun 2026 15:15:00 +0200 Subject: [PATCH 2/5] fix(core): validate CTAPHID response channels --- .../src/Protocols/Fido/Hid/FidoHidProtocol.cs | 32 ++++++++- .../Fido/Hid/FidoHidProtocolTests.cs | 68 +++++++++++++++++++ 2 files changed, 97 insertions(+), 3 deletions(-) diff --git a/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs b/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs index 3ff6b6fd0..40178a8d7 100644 --- a/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs +++ b/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs @@ -247,12 +247,14 @@ 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)) { _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"); @@ -275,7 +277,7 @@ private async Task> ReceiveResponse( while (bytesReceived < responseLength) { var contPacket = await _connection.ReceiveAsync(cancellationToken).ConfigureAwait(false); - ValidateContinuationPacket(contPacket.Span, expectedSequence); + ValidateContinuationPacket(contPacket.Span, channelId, expectedSequence); var contDataLength = Math.Min( responseLength - bytesReceived, CtapConstants.ContinuationDataSize); @@ -346,11 +348,35 @@ private static byte[] ConstructContinuationPacket(uint channelId, byte sequence, private static byte GetPacketCommand(ReadOnlySpan packet) => (byte)(packet[4] & ~CtapConstants.InitPacketMask); - private static void ValidateContinuationPacket(ReadOnlySpan packet, byte expectedSequence) + private static bool IsKeepAlivePacket(ReadOnlySpan packet) => + packet.Length >= CtapConstants.InitHeaderSize && + GetPacketCommand(packet) == CtapConstants.CtapHidKeepAlive; + + private static void ValidateInitPacket(ReadOnlySpan packet, uint channelId) + { + if (packet.Length < CtapConstants.InitHeaderSize) + throw new InvalidOperationException("CTAP HID init packet is too short"); + + 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.ContinuationHeaderSize) throw new InvalidOperationException("CTAP HID continuation packet is too short"); + 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"); 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 index 764ba8033..049e8c933 100644 --- 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 @@ -52,6 +52,74 @@ 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("too short", 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() { From 492171f4fdf99c81dd2ca5acbb9d2ceff584c5b1 Mon Sep 17 00:00:00 2001 From: Dennis Dyall Date: Tue, 30 Jun 2026 01:05:21 +0200 Subject: [PATCH 3/5] fix(core): require init bit for ctaphid keepalive --- .../src/Protocols/Fido/Hid/FidoHidProtocol.cs | 1 + .../Protocols/Fido/Hid/FidoHidProtocolTests.cs | 18 ++++++++++++++++++ src/Fido2/src/Ctap/CtapStatus.cs | 6 ++++++ 3 files changed, 25 insertions(+) diff --git a/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs b/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs index 40178a8d7..df0a1b344 100644 --- a/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs +++ b/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs @@ -350,6 +350,7 @@ private static byte GetPacketCommand(ReadOnlySpan packet) => 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) 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 index 049e8c933..be529f4ab 100644 --- 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 @@ -141,6 +141,24 @@ public async Task SendVendorCommandAsync_ResponseWithExpectedContinuationSequenc 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]; diff --git a/src/Fido2/src/Ctap/CtapStatus.cs b/src/Fido2/src/Ctap/CtapStatus.cs index c7baa5d12..2f2fd5d25 100644 --- a/src/Fido2/src/Ctap/CtapStatus.cs +++ b/src/Fido2/src/Ctap/CtapStatus.cs @@ -212,6 +212,12 @@ public enum CtapStatus : byte /// PuatRequired = 0x36, + /// + /// A pinUvAuthToken is required for the selected operation. + /// + [Obsolete("Use PuatRequired instead.", false)] + PuvathRequired = PuatRequired, + /// /// PIN policy violation. Currently only enforces minimum length. /// From cc3757161cf664953e1996e69bcefbca215e01f4 Mon Sep 17 00:00:00 2001 From: Dennis Dyall Date: Tue, 30 Jun 2026 01:16:30 +0200 Subject: [PATCH 4/5] refactor(fido2): keep only canonical puat status --- src/Fido2/src/Ctap/CtapStatus.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/Fido2/src/Ctap/CtapStatus.cs b/src/Fido2/src/Ctap/CtapStatus.cs index 2f2fd5d25..c7baa5d12 100644 --- a/src/Fido2/src/Ctap/CtapStatus.cs +++ b/src/Fido2/src/Ctap/CtapStatus.cs @@ -212,12 +212,6 @@ public enum CtapStatus : byte /// PuatRequired = 0x36, - /// - /// A pinUvAuthToken is required for the selected operation. - /// - [Obsolete("Use PuatRequired instead.", false)] - PuvathRequired = PuatRequired, - /// /// PIN policy violation. Currently only enforces minimum length. /// From 565697ec4c3bab86460f49ca4ee8b5febbe415f0 Mon Sep 17 00:00:00 2001 From: Dennis Dyall Date: Wed, 1 Jul 2026 12:57:27 +0200 Subject: [PATCH 5/5] fix(core): validate ctaphid keepalive framing --- .../src/Protocols/Fido/Hid/FidoHidProtocol.cs | 29 ++++------ .../Fido/Hid/FidoHidProtocolTests.cs | 55 ++++++++++++++++++- 2 files changed, 63 insertions(+), 21 deletions(-) diff --git a/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs b/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs index df0a1b344..5c8453710 100644 --- a/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs +++ b/src/Core/src/Protocols/Fido/Hid/FidoHidProtocol.cs @@ -249,6 +249,7 @@ private async Task> ReceiveResponse( var initPacket = await _connection.ReceiveAsync(cancellationToken).ConfigureAwait(false); while (IsKeepAlivePacket(initPacket.Span)) { + ValidateInitPacket(initPacket.Span, channelId); _logger.LogTrace("Received keep-alive, waiting for response"); initPacket = await _connection.ReceiveAsync(cancellationToken).ConfigureAwait(false); } @@ -263,16 +264,11 @@ 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) { @@ -282,15 +278,10 @@ private async Task> ReceiveResponse( 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++; } @@ -355,8 +346,8 @@ private static bool IsKeepAlivePacket(ReadOnlySpan packet) => private static void ValidateInitPacket(ReadOnlySpan packet, uint channelId) { - if (packet.Length < CtapConstants.InitHeaderSize) - throw new InvalidOperationException("CTAP HID init packet is too short"); + 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) @@ -368,8 +359,8 @@ private static void ValidateInitPacket(ReadOnlySpan packet, uint channelId private static void ValidateContinuationPacket(ReadOnlySpan packet, uint channelId, byte expectedSequence) { - if (packet.Length < CtapConstants.ContinuationHeaderSize) - throw new InvalidOperationException("CTAP HID continuation packet is too short"); + 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) @@ -440,4 +431,4 @@ public void Dispose() _connection.Dispose(); _disposed = true; } -} +} \ No newline at end of file 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 index be529f4ab..99255bf0c 100644 --- 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 @@ -98,7 +98,58 @@ public async Task SendVendorCommandAsync_ResponseWithShortInitPacket_ThrowsInval var ex = await Assert.ThrowsAsync(() => protocol.SendVendorCommandAsync(CtapConstants.CtapVendorFirst, ReadOnlyMemory.Empty, TestContext.Current.CancellationToken)); - Assert.Contains("too short", ex.Message, StringComparison.OrdinalIgnoreCase); + 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] @@ -244,4 +295,4 @@ private byte[] CreateInitResponse() return CreateInitPacket(CtapConstants.BroadcastChannelId, CtapConstants.CtapHidInit, payload); } } -} +} \ No newline at end of file