Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optional client certificate #354

Merged
merged 5 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 12 additions & 10 deletions src/main/java/winstone/AbstractSecuredConnectorFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

package winstone;

import java.util.Locale;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import winstone.cmdline.Option;
Expand Down Expand Up @@ -118,16 +119,17 @@
"HttpsListener.ExcludeCiphers", //
Arrays.asList(ssl.getExcludeCipherSuites()));

/*
* If true, request the client certificate ala "SSLVerifyClient require" Apache directive.
* If false, which is the default, don't do so.
* Technically speaking, there's the equivalent of "SSLVerifyClient optional", but IE doesn't
* recognize it and it always prompt the certificate chooser dialog box, so in practice
* it's useless.
* <p>
* See http://hudson.361315.n4.nabble.com/winstone-container-and-ssl-td383501.html for this failure mode in IE.
*/
ssl.setNeedClientAuth(Option.HTTPS_VERIFY_CLIENT.get(args));
switch (Option.HTTPS_VERIFY_CLIENT.get(args).toLowerCase(Locale.ROOT)) {

Check warning on line 122 in src/main/java/winstone/AbstractSecuredConnectorFactory.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 122 is only partially covered, 2 branches are missing
case "yes":
case "true":
ssl.setNeedClientAuth(true);
break;
case "optional":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far this option is undocumented but I can add it

ssl.setWantClientAuth(true);
break;

Check warning on line 129 in src/main/java/winstone/AbstractSecuredConnectorFactory.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 125-129 are not covered by tests
default:
break;
mawinter69 marked this conversation as resolved.
Show resolved Hide resolved
}
return ssl;
} catch (Throwable err) {
throw new WinstoneException(SSL_RESOURCES
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/winstone/cmdline/Option.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public static List<Option<?>> all(Class<?> clazz) {
public static final OString HTTPS_KEY_STORE_PASSWORD=string("httpsKeyStorePassword");
public static final OString HTTPS_PRIVATE_KEY_PASSWORD=string("httpsPrivateKeyPassword");
public static final OString HTTPS_KEY_MANAGER_TYPE=string("httpsKeyManagerType","SunX509");
public static final OBoolean HTTPS_VERIFY_CLIENT=bool("httpsVerifyClient",false);
public static final OString HTTPS_VERIFY_CLIENT=string("httpsVerifyClient","false");
public static final OString HTTPS_CERTIFICATE_ALIAS=string("httpsCertificateAlias");
public static final OString HTTPS_EXCLUDE_PROTOCOLS=string("excludeProtocols", "SSL, SSLv2, SSLv2Hello, SSLv3");
public static final OString HTTPS_EXCLUDE_CIPHER_SUITES=string("excludeCipherSuites");
Expand Down