Fix send-gate ticket leak and chunk-size overflow on the data channel send path

A data channel send that WriteSymmetricMessage refuses with limitsExceeded
returned without completing the send-gate ticket it had been handed. The
ticket then became the tail every later send chains behind, so MSG, OPN and
CLO for every Session on that SecureChannel stalled for good. Nothing
recovered from it: the scheduler logs a send fault rather than faulting the
channel, so the stall was silent. The ticket is now released on every path
out. Releasing twice is safe because the ticket completes under Interlocked.

MaxDataChannelBodySize approximated the chunk budget instead of computing it,
and missed the cipher block rounding WriteSymmetricMessage applies: a 65535
byte send buffer under a 16 byte block loses its last 15 bytes. A body at the
advertised size therefore spilled into a second chunk, which carries the
Intermediate chunk type that the frame codec rejects, after the SequenceNumber
had already been spent. It now mirrors that arithmetic.

The audio sample wrote to a fixed name under the shared temp directory and
truncated whatever was already there, so a local user could pre-create the
path as a symlink and have the sample overwrite the target. It now writes into
a directory created for the run and opens the file with CreateNew.

Both channel fixes are covered by tests proven to fail without them.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c

samples/Core/ConsoleDataChannelAudio/AudioSink.cs

@@ -66,15 +66,14 @@ namespace Quickstarts.ConsoleDataChannelAudio
         /// writes a WAV there, which still demonstrates that the bytes arrived
         /// intact and in order.
         /// </remarks>
-        /// <param name="wavPath">Where to write when playback is unavailable.</param>
-        public static IAudioSink Create(string wavPath)
+        public static IAudioSink Create()
         {
             if (OperatingSystem.IsWindows())
             {
                 return new PlaybackAudioSink();
             }
 
-            return new WaveFileAudioSink(wavPath);
+            return new WaveFileAudioSink();
         }
     }
 
@@ -134,12 +133,26 @@ namespace Quickstarts.ConsoleDataChannelAudio
         /// <summary>
         /// Creates the sink and opens the file.
         /// </summary>
-        /// <param name="path">The file to write.</param>
-        public WaveFileAudioSink(string path)
+        /// <remarks>
+        /// The file goes in a directory created fresh for this run rather than
+        /// at a predictable path under the shared temp directory, and it is
+        /// opened with CreateNew. A fixed name in a world-writable directory
+        /// can be pre-created as a symlink by any local user, and an open that
+        /// truncates would then follow it and overwrite the target.
+        /// </remarks>
+        public WaveFileAudioSink()
         {
-            m_path = path;
+            DirectoryInfo directory = Directory.CreateTempSubdirectory("ConsoleDataChannelAudio");
+            m_path = Path.Combine(directory.FullName, "audio.wav");
+
+            var stream = new FileStream(
+                m_path,
+                FileMode.CreateNew,
+                FileAccess.Write,
+                FileShare.Read);
+
             m_writer = new WaveFileWriter(
-                path,
+                stream,
                 new WaveFormat(
                     AudioFormat.SampleRate,
                     AudioFormat.BitsPerSample,

samples/Core/ConsoleDataChannelAudio/Program.cs

@@ -168,11 +168,7 @@ namespace Quickstarts.ConsoleDataChannelAudio
                     opened.RevisedTransportChannelId);
                 channels.MarkOpen(opened.ChannelId);
 
-                string wavPath = Path.Combine(
-                    Path.GetTempPath(),
-                    "ConsoleDataChannelAudio.wav");
-
-                using IAudioSink player = AudioSink.Create(wavPath);
+                using IAudioSink player = AudioSink.Create();
 
                 Console.WriteLine();
                 Console.WriteLine("OPC UA data channel audio streaming");

src/Opc.Ua.Core/Stack/Tcp/UaSCBinaryChannel.DataChannels.cs

@@ -124,12 +124,44 @@ namespace Opc.Ua.Bindings
         /// The largest secured body a data channel frame may occupy on this
         /// SecureChannel.
         /// </summary>
+        /// <remarks>
+        /// Computed with the arithmetic <c>WriteSymmetricMessage</c> uses to
+        /// split a message into chunks rather than an approximation of it,
+        /// because a frame at this size has to fit exactly one chunk. A second
+        /// chunk would carry the Intermediate chunk type, which the frame codec
+        /// rejects outright, and the send would be refused after the
+        /// SequenceNumber had already been claimed. The part an approximation
+        /// misses is the cipher block rounding: a 65535 byte send buffer under
+        /// a 16 byte block cipher loses its last 15 bytes, so a body sized
+        /// against the raw buffer overflows into a second chunk.
+        /// </remarks>
         public int MaxDataChannelBodySize
-            => DataChannelFrameCodec.MaxPayload(
-                DataChannelFramingMode.Inline,
-                SendBufferSize,
-                SymmetricSignatureSize + 2,
-                withDeadline: true);
+        {
+            get
+            {
+                int blockSize = EncryptionBlockSize > 0 ? EncryptionBlockSize : 1;
+                int maxCipherTextSize = SendBufferSize - TcpMessageLimits.SymmetricHeaderSize;
+                int maxPlainTextSize = maxCipherTextSize / blockSize * blockSize;
+
+                // Conservative while no token is in force: assume padding is
+                // present, so the advertised size is never larger than what a
+                // later token can actually carry.
+                int paddingCountSize =
+                    SecurityMode != MessageSecurityMode.SignAndEncrypt ||
+                    CurrentToken?.SecurityPolicy?.NoSymmetricEncryptionPadding == true
+                        ? 0
+                        : blockSize > byte.MaxValue ? 2 : 1;
+
+                int available = maxPlainTextSize -
+                    SymmetricSignatureSize -
+                    TcpMessageLimits.SequenceHeaderSize -
+                    paddingCountSize -
+                    DataChannelConstants.StreamHeaderSize -
+                    DataChannelConstants.DeadlineSize;
+
+                return available > 0 ? available : 0;
+            }
+        }
 
         /// <summary>
         /// Routes an incoming STR chunk to the data channels.
@@ -248,7 +280,6 @@ namespace Opc.Ua.Bindings
             byte[] encoded = BufferManager.TakeBuffer(size, nameof(SendDataChannelFrameAsync), ct);
             BufferCollection? chunks = null;
             SendGateTicket? sendTicket = null;
-            bool sendTurnAcquired = false;
 
             try
             {
@@ -291,14 +322,21 @@ namespace Opc.Ua.Bindings
                         "The transport was closed by the remote application.");
 
                 await AwaitSendTurnAsync(sendTicket, ct).ConfigureAwait(false);
-                sendTurnAcquired = true;
                 await transport.SendChunkAsync(chunks, ct).ConfigureAwait(false);
             }
             finally
             {
-                if (sendTurnAcquired)
+                // The ticket is released on every path out, not only the one
+                // that reached the transport. WriteSymmetricMessage hands its
+                // ticket to the caller even when it reports limitsExceeded, and
+                // a ticket that is never completed becomes the tail every later
+                // send chains behind — which would stall MSG, OPN and CLO for
+                // every Session on this SecureChannel, silently and for good,
+                // because the scheduler logs a send fault rather than faulting
+                // the channel.
+                if (sendTicket != null)
                 {
-                    ReleaseSendTicket(sendTicket!);
+                    ReleaseSendTicket(sendTicket);
                 }
 
                 chunks?.Release(BufferManager, nameof(SendDataChannelFrameAsync));

tests/Opc.Ua.Core.DataChannels.Tests/UaSCDataChannelDispatchTests.cs

@@ -29,6 +29,7 @@
 
 using System;
 using System.Buffers.Binary;
+using System.Collections;
 using System.Collections.Generic;
 using System.Linq;
 using System.Net;
@@ -398,6 +399,99 @@ namespace Opc.Ua.Core.DataChannels.Tests
             });
         }
 
