SCAN4NET-244 Add logging for ServerCertificateValidationChain #2324
Conversation
hashicorp-vault-sonar-prod
bot
changed the title
| logger.LogWarning(Resources.WARN_TrustStore_Chain_RootCertificateNotFound, | ||
| rootInChain.Certificate.Issuer, | ||
| rootInChain.Certificate.Thumbprint, | ||
| trustStoreFile, | ||
| SonarProperties.TruststorePath); | ||
| return false; |
There was a problem hiding this comment.
This code path is untestable and likely unreachable (because it requires a exclusively "partial chain" error when the callback is invoked and a sub-sequent exclusively "untrusted root" error here which can only be achieved if the root ca is in the truststore file).
I would still keep the code path as is, just to be safe. If we ever encounter this in some logs, we can try to figure out the environment (most likely some OS specific scenario) in which this could happen.
There was a problem hiding this comment.
Can you add a comment there?
| logger.AssertDebugLogged($""" | ||
| The remote server certificate is not trusted by the operating system. The scanner is checking the certificate against the certificates provided by the file '{trustStoreFile.FileName}' (specified via the sonar.scanner.truststorePath parameter or its default value). | ||
| """); | ||
| logger.Warnings.Should().ContainSingle(because: "the warning is either WARN_TrustStore_Chain_Invalid or WARN_TrustStore_OtherChainStatus depending on the environment."); |
There was a problem hiding this comment.
The code path taken is different in CI and locally. It doesn't really matter what code path fails at the end, so having at least some warning is good enough).
See this CI failure for the two warnings that are issued locally and in CI:
https://dev.azure.com/sonarsource/DotNetTeam%20Project/_build/results?buildId=110200&view=logs&j=3635ba3c-75de-526b-7d9e-c1be443d4750&t=8957dee2-f992-5acc-d4ff-d7b1b1efb951&l=193
|
|
||
| * {x.StatusInformation.TrimEnd()} | ||
| """), x => x.ToString())); | ||
| logger.LogWarning(Resources.WARN_TrustStore_OtherChainStatus, SonarProperties.TruststorePath, ChainStatusAsBulletList(chain)); |
There was a problem hiding this comment.
I should have probably asked this before, my understanding is whenever we get an SSL trust issue, we will stop the execution of the scanner, then why do we log the messages as warnings?
There was a problem hiding this comment.
Its a warning followed by a terminating error next. I don't know what is best practice in such a case, but a warning seems fine to me.
| logger.LogWarning(Resources.WARN_TrustStore_Chain_RootCertificateNotFound, | ||
| rootInChain.Certificate.Issuer, | ||
| rootInChain.Certificate.Thumbprint, | ||
| trustStoreFile, | ||
| SonarProperties.TruststorePath); | ||
| return false; |
There was a problem hiding this comment.
Can you add a comment there?
|
pavel-mikula-sonarsource
merged commit 5caa02c
into
feature/MMF_4168
SCAN4NET-244
Part of SCAN4NET-209