You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2020/05/18 22:29:21 UTC
[GitHub] [maven-wagon] spyhunter99 opened a new pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
spyhunter99 opened a new pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] mthmulders commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427049478
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -593,13 +603,189 @@ private static CloseableHttpClient createClient()
*/
private BasicAuthScope proxyAuth;
+ /**
+ * initializes a custom http client given user specified keystore/truststore
+ * information from settings.xml
+ * see httsp://issues.apache.org/jira/browser/MNG-5583
+ * @return a client
+ * @throws SSLInitializationException if there's an issue loading keystore/truststore
+ */
+ private CloseableHttpClient initilaizeLocalHttpClientWithCustomSslSocketFactory() throws SSLInitializationException
+ {
+ String sslProtocolsStr = System.getProperty( "https.protocols" );
+ String cipherSuitesStr = System.getProperty( "https.cipherSuites" );
+ String[] sslProtocols = sslProtocolsStr != null ? sslProtocolsStr.split( " *, *" ) : null;
+ String[] cipherSuites = cipherSuitesStr != null ? cipherSuitesStr.split( " *, *" ) : null;
+
+ SSLSocketFactory socketFactory = null;
+ TrustManager[] trustManagers = null;
+ KeyManager[] keyManagers = null;
+ try
+ {
+ if ( authenticationInfo.getTrustStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getTrustStoreType() == null ? "JKS"
+ : authenticationInfo.getTrustStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ //also a null password is fine with windows and even with JKS files
+ char[] keyPass = authenticationInfo.getTrustStorePassword() != null
+ ? authenticationInfo.getTrustStorePassword().toCharArray()
+ : null;
+ keystore.load( fis, keyPass );
+ if ( keyPass != null )
Review comment:
I assume you're doing this to clean up the password from memory, but it currently still resides in the `authenticationInfo` variable. So I think it has little to no effect.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427644990
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -593,13 +603,189 @@ private static CloseableHttpClient createClient()
*/
private BasicAuthScope proxyAuth;
+ /**
+ * initializes a custom http client given user specified keystore/truststore
+ * information from settings.xml
+ * see httsp://issues.apache.org/jira/browser/MNG-5583
+ * @return a client
+ * @throws SSLInitializationException if there's an issue loading keystore/truststore
+ */
+ private CloseableHttpClient initilaizeLocalHttpClientWithCustomSslSocketFactory() throws SSLInitializationException
+ {
+ String sslProtocolsStr = System.getProperty( "https.protocols" );
+ String cipherSuitesStr = System.getProperty( "https.cipherSuites" );
+ String[] sslProtocols = sslProtocolsStr != null ? sslProtocolsStr.split( " *, *" ) : null;
+ String[] cipherSuites = cipherSuitesStr != null ? cipherSuitesStr.split( " *, *" ) : null;
+
+ SSLSocketFactory socketFactory = null;
+ TrustManager[] trustManagers = null;
+ KeyManager[] keyManagers = null;
+ try
+ {
+ if ( authenticationInfo.getTrustStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getTrustStoreType() == null ? "JKS"
+ : authenticationInfo.getTrustStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ //also a null password is fine with windows and even with JKS files
+ char[] keyPass = authenticationInfo.getTrustStorePassword() != null
+ ? authenticationInfo.getTrustStorePassword().toCharArray()
+ : null;
+ keystore.load( fis, keyPass );
+ if ( keyPass != null )
+ {
+ for ( int i = 0; i < keyPass.length; i++ )
+ {
+ keyPass[i] = 0;
+ }
+ }
+ if ( fis != null )
+ {
+ fis.close();
+ }
+ String alg = KeyManagerFactory.getDefaultAlgorithm();
+ TrustManagerFactory fac = TrustManagerFactory.getInstance( alg );
+ fac.init( keystore );
+ trustManagers = fac.getTrustManagers();
+ }
+
+ if ( authenticationInfo.getKeyStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getKeyStoreType() == null ? "JKS"
+ : authenticationInfo.getKeyStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ String alg = KeyManagerFactory.getDefaultAlgorithm();
+ char[] keyStorePass = authenticationInfo.getKeyStorePassword() != null
+ ? authenticationInfo.getKeyStorePassword().toCharArray()
+ : null;
+ char[] keyPass = authenticationInfo.getKeyPassword() != null
+ ? authenticationInfo.getKeyPassword().toCharArray()
+ : null;
+ KeyManagerFactory fac = KeyManagerFactory.getInstance( alg );
+ keystore.load( fis, keyStorePass );
+ fac.init( keystore, keyPass );
+ if ( keyPass != null )
+ {
+ for ( int i = 0; i < keyPass.length; i++ )
+ {
+ keyPass[i] = 0;
+ }
+ }
+ keyPass = null;
+ String alias = authenticationInfo.getKeyAlias();
+ if ( alias != null )
+ {
+ //borrowed from tomcat's code
+ //user has explicitly specified a key alias, great.
+ //let's make sure it exists in the key store that it has a
+ //key pair
+ if ( keystore.containsAlias( alias ) )
+ {
+ if ( !keystore.isKeyEntry( alias ) )
+ {
+ alias = null;
+ }
+ }
+ else if ( keystore.containsAlias( alias.toLowerCase() ) )
Review comment:
this was borrowed from tomcat's code base, apparently in certain situational, key aliases are case folded. i've never seen it in the wild either, it's always been case sensitive but i figured there was a reason for this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#issuecomment-632377448
regarding unit tests, whats the difference between the regular http wagon and the light weight version?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427658047
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -593,13 +603,189 @@ private static CloseableHttpClient createClient()
*/
private BasicAuthScope proxyAuth;
+ /**
+ * initializes a custom http client given user specified keystore/truststore
+ * information from settings.xml
+ * see httsp://issues.apache.org/jira/browser/MNG-5583
+ * @return a client
+ * @throws SSLInitializationException if there's an issue loading keystore/truststore
+ */
+ private CloseableHttpClient initilaizeLocalHttpClientWithCustomSslSocketFactory() throws SSLInitializationException
+ {
+ String sslProtocolsStr = System.getProperty( "https.protocols" );
+ String cipherSuitesStr = System.getProperty( "https.cipherSuites" );
+ String[] sslProtocols = sslProtocolsStr != null ? sslProtocolsStr.split( " *, *" ) : null;
+ String[] cipherSuites = cipherSuitesStr != null ? cipherSuitesStr.split( " *, *" ) : null;
+
+ SSLSocketFactory socketFactory = null;
+ TrustManager[] trustManagers = null;
+ KeyManager[] keyManagers = null;
+ try
+ {
+ if ( authenticationInfo.getTrustStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getTrustStoreType() == null ? "JKS"
+ : authenticationInfo.getTrustStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
Review comment:
ok, i was unsure of the java min version for wagon so i went with the safest route.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 removed a comment on pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 removed a comment on pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#issuecomment-630471431
hang on,forgot i had disabled checkstyle. sorry stand by
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427648575
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -382,12 +390,12 @@ private static PoolingHttpClientConnectionManager createConnManager()
{
sslConnectionSocketFactory =
new SSLConnectionSocketFactory( HttpsURLConnection.getDefaultSSLSocketFactory(), sslProtocols,
- cipherSuites,
+ cipherSuites,
SSLConnectionSocketFactory.BROWSER_COMPATIBLE_HOSTNAME_VERIFIER );
}
-
+
Registry<ConnectionSocketFactory> registry = RegistryBuilder.<ConnectionSocketFactory>create().register( "http",
- PlainConnectionSocketFactory.INSTANCE ).register(
+ PlainConnectionSocketFactory.INSTANCE ).register(
Review comment:
what's your preference for addressing this? with everything on one line, it was triggering other checkstyle rules for max length
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427635297
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/authentication/AuthenticationInfo.java
##########
@@ -51,7 +52,300 @@
* The absolute path to private key file
*/
private String privateKey;
+
+ /**
+ *
Review comment:
np, someone should tell the netbeans folks then. auto formatter does that. If it bothers you all that much, add it to the check style xml file
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427657170
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -593,13 +603,189 @@ private static CloseableHttpClient createClient()
*/
private BasicAuthScope proxyAuth;
+ /**
+ * initializes a custom http client given user specified keystore/truststore
+ * information from settings.xml
+ * see httsp://issues.apache.org/jira/browser/MNG-5583
+ * @return a client
+ * @throws SSLInitializationException if there's an issue loading keystore/truststore
+ */
+ private CloseableHttpClient initilaizeLocalHttpClientWithCustomSslSocketFactory() throws SSLInitializationException
+ {
+ String sslProtocolsStr = System.getProperty( "https.protocols" );
+ String cipherSuitesStr = System.getProperty( "https.cipherSuites" );
+ String[] sslProtocols = sslProtocolsStr != null ? sslProtocolsStr.split( " *, *" ) : null;
+ String[] cipherSuites = cipherSuitesStr != null ? cipherSuitesStr.split( " *, *" ) : null;
+
+ SSLSocketFactory socketFactory = null;
+ TrustManager[] trustManagers = null;
+ KeyManager[] keyManagers = null;
+ try
+ {
+ if ( authenticationInfo.getTrustStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getTrustStoreType() == null ? "JKS"
+ : authenticationInfo.getTrustStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ //also a null password is fine with windows and even with JKS files
+ char[] keyPass = authenticationInfo.getTrustStorePassword() != null
+ ? authenticationInfo.getTrustStorePassword().toCharArray()
+ : null;
+ keystore.load( fis, keyPass );
+ if ( keyPass != null )
Review comment:
true, i'll remove it if you want
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#issuecomment-633298661
anything else that needs to changed on this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r430018381
##########
File path: wagon-providers/wagon-http-lightweight/src/test/java/org/apache/maven/wagon/providers/http/TckTest.java
##########
@@ -20,6 +20,7 @@
*/
import org.apache.maven.wagon.tck.http.GetWagonTests;
+import org.apache.maven.wagon.tck.http.HttpsClientCertGetWagonTests;
Review comment:
good catch, i think i had attempted to enable that test profile for the lightweight provider, but considering your explanation of when it's used, it's out of scope anyhow.
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/JSSEKeyManager.java
##########
@@ -0,0 +1,139 @@
+package org.apache.maven.wagon.shared.http;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.net.Socket;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.cert.X509Certificate;
+
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import javax.net.ssl.X509KeyManager;
+
+/**
+ * This was borrowed from Apache Tomcat
Review comment:
Doesn't look like it. I'll see if I can whip one up. The purpose of the class is to force a selection for a specific certificate. The default JRE implementation will pick the key pair that matches the server's criteria as part of the hand shake. If you have more than one that matches the criteria but only a specific one will allow access, you're hosed.
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/JSSEKeyManager.java
##########
@@ -0,0 +1,139 @@
+package org.apache.maven.wagon.shared.http;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.net.Socket;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.cert.X509Certificate;
+
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import javax.net.ssl.X509KeyManager;
+
+/**
+ * This was borrowed from Apache Tomcat
Review comment:
Doesn't look like it. I'll see if I can whip one up. The purpose of the class is to force a selection for a specific certificate. The default JRE implementation will pick the first key pair that matches the server's criteria as part of the hand shake. If you have more than one that matches the criteria but only a specific one will allow access, you're hosed.
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/JSSEKeyManager.java
##########
@@ -0,0 +1,139 @@
+package org.apache.maven.wagon.shared.http;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.net.Socket;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.cert.X509Certificate;
+
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import javax.net.ssl.X509KeyManager;
+
+/**
+ * This was borrowed from Apache Tomcat
Review comment:
actually i'm glad you brought this up, definitely needs a test case. I shouldn't have copied tomcat's code without reading the details. It's purpose is to select the server certificate to for when a single tomcat server hosts multiple domains all in the same key store. We actually need the opposite, client certificate selection.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#issuecomment-632410550
>
>
> > regarding unit tests, whats the difference between the regular http wagon and the light weight version?
>
> Bad naming. HttpWagon uses Apache HttpClient, LightweightClient uses `HttpURLConnection`.
perhaps i asked the wrong question. In what context is the lightweight version used vs the httpwagon version? i'm trying to figure out if i need to (or should) add the pki support mechanisms to the lightweight setup
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] mthmulders commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427749684
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/repository/Repository.java
##########
@@ -19,12 +19,12 @@
* under the License.
*/
+import java.util.Properties;
Review comment:
I think you missed one over here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] mthmulders commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427050406
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -593,13 +603,189 @@ private static CloseableHttpClient createClient()
*/
private BasicAuthScope proxyAuth;
+ /**
+ * initializes a custom http client given user specified keystore/truststore
+ * information from settings.xml
+ * see httsp://issues.apache.org/jira/browser/MNG-5583
+ * @return a client
+ * @throws SSLInitializationException if there's an issue loading keystore/truststore
+ */
+ private CloseableHttpClient initilaizeLocalHttpClientWithCustomSslSocketFactory() throws SSLInitializationException
+ {
+ String sslProtocolsStr = System.getProperty( "https.protocols" );
+ String cipherSuitesStr = System.getProperty( "https.cipherSuites" );
+ String[] sslProtocols = sslProtocolsStr != null ? sslProtocolsStr.split( " *, *" ) : null;
+ String[] cipherSuites = cipherSuitesStr != null ? cipherSuitesStr.split( " *, *" ) : null;
+
+ SSLSocketFactory socketFactory = null;
+ TrustManager[] trustManagers = null;
+ KeyManager[] keyManagers = null;
+ try
+ {
+ if ( authenticationInfo.getTrustStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getTrustStoreType() == null ? "JKS"
+ : authenticationInfo.getTrustStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ //also a null password is fine with windows and even with JKS files
+ char[] keyPass = authenticationInfo.getTrustStorePassword() != null
+ ? authenticationInfo.getTrustStorePassword().toCharArray()
+ : null;
+ keystore.load( fis, keyPass );
+ if ( keyPass != null )
+ {
+ for ( int i = 0; i < keyPass.length; i++ )
+ {
+ keyPass[i] = 0;
+ }
+ }
+ if ( fis != null )
+ {
+ fis.close();
+ }
+ String alg = KeyManagerFactory.getDefaultAlgorithm();
+ TrustManagerFactory fac = TrustManagerFactory.getInstance( alg );
+ fac.init( keystore );
+ trustManagers = fac.getTrustManagers();
+ }
+
+ if ( authenticationInfo.getKeyStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getKeyStoreType() == null ? "JKS"
+ : authenticationInfo.getKeyStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ String alg = KeyManagerFactory.getDefaultAlgorithm();
+ char[] keyStorePass = authenticationInfo.getKeyStorePassword() != null
+ ? authenticationInfo.getKeyStorePassword().toCharArray()
+ : null;
+ char[] keyPass = authenticationInfo.getKeyPassword() != null
+ ? authenticationInfo.getKeyPassword().toCharArray()
+ : null;
+ KeyManagerFactory fac = KeyManagerFactory.getInstance( alg );
+ keystore.load( fis, keyStorePass );
+ fac.init( keystore, keyPass );
+ if ( keyPass != null )
+ {
+ for ( int i = 0; i < keyPass.length; i++ )
+ {
+ keyPass[i] = 0;
+ }
+ }
+ keyPass = null;
+ String alias = authenticationInfo.getKeyAlias();
+ if ( alias != null )
+ {
+ //borrowed from tomcat's code
+ //user has explicitly specified a key alias, great.
+ //let's make sure it exists in the key store that it has a
+ //key pair
+ if ( keystore.containsAlias( alias ) )
+ {
+ if ( !keystore.isKeyEntry( alias ) )
+ {
+ alias = null;
+ }
+ }
+ else if ( keystore.containsAlias( alias.toLowerCase() ) )
Review comment:
Why this additional check on the lower-cased variant?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] mthmulders commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427750009
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -593,6 +610,141 @@ private static CloseableHttpClient createClient()
*/
private BasicAuthScope proxyAuth;
+ /**
+ * initializes a custom http client given user specified keystore/truststore
+ * information from settings.xml
+ * see https://issues.apache.org/jira/browser/MNG-5583
+ * @return a client
+ * @throws SSLInitializationException if there's an issue loading keystore/truststore
+ */
+ private CloseableHttpClient initilaizeLocalHttpClientWithCustomSslSocketFactory() throws SSLInitializationException
+ {
+ String sslProtocolsStr = System.getProperty( "https.protocols" );
+ String cipherSuitesStr = System.getProperty( "https.cipherSuites" );
+ String[] sslProtocols = sslProtocolsStr != null ? sslProtocolsStr.split( " *, *" ) : null;
+ String[] cipherSuites = cipherSuitesStr != null ? cipherSuitesStr.split( " *, *" ) : null;
+
+ SSLSocketFactory socketFactory = null;
+ TrustManager[] trustManagers = null;
+ KeyManager[] keyManagers = null;
+ try
+ {
+ if ( authenticationInfo.getTrustStore() != null )
+ {
+ KeyStore keystore = initializeKeyStore( authenticationInfo.getTrustStoreType(),
Review comment:
Conceptually, the variable of type `Keystore` here is a _trust store_, but it's no big deal to me.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r428988352
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/authentication/AuthenticationInfo.java
##########
@@ -51,7 +52,300 @@
* The absolute path to private key file
*/
private String privateKey;
+
+ /**
+ *
+ *
+ * The path to the trust store. If not defined, the JRE's cacert store is
+ * used.
+ *
+ *
+ */
+ private String trustStore;
+
+ /**
+ *
+ *
+ * The password to the trust store.
+ *
+ *
+ */
+ private String trustStorePassword;
Review comment:
i agree with @michael-o otherwise i would have used char[]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] mthmulders commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427051853
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -593,13 +603,189 @@ private static CloseableHttpClient createClient()
*/
private BasicAuthScope proxyAuth;
+ /**
+ * initializes a custom http client given user specified keystore/truststore
+ * information from settings.xml
+ * see httsp://issues.apache.org/jira/browser/MNG-5583
+ * @return a client
+ * @throws SSLInitializationException if there's an issue loading keystore/truststore
+ */
+ private CloseableHttpClient initilaizeLocalHttpClientWithCustomSslSocketFactory() throws SSLInitializationException
+ {
+ String sslProtocolsStr = System.getProperty( "https.protocols" );
+ String cipherSuitesStr = System.getProperty( "https.cipherSuites" );
+ String[] sslProtocols = sslProtocolsStr != null ? sslProtocolsStr.split( " *, *" ) : null;
+ String[] cipherSuites = cipherSuitesStr != null ? cipherSuitesStr.split( " *, *" ) : null;
+
+ SSLSocketFactory socketFactory = null;
+ TrustManager[] trustManagers = null;
+ KeyManager[] keyManagers = null;
+ try
+ {
+ if ( authenticationInfo.getTrustStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getTrustStoreType() == null ? "JKS"
+ : authenticationInfo.getTrustStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
Review comment:
Using Java 7's `try-with-resources` might make the code from line 632 - 652 a little more compact.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427651069
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/AbstractWagon.java
##########
@@ -36,21 +49,6 @@
import org.apache.maven.wagon.resource.Resource;
import org.codehaus.plexus.util.IOUtil;
-import java.io.File;
Review comment:
artifact of netbean's fix imports. i'll roll it back
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] michael-o commented on pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#issuecomment-632384688
>
>
> regarding unit tests, whats the difference between the regular http wagon and the light weight version?
Bad naming. HttpWagon uses Apache HttpClient, LightweightClient uses `HttpURLConnection`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] mthmulders commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427052192
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/authentication/AuthenticationInfo.java
##########
@@ -24,9 +24,10 @@
/**
* This class holds the set of properties used when instance of the <code>Wagon</code>
* will use during login operation.
+ * <br>May 2020, added PKI settings, see MNG-5583
Review comment:
I'm not sure we need a changelog in the Javadoc.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] michael-o commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427585772
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/AbstractWagon.java
##########
@@ -36,21 +49,6 @@
import org.apache.maven.wagon.resource.Resource;
import org.codehaus.plexus.util.IOUtil;
-import java.io.File;
Review comment:
Skip the reformat, unrelated.
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/authentication/AuthenticationInfo.java
##########
@@ -51,7 +52,300 @@
* The absolute path to private key file
*/
private String privateKey;
+
+ /**
+ *
Review comment:
Same here
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/authentication/AuthenticationInfo.java
##########
@@ -24,9 +24,10 @@
/**
* This class holds the set of properties used when instance of the <code>Wagon</code>
* will use during login operation.
+ * <br>May 2020, added PKI settings, see MNG-5583
*
* @author <a href="michal.maczka@dimatics.com">Michal Maczka</a>
- *
+ *
Review comment:
Unrelated
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/authentication/AuthenticationInfo.java
##########
@@ -24,9 +24,10 @@
/**
* This class holds the set of properties used when instance of the <code>Wagon</code>
* will use during login operation.
+ * <br>May 2020, added PKI settings, see MNG-5583
Review comment:
We have version control. This should be at most in the commit message.
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/repository/Repository.java
##########
@@ -19,13 +19,12 @@
* under the License.
*/
+import java.io.Serializable;
+import java.util.Properties;
import org.apache.maven.wagon.PathUtils;
import org.apache.maven.wagon.WagonConstants;
import org.codehaus.plexus.util.StringUtils;
-import java.io.Serializable;
Review comment:
Same here.
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -80,37 +114,11 @@
import org.apache.maven.wagon.proxy.ProxyInfo;
import org.apache.maven.wagon.repository.Repository;
import org.apache.maven.wagon.resource.Resource;
-import org.codehaus.plexus.util.StringUtils;
-
-import javax.net.ssl.HttpsURLConnection;
Review comment:
same here
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -593,13 +603,189 @@ private static CloseableHttpClient createClient()
*/
private BasicAuthScope proxyAuth;
+ /**
+ * initializes a custom http client given user specified keystore/truststore
+ * information from settings.xml
+ * see httsp://issues.apache.org/jira/browser/MNG-5583
+ * @return a client
+ * @throws SSLInitializationException if there's an issue loading keystore/truststore
+ */
+ private CloseableHttpClient initilaizeLocalHttpClientWithCustomSslSocketFactory() throws SSLInitializationException
+ {
+ String sslProtocolsStr = System.getProperty( "https.protocols" );
+ String cipherSuitesStr = System.getProperty( "https.cipherSuites" );
+ String[] sslProtocols = sslProtocolsStr != null ? sslProtocolsStr.split( " *, *" ) : null;
+ String[] cipherSuites = cipherSuitesStr != null ? cipherSuitesStr.split( " *, *" ) : null;
+
+ SSLSocketFactory socketFactory = null;
+ TrustManager[] trustManagers = null;
+ KeyManager[] keyManagers = null;
+ try
+ {
+ if ( authenticationInfo.getTrustStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getTrustStoreType() == null ? "JKS"
+ : authenticationInfo.getTrustStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ //also a null password is fine with windows and even with JKS files
+ char[] keyPass = authenticationInfo.getTrustStorePassword() != null
+ ? authenticationInfo.getTrustStorePassword().toCharArray()
+ : null;
+ keystore.load( fis, keyPass );
+ if ( keyPass != null )
+ {
+ for ( int i = 0; i < keyPass.length; i++ )
+ {
+ keyPass[i] = 0;
+ }
+ }
+ if ( fis != null )
+ {
+ fis.close();
+ }
+ String alg = KeyManagerFactory.getDefaultAlgorithm();
+ TrustManagerFactory fac = TrustManagerFactory.getInstance( alg );
+ fac.init( keystore );
+ trustManagers = fac.getTrustManagers();
+ }
+
+ if ( authenticationInfo.getKeyStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getKeyStoreType() == null ? "JKS"
+ : authenticationInfo.getKeyStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ String alg = KeyManagerFactory.getDefaultAlgorithm();
+ char[] keyStorePass = authenticationInfo.getKeyStorePassword() != null
+ ? authenticationInfo.getKeyStorePassword().toCharArray()
+ : null;
+ char[] keyPass = authenticationInfo.getKeyPassword() != null
+ ? authenticationInfo.getKeyPassword().toCharArray()
+ : null;
+ KeyManagerFactory fac = KeyManagerFactory.getInstance( alg );
+ keystore.load( fis, keyStorePass );
+ fac.init( keystore, keyPass );
+ if ( keyPass != null )
+ {
+ for ( int i = 0; i < keyPass.length; i++ )
+ {
+ keyPass[i] = 0;
+ }
+ }
+ keyPass = null;
+ String alias = authenticationInfo.getKeyAlias();
+ if ( alias != null )
+ {
+ //borrowed from tomcat's code
+ //user has explicitly specified a key alias, great.
+ //let's make sure it exists in the key store that it has a
+ //key pair
+ if ( keystore.containsAlias( alias ) )
+ {
+ if ( !keystore.isKeyEntry( alias ) )
+ {
+ alias = null;
+ }
+ }
+ else if ( keystore.containsAlias( alias.toLowerCase() ) )
+ {
+ alias = alias.toLowerCase();
Review comment:
Prone to locale issues.
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -382,12 +390,12 @@ private static PoolingHttpClientConnectionManager createConnManager()
{
sslConnectionSocketFactory =
new SSLConnectionSocketFactory( HttpsURLConnection.getDefaultSSLSocketFactory(), sslProtocols,
- cipherSuites,
+ cipherSuites,
SSLConnectionSocketFactory.BROWSER_COMPATIBLE_HOSTNAME_VERIFIER );
}
-
+
Registry<ConnectionSocketFactory> registry = RegistryBuilder.<ConnectionSocketFactory>create().register( "http",
- PlainConnectionSocketFactory.INSTANCE ).register(
+ PlainConnectionSocketFactory.INSTANCE ).register(
Review comment:
Drop reformat
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/authentication/AuthenticationInfo.java
##########
@@ -51,7 +52,300 @@
* The absolute path to private key file
*/
private String privateKey;
+
+ /**
+ *
+ *
+ * The path to the trust store. If not defined, the JRE's cacert store is
+ * used.
+ *
+ *
+ */
+ private String trustStore;
+
+ /**
+ *
+ *
+ * The password to the trust store.
+ *
+ *
+ */
+ private String trustStorePassword;
Review comment:
The upstream model does not allow that. Given that other password fields are strings too, this is acceptable, but not ideal.
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/authentication/AuthenticationInfo.java
##########
@@ -51,7 +52,300 @@
* The absolute path to private key file
*/
private String privateKey;
+
+ /**
+ *
+ *
+ * The path to the trust store. If not defined, the JRE's cacert store is
+ * used.
+ *
+ *
+ */
+ private String trustStore;
+
+ /**
+ *
+ *
+ * The password to the trust store.
+ *
+ *
+ */
+ private String trustStorePassword;
+
+ /**
+ *
+ *
+ * The type of trust store, default is JKS
+ *
+ * .
+ */
+ private String trustStoreType;
+
+ /**
+ *
+ *
+ * The path to the keystore used for authentication purposes, or null
+ *
+ * .
+ */
+ private String keyStore;
+
+ /**
+ *
+ *
+ * Keystore password, can be null
+ *
+ * .
+ */
+ private String keyStorePassword;
+
+ /**
+ *
+ *
+ * Keystore if the key store has multiple key pairs, this can be used to
+ * explicitly select a specific certificate via it's alias. If null, the
+ * most appropriate certificate is automatically selected by the SSL Factory
+ *
+ * .
+ */
+ private String keyAlias;
+
+ /**
+ *
+ *
+ * The password to unlock the key, can be null
+ *
+ * .
+ */
+ private String keyPassword;
Review comment:
Same here
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -593,13 +603,189 @@ private static CloseableHttpClient createClient()
*/
private BasicAuthScope proxyAuth;
+ /**
+ * initializes a custom http client given user specified keystore/truststore
+ * information from settings.xml
+ * see httsp://issues.apache.org/jira/browser/MNG-5583
+ * @return a client
+ * @throws SSLInitializationException if there's an issue loading keystore/truststore
+ */
+ private CloseableHttpClient initilaizeLocalHttpClientWithCustomSslSocketFactory() throws SSLInitializationException
+ {
+ String sslProtocolsStr = System.getProperty( "https.protocols" );
+ String cipherSuitesStr = System.getProperty( "https.cipherSuites" );
+ String[] sslProtocols = sslProtocolsStr != null ? sslProtocolsStr.split( " *, *" ) : null;
+ String[] cipherSuites = cipherSuitesStr != null ? cipherSuitesStr.split( " *, *" ) : null;
+
+ SSLSocketFactory socketFactory = null;
+ TrustManager[] trustManagers = null;
+ KeyManager[] keyManagers = null;
+ try
+ {
+ if ( authenticationInfo.getTrustStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getTrustStoreType() == null ? "JKS"
+ : authenticationInfo.getTrustStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ //also a null password is fine with windows and even with JKS files
+ char[] keyPass = authenticationInfo.getTrustStorePassword() != null
+ ? authenticationInfo.getTrustStorePassword().toCharArray()
+ : null;
+ keystore.load( fis, keyPass );
+ if ( keyPass != null )
+ {
+ for ( int i = 0; i < keyPass.length; i++ )
+ {
+ keyPass[i] = 0;
+ }
+ }
+ if ( fis != null )
+ {
+ fis.close();
+ }
+ String alg = KeyManagerFactory.getDefaultAlgorithm();
+ TrustManagerFactory fac = TrustManagerFactory.getInstance( alg );
+ fac.init( keystore );
+ trustManagers = fac.getTrustManagers();
+ }
+
+ if ( authenticationInfo.getKeyStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getKeyStoreType() == null ? "JKS"
+ : authenticationInfo.getKeyStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ String alg = KeyManagerFactory.getDefaultAlgorithm();
+ char[] keyStorePass = authenticationInfo.getKeyStorePassword() != null
+ ? authenticationInfo.getKeyStorePassword().toCharArray()
+ : null;
+ char[] keyPass = authenticationInfo.getKeyPassword() != null
+ ? authenticationInfo.getKeyPassword().toCharArray()
+ : null;
+ KeyManagerFactory fac = KeyManagerFactory.getInstance( alg );
+ keystore.load( fis, keyStorePass );
+ fac.init( keystore, keyPass );
+ if ( keyPass != null )
+ {
+ for ( int i = 0; i < keyPass.length; i++ )
+ {
+ keyPass[i] = 0;
+ }
+ }
+ keyPass = null;
+ String alias = authenticationInfo.getKeyAlias();
+ if ( alias != null )
+ {
+ //borrowed from tomcat's code
+ //user has explicitly specified a key alias, great.
+ //let's make sure it exists in the key store that it has a
+ //key pair
+ if ( keystore.containsAlias( alias ) )
+ {
+ if ( !keystore.isKeyEntry( alias ) )
+ {
+ alias = null;
+ }
+ }
+ else if ( keystore.containsAlias( alias.toLowerCase() ) )
+ {
+ alias = alias.toLowerCase();
+ if ( !keystore.isKeyEntry( alias ) )
+ {
+ alias = null;
+ }
+ }
+ if ( alias == null )
+ {
+ //TODO this should be I18N
+ throw new IOException( "key alias not found" );
+ }
+ //ok we found the alias with a key.
+ }
+
+ keyManagers = fac.getKeyManagers();
+ if ( alias != null )
+ {
+ //filter down the key managers to just the user's selected cert.
+ for ( int k = 0; k < keyManagers.length; k++ )
+ {
+ if ( keyManagers[k] instanceof X509KeyManager )
+ {
+ keyManagers[k] = new JSSEKeyManager( (X509KeyManager) keyManagers[k], alias );
+ }
+ }
+ }
+
+ }
+ final SSLContext context = SSLContexts.custom().useSSL().build();
+ context.init( keyManagers, trustManagers, new SecureRandom() );
+ socketFactory = context.getSocketFactory();
+ if ( socketFactory == null )
+ {
+ socketFactory = HttpsURLConnection.getDefaultSSLSocketFactory();
+ }
+ }
+ catch ( Exception ex )
+ {
+ throw new SSLInitializationException( ex.getMessage(), ex );
+ }
+
+ SSLConnectionSocketFactory sslConnectionSocketFactory = new SSLConnectionSocketFactory(
+ socketFactory,
+ sslProtocols,
+ cipherSuites,
+ SSLConnectionSocketFactory.BROWSER_COMPATIBLE_HOSTNAME_VERIFIER );
+
+ Registry<ConnectionSocketFactory> registry = RegistryBuilder.
+ <ConnectionSocketFactory>create().register( "http",
+ PlainConnectionSocketFactory.INSTANCE ).register(
+ "https", sslConnectionSocketFactory ).build();
+
+ PoolingHttpClientConnectionManager connManager =
+ new PoolingHttpClientConnectionManager( registry, null, null, null, CONN_TTL, TimeUnit.SECONDS );
+ if ( persistentPool )
+ {
+ connManager.setDefaultMaxPerRoute( MAX_CONN_PER_ROUTE );
+ connManager.setMaxTotal( MAX_CONN_TOTAL );
+ }
+ else
+ {
+ connManager.setMaxTotal( 1 );
+ }
+
+ CloseableHttpClient httpClientWithCustomSslSocketFactory = HttpClientBuilder.create()
+ .useSystemProperties()
+ .disableConnectionState()
+ .setConnectionManager( connManager )
+ .setRetryHandler( createRetryHandler() )
+ .setServiceUnavailableRetryStrategy( createServiceUnavailableRetryStrategy() )
+ .setDefaultAuthSchemeRegistry( createAuthSchemeRegistry() )
+ .setRedirectStrategy( new WagonRedirectStrategy() )
+ .build();
+ return httpClientWithCustomSslSocketFactory;
+ }
+
+
public void openConnectionInternal()
{
repository.setUrl( getURL( repository ) );
credentialsProvider = new BasicCredentialsProvider();
authCache = new BasicAuthCache();
-
+
Review comment:
trailing ws
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -614,11 +800,23 @@ public void openConnectionInternal()
getRepository().getPort() );
credentialsProvider.setCredentials( targetScope, creds );
}
+ //MNG-5583 per endpoint PKI authentication
+ if ( authenticationInfo.getTrustStore() != null || authenticationInfo.getKeyStore() != null )
+ {
+ //cache the client for this specific server host:port combination
+ String key = repository.getProtocol() + repository.getHost() + repository.getPort();
+ if ( !httpClientWithCustomSslSocketFactoryCache.containsKey( key ) )
+ {
+ httpClientWithCustomSslSocketFactoryCache.put( key,
+ initilaizeLocalHttpClientWithCustomSslSocketFactory() );
+ }
+ }
}
ProxyInfo proxyInfo = getProxyInfo( getRepository().getProtocol(), getRepository().getHost() );
if ( proxyInfo != null )
{
+ //TODO add PKI support?
Review comment:
This would require two things:
* A proxy which speaks HTTPS as its protocol
* The proxy from above + TLS mutual auth supported
If this cannot be set up with OSS tools no need to add a TODO here. Maybe Apache's mod_proxy might be tried as forwarding proxy.
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/repository/Repository.java
##########
@@ -65,7 +64,7 @@
private String username = null;
private String password = null;
-
+
Review comment:
traling ws
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -593,13 +603,189 @@ private static CloseableHttpClient createClient()
*/
private BasicAuthScope proxyAuth;
+ /**
+ * initializes a custom http client given user specified keystore/truststore
+ * information from settings.xml
+ * see httsp://issues.apache.org/jira/browser/MNG-5583
Review comment:
Typo
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -382,12 +390,12 @@ private static PoolingHttpClientConnectionManager createConnManager()
{
sslConnectionSocketFactory =
new SSLConnectionSocketFactory( HttpsURLConnection.getDefaultSSLSocketFactory(), sslProtocols,
- cipherSuites,
+ cipherSuites,
Review comment:
Drop reformat
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -593,13 +603,189 @@ private static CloseableHttpClient createClient()
*/
private BasicAuthScope proxyAuth;
+ /**
+ * initializes a custom http client given user specified keystore/truststore
+ * information from settings.xml
+ * see httsp://issues.apache.org/jira/browser/MNG-5583
+ * @return a client
+ * @throws SSLInitializationException if there's an issue loading keystore/truststore
+ */
+ private CloseableHttpClient initilaizeLocalHttpClientWithCustomSslSocketFactory() throws SSLInitializationException
+ {
+ String sslProtocolsStr = System.getProperty( "https.protocols" );
+ String cipherSuitesStr = System.getProperty( "https.cipherSuites" );
+ String[] sslProtocols = sslProtocolsStr != null ? sslProtocolsStr.split( " *, *" ) : null;
+ String[] cipherSuites = cipherSuitesStr != null ? cipherSuitesStr.split( " *, *" ) : null;
+
+ SSLSocketFactory socketFactory = null;
+ TrustManager[] trustManagers = null;
+ KeyManager[] keyManagers = null;
+ try
+ {
+ if ( authenticationInfo.getTrustStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getTrustStoreType() == null ? "JKS"
+ : authenticationInfo.getTrustStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ //also a null password is fine with windows and even with JKS files
+ char[] keyPass = authenticationInfo.getTrustStorePassword() != null
+ ? authenticationInfo.getTrustStorePassword().toCharArray()
+ : null;
+ keystore.load( fis, keyPass );
+ if ( keyPass != null )
+ {
+ for ( int i = 0; i < keyPass.length; i++ )
+ {
+ keyPass[i] = 0;
+ }
+ }
+ if ( fis != null )
+ {
+ fis.close();
+ }
+ String alg = KeyManagerFactory.getDefaultAlgorithm();
+ TrustManagerFactory fac = TrustManagerFactory.getInstance( alg );
+ fac.init( keystore );
+ trustManagers = fac.getTrustManagers();
+ }
+
+ if ( authenticationInfo.getKeyStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getKeyStoreType() == null ? "JKS"
+ : authenticationInfo.getKeyStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ String alg = KeyManagerFactory.getDefaultAlgorithm();
+ char[] keyStorePass = authenticationInfo.getKeyStorePassword() != null
+ ? authenticationInfo.getKeyStorePassword().toCharArray()
+ : null;
+ char[] keyPass = authenticationInfo.getKeyPassword() != null
+ ? authenticationInfo.getKeyPassword().toCharArray()
+ : null;
+ KeyManagerFactory fac = KeyManagerFactory.getInstance( alg );
+ keystore.load( fis, keyStorePass );
+ fac.init( keystore, keyPass );
+ if ( keyPass != null )
+ {
+ for ( int i = 0; i < keyPass.length; i++ )
+ {
+ keyPass[i] = 0;
+ }
+ }
+ keyPass = null;
+ String alias = authenticationInfo.getKeyAlias();
+ if ( alias != null )
+ {
+ //borrowed from tomcat's code
+ //user has explicitly specified a key alias, great.
+ //let's make sure it exists in the key store that it has a
+ //key pair
+ if ( keystore.containsAlias( alias ) )
+ {
+ if ( !keystore.isKeyEntry( alias ) )
+ {
+ alias = null;
+ }
+ }
+ else if ( keystore.containsAlias( alias.toLowerCase() ) )
+ {
+ alias = alias.toLowerCase();
+ if ( !keystore.isKeyEntry( alias ) )
+ {
+ alias = null;
+ }
+ }
+ if ( alias == null )
+ {
+ //TODO this should be I18N
+ throw new IOException( "key alias not found" );
Review comment:
Why is that an IOE?
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/authentication/AuthenticationInfo.java
##########
@@ -51,7 +52,300 @@
* The absolute path to private key file
*/
private String privateKey;
+
+ /**
+ *
+ *
+ * The path to the trust store. If not defined, the JRE's cacert store is
+ * used.
+ *
+ *
+ */
+ private String trustStore;
+
+ /**
+ *
+ *
+ * The password to the trust store.
+ *
+ *
+ */
+ private String trustStorePassword;
+
+ /**
+ *
+ *
+ * The type of trust store, default is JKS
+ *
+ * .
+ */
+ private String trustStoreType;
+
+ /**
+ *
+ *
+ * The path to the keystore used for authentication purposes, or null
+ *
+ * .
+ */
+ private String keyStore;
+
+ /**
+ *
+ *
+ * Keystore password, can be null
+ *
+ * .
+ */
+ private String keyStorePassword;
+
+ /**
+ *
+ *
+ * Keystore if the key store has multiple key pairs, this can be used to
+ * explicitly select a specific certificate via it's alias. If null, the
+ * most appropriate certificate is automatically selected by the SSL Factory
+ *
+ * .
+ */
+ private String keyAlias;
+
+ /**
+ *
+ *
+ * The password to unlock the key, can be null
+ *
+ * .
+ */
+ private String keyPassword;
+
+ /**
+ *
+ *
+ * The key store type, defaults to JKS
+ *
+ * .
+ */
+ private String keyStoreType;
+
+ /**
+ *
+ *
+ * The path to the trust store. If not defined, the JRE's cacert store is
+ * used.
+ *
+ *
+ * @return path, name or null
+ */
+ public String getTrustStore()
+ {
+ return trustStore;
+ }
+
+ /**
+ *
+ *
+ * The path to the trust store. If not defined, the JRE's cacert store is
+ * used.
+ *
+ *
+ * @param trustStore path name or null
+ */
+ public void setTrustStore( String trustStore )
+ {
+ this.trustStore = trustStore;
+ }
+
+ /**
+ *
+ *
+ * The password to the trust store.
+ *
+ *
+ * @return password or null
+ */
+ public String getTrustStorePassword()
+ {
+ return trustStorePassword;
+ }
+
+ /**
+ *
+ *
+ * The password to the trust store.
+ *
+ *
+ * @param trustStorePassword password or null
+ */
+ public void setTrustStorePassword( String trustStorePassword )
+ {
+ this.trustStorePassword = trustStorePassword;
+ }
+
+ /**
+ *
+ *
+ * The type of trust store, default is JKS
+ *
+ * .
+ *
+ * @return type
+ */
+ public String getTrustStoreType()
+ {
+ return trustStoreType;
+ }
+ /**
+ *
+ *
+ * The type of trust store, default is JKS
+ *
+ * .
+ *
+ * @param trustStoreType key store type
+ */
+ public void setTrustStoreType( String trustStoreType )
+ {
+ this.trustStoreType = trustStoreType;
+ }
+
+ /**
+ *
+ *
+ * The path to the keystore used for authentication purposes, or null
+ *
+ * .
+ *
+ * @return path, named keystore (such as MY) or null
Review comment:
Bad example, if at all `Windows-MY`.
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/repository/Repository.java
##########
@@ -53,7 +52,7 @@
private String url;
- private RepositoryPermissions permissions;
+ private RepositoryPermissions permissions;
Review comment:
traling ws
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] mthmulders commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r429754516
##########
File path: wagon-providers/wagon-http-lightweight/src/test/java/org/apache/maven/wagon/providers/http/TckTest.java
##########
@@ -20,6 +20,7 @@
*/
import org.apache.maven.wagon.tck.http.GetWagonTests;
+import org.apache.maven.wagon.tck.http.HttpsClientCertGetWagonTests;
Review comment:
I don't see why this is necessary. Could you please explain?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] mthmulders commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r429754252
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/JSSEKeyManager.java
##########
@@ -0,0 +1,139 @@
+package org.apache.maven.wagon.shared.http;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.net.Socket;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.cert.X509Certificate;
+
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import javax.net.ssl.X509KeyManager;
+
+/**
+ * This was borrowed from Apache Tomcat
Review comment:
Does Tomcat also have a unit test for this class? If so, I think we might want to borrow that as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427646906
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -614,11 +800,23 @@ public void openConnectionInternal()
getRepository().getPort() );
credentialsProvider.setCredentials( targetScope, creds );
}
+ //MNG-5583 per endpoint PKI authentication
+ if ( authenticationInfo.getTrustStore() != null || authenticationInfo.getKeyStore() != null )
+ {
+ //cache the client for this specific server host:port combination
+ String key = repository.getProtocol() + repository.getHost() + repository.getPort();
+ if ( !httpClientWithCustomSslSocketFactoryCache.containsKey( key ) )
+ {
+ httpClientWithCustomSslSocketFactoryCache.put( key,
+ initilaizeLocalHttpClientWithCustomSslSocketFactory() );
+ }
+ }
}
ProxyInfo proxyInfo = getProxyInfo( getRepository().getProtocol(), getRepository().getHost() );
if ( proxyInfo != null )
{
+ //TODO add PKI support?
Review comment:
i'm pretty sure it's possible, but i'll remove the todo if you like.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] mthmulders commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427050142
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -593,13 +603,189 @@ private static CloseableHttpClient createClient()
*/
private BasicAuthScope proxyAuth;
+ /**
+ * initializes a custom http client given user specified keystore/truststore
+ * information from settings.xml
+ * see httsp://issues.apache.org/jira/browser/MNG-5583
+ * @return a client
+ * @throws SSLInitializationException if there's an issue loading keystore/truststore
+ */
+ private CloseableHttpClient initilaizeLocalHttpClientWithCustomSslSocketFactory() throws SSLInitializationException
+ {
+ String sslProtocolsStr = System.getProperty( "https.protocols" );
+ String cipherSuitesStr = System.getProperty( "https.cipherSuites" );
+ String[] sslProtocols = sslProtocolsStr != null ? sslProtocolsStr.split( " *, *" ) : null;
+ String[] cipherSuites = cipherSuitesStr != null ? cipherSuitesStr.split( " *, *" ) : null;
+
+ SSLSocketFactory socketFactory = null;
+ TrustManager[] trustManagers = null;
+ KeyManager[] keyManagers = null;
+ try
+ {
+ if ( authenticationInfo.getTrustStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getTrustStoreType() == null ? "JKS"
+ : authenticationInfo.getTrustStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ //also a null password is fine with windows and even with JKS files
+ char[] keyPass = authenticationInfo.getTrustStorePassword() != null
+ ? authenticationInfo.getTrustStorePassword().toCharArray()
+ : null;
+ keystore.load( fis, keyPass );
+ if ( keyPass != null )
+ {
+ for ( int i = 0; i < keyPass.length; i++ )
+ {
+ keyPass[i] = 0;
+ }
+ }
+ if ( fis != null )
+ {
+ fis.close();
+ }
+ String alg = KeyManagerFactory.getDefaultAlgorithm();
+ TrustManagerFactory fac = TrustManagerFactory.getInstance( alg );
+ fac.init( keystore );
+ trustManagers = fac.getTrustManagers();
+ }
+
+ if ( authenticationInfo.getKeyStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getKeyStoreType() == null ? "JKS"
Review comment:
As far as I can see, the code for loading the Keystore is identical to lines 627 - 652 (roughly). Can't we refactor this into a method?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427650058
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/authentication/AuthenticationInfo.java
##########
@@ -51,7 +52,300 @@
* The absolute path to private key file
*/
private String privateKey;
+
+ /**
+ *
+ *
+ * The path to the trust store. If not defined, the JRE's cacert store is
+ * used.
+ *
+ *
+ */
+ private String trustStore;
+
+ /**
+ *
+ *
+ * The password to the trust store.
+ *
+ *
+ */
+ private String trustStorePassword;
+
+ /**
+ *
+ *
+ * The type of trust store, default is JKS
+ *
+ * .
+ */
+ private String trustStoreType;
+
+ /**
+ *
+ *
+ * The path to the keystore used for authentication purposes, or null
+ *
+ * .
+ */
+ private String keyStore;
+
+ /**
+ *
+ *
+ * Keystore password, can be null
+ *
+ * .
+ */
+ private String keyStorePassword;
+
+ /**
+ *
+ *
+ * Keystore if the key store has multiple key pairs, this can be used to
+ * explicitly select a specific certificate via it's alias. If null, the
+ * most appropriate certificate is automatically selected by the SSL Factory
+ *
+ * .
+ */
+ private String keyAlias;
+
+ /**
+ *
+ *
+ * The password to unlock the key, can be null
+ *
+ * .
+ */
+ private String keyPassword;
+
+ /**
+ *
+ *
+ * The key store type, defaults to JKS
+ *
+ * .
+ */
+ private String keyStoreType;
+
+ /**
+ *
+ *
+ * The path to the trust store. If not defined, the JRE's cacert store is
+ * used.
+ *
+ *
+ * @return path, name or null
+ */
+ public String getTrustStore()
+ {
+ return trustStore;
+ }
+
+ /**
+ *
+ *
+ * The path to the trust store. If not defined, the JRE's cacert store is
+ * used.
+ *
+ *
+ * @param trustStore path name or null
+ */
+ public void setTrustStore( String trustStore )
+ {
+ this.trustStore = trustStore;
+ }
+
+ /**
+ *
+ *
+ * The password to the trust store.
+ *
+ *
+ * @return password or null
+ */
+ public String getTrustStorePassword()
+ {
+ return trustStorePassword;
+ }
+
+ /**
+ *
+ *
+ * The password to the trust store.
+ *
+ *
+ * @param trustStorePassword password or null
+ */
+ public void setTrustStorePassword( String trustStorePassword )
+ {
+ this.trustStorePassword = trustStorePassword;
+ }
+
+ /**
+ *
+ *
+ * The type of trust store, default is JKS
+ *
+ * .
+ *
+ * @return type
+ */
+ public String getTrustStoreType()
+ {
+ return trustStoreType;
+ }
+ /**
+ *
+ *
+ * The type of trust store, default is JKS
+ *
+ * .
+ *
+ * @param trustStoreType key store type
+ */
+ public void setTrustStoreType( String trustStoreType )
+ {
+ this.trustStoreType = trustStoreType;
+ }
+
+ /**
+ *
+ *
+ * The path to the keystore used for authentication purposes, or null
+ *
+ * .
+ *
+ * @return path, named keystore (such as MY) or null
Review comment:
ok i'll remove the reference, but windows-my, while it is obviously a platform specific thing, as is the mac specific version, does work and is (in some cases) the only way to access specific certificates
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427647826
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -593,13 +603,189 @@ private static CloseableHttpClient createClient()
*/
private BasicAuthScope proxyAuth;
+ /**
+ * initializes a custom http client given user specified keystore/truststore
+ * information from settings.xml
+ * see httsp://issues.apache.org/jira/browser/MNG-5583
+ * @return a client
+ * @throws SSLInitializationException if there's an issue loading keystore/truststore
+ */
+ private CloseableHttpClient initilaizeLocalHttpClientWithCustomSslSocketFactory() throws SSLInitializationException
+ {
+ String sslProtocolsStr = System.getProperty( "https.protocols" );
+ String cipherSuitesStr = System.getProperty( "https.cipherSuites" );
+ String[] sslProtocols = sslProtocolsStr != null ? sslProtocolsStr.split( " *, *" ) : null;
+ String[] cipherSuites = cipherSuitesStr != null ? cipherSuitesStr.split( " *, *" ) : null;
+
+ SSLSocketFactory socketFactory = null;
+ TrustManager[] trustManagers = null;
+ KeyManager[] keyManagers = null;
+ try
+ {
+ if ( authenticationInfo.getTrustStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getTrustStoreType() == null ? "JKS"
+ : authenticationInfo.getTrustStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ //also a null password is fine with windows and even with JKS files
+ char[] keyPass = authenticationInfo.getTrustStorePassword() != null
+ ? authenticationInfo.getTrustStorePassword().toCharArray()
+ : null;
+ keystore.load( fis, keyPass );
+ if ( keyPass != null )
+ {
+ for ( int i = 0; i < keyPass.length; i++ )
+ {
+ keyPass[i] = 0;
+ }
+ }
+ if ( fis != null )
+ {
+ fis.close();
+ }
+ String alg = KeyManagerFactory.getDefaultAlgorithm();
+ TrustManagerFactory fac = TrustManagerFactory.getInstance( alg );
+ fac.init( keystore );
+ trustManagers = fac.getTrustManagers();
+ }
+
+ if ( authenticationInfo.getKeyStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getKeyStoreType() == null ? "JKS"
+ : authenticationInfo.getKeyStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ String alg = KeyManagerFactory.getDefaultAlgorithm();
+ char[] keyStorePass = authenticationInfo.getKeyStorePassword() != null
+ ? authenticationInfo.getKeyStorePassword().toCharArray()
+ : null;
+ char[] keyPass = authenticationInfo.getKeyPassword() != null
+ ? authenticationInfo.getKeyPassword().toCharArray()
+ : null;
+ KeyManagerFactory fac = KeyManagerFactory.getInstance( alg );
+ keystore.load( fis, keyStorePass );
+ fac.init( keystore, keyPass );
+ if ( keyPass != null )
+ {
+ for ( int i = 0; i < keyPass.length; i++ )
+ {
+ keyPass[i] = 0;
+ }
+ }
+ keyPass = null;
+ String alias = authenticationInfo.getKeyAlias();
+ if ( alias != null )
+ {
+ //borrowed from tomcat's code
+ //user has explicitly specified a key alias, great.
+ //let's make sure it exists in the key store that it has a
+ //key pair
+ if ( keystore.containsAlias( alias ) )
+ {
+ if ( !keystore.isKeyEntry( alias ) )
+ {
+ alias = null;
+ }
+ }
+ else if ( keystore.containsAlias( alias.toLowerCase() ) )
+ {
+ alias = alias.toLowerCase();
+ if ( !keystore.isKeyEntry( alias ) )
+ {
+ alias = null;
+ }
+ }
+ if ( alias == null )
+ {
+ //TODO this should be I18N
+ throw new IOException( "key alias not found" );
Review comment:
i can change to SSLInitializationException, would that work?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427644990
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -593,13 +603,189 @@ private static CloseableHttpClient createClient()
*/
private BasicAuthScope proxyAuth;
+ /**
+ * initializes a custom http client given user specified keystore/truststore
+ * information from settings.xml
+ * see httsp://issues.apache.org/jira/browser/MNG-5583
+ * @return a client
+ * @throws SSLInitializationException if there's an issue loading keystore/truststore
+ */
+ private CloseableHttpClient initilaizeLocalHttpClientWithCustomSslSocketFactory() throws SSLInitializationException
+ {
+ String sslProtocolsStr = System.getProperty( "https.protocols" );
+ String cipherSuitesStr = System.getProperty( "https.cipherSuites" );
+ String[] sslProtocols = sslProtocolsStr != null ? sslProtocolsStr.split( " *, *" ) : null;
+ String[] cipherSuites = cipherSuitesStr != null ? cipherSuitesStr.split( " *, *" ) : null;
+
+ SSLSocketFactory socketFactory = null;
+ TrustManager[] trustManagers = null;
+ KeyManager[] keyManagers = null;
+ try
+ {
+ if ( authenticationInfo.getTrustStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getTrustStoreType() == null ? "JKS"
+ : authenticationInfo.getTrustStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ //also a null password is fine with windows and even with JKS files
+ char[] keyPass = authenticationInfo.getTrustStorePassword() != null
+ ? authenticationInfo.getTrustStorePassword().toCharArray()
+ : null;
+ keystore.load( fis, keyPass );
+ if ( keyPass != null )
+ {
+ for ( int i = 0; i < keyPass.length; i++ )
+ {
+ keyPass[i] = 0;
+ }
+ }
+ if ( fis != null )
+ {
+ fis.close();
+ }
+ String alg = KeyManagerFactory.getDefaultAlgorithm();
+ TrustManagerFactory fac = TrustManagerFactory.getInstance( alg );
+ fac.init( keystore );
+ trustManagers = fac.getTrustManagers();
+ }
+
+ if ( authenticationInfo.getKeyStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getKeyStoreType() == null ? "JKS"
+ : authenticationInfo.getKeyStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ String alg = KeyManagerFactory.getDefaultAlgorithm();
+ char[] keyStorePass = authenticationInfo.getKeyStorePassword() != null
+ ? authenticationInfo.getKeyStorePassword().toCharArray()
+ : null;
+ char[] keyPass = authenticationInfo.getKeyPassword() != null
+ ? authenticationInfo.getKeyPassword().toCharArray()
+ : null;
+ KeyManagerFactory fac = KeyManagerFactory.getInstance( alg );
+ keystore.load( fis, keyStorePass );
+ fac.init( keystore, keyPass );
+ if ( keyPass != null )
+ {
+ for ( int i = 0; i < keyPass.length; i++ )
+ {
+ keyPass[i] = 0;
+ }
+ }
+ keyPass = null;
+ String alias = authenticationInfo.getKeyAlias();
+ if ( alias != null )
+ {
+ //borrowed from tomcat's code
+ //user has explicitly specified a key alias, great.
+ //let's make sure it exists in the key store that it has a
+ //key pair
+ if ( keystore.containsAlias( alias ) )
+ {
+ if ( !keystore.isKeyEntry( alias ) )
+ {
+ alias = null;
+ }
+ }
+ else if ( keystore.containsAlias( alias.toLowerCase() ) )
Review comment:
this was borrowed from tomcat's code base, apparently in certain situational, key aliases are case folded. i've never seen it in the wild either, it's always been case sensitive but i figured there was a reason for this. lmk if you want it stricken from the record
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] mthmulders commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427047648
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/authentication/AuthenticationInfo.java
##########
@@ -51,7 +52,300 @@
* The absolute path to private key file
*/
private String privateKey;
+
+ /**
+ *
Review comment:
Nit-picking: I don't like the superfluous `*` lines. Also, I think the Javadoc specs say they shouldn't be there.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] michael-o commented on pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#issuecomment-632530819
>
>
> > > regarding unit tests, whats the difference between the regular http wagon and the light weight version?
> >
> >
> > Bad naming. HttpWagon uses Apache HttpClient, LightweightClient uses `HttpURLConnection`.
>
> perhaps i asked the wrong question. In what context is the lightweight version used vs the httpwagon version? i'm trying to figure out if i need to (or should) add the pki support mechanisms to the lightweight setup
Not in Maven itself. It is a user desicion to swap the HTTP client backend for Wagon. The LightweightWagon has flaws as documented in JIRA.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] michael-o commented on pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#issuecomment-633482529
Since this is a massive change, I have to complete the current Wagon release first and then will pick this up in June.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#issuecomment-631166281
FYI i'm going to try and add some new unit tests for this to have a more reproducible setup rather than the hacked up sonatype nexus server i'm testing against
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r428990815
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -593,13 +603,189 @@ private static CloseableHttpClient createClient()
*/
private BasicAuthScope proxyAuth;
+ /**
+ * initializes a custom http client given user specified keystore/truststore
+ * information from settings.xml
+ * see httsp://issues.apache.org/jira/browser/MNG-5583
+ * @return a client
+ * @throws SSLInitializationException if there's an issue loading keystore/truststore
+ */
+ private CloseableHttpClient initilaizeLocalHttpClientWithCustomSslSocketFactory() throws SSLInitializationException
+ {
+ String sslProtocolsStr = System.getProperty( "https.protocols" );
+ String cipherSuitesStr = System.getProperty( "https.cipherSuites" );
+ String[] sslProtocols = sslProtocolsStr != null ? sslProtocolsStr.split( " *, *" ) : null;
+ String[] cipherSuites = cipherSuitesStr != null ? cipherSuitesStr.split( " *, *" ) : null;
+
+ SSLSocketFactory socketFactory = null;
+ TrustManager[] trustManagers = null;
+ KeyManager[] keyManagers = null;
+ try
+ {
+ if ( authenticationInfo.getTrustStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getTrustStoreType() == null ? "JKS"
+ : authenticationInfo.getTrustStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ //also a null password is fine with windows and even with JKS files
+ char[] keyPass = authenticationInfo.getTrustStorePassword() != null
+ ? authenticationInfo.getTrustStorePassword().toCharArray()
+ : null;
+ keystore.load( fis, keyPass );
+ if ( keyPass != null )
+ {
+ for ( int i = 0; i < keyPass.length; i++ )
+ {
+ keyPass[i] = 0;
+ }
+ }
+ if ( fis != null )
+ {
+ fis.close();
+ }
+ String alg = KeyManagerFactory.getDefaultAlgorithm();
+ TrustManagerFactory fac = TrustManagerFactory.getInstance( alg );
+ fac.init( keystore );
+ trustManagers = fac.getTrustManagers();
+ }
+
+ if ( authenticationInfo.getKeyStore() != null )
+ {
+ KeyStore keystore = KeyStore.getInstance( authenticationInfo.getKeyStoreType() == null ? "JKS"
+ : authenticationInfo.getKeyStoreType() );
+ FileInputStream fis = null;
+ //on windows platforms, the truststoreType of "WINDOWS" mounts
+ //to the windows certificate store, so a null input stream is just fine
+ File file = new File( ( authenticationInfo.getTrustStore() ) );
+ if ( file.exists() )
+ {
+ fis = new FileInputStream( file );
+ }
+ String alg = KeyManagerFactory.getDefaultAlgorithm();
+ char[] keyStorePass = authenticationInfo.getKeyStorePassword() != null
+ ? authenticationInfo.getKeyStorePassword().toCharArray()
+ : null;
+ char[] keyPass = authenticationInfo.getKeyPassword() != null
+ ? authenticationInfo.getKeyPassword().toCharArray()
+ : null;
+ KeyManagerFactory fac = KeyManagerFactory.getInstance( alg );
+ keystore.load( fis, keyStorePass );
+ fac.init( keystore, keyPass );
+ if ( keyPass != null )
+ {
+ for ( int i = 0; i < keyPass.length; i++ )
+ {
+ keyPass[i] = 0;
+ }
+ }
+ keyPass = null;
+ String alias = authenticationInfo.getKeyAlias();
+ if ( alias != null )
+ {
+ //borrowed from tomcat's code
+ //user has explicitly specified a key alias, great.
+ //let's make sure it exists in the key store that it has a
+ //key pair
+ if ( keystore.containsAlias( alias ) )
+ {
+ if ( !keystore.isKeyEntry( alias ) )
+ {
+ alias = null;
+ }
+ }
+ else if ( keystore.containsAlias( alias.toLowerCase() ) )
Review comment:
here's the link
https://github.com/apache/tomcat/blob/a6aad15a470f6f69bcc58c53729dc4e787a536ed/java/org/apache/tomcat/util/net/SSLUtilBase.java#L371
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] mthmulders commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427048491
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/authentication/AuthenticationInfo.java
##########
@@ -51,7 +52,300 @@
* The absolute path to private key file
*/
private String privateKey;
+
+ /**
+ *
+ *
+ * The path to the trust store. If not defined, the JRE's cacert store is
+ * used.
+ *
+ *
+ */
+ private String trustStore;
+
+ /**
+ *
+ *
+ * The password to the trust store.
+ *
+ *
+ */
+ private String trustStorePassword;
+
+ /**
+ *
+ *
+ * The type of trust store, default is JKS
+ *
+ * .
+ */
+ private String trustStoreType;
+
+ /**
+ *
+ *
+ * The path to the keystore used for authentication purposes, or null
+ *
+ * .
+ */
+ private String keyStore;
+
+ /**
+ *
+ *
+ * Keystore password, can be null
+ *
+ * .
+ */
+ private String keyStorePassword;
+
+ /**
+ *
+ *
+ * Keystore if the key store has multiple key pairs, this can be used to
+ * explicitly select a specific certificate via it's alias. If null, the
+ * most appropriate certificate is automatically selected by the SSL Factory
+ *
+ * .
+ */
+ private String keyAlias;
+
+ /**
+ *
+ *
+ * The password to unlock the key, can be null
+ *
+ * .
+ */
+ private String keyPassword;
Review comment:
This field contains a password. We must convert it to `char[]` later on, so can't we store it as a `char[]`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] mthmulders commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427048402
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/authentication/AuthenticationInfo.java
##########
@@ -51,7 +52,300 @@
* The absolute path to private key file
*/
private String privateKey;
+
+ /**
+ *
+ *
+ * The path to the trust store. If not defined, the JRE's cacert store is
+ * used.
+ *
+ *
+ */
+ private String trustStore;
+
+ /**
+ *
+ *
+ * The password to the trust store.
+ *
+ *
+ */
+ private String trustStorePassword;
Review comment:
This field contains a password. We must convert it to `char[]` later on, so can't we store it as a `char[]`?
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/authentication/AuthenticationInfo.java
##########
@@ -51,7 +52,300 @@
* The absolute path to private key file
*/
private String privateKey;
+
+ /**
+ *
+ *
+ * The path to the trust store. If not defined, the JRE's cacert store is
+ * used.
+ *
+ *
+ */
+ private String trustStore;
+
+ /**
+ *
+ *
+ * The password to the trust store.
+ *
+ *
+ */
+ private String trustStorePassword;
+
+ /**
+ *
+ *
+ * The type of trust store, default is JKS
+ *
+ * .
+ */
+ private String trustStoreType;
+
+ /**
+ *
+ *
+ * The path to the keystore used for authentication purposes, or null
+ *
+ * .
+ */
+ private String keyStore;
+
+ /**
+ *
+ *
+ * Keystore password, can be null
+ *
+ * .
+ */
+ private String keyStorePassword;
Review comment:
This field contains a password. We must convert it to `char[]` later on, so can't we store it as a `char[]`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] mthmulders commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r429754973
##########
File path: wagon-tcks/wagon-tck-http/src/main/java/org/apache/maven/wagon/tck/http/GetWagonTests.java
##########
@@ -545,4 +549,9 @@ protected void testErrorHandling( final int code )
// expected
}
}
+
+ protected AuthenticationInfo getAuthIfNeeded( AuthenticationInfo info ) throws Exception
Review comment:
What is the purpose of this method?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427646906
##########
File path: wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
##########
@@ -614,11 +800,23 @@ public void openConnectionInternal()
getRepository().getPort() );
credentialsProvider.setCredentials( targetScope, creds );
}
+ //MNG-5583 per endpoint PKI authentication
+ if ( authenticationInfo.getTrustStore() != null || authenticationInfo.getKeyStore() != null )
+ {
+ //cache the client for this specific server host:port combination
+ String key = repository.getProtocol() + repository.getHost() + repository.getPort();
+ if ( !httpClientWithCustomSslSocketFactoryCache.containsKey( key ) )
+ {
+ httpClientWithCustomSslSocketFactoryCache.put( key,
+ initilaizeLocalHttpClientWithCustomSslSocketFactory() );
+ }
+ }
}
ProxyInfo proxyInfo = getProxyInfo( getRepository().getProtocol(), getRepository().getHost() );
if ( proxyInfo != null )
{
+ //TODO add PKI support?
Review comment:
i'm pretty sure it's possible, but i'll remove the todo if you like. definitely a separate jira
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#issuecomment-630471431
hang on,forgot i had disabled checkstyle. sorry stand by
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-wagon] spyhunter99 commented on a change in pull request #67: Feature/MNG-5583 per endpoint support for PKI authentication
Posted by GitBox <gi...@apache.org>.
spyhunter99 commented on a change in pull request #67:
URL: https://github.com/apache/maven-wagon/pull/67#discussion_r427635895
##########
File path: wagon-provider-api/src/main/java/org/apache/maven/wagon/authentication/AuthenticationInfo.java
##########
@@ -51,7 +52,300 @@
* The absolute path to private key file
*/
private String privateKey;
+
+ /**
+ *
Review comment:
actually i think it was from the maven core mdo > java class generator that did that and i just copied and pasted... no worries
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org