+        /// <summary>
+        /// A refused frame must not leave its send-gate ticket outstanding.
+        /// </summary>
+        /// <remarks>
+        /// WriteSymmetricMessage hands its ticket to the caller even when it
+        /// reports limitsExceeded, so a refusal that returns without completing
+        /// the ticket leaves it as the tail every later send chains behind.
+        /// Nothing recovers from that: DataChannelManager logs a scheduler
+        /// fault rather than faulting the channel, so MSG, OPN and CLO for
+        /// every Session on this SecureChannel stall silently and for good.
+        /// The ticket count is asserted directly because it is the invariant
+        /// that was broken; a later send would only observe it as a hang.
+        /// </remarks>
+        [Test]
+        public void ARefusedOversizedFrameLeavesNoOutstandingSendTicket()
+        {
+            using var channel = TestChannel.Create("str-dispatch-gate");
+            var transport = new CapturingByteTransport();
+            channel.AttachTransport(transport);
+            channel.Activate(SecureChannelId, TokenId);
+            channel.SetMaxResponseMessageSize(16);
+
+            Assert.That(channel.OutstandingSendTickets, Is.Zero, "The gate did not start clear.");
+
+            Assert.ThrowsAsync<ServiceResultException>(
+                async () => await SendFrameAsync(
+                        channel,
+                        DataChannelFrame.Data(
+                            DataChannelId,
+                            1,
+                            DataChannelFrameFlags.MessageStart,
+                            new byte[64]))
+                    .ConfigureAwait(false));
+
+            Assert.That(
+                channel.OutstandingSendTickets,
+                Is.Zero,
+                "The refused frame leaked its send-gate ticket, which stalls every later " +
+                    "send on this SecureChannel including MSG, OPN and CLO.");
+        }
+
+        /// <summary>
+        /// The advertised maximum body has to be what one chunk can carry.
+        /// </summary>
+        /// <remarks>
+        /// A body larger than one chunk is split, and the second chunk carries
+        /// the Intermediate chunk type, which the frame codec rejects — after
+        /// the SequenceNumber has already been claimed. The advertised size is
+        /// therefore recomputed here from the security policy with
+        /// WriteSymmetricMessage's own arithmetic, whose cipher block rounding
+        /// an approximation of the buffer size misses: a 65535 byte send buffer
+        /// under a 16 byte block loses its last 15 bytes.
+        /// </remarks>
+        [TestCase(SecurityPolicies.Basic256Sha256)]
+        [TestCase(SecurityPolicies.Aes128_Sha256_RsaOaep)]
+        [TestCase(SecurityPolicies.Aes256_Sha256_RsaPss)]
+        public void TheAdvertisedMaximumBodyIsWhatOneChunkCanCarry(string securityPolicyUri)
+        {
+            using var channel = TestChannel.Create(
+                "str-dispatch-maxframe",
+                MessageSecurityMode.SignAndEncrypt,
+                securityPolicyUri);
+
+            SecurityPolicyInfo policy = SecurityPolicies.GetInfo(securityPolicyUri)!;
+            int blockSize = policy.InitializationVectorLength != 0
+                ? policy.InitializationVectorLength
+                : 1;
+            int paddingCountSize = policy.NoSymmetricEncryptionPadding
+                ? 0
+                : blockSize > byte.MaxValue ? 2 : 1;
+
+            // WriteSymmetricMessage rounds the cipher text down to whole blocks
+            // before it subtracts the footers.
+            int maxPlainTextSize =
+                (channel.SendBuffer - TcpMessageLimits.SymmetricHeaderSize) /
+                blockSize *
+                blockSize;
+
+            int expected = maxPlainTextSize -
+                policy.SymmetricSignatureLength -
+                TcpMessageLimits.SequenceHeaderSize -
+                paddingCountSize -
+                DataChannelConstants.StreamHeaderSize -
+                DataChannelConstants.DeadlineSize;
+
+            Assert.That(
+                channel.MaxDataChannelBodySize,
+                Is.EqualTo(expected),
+                "A body of the advertised size does not fit the single chunk " +
+                    "WriteSymmetricMessage would build for it, so the frame is split " +
+                    "and then refused as Intermediate after the SequenceNumber is spent.");
+        }
+
         private static DataChannel OpenSink(TestChannel channel)
         {
             DataChannelManager manager = channel.EnableDataChannels(
@@ -474,6 +568,8 @@ namespace Opc.Ua.Core.DataChannels.Tests
                 string contextId,
                 BufferManager bufferManager,
                 ChannelQuotas quotas,
+                MessageSecurityMode securityMode,
+                string securityPolicyUri,
                 ITelemetryContext telemetry)
                 : base(
                     contextId,
@@ -481,8 +577,8 @@ namespace Opc.Ua.Core.DataChannels.Tests
                     quotas,
                     serverCertificates: null,
                     endpoints: null,
-                    securityMode: MessageSecurityMode.None,
-                    securityPolicyUri: SecurityPolicies.None,
+                    securityMode: securityMode,
+                    securityPolicyUri: securityPolicyUri,
                     telemetry: telemetry)
             {
             }
@@ -495,14 +591,29 @@ namespace Opc.Ua.Core.DataChannels.Tests
 
             public int SequenceNumbersIssued => m_sequenceNumbersIssued;
 
+            public int SendBuffer => SendBufferSize;
+
             public static TestChannel Create(string contextId)
+            {
+                return Create(contextId, MessageSecurityMode.None, SecurityPolicies.None);
+            }
+
+            public static TestChannel Create(
+                string contextId,
+                MessageSecurityMode securityMode,
+                string securityPolicyUri)
             {
                 ITelemetryContext telemetry = NUnitTelemetryContext.Create();
-                return new TestChannel(
+                var channel = new TestChannel(
                     contextId,
                     new BufferManager(contextId, TcpMessageLimits.DefaultMaxBufferSize, telemetry),
                     new ChannelQuotas(ServiceMessageContext.CreateEmpty(telemetry)),
+                    securityMode,
+                    securityPolicyUri,
                     telemetry);
+
+                channel.CalculateSymmetricKeySizes();
+                return channel;
             }
 
             public void Activate(uint channelId, uint tokenId)
@@ -546,6 +657,23 @@ namespace Opc.Ua.Core.DataChannels.Tests
                 MaxResponseChunkCount = 1;
             }
 
+            /// <summary>
+            /// How many send-gate tickets have been issued and not completed.
+            /// A ticket that outlives its send blocks every later send.
+            /// </summary>
+            public int OutstandingSendTickets
+            {
+                get
+                {
+                    FieldInfo ticketsField = typeof(UaSCUaBinaryChannel).GetField(
+                            "m_sendGateTickets",
+                            BindingFlags.Instance | BindingFlags.NonPublic)
+                        ?? throw new AssertionException("m_sendGateTickets was not found.");
+
+                    return ((IEnumerable)ticketsField.GetValue(this)!).Cast<object>().Count();
+                }
+            }
+
             public void SetNextSendSequenceNumber(uint sequenceNumber)
             {
                 FieldInfo field = typeof(UaSCUaBinaryChannel).GetField(