Dead letter message when CLR is offline (#658)

- If certificate cannot be validated because of network related issues with CLR then it will threat it as an unhandeled exception instead of place it on the senders error queue. It will retry the message and if the problem presist then it will eventually be dead lettered. Dead lettered messages on receivers queue can be moved back to normal queue and retried when the issue have been resolved
- Added support to dead letter messages
- Added X509Chain Mock to tests and fixed failing and ignored tests on build server. Removed ignore and X509Chain test category because these tests runs on build server now.
- Removed X509Chain test category after fixing the test to run on build
server using X509Chain Mock
- Remove calles of obsolete methods in MessagingException that were not
  in use either
- dist: Bump version number to 5.1.0

Author: ArneHB

Directory.build.props

@@ -1,7 +1,7 @@
 <Project>
   <PropertyGroup>
     <LangVersion>10.0</LangVersion>
-    <Version>5.0.10</Version>
+    <Version>5.1.0</Version>
   </PropertyGroup>
   <PropertyGroup>
     <Authors>Norsk Helsenett SF</Authors>

src/Helsenorge.Messaging/Abstractions/MessagingException.cs

@@ -139,6 +139,14 @@ public static class EventIds
         /// Invalid HER-id.
         /// </summary>
         public static EventId CouldNotVerifyCertificate = new EventId(41, EventIdName);
+        /// <summary>
+        /// Remote certificate has been revoked.
+        /// </summary>
+        public static EventId RemoteCertificateRevocationOffline = new EventId(42, EventIdName);
+        /// <summary>
+        /// Local certificate has been revoked.
+        /// </summary>
+        public static EventId LocalCertificateRevocationOffline = new EventId(43, EventIdName);
 
         /// <summary>
         /// Event Id used for informational purposes when starting/ending the Receive process.
@@ -207,22 +215,5 @@ public MessagingException(string message) : base(message){}
         /// Constructor
         /// </summary>
         public MessagingException(string message, Exception inner) : base(message, inner){}
-        /// <summary>
-        /// Constructor
-        /// </summary>
-        protected MessagingException(
-            SerializationInfo info,
-            StreamingContext context) : base(info, context){}
-        /// <summary>
-        /// 
-        /// </summary>
-        /// <param name="info"></param>
-        /// <param name="context"></param>
-        // ReSharper disable once RedundantOverridenMember
-        public override void GetObjectData(SerializationInfo info, StreamingContext context)
-        {
-            // code analysis trigger if this is not present
-            base.GetObjectData(info, context);
-        }
     }
 }

src/Helsenorge.Messaging/Amqp/AmqpCore.cs

@@ -469,6 +469,27 @@ internal static void RemoveProcessedMessageFromQueue(IAmqpMessage message)
 
             message.Complete();
         }
+
+        /// <summary>
+        /// Moves the message from current queue to the dead letter queue
+        /// </summary>
+        /// <param name="logger"></param>
+        /// <param name="id"></param>
+        /// <param name="message"></param>
+        /// <param name="description"></param>
+        /// <param name="ex"></param>
+        internal async void DeadLetterProcessedMessageAsync(
+            ILogger logger,
+            EventId id, 
+            IAmqpMessage message,
+            string description,
+            Exception ex = null)
+        {
+            if (message == null) throw new ArgumentNullException(nameof(message));
+
+            logger.LogWarning(id, ex, description);
+            await message.RejectAsync();
+        }
         /// <summary>
         /// Registers an alternate messaging factory
         /// </summary>

src/Helsenorge.Messaging/Amqp/AmqpMessage.cs

@@ -323,9 +323,9 @@ public IAmqpMessage Clone(bool includePayload = true)
 
         public async Task CompleteAsync() => await CompleteActionAsync.Invoke().ConfigureAwait(false);
 
-        public void Reject() => ReleaseAction.Invoke();
+        public void Reject() => RejectAction.Invoke();
 
-        public async Task RejectAsync() => await ReleaseActionAsync.Invoke().ConfigureAwait(false);
+        public async Task RejectAsync() => await RejectActionAsync.Invoke().ConfigureAwait(false);
 
         public void Release() => ReleaseAction.Invoke();
 

src/Helsenorge.Messaging/Amqp/Receivers/MessageListener.cs

@@ -254,7 +254,10 @@ private async Task<IncomingMessage> HandleRawMessageAsync(IAmqpMessage message,
                 stopwatch.Stop();
                 return incomingMessage;
             }
