diff --git a/samples/Core/ConsoleDataChannelAudio/AudioSink.cs b/samples/Core/ConsoleDataChannelAudio/AudioSink.cs index 3256d4aa2c..df3778c2ab 100644 --- a/samples/Core/ConsoleDataChannelAudio/AudioSink.cs +++ b/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. /// - /// Where to write when playback is unavailable. - 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 /// /// Creates the sink and opens the file. /// - /// The file to write. - public WaveFileAudioSink(string path) + /// + /// 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. + /// + 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, diff --git a/samples/Core/ConsoleDataChannelAudio/Program.cs b/samples/Core/ConsoleDataChannelAudio/Program.cs index a8bbe724d3..3c066e6b91 100644 --- a/samples/Core/ConsoleDataChannelAudio/Program.cs +++ b/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"); diff --git a/src/Opc.Ua.Core/Stack/Tcp/UaSCBinaryChannel.DataChannels.cs b/src/Opc.Ua.Core/Stack/Tcp/UaSCBinaryChannel.DataChannels.cs index 15c118d4f4..387bf8bba5 100644 --- a/src/Opc.Ua.Core/Stack/Tcp/UaSCBinaryChannel.DataChannels.cs +++ b/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. /// + /// + /// Computed with the arithmetic WriteSymmetricMessage 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. + /// 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; + } + } /// /// 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)); diff --git a/tests/Opc.Ua.Core.DataChannels.Tests/UaSCDataChannelDispatchTests.cs b/tests/Opc.Ua.Core.DataChannels.Tests/UaSCDataChannelDispatchTests.cs index fbf0836cac..0427d1af11 100644 --- a/tests/Opc.Ua.Core.DataChannels.Tests/UaSCDataChannelDispatchTests.cs +++ b/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 }); } + /// + /// A refused frame must not leave its send-gate ticket outstanding. + /// + /// + /// 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. + /// + [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( + 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."); + } + + /// + /// The advertised maximum body has to be what one chunk can carry. + /// + /// + /// 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. + /// + [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; } + /// + /// How many send-gate tickets have been issued and not completed. + /// A ticket that outlives its send blocks every later send. + /// + 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().Count(); + } + } + public void SetNextSendSequenceNumber(uint sequenceNumber) { FieldInfo field = typeof(UaSCUaBinaryChannel).GetField(