You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2021/05/21 07:23:38 UTC
[GitHub] [solr] janhoy opened a new pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
janhoy opened a new pull request #139:
URL: https://github.com/apache/solr/pull/139
https://issues.apache.org/jira/browse/SOLR-15423
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] madrob commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r645835911
##########
File path: solr/solr-ref-guide/src/jwt-authentication-plugin.adoc
##########
@@ -158,12 +161,18 @@ Let's comment on this config:
<11> Configure the issuer id. Will be used for validating tokens. A token's 'iss' claim must match one of the configured issuer IDs.
<12> Configure the audience claim. A token's 'aud' claim must match 'aud' for one of the configured issuers.
<13> This issuer is auto configured through discovery, so 'iss' and JWK settings are not required
+<14> Provides SSL certificates to trust to support self-signed SSL in the issuers.
=== Using non SSL URLs
In production environments you should always use SSL protected HTTPS connections, otherwise you open yourself up to attacks.
However, in development, it may be useful to use regular http urls, and bypass the
security check that Solr performs. To support this you can set the environment variable `solr.auth.jwt.allowOutboundHttp=true`.
+=== Trusting the IdP server
+All communication with issuer server (IdP) is done over HTTPS. If the issuer uses a self signed SSL certificate, the connection will fail since the plugin enforces verification of SSL certificates on outbound traffic. By default, the root certificates present in Java's TrustStore (default or custom provided) are consulted. If you do not want to place the IdP certificates in Java's TrustStore, you can configure this plugin with either `trustedCertsFile` or `trustedCerts` options to provide a set of certificates to use when talking to IdP. This has the extra benefit of also working even if Solr is not started in SSL mode.
+
+Please configure either `trustedCerts` or `trustedCertsFile`, not both.
Review comment:
Stronger language here, indicating that it will fail if both are configured.
##########
File path: solr/core/src/test/org/apache/solr/security/JWTAuthPluginIntegrationTest.java
##########
@@ -127,13 +135,40 @@ public void tearDown() throws Exception {
super.tearDown();
}
- @Test(expected = IOException.class)
- public void infoRequestWithoutToken() throws Exception {
- get(baseUrl + "/admin/info/system", null);
+ @Test
+ public void extractCertificateFromPem() throws IOException {
Review comment:
Can we have a test where the pen file or directly supplied string is improperly formatted? I'd like to see what kind of error message users can expect in that case (and maybe we should document that, because I suspect it will be cryptic otherwise)
##########
File path: solr/core/src/java/org/apache/solr/security/JWTVerificationkeyResolver.java
##########
@@ -110,7 +112,13 @@ public Key resolveKey(JsonWebSignature jws, List<JsonWebStructure> nestingContex
if (issuerConfig.usesHttpsJwk()) {
keysSource = "[" + String.join(", ", issuerConfig.getJwksUrls()) + "]";
for (HttpsJwks hjwks : issuerConfig.getHttpsJwks()) {
- jsonWebKeys.addAll(hjwks.getJsonWebKeys());
+ try {
+ jsonWebKeys.addAll(hjwks.getJsonWebKeys());
+ } catch (SSLHandshakeException e) {
+ log.warn("Failed to talk SSL with {}, do you have the correct SSL certificate configured?", hjwks.getLocation());
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+ "SSL error when validating auth token. Make sure correct certificates are configured.", e);
Review comment:
Generally not a fan of both logging and throwing, as that likely means that we log twice.
##########
File path: solr/core/src/java/org/apache/solr/security/JWTAuthPlugin.java
##########
@@ -155,8 +175,38 @@ public void init(Map<String, Object> pluginConfig) {
requiredScopes = Arrays.asList(requiredScopesStr.split("\\s+"));
}
+ // Parse custom IDP SSL Cert from either path or string
+ InputStream trustedCertsStream = null;
+ String trustedCertsFile = (String) pluginConfig.get(PARAM_TRUSTED_CERTS_FILE);
+ String trustedCerts = (String) pluginConfig.get(PARAM_TRUSTED_CERTS);
+ if (trustedCertsFile != null && trustedCerts != null) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Found both " + PARAM_TRUSTED_CERTS_FILE + " and " + PARAM_TRUSTED_CERTS + ", please use only one");
+ }
+ if (trustedCertsFile != null) {
+ try {
+ Path trustedCertsPath = Paths.get(trustedCertsFile);
+ if (coreContainer != null) {
+ coreContainer.assertPathAllowed(trustedCertsPath);
+ }
+ trustedCertsStream = Files.newInputStream(trustedCertsPath);
+ log.info("Reading trustedCerts from file {}", trustedCertsFile);
+ } catch (IOException e) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Failed to read file " + trustedCertsFile, e);
+ }
+ }
+ if (trustedCerts != null) {
+ trustedCertsStream = IOUtils.toInputStream(trustedCerts, StandardCharsets.UTF_8);
Review comment:
I'm amazed that this doesn't exist in the JDK. We have StringStream in ContentStreamBase that is similar, maybe we can create something for the common need.
##########
File path: solr/core/src/test/org/apache/solr/security/JWTAuthPluginIntegrationTest.java
##########
@@ -70,54 +89,43 @@
protected static final int NUM_SERVERS = 2;
protected static final int NUM_SHARDS = 2;
protected static final int REPLICATION_FACTOR = 1;
+ private static String mockOAuthToken;
+ private static Path pemFilePath;
+ private static Path wrongPemFilePath;
private final String COLLECTION = "jwtColl";
- private String jwtTestToken;
+ private static String jwtStaticTestToken;
private String baseUrl;
- private JsonWebSignature jws;
- private String jwtTokenWrongSignature;
+ private static JsonWebSignature jws;
+ private static String jwtTokenWrongSignature;
- @Override
- @Before
- public void setUp() throws Exception {
- super.setUp();
-
- configureCluster(NUM_SERVERS)// nodes
- .withSecurityJson(TEST_PATH().resolve("security").resolve("jwt_plugin_jwk_security.json"))
- .addConfig("conf1", TEST_PATH().resolve("configsets").resolve("cloud-minimal").resolve("conf"))
- .withDefaultClusterProperty("useLegacyReplicaAssignment", "false")
- .configure();
- baseUrl = cluster.getRandomJetty(random()).getBaseUrl().toString();
+ private static MockOAuth2Server mockOAuth2Server;
- String jwkJSON = "{\n" +
- " \"kty\": \"RSA\",\n" +
- " \"d\": \"i6pyv2z3o-MlYytWsOr3IE1olu2RXZBzjPRBNgWAP1TlLNaphHEvH5aHhe_CtBAastgFFMuP29CFhaL3_tGczkvWJkSveZQN2AHWHgRShKgoSVMspkhOt3Ghha4CvpnZ9BnQzVHnaBnHDTTTfVgXz7P1ZNBhQY4URG61DKIF-JSSClyh1xKuMoJX0lILXDYGGcjVTZL_hci4IXPPTpOJHV51-pxuO7WU5M9252UYoiYyCJ56ai8N49aKIMsqhdGuO4aWUwsGIW4oQpjtce5eEojCprYl-9rDhTwLAFoBtjy6LvkqlR2Ae5dKZYpStljBjK8PJrBvWZjXAEMDdQ8PuQ\",\n" +
- " \"e\": \"AQAB\",\n" +
- " \"use\": \"sig\",\n" +
- " \"kid\": \"test\",\n" +
- " \"alg\": \"RS256\",\n" +
- " \"n\": \"jeyrvOaZrmKWjyNXt0myAc_pJ1hNt3aRupExJEx1ewPaL9J9HFgSCjMrYxCB1ETO1NDyZ3nSgjZis-jHHDqBxBjRdq_t1E2rkGFaYbxAyKt220Pwgme_SFTB9MXVrFQGkKyjmQeVmOmV6zM3KK8uMdKQJ4aoKmwBcF5Zg7EZdDcKOFgpgva1Jq-FlEsaJ2xrYDYo3KnGcOHIt9_0NQeLsqZbeWYLxYni7uROFncXYV5FhSJCeR4A_rrbwlaCydGxE0ToC_9HNYibUHlkJjqyUhAgORCbNS8JLCJH8NUi5sDdIawK9GTSyvsJXZ-QHqo4cMUuxWV5AJtaRGghuMUfqQ\"\n" +
- "}";
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ // Setup an OAuth2 mock server with SSL
+ Path p12Cert = SolrTestCaseJ4.TEST_PATH().resolve("security").resolve("jwt_plugin_idp_certs.p12");
+ pemFilePath = SolrTestCaseJ4.TEST_PATH().resolve("security").resolve("jwt_plugin_idp_cert.pem");
+ wrongPemFilePath = SolrTestCaseJ4.TEST_PATH().resolve("security").resolve("jwt_plugin_idp_wrongcert.pem");
- PublicJsonWebKey jwk = RsaJsonWebKey.Factory.newPublicJwk(jwkJSON);
- JwtClaims claims = JWTAuthPluginTest.generateClaims();
- jws = new JsonWebSignature();
- jws.setPayload(claims.toJson());
- jws.setKey(jwk.getPrivateKey());
- jws.setKeyIdHeaderValue(jwk.getKeyId());
- jws.setAlgorithmHeaderValue(AlgorithmIdentifiers.RSA_USING_SHA256);
+ mockOAuth2Server = createMockOAuthServer(p12Cert,"secret");
+ mockOAuth2Server.start();
+ mockOAuthToken = mockOAuth2Server.issueToken("default",
+ "myClientId",
+ new DefaultOAuth2TokenCallback()).serialize();
+ initStaticJwt();
+ }
- jwtTestToken = jws.getCompactSerialization();
-
- PublicJsonWebKey jwk2 = RsaJwkGenerator.generateJwk(2048);
- jwk2.setKeyId("k2");
- JsonWebSignature jws2 = new JsonWebSignature();
- jws2.setPayload(claims.toJson());
- jws2.setKey(jwk2.getPrivateKey());
- jws2.setKeyIdHeaderValue(jwk2.getKeyId());
- jws2.setAlgorithmHeaderValue(AlgorithmIdentifiers.RSA_USING_SHA256);
- jwtTokenWrongSignature = jws2.getCompactSerialization();
+ @AfterClass
+ public static void afterClass() throws Exception {
+ if (mockOAuth2Server != null) {
+ mockOAuth2Server.shutdown();
+ }
+ }
- cluster.waitForAllNodes(10);
+ @Override
+ @Before
+ public void setUp() throws Exception {
+ super.setUp();
Review comment:
if all it does it call super, then we don't need this method.
##########
File path: solr/solr-ref-guide/src/jwt-authentication-plugin.adoc
##########
@@ -158,12 +161,18 @@ Let's comment on this config:
<11> Configure the issuer id. Will be used for validating tokens. A token's 'iss' claim must match one of the configured issuer IDs.
<12> Configure the audience claim. A token's 'aud' claim must match 'aud' for one of the configured issuers.
<13> This issuer is auto configured through discovery, so 'iss' and JWK settings are not required
+<14> Provides SSL certificates to trust to support self-signed SSL in the issuers.
=== Using non SSL URLs
In production environments you should always use SSL protected HTTPS connections, otherwise you open yourself up to attacks.
However, in development, it may be useful to use regular http urls, and bypass the
security check that Solr performs. To support this you can set the environment variable `solr.auth.jwt.allowOutboundHttp=true`.
+=== Trusting the IdP server
+All communication with issuer server (IdP) is done over HTTPS. If the issuer uses a self signed SSL certificate, the connection will fail since the plugin enforces verification of SSL certificates on outbound traffic. By default, the root certificates present in Java's TrustStore (default or custom provided) are consulted. If you do not want to place the IdP certificates in Java's TrustStore, you can configure this plugin with either `trustedCertsFile` or `trustedCerts` options to provide a set of certificates to use when talking to IdP. This has the extra benefit of also working even if Solr is not started in SSL mode.
Review comment:
I like this comment.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #139:
URL: https://github.com/apache/solr/pull/139#issuecomment-845731816
Feedback welcome. I could not think of a good way to actually test that the certificates are used (do we have a mock HTTPS server somewhere?).
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r646390022
##########
File path: solr/solr-ref-guide/src/jwt-authentication-plugin.adoc
##########
@@ -158,12 +161,18 @@ Let's comment on this config:
<11> Configure the issuer id. Will be used for validating tokens. A token's 'iss' claim must match one of the configured issuer IDs.
<12> Configure the audience claim. A token's 'aud' claim must match 'aud' for one of the configured issuers.
<13> This issuer is auto configured through discovery, so 'iss' and JWK settings are not required
+<14> Provides SSL certificates to trust to support self-signed SSL in the issuers.
=== Using non SSL URLs
In production environments you should always use SSL protected HTTPS connections, otherwise you open yourself up to attacks.
However, in development, it may be useful to use regular http urls, and bypass the
security check that Solr performs. To support this you can set the environment variable `solr.auth.jwt.allowOutboundHttp=true`.
+=== Trusting the IdP server
+All communication with issuer server (IdP) is done over HTTPS. If the issuer uses a self signed SSL certificate, the connection will fail since the plugin enforces verification of SSL certificates on outbound traffic. By default, the root certificates present in Java's TrustStore (default or custom provided) are consulted. If you do not want to place the IdP certificates in Java's TrustStore, you can configure this plugin with either `trustedCertsFile` or `trustedCerts` options to provide a set of certificates to use when talking to IdP. This has the extra benefit of also working even if Solr is not started in SSL mode.
+
+Please configure either `trustedCerts` or `trustedCertsFile`, not both.
Review comment:
Done. Also rewrote the paragraph above.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] muse-dev[bot] commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
muse-dev[bot] commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r641608294
##########
File path: solr/core/src/test/org/apache/solr/security/JWTAuthPluginIntegrationTest.java
##########
@@ -328,4 +406,56 @@ private void executeCommand(String url, HttpClient cl, String payload, JsonWebSi
});
}
+
+ /**
+ * Creates a security.json string which points to the MockOAuth server using it's well-known URL and trusting its SSL
+ */
+ private static String createMockOAuthSecurityJson(Path pemFilePath) throws IOException {
+ String wellKnown = mockOAuth2Server.wellKnownUrl("default").toString();
+ String pemCert = extractCertificateFromPem(pemFilePath);
+ return "{\n" +
+ " \"authentication\" : {\n" +
+ " \"class\": \"solr.JWTAuthPlugin\",\n" +
+ " \"wellKnownUrl\": \"" + wellKnown + "\",\n" +
+ " \"blockUnknown\": true\n" +
+ " \"trustedCerts\": \"" + pemCert + "\"\n" +
+ " }\n" +
+ "}";
+ }
+
+ private static String extractCertificateFromPem(Path pemFilePath) throws IOException {
+ String raw = Files.readString(pemFilePath).replaceAll("\n", "\\\\n");
+ int from = raw.indexOf("-----BEGIN CERTIFICATE-----");
+ int end = raw.lastIndexOf("-----END CERTIFICATE-----") + 25;
+ return raw.substring(from, end);
+ }
+
+ /**
+ * Create and return a MockOAuth2Server with given SSL certificate
+ * @param pemFilepath path to a pem certificate file
+ * @param secretKeyPass password to secret key
+ */
+ private static MockOAuth2Server createMockOAuthServer(Path pemFilepath, String secretKeyPass) {
+ X509ExtendedKeyManager keyManager = PemUtils.loadIdentityMaterial(pemFilepath, secretKeyPass.toCharArray());
+ X509ExtendedTrustManager trustManager = PemUtils.loadTrustMaterial(pemFilepath);
+ SSLFactory sslFactory = SSLFactory.builder()
+ .withIdentityMaterial(keyManager)
+ .withTrustMaterial(trustManager)
+ .build();
+
+ MockWebServerWrapper mockWebServerWrapper = new MockWebServerWrapper();
+ MockWebServer mockWebServer = mockWebServerWrapper.getMockWebServer();
+ try {
+ SSLContext sslContext = SSLContext.getInstance("TLS");
Review comment:
*opt.semgrep.java.lang.security.audit.weak-ssl-context.weak-ssl-context:* An insecure SSL context was detected. TLS versions 1.0, 1.1, and all SSL versions
are considered weak encryption and are deprecated.
Use SSLContext.getInstance("TLSv1.2") for the best security.
(at-me [in a reply](https://docs.muse.dev/docs/talk-to-muse/) with `help` or `ignore`)
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r646402044
##########
File path: solr/core/build.gradle
##########
@@ -149,5 +149,12 @@ dependencies {
testImplementation('org.mockito:mockito-core', {
exclude group: "net.bytebuddy", module: "byte-buddy-agent"
})
+ testImplementation('no.nav.security:mock-oauth2-server', {
Review comment:
Anyone knows about 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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r646389781
##########
File path: solr/core/src/java/org/apache/solr/security/JWTVerificationkeyResolver.java
##########
@@ -110,7 +112,13 @@ public Key resolveKey(JsonWebSignature jws, List<JsonWebStructure> nestingContex
if (issuerConfig.usesHttpsJwk()) {
keysSource = "[" + String.join(", ", issuerConfig.getJwksUrls()) + "]";
for (HttpsJwks hjwks : issuerConfig.getHttpsJwks()) {
- jsonWebKeys.addAll(hjwks.getJsonWebKeys());
+ try {
+ jsonWebKeys.addAll(hjwks.getJsonWebKeys());
+ } catch (SSLHandshakeException e) {
+ log.warn("Failed to talk SSL with {}, do you have the correct SSL certificate configured?", hjwks.getLocation());
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+ "SSL error when validating auth token. Make sure correct certificates are configured.", e);
Review comment:
Agree, now only throws exception
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] muse-dev[bot] commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
muse-dev[bot] commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r636708465
##########
File path: solr/core/src/java/org/apache/solr/security/JWTAuthPlugin.java
##########
@@ -155,8 +168,33 @@ public void init(Map<String, Object> pluginConfig) {
requiredScopes = Arrays.asList(requiredScopesStr.split("\\s+"));
}
+ // Parse custom IDP SSL Cert from either path or string
+ InputStream trustedCertsStream = null;
+ String trustedCertsFile = (String) pluginConfig.get(PARAM_TRUSTED_CERTS_FILE);
+ String trustedCerts = (String) pluginConfig.get(PARAM_TRUSTED_CERTS);
+ if (trustedCertsFile != null && trustedCerts != null) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Found both " + PARAM_TRUSTED_CERTS_FILE + " and " + PARAM_TRUSTED_CERTS + ", please use only one");
+ }
+ if (trustedCertsFile != null) {
+ try {
+ trustedCertsStream = Files.newInputStream(Paths.get(trustedCertsFile));
Review comment:
*PATH_TRAVERSAL_IN:* This API (java/nio/file/Paths.get(Ljava/lang/String;[Ljava/lang/String;)Ljava/nio/file/Path;) reads a file whose location might be specified by user input [(details)](https://find-sec-bugs.github.io/bugs.htm#PATH_TRAVERSAL_IN)
(at-me [in a reply](https://docs.muse.dev/docs/talk-to-muse/) with `help` or `ignore`)
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r648982175
##########
File path: solr/core/build.gradle
##########
@@ -149,5 +149,12 @@ dependencies {
testImplementation('org.mockito:mockito-core', {
exclude group: "net.bytebuddy", module: "byte-buddy-agent"
})
+ testImplementation('no.nav.security:mock-oauth2-server', {
Review comment:
See https://issues.apache.org/jira/browse/SOLR-15465 for a separate discussion of this topic, leaving it in for now.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #139:
URL: https://github.com/apache/solr/pull/139#issuecomment-848733275
I'm trying to integrate https://github.com/navikt/mock-oauth2-server into the tests (actually open-sourced by a Norwegian government entity). Quite simple, you initialize a `MockOAuth2Server` and then it listens on a separate port and behaves like an Oauth server. I'm struggling to make it talk https though, so that we can finally assert that the new config options actually works.
If this ends up working, we can also add several other end-to-end integration tests for JWT auth (in separate issues).
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #139:
URL: https://github.com/apache/solr/pull/139#issuecomment-850459590
I just integrated mock-oauth2-server, and finally was able to verify that you can configure JWTAuthPlugin with a PEM file and it is accepted by the mock server. Still have some cleanup to do
* Create a negative test with a different cert, to see it failing
* Reconsider use of SslFactory dependency and BouncyCastle to setup SSL. May instead check in a .jks or .p12 file for the tests.
* Other feedback welcome
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r645593512
##########
File path: solr/solr-ref-guide/src/jwt-authentication-plugin.adoc
##########
@@ -158,12 +161,18 @@ Let's comment on this config:
<11> Configure the issuer id. Will be used for validating tokens. A token's 'iss' claim must match one of the configured issuer IDs.
<12> Configure the audience claim. A token's 'aud' claim must match 'aud' for one of the configured issuers.
<13> This issuer is auto configured through discovery, so 'iss' and JWK settings are not required
+<14> Provides SSL certificates to trust to support self-signed SSL in the issuers.
=== Using non SSL URLs
In production environments you should always use SSL protected HTTPS connections, otherwise you open yourself up to attacks.
However, in development, it may be useful to use regular http urls, and bypass the
security check that Solr performs. To support this you can set the environment variable `solr.auth.jwt.allowOutboundHttp=true`.
+=== Trusting the IdP server
+All communication with issuer server (IdP) is done over HTTPS. If the issuer uses a self signed SSL certificate, the connection will fail since the plugin enforces verification of SSL certificates on outbound traffic. By default, the root certificates present in Java's TrustStore (default or custom provided) are consulted. If you do not want to place the IdP certificates in Java's TrustStore, you can configure this plugin with either `trustedCertsFile` or `trustedCerts` options to provide a set of certificates to use when talking to IdP. This has the extra benefit of also working even if Solr is not started in SSL mode.
Review comment:
We should probably highlight here that it could be a benefit to supply a PEM even if the IDP server cert is signed by a global top-level CA found in Java. This is because trusting only ONE specific cert is more secure that trusting all public certs out there :)
##########
File path: solr/core/build.gradle
##########
@@ -149,5 +149,12 @@ dependencies {
testImplementation('org.mockito:mockito-core', {
exclude group: "net.bytebuddy", module: "byte-buddy-agent"
})
+ testImplementation('no.nav.security:mock-oauth2-server', {
Review comment:
I add this as a testImpl dependency, but still I'm forced to add LICENSE and NOTICE files in the `licenses` folder for this and all its dependencies. Why is that necessary, when we do not distribute these jars, they are only used during tests?
There are the new (test) deps:
```
+--- no.nav.security:mock-oauth2-server -> 0.3.3
| +--- org.jetbrains.kotlin:kotlin-stdlib:1.4.32
| | +--- org.jetbrains.kotlin:kotlin-stdlib-common:1.4.32
| | \--- org.jetbrains:annotations:13.0
| +--- org.jetbrains.kotlin:kotlin-reflect:1.4.32
| | \--- org.jetbrains.kotlin:kotlin-stdlib:1.4.32 (*)
| +--- com.fasterxml.jackson.core:jackson-databind:2.12.2 (*)
| +--- io.github.microutils:kotlin-logging:2.0.6
| | \--- io.github.microutils:kotlin-logging-jvm:2.0.6
| | +--- org.jetbrains.kotlin:kotlin-stdlib:1.4.31 -> 1.4.32 (*)
| | \--- org.jetbrains.kotlin:kotlin-stdlib-common:1.4.31 -> 1.4.32
| +--- com.fasterxml.jackson.module:jackson-module-kotlin:2.12.2
| | +--- com.fasterxml.jackson.core:jackson-databind:2.12.2 (*)
| | +--- com.fasterxml.jackson.core:jackson-annotations:2.12.2 (*)
| | +--- org.jetbrains.kotlin:kotlin-reflect:1.4.21 -> 1.4.32 (*)
| | \--- com.fasterxml.jackson:jackson-bom:2.12.2 (*)
| +--- org.freemarker:freemarker:2.3.31
| +--- com.squareup.okhttp3:mockwebserver:4.9.1
| | +--- com.squareup.okhttp3:okhttp:4.9.1
| | | +--- com.squareup.okio:okio:2.8.0
| | | | +--- org.jetbrains.kotlin:kotlin-stdlib:1.4.0 -> 1.4.32 (*)
| | | | \--- org.jetbrains.kotlin:kotlin-stdlib-common:1.4.0 -> 1.4.32
| | | \--- org.jetbrains.kotlin:kotlin-stdlib:1.4.10 -> 1.4.32 (*)
| | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10
| | +--- org.jetbrains.kotlin:kotlin-stdlib:1.4.10 -> 1.4.32 (*)
| | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.4.10
| | \--- org.jetbrains.kotlin:kotlin-stdlib:1.4.10 -> 1.4.32 (*)
| \--- com.nimbusds:oauth2-oidc-sdk:9.2.3
| +--- com.github.stephenc.jcip:jcip-annotations:1.0-1
| +--- com.nimbusds:content-type:2.1
| +--- net.minidev:json-smart:[1.3.1,2.3] -> 2.3
| | \--- net.minidev:accessors-smart:1.2
| | \--- org.ow2.asm:asm:5.0.4 -> 7.2
| +--- com.nimbusds:lang-tag:1.4.4
| \--- com.nimbusds:nimbus-jose-jwt:9.6.1
| \--- com.github.stephenc.jcip:jcip-annotations:1.0-1
```
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy merged pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy merged pull request #139:
URL: https://github.com/apache/solr/pull/139
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r636899014
##########
File path: solr/core/src/java/org/apache/solr/security/JWTAuthPlugin.java
##########
@@ -155,8 +168,33 @@ public void init(Map<String, Object> pluginConfig) {
requiredScopes = Arrays.asList(requiredScopesStr.split("\\s+"));
}
+ // Parse custom IDP SSL Cert from either path or string
+ InputStream trustedCertsStream = null;
+ String trustedCertsFile = (String) pluginConfig.get(PARAM_TRUSTED_CERTS_FILE);
+ String trustedCerts = (String) pluginConfig.get(PARAM_TRUSTED_CERTS);
+ if (trustedCertsFile != null && trustedCerts != null) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Found both " + PARAM_TRUSTED_CERTS_FILE + " and " + PARAM_TRUSTED_CERTS + ", please use only one");
+ }
+ if (trustedCertsFile != null) {
+ try {
+ trustedCertsStream = Files.newInputStream(Paths.get(trustedCertsFile));
Review comment:
Hmm, I tried using `CoreContainer.assertPathAllowed()` but unfortunately AuthPlugins do not have access to coreContainer, so I cannot get access to it. An Auth plugin is at the container level, not the core level, so what is the equivalent of `SolrCoreAware` for such plugins? I.e. that it would get a reference to `CoreContainer`?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r637278099
##########
File path: solr/core/src/java/org/apache/solr/security/JWTAuthPlugin.java
##########
@@ -199,6 +237,28 @@ public void init(Map<String, Object> pluginConfig) {
lastInitTime = Instant.now();
}
+ /**
+ * Tries for find X509 certificates in the input stream in DER or PEM format.
+ * Supports multiple certs in same stream if multiple PEM certs are concatenated.
+ * @param trustedCertsStream input stream with the contents of either PEM (plaintext) or DER (binary) certs
+ * @return collection of found certificates, else throws exception
+ */
+ static Collection<X509Certificate> parsePemToX509Certs(InputStream trustedCertsStream) {
Review comment:
Good idea - moving to CryptoKeys and generalizing a bit
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r646390022
##########
File path: solr/solr-ref-guide/src/jwt-authentication-plugin.adoc
##########
@@ -158,12 +161,18 @@ Let's comment on this config:
<11> Configure the issuer id. Will be used for validating tokens. A token's 'iss' claim must match one of the configured issuer IDs.
<12> Configure the audience claim. A token's 'aud' claim must match 'aud' for one of the configured issuers.
<13> This issuer is auto configured through discovery, so 'iss' and JWK settings are not required
+<14> Provides SSL certificates to trust to support self-signed SSL in the issuers.
=== Using non SSL URLs
In production environments you should always use SSL protected HTTPS connections, otherwise you open yourself up to attacks.
However, in development, it may be useful to use regular http urls, and bypass the
security check that Solr performs. To support this you can set the environment variable `solr.auth.jwt.allowOutboundHttp=true`.
+=== Trusting the IdP server
+All communication with issuer server (IdP) is done over HTTPS. If the issuer uses a self signed SSL certificate, the connection will fail since the plugin enforces verification of SSL certificates on outbound traffic. By default, the root certificates present in Java's TrustStore (default or custom provided) are consulted. If you do not want to place the IdP certificates in Java's TrustStore, you can configure this plugin with either `trustedCertsFile` or `trustedCerts` options to provide a set of certificates to use when talking to IdP. This has the extra benefit of also working even if Solr is not started in SSL mode.
+
+Please configure either `trustedCerts` or `trustedCertsFile`, not both.
Review comment:
Done.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r637277985
##########
File path: solr/core/src/java/org/apache/solr/security/JWTIssuerConfig.java
##########
@@ -335,33 +346,49 @@ public boolean isValid() {
return jwkConfigured > 0;
}
+ public void setTrustedCerts(Collection<X509Certificate> trustedCerts) {
+ this.trustedCerts = trustedCerts;
+ }
+
/**
*
*/
static class HttpsJwksFactory {
private final long jwkCacheDuration;
private final long refreshReprieveThreshold;
+ private Collection<X509Certificate> trustedCerts;
public HttpsJwksFactory(long jwkCacheDuration, long refreshReprieveThreshold) {
this.jwkCacheDuration = jwkCacheDuration;
this.refreshReprieveThreshold = refreshReprieveThreshold;
}
-
+
+ public HttpsJwksFactory(long jwkCacheDuration, long refreshReprieveThreshold, Collection<X509Certificate> trustedCerts) {
+ this.jwkCacheDuration = jwkCacheDuration;
+ this.refreshReprieveThreshold = refreshReprieveThreshold;
+ this.trustedCerts = trustedCerts;
+ }
+
/**
* While the class name is HttpsJwks, it actually works with plain http formatted url as well.
+ *
* @param url the Url to connect to for JWK details.
*/
private HttpsJwks create(String url) {
try {
URL jwksUrl = new URL(url);
checkAllowOutboundHttpConnections(PARAM_JWKS_URL, jwksUrl);
-
} catch (MalformedURLException e) {
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Url " + url + " configured in " + PARAM_JWKS_URL + " is not a valid URL");
}
HttpsJwks httpsJkws = new HttpsJwks(url);
httpsJkws.setDefaultCacheDuration(jwkCacheDuration);
httpsJkws.setRefreshReprieveThreshold(refreshReprieveThreshold);
+ if (trustedCerts != null) {
+ Get getWithCustomTrust = new Get();
+ getWithCustomTrust.setTrustedCertificates(trustedCerts);
+ log.info("Trusting custom SSL certificate(s) for the IdP");
Review comment:
I moved ths to the first config parsing in JwtAuthPlugin instead. It will print on every re-initialization / change of security.json.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r646401624
##########
File path: solr/core/src/test/org/apache/solr/security/JWTAuthPluginIntegrationTest.java
##########
@@ -127,13 +135,40 @@ public void tearDown() throws Exception {
super.tearDown();
}
- @Test(expected = IOException.class)
- public void infoRequestWithoutToken() throws Exception {
- get(baseUrl + "/admin/info/system", null);
+ @Test
+ public void extractCertificateFromPem() throws IOException {
Review comment:
I added two tests to JWTAuthPluginTest, verifying that init() fails. No need for integration test of all these?
If you load a `security.json` to ZK with bogous `trustedCertsFile` or `trustedCerts` (or other config) while Solr is running, the JWT plugin `init()` will throw and exception and auth will not be enabled/changed. Here are log messages you can expect to see prior to exception:
> Reading trustedCerts from file {}
or
> Reading trustedCerts PEM from configuration string
In the exception stacktrace itself you'll see either
> Wrong type of certificates. Must be DER or PEM format
or
> Failed loading certificate(s) from input stream
If you start Solr from scratch with a bogous config, I think the server will fail during CoreContainer init and not start at all. The fact that users need to monitor Solr logs when pushing new security.json is unfortunate. It is easy to make mistakes when automating this with scripts. You may start Solr, then apply security.json which fails, and then you have an unsecured cluster without even knowing :(
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #139:
URL: https://github.com/apache/solr/pull/139#issuecomment-858508055
> BUILD SUCCESSFUL in 36m 16s
I'll merge this now
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r646401624
##########
File path: solr/core/src/test/org/apache/solr/security/JWTAuthPluginIntegrationTest.java
##########
@@ -127,13 +135,40 @@ public void tearDown() throws Exception {
super.tearDown();
}
- @Test(expected = IOException.class)
- public void infoRequestWithoutToken() throws Exception {
- get(baseUrl + "/admin/info/system", null);
+ @Test
+ public void extractCertificateFromPem() throws IOException {
Review comment:
I added two tests to JWTAuthPluginTest, verifying that init() fails. No need for integration test of all these?
If you load a `security.json` to ZK with bogous `trustedCertsFile` or `trustedCerts`, the JWT plugin will throw and exception and auth will not be enabled. Here are log messages you can expect to see prior to exception:
> Reading trustedCerts from file {}
or
> Reading trustedCerts PEM from configuration string
In the exception stacktrace itself you'll see either
> Wrong type of certificates. Must be DER or PEM format
or
> Failed loading certificate(s) from input stream
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #139:
URL: https://github.com/apache/solr/pull/139#issuecomment-846307923
Committed latest changes. Please resolve outstanding conversations if you're happy. Still would like some integration test to check the actual certificate usage. Think I'll try to spin up a MockIdp listening on some SSL socket...
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r637277310
##########
File path: solr/solr-ref-guide/src/jwt-authentication-plugin.adoc
##########
@@ -54,6 +54,8 @@ rolesClaim ; What claim id to pull user roles from. The claim must the
claimsMatch ; JSON object of claims (key) that must match a regular expression (value). Example: `{ "foo" : "A|B" }` will require the `foo` claim to be either "A" or "B". ;
adminUiScope ; Define what scope is requested when logging in from Admin UI ; If not defined, the first scope from `scope` parameter is used
redirectUris ; Valid location(s) for redirect after external authentication. Takes a string or array of strings. Must be the base URL of Solr, e.g., https://solr1.example.com:8983/solr/ and must match the list of redirect URIs registered with the Identity Provider beforehand. ; Defaults to empty list, i.e., any node is assumed to be a valid redirect target.
+trustedCerts ; One or more X.509 SSL certificates in plaintext PEM or PKCS#7 formats, that should be trusted when talking to IdPs. Newlines must be replaced with `\n`. ; Defaults to Java truststore
Review comment:
I added a separate paragraph explaining more, and linked to it from the table.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r637276860
##########
File path: solr/core/src/test/org/apache/solr/security/JWTAuthPluginTest.java
##########
@@ -477,4 +484,51 @@ public void initWithToplevelAndIssuersCombined() {
// Top-level (name=PRIMARY) issuer config does not need "iss" for back compat
assertNull(plugin.getPrimaryIssuer().getIss());
}
+
+ @Test
+ public void initWithIdpCertString() {
+ HashMap<String, Object> authConf = new HashMap<>();
+ authConf.put("jwksUrl", "https://127.0.0.1:9999/foo.jwk");
+ authConf.put("trustedCerts", trustedPemCert);
+ plugin = new JWTAuthPlugin();
+ plugin.init(authConf);
+ }
+
+ @Test
+ public void initWithIdpCertPath() {
+ HashMap<String, Object> authConf = new HashMap<>();
+ authConf.put("jwksUrl", "https://127.0.0.1:9999/foo.jwk");
+ authConf.put("trustedCertsFile", TEST_PATH().resolve("security").resolve("jwt_idpCert.pem").toString());
+ plugin = new JWTAuthPlugin();
+ plugin.init(authConf);
Review comment:
I added a test to check that there are 2 X509 certs in IssuerConfig. But still need to test somehow that the certs are actually used on outbound traffic to WellKnown endpoint as well as JWKS endpoint.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #139:
URL: https://github.com/apache/solr/pull/139#issuecomment-854719232
I have now addressed my two points
* Create a negative test with a different cert, to see it failing
* Remove need for BouncyCastle by importing pcs12 file directly in test
I have also added license and sha files for all new test dependencies and made precommit pass.
I think this is now ready for review.
It's a large PR, but most files are really SHA, LICENSE and NOTICE files.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r637238036
##########
File path: solr/core/src/java/org/apache/solr/security/JWTAuthPlugin.java
##########
@@ -155,8 +168,33 @@ public void init(Map<String, Object> pluginConfig) {
requiredScopes = Arrays.asList(requiredScopesStr.split("\\s+"));
}
+ // Parse custom IDP SSL Cert from either path or string
+ InputStream trustedCertsStream = null;
+ String trustedCertsFile = (String) pluginConfig.get(PARAM_TRUSTED_CERTS_FILE);
+ String trustedCerts = (String) pluginConfig.get(PARAM_TRUSTED_CERTS);
+ if (trustedCertsFile != null && trustedCerts != null) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Found both " + PARAM_TRUSTED_CERTS_FILE + " and " + PARAM_TRUSTED_CERTS + ", please use only one");
+ }
+ if (trustedCertsFile != null) {
+ try {
+ trustedCertsStream = Files.newInputStream(Paths.get(trustedCertsFile));
Review comment:
Thanks a lot - it actually worked!!
Solr should probably replace its dependency injection system at some point :)
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r646390578
##########
File path: solr/core/src/java/org/apache/solr/security/JWTAuthPlugin.java
##########
@@ -155,8 +175,38 @@ public void init(Map<String, Object> pluginConfig) {
requiredScopes = Arrays.asList(requiredScopesStr.split("\\s+"));
}
+ // Parse custom IDP SSL Cert from either path or string
+ InputStream trustedCertsStream = null;
+ String trustedCertsFile = (String) pluginConfig.get(PARAM_TRUSTED_CERTS_FILE);
+ String trustedCerts = (String) pluginConfig.get(PARAM_TRUSTED_CERTS);
+ if (trustedCertsFile != null && trustedCerts != null) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Found both " + PARAM_TRUSTED_CERTS_FILE + " and " + PARAM_TRUSTED_CERTS + ", please use only one");
+ }
+ if (trustedCertsFile != null) {
+ try {
+ Path trustedCertsPath = Paths.get(trustedCertsFile);
+ if (coreContainer != null) {
+ coreContainer.assertPathAllowed(trustedCertsPath);
+ }
+ trustedCertsStream = Files.newInputStream(trustedCertsPath);
+ log.info("Reading trustedCerts from file {}", trustedCertsFile);
+ } catch (IOException e) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Failed to read file " + trustedCertsFile, e);
+ }
+ }
+ if (trustedCerts != null) {
+ trustedCertsStream = IOUtils.toInputStream(trustedCerts, StandardCharsets.UTF_8);
Review comment:
Is there a problem using IOUtils for this?
##########
File path: solr/core/src/test/org/apache/solr/security/JWTAuthPluginIntegrationTest.java
##########
@@ -70,54 +89,43 @@
protected static final int NUM_SERVERS = 2;
protected static final int NUM_SHARDS = 2;
protected static final int REPLICATION_FACTOR = 1;
+ private static String mockOAuthToken;
+ private static Path pemFilePath;
+ private static Path wrongPemFilePath;
private final String COLLECTION = "jwtColl";
- private String jwtTestToken;
+ private static String jwtStaticTestToken;
private String baseUrl;
- private JsonWebSignature jws;
- private String jwtTokenWrongSignature;
+ private static JsonWebSignature jws;
+ private static String jwtTokenWrongSignature;
- @Override
- @Before
- public void setUp() throws Exception {
- super.setUp();
-
- configureCluster(NUM_SERVERS)// nodes
- .withSecurityJson(TEST_PATH().resolve("security").resolve("jwt_plugin_jwk_security.json"))
- .addConfig("conf1", TEST_PATH().resolve("configsets").resolve("cloud-minimal").resolve("conf"))
- .withDefaultClusterProperty("useLegacyReplicaAssignment", "false")
- .configure();
- baseUrl = cluster.getRandomJetty(random()).getBaseUrl().toString();
+ private static MockOAuth2Server mockOAuth2Server;
- String jwkJSON = "{\n" +
- " \"kty\": \"RSA\",\n" +
- " \"d\": \"i6pyv2z3o-MlYytWsOr3IE1olu2RXZBzjPRBNgWAP1TlLNaphHEvH5aHhe_CtBAastgFFMuP29CFhaL3_tGczkvWJkSveZQN2AHWHgRShKgoSVMspkhOt3Ghha4CvpnZ9BnQzVHnaBnHDTTTfVgXz7P1ZNBhQY4URG61DKIF-JSSClyh1xKuMoJX0lILXDYGGcjVTZL_hci4IXPPTpOJHV51-pxuO7WU5M9252UYoiYyCJ56ai8N49aKIMsqhdGuO4aWUwsGIW4oQpjtce5eEojCprYl-9rDhTwLAFoBtjy6LvkqlR2Ae5dKZYpStljBjK8PJrBvWZjXAEMDdQ8PuQ\",\n" +
- " \"e\": \"AQAB\",\n" +
- " \"use\": \"sig\",\n" +
- " \"kid\": \"test\",\n" +
- " \"alg\": \"RS256\",\n" +
- " \"n\": \"jeyrvOaZrmKWjyNXt0myAc_pJ1hNt3aRupExJEx1ewPaL9J9HFgSCjMrYxCB1ETO1NDyZ3nSgjZis-jHHDqBxBjRdq_t1E2rkGFaYbxAyKt220Pwgme_SFTB9MXVrFQGkKyjmQeVmOmV6zM3KK8uMdKQJ4aoKmwBcF5Zg7EZdDcKOFgpgva1Jq-FlEsaJ2xrYDYo3KnGcOHIt9_0NQeLsqZbeWYLxYni7uROFncXYV5FhSJCeR4A_rrbwlaCydGxE0ToC_9HNYibUHlkJjqyUhAgORCbNS8JLCJH8NUi5sDdIawK9GTSyvsJXZ-QHqo4cMUuxWV5AJtaRGghuMUfqQ\"\n" +
- "}";
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ // Setup an OAuth2 mock server with SSL
+ Path p12Cert = SolrTestCaseJ4.TEST_PATH().resolve("security").resolve("jwt_plugin_idp_certs.p12");
+ pemFilePath = SolrTestCaseJ4.TEST_PATH().resolve("security").resolve("jwt_plugin_idp_cert.pem");
+ wrongPemFilePath = SolrTestCaseJ4.TEST_PATH().resolve("security").resolve("jwt_plugin_idp_wrongcert.pem");
- PublicJsonWebKey jwk = RsaJsonWebKey.Factory.newPublicJwk(jwkJSON);
- JwtClaims claims = JWTAuthPluginTest.generateClaims();
- jws = new JsonWebSignature();
- jws.setPayload(claims.toJson());
- jws.setKey(jwk.getPrivateKey());
- jws.setKeyIdHeaderValue(jwk.getKeyId());
- jws.setAlgorithmHeaderValue(AlgorithmIdentifiers.RSA_USING_SHA256);
+ mockOAuth2Server = createMockOAuthServer(p12Cert,"secret");
+ mockOAuth2Server.start();
+ mockOAuthToken = mockOAuth2Server.issueToken("default",
+ "myClientId",
+ new DefaultOAuth2TokenCallback()).serialize();
+ initStaticJwt();
+ }
- jwtTestToken = jws.getCompactSerialization();
-
- PublicJsonWebKey jwk2 = RsaJwkGenerator.generateJwk(2048);
- jwk2.setKeyId("k2");
- JsonWebSignature jws2 = new JsonWebSignature();
- jws2.setPayload(claims.toJson());
- jws2.setKey(jwk2.getPrivateKey());
- jws2.setKeyIdHeaderValue(jwk2.getKeyId());
- jws2.setAlgorithmHeaderValue(AlgorithmIdentifiers.RSA_USING_SHA256);
- jwtTokenWrongSignature = jws2.getCompactSerialization();
+ @AfterClass
+ public static void afterClass() throws Exception {
+ if (mockOAuth2Server != null) {
+ mockOAuth2Server.shutdown();
+ }
+ }
- cluster.waitForAllNodes(10);
+ @Override
+ @Before
+ public void setUp() throws Exception {
+ super.setUp();
Review comment:
Agree, removed it.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] madrob commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r636967838
##########
File path: solr/core/src/java/org/apache/solr/security/JWTAuthPlugin.java
##########
@@ -155,8 +168,33 @@ public void init(Map<String, Object> pluginConfig) {
requiredScopes = Arrays.asList(requiredScopesStr.split("\\s+"));
}
+ // Parse custom IDP SSL Cert from either path or string
+ InputStream trustedCertsStream = null;
+ String trustedCertsFile = (String) pluginConfig.get(PARAM_TRUSTED_CERTS_FILE);
+ String trustedCerts = (String) pluginConfig.get(PARAM_TRUSTED_CERTS);
+ if (trustedCertsFile != null && trustedCerts != null) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Found both " + PARAM_TRUSTED_CERTS_FILE + " and " + PARAM_TRUSTED_CERTS + ", please use only one");
+ }
+ if (trustedCertsFile != null) {
+ try {
+ trustedCertsStream = Files.newInputStream(Paths.get(trustedCertsFile));
Review comment:
CoreContainer:517 attempts to create an authentication plugin using a constructor with the CoreContainer as a parameter first, then falls back to a zero-arg constructor. I think you can just add this constructor and hold a reference to the container.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] madrob commented on a change in pull request #139: SOLR-15423 JWTAuthPlugin support for custom truststore
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #139:
URL: https://github.com/apache/solr/pull/139#discussion_r636972401
##########
File path: solr/core/src/java/org/apache/solr/security/JWTAuthPlugin.java
##########
@@ -199,6 +237,28 @@ public void init(Map<String, Object> pluginConfig) {
lastInitTime = Instant.now();
}
+ /**
+ * Tries for find X509 certificates in the input stream in DER or PEM format.
+ * Supports multiple certs in same stream if multiple PEM certs are concatenated.
+ * @param trustedCertsStream input stream with the contents of either PEM (plaintext) or DER (binary) certs
+ * @return collection of found certificates, else throws exception
+ */
+ static Collection<X509Certificate> parsePemToX509Certs(InputStream trustedCertsStream) {
Review comment:
Maybe this belongs in CryptoKeys? Or some other util class?
##########
File path: solr/core/src/java/org/apache/solr/security/JWTIssuerConfig.java
##########
@@ -335,33 +346,49 @@ public boolean isValid() {
return jwkConfigured > 0;
}
+ public void setTrustedCerts(Collection<X509Certificate> trustedCerts) {
+ this.trustedCerts = trustedCerts;
+ }
+
/**
*
*/
static class HttpsJwksFactory {
private final long jwkCacheDuration;
private final long refreshReprieveThreshold;
+ private Collection<X509Certificate> trustedCerts;
public HttpsJwksFactory(long jwkCacheDuration, long refreshReprieveThreshold) {
this.jwkCacheDuration = jwkCacheDuration;
this.refreshReprieveThreshold = refreshReprieveThreshold;
}
-
+
+ public HttpsJwksFactory(long jwkCacheDuration, long refreshReprieveThreshold, Collection<X509Certificate> trustedCerts) {
+ this.jwkCacheDuration = jwkCacheDuration;
+ this.refreshReprieveThreshold = refreshReprieveThreshold;
+ this.trustedCerts = trustedCerts;
+ }
+
/**
* While the class name is HttpsJwks, it actually works with plain http formatted url as well.
+ *
* @param url the Url to connect to for JWK details.
*/
private HttpsJwks create(String url) {
try {
URL jwksUrl = new URL(url);
checkAllowOutboundHttpConnections(PARAM_JWKS_URL, jwksUrl);
-
} catch (MalformedURLException e) {
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Url " + url + " configured in " + PARAM_JWKS_URL + " is not a valid URL");
}
HttpsJwks httpsJkws = new HttpsJwks(url);
httpsJkws.setDefaultCacheDuration(jwkCacheDuration);
httpsJkws.setRefreshReprieveThreshold(refreshReprieveThreshold);
+ if (trustedCerts != null) {
+ Get getWithCustomTrust = new Get();
+ getWithCustomTrust.setTrustedCertificates(trustedCerts);
+ log.info("Trusting custom SSL certificate(s) for the IdP");
Review comment:
How often does this happen? Is it per user request? Per user session? Might be too verbose.
##########
File path: solr/solr-ref-guide/src/jwt-authentication-plugin.adoc
##########
@@ -54,6 +54,8 @@ rolesClaim ; What claim id to pull user roles from. The claim must the
claimsMatch ; JSON object of claims (key) that must match a regular expression (value). Example: `{ "foo" : "A|B" }` will require the `foo` claim to be either "A" or "B". ;
adminUiScope ; Define what scope is requested when logging in from Admin UI ; If not defined, the first scope from `scope` parameter is used
redirectUris ; Valid location(s) for redirect after external authentication. Takes a string or array of strings. Must be the base URL of Solr, e.g., https://solr1.example.com:8983/solr/ and must match the list of redirect URIs registered with the Identity Provider beforehand. ; Defaults to empty list, i.e., any node is assumed to be a valid redirect target.
+trustedCerts ; One or more X.509 SSL certificates in plaintext PEM or PKCS#7 formats, that should be trusted when talking to IdPs. Newlines must be replaced with `\n`. ; Defaults to Java truststore
Review comment:
I'd like to see more explanation when somebody would need this, similar to what was described in the JIRA, e.g. explain that Jetty's TrustStore is ignored if running with HTTP with an SSL-terminating proxy/LB/ingress in front.
##########
File path: solr/core/src/test/org/apache/solr/security/JWTAuthPluginTest.java
##########
@@ -477,4 +484,51 @@ public void initWithToplevelAndIssuersCombined() {
// Top-level (name=PRIMARY) issuer config does not need "iss" for back compat
assertNull(plugin.getPrimaryIssuer().getIss());
}
+
+ @Test
+ public void initWithIdpCertString() {
+ HashMap<String, Object> authConf = new HashMap<>();
+ authConf.put("jwksUrl", "https://127.0.0.1:9999/foo.jwk");
+ authConf.put("trustedCerts", trustedPemCert);
+ plugin = new JWTAuthPlugin();
+ plugin.init(authConf);
+ }
+
+ @Test
+ public void initWithIdpCertPath() {
+ HashMap<String, Object> authConf = new HashMap<>();
+ authConf.put("jwksUrl", "https://127.0.0.1:9999/foo.jwk");
+ authConf.put("trustedCertsFile", TEST_PATH().resolve("security").resolve("jwt_idpCert.pem").toString());
+ plugin = new JWTAuthPlugin();
+ plugin.init(authConf);
Review comment:
Anything we can do to assert that the generated factory has the correct trust store?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org