SCAN4NET-227 Use system trusted certificate or JVM certificate store #2330
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -24,19 +24,75 @@ namespace SonarScanner.MSBuild.PreProcessor.AnalysisConfigProcessing.Processors; | |||||
| /// Map property name to another property name and/or value to pass them through the SONAR_SCANNER_OPTS | ||||||
| /// environment variable to Scanner CLI 5. | ||||||
| /// </summary> | ||||||
| public class TruststorePropertiesProcessor( | ||||||
| ProcessedArgs localSettings, | ||||||
| IDictionary<string, string> serverProperties, | ||||||
| IFileWrapper fileWrapper, | ||||||
| IOperatingSystemProvider operatingSystemProvider) | ||||||
| : AnalysisConfigProcessorBase(localSettings, serverProperties) | ||||||
| { | ||||||
| const string JAVAX_NET_SSL_TRUST_STORE = "javax.net.ssl.trustStore"; | ||||||
| const string JAVAX_NET_SSL_TRUST_STORE_TYPE = "javax.net.ssl.trustStoreType"; | ||||||
| const string JAVAX_NET_SSL_TRUST_STORE_PASSWORD = "javax.net.ssl.trustStorePassword"; | ||||||
|
|
||||||
| public override void Update(AnalysisConfig config) | ||||||
| { | ||||||
| if (new Uri(LocalSettings.ServerInfo.ServerUrl).Scheme != "https" | ||||||
|
There was a problem hiding this comment. This constructor throws. Consider using There was a problem hiding this comment.
Suggested change
|
||||||
| || LocalSettings.ServerInfo.ServerUrl is SonarPropertiesDefault.SonarcloudUrl) | ||||||
| { | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| var truststorePath = PropertyValueOrDefault(SonarProperties.TruststorePath, LocalSettings.TruststorePath); | ||||||
| var truststorePassword = PropertyValueOrDefault(SonarProperties.TruststorePassword, LocalSettings.TruststorePassword); | ||||||
|
|
||||||
| if (truststorePath is null) | ||||||
| { | ||||||
| if (operatingSystemProvider.IsUnix()) | ||||||
| { | ||||||
| truststorePath = FindJavaTruststorePath(); | ||||||
| truststorePassword ??= TruststorePasswordFromEnvironment(); | ||||||
| } | ||||||
| else | ||||||
| { | ||||||
| MapProperty(config, JAVAX_NET_SSL_TRUST_STORE_TYPE, "Windows-ROOT"); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| MapProperty(config, JAVAX_NET_SSL_TRUST_STORE, truststorePath, ConvertToJavaPath, EnsureSurroundedByQuotes); | ||||||
| config.LocalSettings.RemoveAll(x => x.Id is SonarProperties.TruststorePath or SonarProperties.TruststorePassword); | ||||||
|
|
||||||
| if (truststorePassword is not null) | ||||||
| { | ||||||
| MapProperty(config, JAVAX_NET_SSL_TRUST_STORE_PASSWORD, truststorePassword, EnsureSurroundedByQuotes); | ||||||
| config.LocalSettings.Add(new Property(SonarProperties.TruststorePassword, truststorePassword)); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| private static string TruststorePasswordFromEnvironment() | ||||||
| { | ||||||
| var sonarScannerOpts = Environment.GetEnvironmentVariable("SONAR_SCANNER_OPTS"); | ||||||
| if (sonarScannerOpts is null) | ||||||
| { | ||||||
| return null; | ||||||
| } | ||||||
|
|
||||||
| return sonarScannerOpts.Split(' ').FirstOrDefault(x => x.StartsWith($"-D{JAVAX_NET_SSL_TRUST_STORE_PASSWORD}=")) is { } truststorePassword | ||||||
|
There was a problem hiding this comment. I think it is safe to specify
Suggested change
There was a problem hiding this comment. I am not sure, I think the properties are case sensitive. |
||||||
| ? truststorePassword.Substring($"-D{JAVAX_NET_SSL_TRUST_STORE_PASSWORD}=".Length) | ||||||
| : null; | ||||||
| } | ||||||
|
|
||||||
| private string FindJavaTruststorePath() | ||||||
| { | ||||||
| var javaHome = Environment.GetEnvironmentVariable("JAVA_HOME"); | ||||||
| if (javaHome is null) | ||||||
| { | ||||||
| // TODO: propagate the error? | ||||||
|
There was a problem hiding this comment. I don't think so. A debug log should be enough. There was a problem hiding this comment. It will not fail before the end step then. There was a problem hiding this comment. If the JRE provisioning is enabled, There was a problem hiding this comment. If this is from letsencrypt, it should be trusted without giving a store. I would be for failing the execution to follow the fail fast principle but I am not 100% sure that in this case there is no scenario where it would work. There was a problem hiding this comment. The scenario is:
-> We would fail the begin step, even though everything would just work. There was a problem hiding this comment. Can we safely assume that the If we take the same scenario with JAVA_HOME set, we will tell the provisioned JRE to use the local There was a problem hiding this comment. The root ca certs change on a regular basis (see e.g. here https://learn.microsoft.com/en-us/security/trusted-root/release-notes). It is likely that we will have different CA roots in both locations. There was a problem hiding this comment. Note: I think this is a problem the scanner-cli may have as well. There was a problem hiding this comment. Also: As a user, you can install custom root CAs in your JAVA_HOME, but you can not install root CAs in the provisioned JRE. Therefore, we should prefer JAVA_HOME over the provisioned JRE. There was a problem hiding this comment. Sounds good to me, I will only do a log. |
||||||
| return null; | ||||||
| } | ||||||
|
|
||||||
| var javaTruststorePath = Path.Combine(javaHome, "lib", "security", "cacerts"); | ||||||
|
There was a problem hiding this comment. Please add a link to some kind of documentation or ticket about the source of these path. |
||||||
| return fileWrapper.Exists(javaTruststorePath) ? javaTruststorePath : null; | ||||||
| } | ||||||
|
|
||||||
| private string PropertyValueOrDefault(string propertyName, string defaultValue) => | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could additionally assert that
fileWrapper.Exists(javaHomeCacerts)was called once.