-            catch (CertificateException ex)
+            // Network related issues when validating certificate (e.g. CRL offline) is considered temporary
+            // This is why when we get validation error of type RevokedOffline then process it as unhandeled exception
+            // Unhandeled exception will place it on dead letter (after given retries) and it can then be reprocesses when issue has been resolved
+            catch (CertificateException ex) when (ex.Error != CertificateErrors.RevokedOffline)
             {
                 Logger.LogWarning($"{ex.Description}. MessageFunction: {message.MessageFunction} " +
                   $"FromHerId: {message.FromHerId} ToHerId: {message.ToHerId} CpaId: {message.CpaId} " +
@@ -263,7 +266,7 @@ private async Task<IncomingMessage> HandleRawMessageAsync(IAmqpMessage message,
                 await AmqpCore.ReportErrorToExternalSenderAsync(Logger, ex.EventId, message, ex.ErrorCode, ex.Description, ex.AdditionalInformation).ConfigureAwait(false);
                 await MessagingNotification.NotifyHandledExceptionAsync(message, ex).ConfigureAwait(false);
             }
-            catch (SecurityException ex)
+            catch (SecurityException ex) when (ex is not CertificateException)
             {
                 await AmqpCore.ReportErrorToExternalSenderAsync(Logger, EventIds.RemoteCertificate, message, "transport:invalid-certificate", ex.Message, null, ex).ConfigureAwait(false);
                 await MessagingNotification.NotifyHandledExceptionAsync(message, ex).ConfigureAwait(false);
@@ -493,6 +496,9 @@ private void ReportErrorOnRemoteCertificate(X509Certificate2 certificate,
                 case CertificateErrors.RevokedUnknown:
                     throw new CertificateException(error, "transport:revoked-certificate", "Unable to determine revocation status",
                         EventIds.RemoteCertificateRevocation, AdditionalInformation(certificate));
+                case CertificateErrors.RevokedOffline:
+                    throw new CertificateException(error, "transport:revoked-certificate-offline", "Unable to determine revocation status. Service is unavailable",
+                        EventIds.RemoteCertificateRevocationOffline, AdditionalInformation(certificate));
                 default: // since the value is bit-coded
                     throw new CertificateException(error, "transport:invalid-certificate", "More than one error with certificate",
                         EventIds.RemoteCertificate, AdditionalInformation(certificate));
@@ -536,6 +542,10 @@ private void ReportErrorOnLocalCertificate(X509Certificate2 certificate, Certifi
                     description = "Certificate is missing";
                     id = EventIds.LocalCertificate;
                     break;
+                case CertificateErrors.RevokedOffline:
+                    description = "Unable to determine revocation status. Service is unavailable";
+                    id = EventIds.LocalCertificateRevocationOffline;
+                    break;
                 default: // since the value is bit-coded
                     description = "More than one error with certificate";
                     id = EventIds.LocalCertificate;

src/Helsenorge.Registries/Abstractions/CertificateErrors.cs

@@ -38,12 +38,16 @@ public enum CertificateErrors
         /// </summary>
         Revoked = 8,
         /// <summary>
-        /// Unable to determine revocation status. Service may be unavailable
+        /// Unable to determine revocation status
         /// </summary>
         RevokedUnknown = 16,
         /// <summary>
         /// The certificate is missing
         /// </summary>
-        Missing = 32
+        Missing = 32,
+        /// <summary>
+        /// Unable to determine revocation status. Service is unavailable
+        /// </summary>
+        RevokedOffline = 64
     }
 }

src/Helsenorge.Registries/CertificateValidator.cs

@@ -9,6 +9,7 @@
 using System;
 using System.Security.Cryptography.X509Certificates;
 using Helsenorge.Registries.Abstractions;
+using Helsenorge.Registries.Utilities;
 
 namespace Helsenorge.Registries
 {
@@ -18,6 +19,7 @@ namespace Helsenorge.Registries
     public class CertificateValidator : ICertificateValidator
     {
         private readonly bool _useOnlineRevocationCheck;
+        private readonly IX509Chain _chain;
 
         /// <summary>
         /// CertificateValidator constructor
@@ -26,7 +28,20 @@ public class CertificateValidator : ICertificateValidator
         public CertificateValidator(bool useOnlineRevocationCheck = true)
         {
             _useOnlineRevocationCheck = useOnlineRevocationCheck;
+            _chain = new X509ChainWrapper();
         }
+
+        /// <summary>
+        /// CertificateValidator constructor
+        /// </summary>
+        /// <param name="chain">You can set your own X509Chain.</param>
+        /// <param name="useOnlineRevocationCheck">Should online certificate revocation list be used. Optional, default true.</param>
+        internal CertificateValidator(IX509Chain chain, bool useOnlineRevocationCheck = true)
+        {
+            _useOnlineRevocationCheck = useOnlineRevocationCheck;
+            _chain = chain;
+        }
+
         /// <summary>
         /// Validates the provided certificate
         /// </summary>
@@ -49,30 +64,27 @@ public CertificateErrors Validate(X509Certificate2 certificate, X509KeyUsageFlag
                 result |= CertificateErrors.EndDate;
             }
 
-            if(!certificate.HasKeyUsage(usage))
+            if (!certificate.HasKeyUsage(usage))
                 result |= CertificateErrors.Usage;
 
-            var chain = new X509Chain
-            {
-                ChainPolicy =
-                {
-                    RevocationMode = _useOnlineRevocationCheck ? X509RevocationMode.Online : X509RevocationMode.NoCheck,
-                    RevocationFlag = X509RevocationFlag.EntireChain,
-                    UrlRetrievalTimeout = TimeSpan.FromSeconds(30),
-                    VerificationTime = DateTime.Now,
-                }
-            };
 
-            using (chain)
+            _chain.ChainPolicy.RevocationMode = _useOnlineRevocationCheck ? X509RevocationMode.Online : X509RevocationMode.NoCheck;
+            _chain.ChainPolicy.RevocationFlag = X509RevocationFlag.EntireChain;
+            _chain.ChainPolicy.UrlRetrievalTimeout = TimeSpan.FromSeconds(30);
+            _chain.ChainPolicy.VerificationTime = DateTime.Now;
+
+            using (_chain)
             {
-                if (chain.Build(certificate)) return result;
+                if (_chain.Build(certificate)) return result;
 
-                foreach (var status in chain.ChainStatus)
+                foreach (var status in _chain.ChainStatus)
                 {
                     // ReSharper disable once SwitchStatementMissingSomeCases
                     switch (status.Status)
                     {
                         case X509ChainStatusFlags.OfflineRevocation:
+                            result |= CertificateErrors.RevokedOffline;
+                            break;
                         case X509ChainStatusFlags.RevocationStatusUnknown:
                             result |= CertificateErrors.RevokedUnknown;
                             break;

src/Helsenorge.Registries/Utilities/IX509Chain.cs

@@ -0,0 +1,12 @@
+using System;
+using System.Security.Cryptography.X509Certificates;
+
+namespace Helsenorge.Registries.Utilities
+{
+    internal interface IX509Chain : IDisposable
+    {
+        internal X509ChainPolicy ChainPolicy { get; }
+        internal X509ChainStatus[] ChainStatus { get; }
+        internal bool Build(X509Certificate2 certificate);
+    }
+}

src/Helsenorge.Registries/Utilities/X509ChainWrapper.cs

@@ -0,0 +1,28 @@
+using System.Security.Cryptography.X509Certificates;
+
+namespace Helsenorge.Registries.Utilities
+{
+    internal class X509ChainWrapper : IX509Chain
+    {
+        private readonly X509Chain _chain;
+
+        internal X509ChainWrapper()
+        {
+            _chain = new X509Chain();
+        }
+
+        public X509ChainPolicy ChainPolicy => _chain.ChainPolicy;
+
+        public X509ChainStatus[] ChainStatus => _chain.ChainStatus;
+
+        public bool Build(X509Certificate2 certificate)
+        {
+            return _chain.Build(certificate);
+        }
+
+        public void Dispose()
+        {
+            _chain.Dispose();
+        }
+    }
+}

test/Helsenorge.Messaging.Tests.Mocks/MockFactory.cs

@@ -66,17 +66,20 @@ public MockCommunicationParty(MockFactory factory, int herId)
             Synchronous = new MockQueue($"{herId}_sync");
             Error = new MockQueue($"{herId}_error");
             SynchronousReply = new MockQueue($"{herId}_syncreply");
-            
+            DeadLetter = new MockQueue($"{herId}_dl");
+
             factory.Qeueues.Add(Asynchronous.Name, Asynchronous.Messages);
             factory.Qeueues.Add(Synchronous.Name, Synchronous.Messages);
             factory.Qeueues.Add(Error.Name, Error.Messages);
             factory.Qeueues.Add(SynchronousReply.Name, SynchronousReply.Messages);
+            factory.Qeueues.Add(DeadLetter.Name, DeadLetter.Messages);
         }
 
         public MockQueue Asynchronous { get; }
         public MockQueue Synchronous { get; }
         public MockQueue Error { get; }
         public MockQueue SynchronousReply { get; }
+        public MockQueue DeadLetter { get; }
 
     }
 

test/Helsenorge.Messaging.Tests.Mocks/MockMessage.cs

@@ -78,22 +78,14 @@ public Task RelaseAsync()
 
         public void Reject()
         {
+            Queue.Remove(this);
+            DeadLetterQueue.Add(this);
         }
 
         public Task RejectAsync()
-        {
-            return Task.CompletedTask;
-        }
-
-        public void DeadLetter()
         {
             Queue.Remove(this);
             DeadLetterQueue.Add(this);
-        }
-
-        public Task DeadLetterAsync()
-        {
-            DeadLetter();
             return Task.CompletedTask;
         }
 

test/Helsenorge.Messaging.Tests/Amqp/Receivers/AsynchronousReceiveTests.cs

@@ -71,7 +71,7 @@ await RunAsynchronousReceive(
                 messageModification: (m) => { });
         }
 
-        [TestMethod, TestCategory("X509Chain")]
+        [TestMethod]
         public async Task Asynchronous_Receive_CertificateSignError()
         {
             Exception receiveException = null;
@@ -550,6 +550,29 @@ await RunAsynchronousReceive(
                   },
                   messageModification: (m) => { });
         }
+
+        [TestMethod]
+        public async Task Asynchronous_Receive_RemoteCertificateOfflineRevocation()
+        {
+            CertificateValidator.SetError(
+                (c, u) => (u == X509KeyUsageFlags.NonRepudiation) ? CertificateErrors.RevokedOffline : CertificateErrors.None);
+
+            await RunAsynchronousReceive(
+                postValidation: () =>
+                {
+                    Assert.AreEqual(1, MockFactory.Helsenorge.Asynchronous.Messages.Count);
+                    Assert.AreEqual(0, MockFactory.OtherParty.Error.Messages.Count);
+                    Assert.IsNotNull(MockLoggerProvider.FindEntry(EventIds.UnknownError));
+                },
+                wait: () => _unhandledExceptionCalled,
+                received: (m) =>
+                {
+                    Assert.IsTrue(m.SignatureError != CertificateErrors.None);
+                    return Task.CompletedTask;
+                },
+                messageModification: (m) => { });
+        }
+
         [TestMethod]
         public async Task Asynchronous_Receive_RemoteCertificateMultiple()
         {
@@ -788,6 +811,7 @@ private MockMessage CreateAsynchronousMessage()
                 TimeToLive = TimeSpan.FromSeconds(15),
                 ReplyTo = MockFactory.OtherParty.Asynchronous.Name,
                 Queue = MockFactory.Helsenorge.Asynchronous.Messages,
+                DeadLetterQueue = MockFactory.Helsenorge.DeadLetter.Messages
             };
         }
 
@@ -810,6 +834,7 @@ private MockMessage CreateAsynchronousMessageProtected()
                 TimeToLive = TimeSpan.FromSeconds(15),
                 ReplyTo = MockFactory.OtherParty.Asynchronous.Name,
                 Queue = MockFactory.Helsenorge.Asynchronous.Messages,
+                DeadLetterQueue = MockFactory.Helsenorge.DeadLetter.Messages
             };
         }
     }

test/Helsenorge.Messaging.Tests/Amqp/Receivers/ErrorReceiveTests.cs

@@ -138,6 +138,7 @@ private MockMessage CreateMockMessage(XDocument content = null)
                 TimeToLive = TimeSpan.FromSeconds(15),
                 ReplyTo = MockFactory.OtherParty.Synchronous.Name,
                 Queue = MockFactory.Helsenorge.Error.Messages,
+                DeadLetterQueue = MockFactory.Helsenorge.DeadLetter.Messages
             };
         }
     }