Skip to content

Commit

Permalink
Added logging of revoked Certificate and source of the certificate
Browse files Browse the repository at this point in the history
- Added logging of status and information when we get revoked cerificate from certificate validation
- Added logging when we retrive profile from CPA, CPP or Cache to determind where the source of the certificate comes from
  • Loading branch information
ArneHB committed Jun 5, 2024
1 parent 75b345d commit 88ec012
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 14 deletions.
2 changes: 1 addition & 1 deletion Directory.build.props
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project>
<PropertyGroup>
<LangVersion>10.0</LangVersion>
<Version>5.1.0</Version>
<Version>5.1.1</Version>
</PropertyGroup>
<PropertyGroup>
<Authors>Norsk Helsenett SF</Authors>
Expand Down
2 changes: 1 addition & 1 deletion src/Helsenorge.Messaging/MessagingCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ internal IMessageProtection GetDefaultMessageProtection()

internal ICertificateValidator GetDefaultCertificateValidator()
{
return new CertificateValidator(Settings.UseOnlineRevocationCheck);
return new CertificateValidator(_logger, Settings.UseOnlineRevocationCheck);
}

private static IDictionary<string, object> GetSystemInformation()
Expand Down
15 changes: 13 additions & 2 deletions src/Helsenorge.Registries/CertificateValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using System.Security.Cryptography.X509Certificates;
using Helsenorge.Registries.Abstractions;
using Helsenorge.Registries.Utilities;
using Microsoft.Extensions.Logging;

namespace Helsenorge.Registries
{
Expand All @@ -18,15 +19,18 @@ namespace Helsenorge.Registries
/// </summary>
public class CertificateValidator : ICertificateValidator
{
private readonly ILogger _logger;
private readonly bool _useOnlineRevocationCheck;
private readonly IX509Chain _chain;

/// <summary>
/// CertificateValidator constructor
/// </summary>
/// <param name="logger">Default logger</param>
/// <param name="useOnlineRevocationCheck">Should online certificate revocation list be used. Optional, default true.</param>
public CertificateValidator(bool useOnlineRevocationCheck = true)
public CertificateValidator(ILogger logger, bool useOnlineRevocationCheck = true)
{
_logger = logger;
_useOnlineRevocationCheck = useOnlineRevocationCheck;
_chain = new X509ChainWrapper();
}
Expand All @@ -35,11 +39,13 @@ public CertificateValidator(bool useOnlineRevocationCheck = true)
/// CertificateValidator constructor
/// </summary>
/// <param name="chain">You can set your own X509Chain.</param>
/// <param name="logger">Default logger</param>
/// <param name="useOnlineRevocationCheck">Should online certificate revocation list be used. Optional, default true.</param>
internal CertificateValidator(IX509Chain chain, bool useOnlineRevocationCheck = true)
internal CertificateValidator(IX509Chain chain, ILogger logger, bool useOnlineRevocationCheck = true)
{
_useOnlineRevocationCheck = useOnlineRevocationCheck;
_chain = chain;
_logger = logger;
}

/// <summary>
Expand Down Expand Up @@ -79,6 +85,8 @@ public CertificateErrors Validate(X509Certificate2 certificate, X509KeyUsageFlag

foreach (var status in _chain.ChainStatus)
{
_logger.LogInformation("Certificate chain validation. " +
$"Status Information: {status.StatusInformation} Status Flag: {status.Status}");
// ReSharper disable once SwitchStatementMissingSomeCases
switch (status.Status)
{
Expand All @@ -93,6 +101,9 @@ public CertificateErrors Validate(X509Certificate2 certificate, X509KeyUsageFlag
break;
}
}
if (result != CertificateErrors.None)
_logger.LogWarning($"Certificate chain validation failed. Certificate: {certificate.Subject} Result: {result}");

return result;
}
}
Expand Down
8 changes: 7 additions & 1 deletion src/Helsenorge.Registries/CollaborationProtocolRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public CollaborationProtocolRegistry(
_addressRegistry = addressRegistry;
_logger = logger;
_invoker = new SoapServiceInvoker(settings.WcfConfiguration);
CertificateValidator = new CertificateValidator(_settings.UseOnlineRevocationCheck);
CertificateValidator = new CertificateValidator(logger, _settings.UseOnlineRevocationCheck);
}

/// <inheritdoc cref="FindProtocolForCounterpartyAsync"/>
Expand All @@ -82,11 +82,13 @@ public async Task<CollaborationProtocolProfile> FindProtocolForCounterpartyAsync
// if the certificates are valid, only then do we return a value from the cache
if (errors == CertificateErrors.None)
{
_logger.LogInformation($"{key} - Retrive from cache");

Check failure

Code scanning / CodeQL

Clear text storage of sensitive information High

This stores sensitive data returned by
access to local variable encodedPassword : String
as clear text.
return result;
}
}
try
{
_logger.LogInformation($"{key} - Retrive from registry");

Check failure

Code scanning / CodeQL

Clear text storage of sensitive information High

This stores sensitive data returned by
access to local variable encodedPassword : String
as clear text.
xmlString = await FindProtocolForCounterparty(counterpartyHerId).ConfigureAwait(false);
}
catch (FaultException<CPAService.GenericFault> ex)
Expand Down Expand Up @@ -156,6 +158,7 @@ public async Task<CollaborationProtocolProfile> FindAgreementByIdAsync(Guid id,
// if the certificates are valid, only then do we return a value from the cache
if (errors == CertificateErrors.None)
{
_logger.LogInformation($"{key} - Retrive from cache");
return result;
}
}
Expand All @@ -164,6 +167,7 @@ public async Task<CollaborationProtocolProfile> FindAgreementByIdAsync(Guid id,

try
{
_logger.LogInformation($"{key} - Retrive from registry");
details = await FindAgreementById(id).ConfigureAwait(false);
}
catch (FaultException ex)
Expand Down Expand Up @@ -220,6 +224,7 @@ public async Task<CollaborationProtocolProfile> FindAgreementForCounterpartyAsyn
// if the certificates are valid, only then do we return a value from the cache
if (errors == CertificateErrors.None)
{
_logger.LogInformation($"{key} - Retrive from cache");

Check failure

Code scanning / CodeQL

Clear text storage of sensitive information High

This stores sensitive data returned by
access to local variable encodedPassword : String
as clear text.
return result;
}
}
Expand All @@ -228,6 +233,7 @@ public async Task<CollaborationProtocolProfile> FindAgreementForCounterpartyAsyn

try
{
_logger.LogInformation($"{key} - Retrive from registry");

Check failure

Code scanning / CodeQL

Clear text storage of sensitive information High

This stores sensitive data returned by
access to local variable encodedPassword : String
as clear text.
details = await FindAgreementForCounterparty(myHerId, counterpartyHerId).ConfigureAwait(false);
}
catch (FaultException ex)
Expand Down
33 changes: 24 additions & 9 deletions test/Helsenorge.Registries.Tests/CertificateValidatorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,48 +9,63 @@
using System.Security.Cryptography.X509Certificates;
using Helsenorge.Registries.Abstractions;
using Helsenorge.Registries.Tests.Mocks;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using ILogger = Microsoft.Extensions.Logging.ILogger;

namespace Helsenorge.Registries.Tests
{
[TestClass]
public class CertificateValidatorTests
{
private ILogger _logger;

[TestInitialize]
public void Setup()
{
var serviceCollection = new ServiceCollection();
serviceCollection.AddLogging(loggingBuilder => loggingBuilder.AddDebug());
var provider = serviceCollection.BuildServiceProvider();
var loggerFactory = provider.GetRequiredService<ILoggerFactory>();
_logger = loggerFactory.CreateLogger<CertificateValidatorTests>();
}

[TestMethod]
public void CertificateValidation_ArgumentNullException()
{
var validator = new CertificateValidator(new MockX509Chain());
var validator = new CertificateValidator(new MockX509Chain(), _logger);
var error = validator.Validate(null, X509KeyUsageFlags.NonRepudiation);
Assert.AreEqual(CertificateErrors.Missing, error);
}
[TestMethod]
public void CertificateValidation_None()
{
var validator = new CertificateValidator(new MockX509Chain());
var validator = new CertificateValidator(new MockX509Chain(), _logger);
var error = validator.Validate(TestCertificates.CounterpartyPublicSignature,
X509KeyUsageFlags.NonRepudiation);
Assert.AreEqual(CertificateErrors.None, error);
}
[TestMethod]
public void CertificateValidation_StartDate()
{
var validator = new CertificateValidator(new MockX509Chain());
var validator = new CertificateValidator(new MockX509Chain(), _logger);
var error = validator.Validate(TestCertificates.CounterpartyPublicSignatureInvalidStart,
X509KeyUsageFlags.NonRepudiation);
Assert.AreEqual(CertificateErrors.StartDate, error);
}
[TestMethod]
public void CertificateValidation_EndDate()
{
var validator = new CertificateValidator(new MockX509Chain());
var validator = new CertificateValidator(new MockX509Chain(), _logger);
var error = validator.Validate(TestCertificates.CounterpartyPublicSignatureInvalidEnd,
X509KeyUsageFlags.NonRepudiation);
Assert.AreEqual(CertificateErrors.EndDate, error);
}
[TestMethod]
public void CertificateValidation_Usage()
{
var validator = new CertificateValidator(new MockX509Chain());
var validator = new CertificateValidator(new MockX509Chain(), _logger);
var error = validator.Validate(TestCertificates.CounterpartyPublicSignature,
X509KeyUsageFlags.KeyEncipherment);
Assert.AreEqual(CertificateErrors.Usage, error);
Expand All @@ -65,7 +80,7 @@ public void X509Certificate2Extensions_KeyUsage()
[TestMethod]
public void CertificateValidation_Multiple()
{
var validator = new CertificateValidator(new MockX509Chain());
var validator = new CertificateValidator(new MockX509Chain(), _logger);
var error = validator.Validate(TestCertificates.CounterpartyPublicSignatureInvalidStart,
X509KeyUsageFlags.KeyEncipherment);
Assert.AreEqual(CertificateErrors.StartDate | CertificateErrors.Usage, error);
Expand All @@ -86,7 +101,7 @@ public void CertificateValidation_RevokeOffline()
StatusInformation = "Offline revocation"
}
});
var validator = new CertificateValidator(mockChain);
var validator = new CertificateValidator(mockChain, _logger);
var error = validator.Validate(testCertificate, usage);
Assert.AreEqual(CertificateErrors.RevokedOffline, error);
}
Expand All @@ -111,7 +126,7 @@ public void CertificateValidation_RevokeMultiple()
StatusInformation = "Revoked"
}
});
var validator = new CertificateValidator(mockChain);
var validator = new CertificateValidator(mockChain, _logger);
var error = validator.Validate(testCertificate, usage);
Assert.AreEqual(CertificateErrors.RevokedOffline | CertificateErrors.Revoked, error);
}
Expand All @@ -136,7 +151,7 @@ public void CertificateValidation_ChainStatusOther()
StatusInformation = "Invalid extension"
}
});
var validator = new CertificateValidator(mockChain);
var validator = new CertificateValidator(mockChain, _logger);
var error = validator.Validate(testCertificate, usage);
Assert.AreEqual(CertificateErrors.None, error);
}
Expand Down

0 comments on commit 88ec012

Please sign in to comment.