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 2023/01/06 20:27:19 UTC
[GitHub] [solr] risdenk opened a new pull request, #1279: SOLR-16613: CryptoKeys should handle RSA padding for OpenJ9
risdenk opened a new pull request, #1279:
URL: https://github.com/apache/solr/pull/1279
https://issues.apache.org/jira/browse/SOLR-16613
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
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] magibney commented on a diff in pull request #1279: SOLR-16613: CryptoKeys should handle RSA padding for OpenJ9
Posted by GitBox <gi...@apache.org>.
magibney commented on code in PR #1279:
URL: https://github.com/apache/solr/pull/1279#discussion_r1063821975
##########
solr/core/src/java/org/apache/solr/util/CryptoKeys.java:
##########
@@ -295,15 +304,28 @@ public PublicKey getPublicKey() {
return publicKey;
}
- public byte[] encrypt(ByteBuffer buffer) {
Review Comment:
This is a public method on a public utility class though; I wonder if the intention was to have this used flexibly from elsewhere? This could in principle break plugin code ...
This change seems good still, but worth explicitly calling out this possible concern.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
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] risdenk commented on a diff in pull request #1279: SOLR-16613: CryptoKeys should handle RSA padding for OpenJ9
Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1279:
URL: https://github.com/apache/solr/pull/1279#discussion_r1064711398
##########
solr/core/src/java/org/apache/solr/util/CryptoKeys.java:
##########
@@ -295,15 +304,28 @@ public PublicKey getPublicKey() {
return publicKey;
}
- public byte[] encrypt(ByteBuffer buffer) {
Review Comment:
Yea I'm just going to back out this change.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
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] risdenk commented on pull request #1279: SOLR-16613: CryptoKeys should handle RSA padding for OpenJ9
Posted by GitBox <gi...@apache.org>.
risdenk commented on PR #1279:
URL: https://github.com/apache/solr/pull/1279#issuecomment-1374120814
> Should we switch the algorithm from no padding to something with padding?
probably at some point. I don't really want to tackle that with this. This at least cleans up the build failures.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
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] risdenk commented on a diff in pull request #1279: SOLR-16613: CryptoKeys should handle RSA padding for OpenJ9
Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1279:
URL: https://github.com/apache/solr/pull/1279#discussion_r1064728436
##########
solr/core/src/java/org/apache/solr/util/CryptoKeys.java:
##########
@@ -295,15 +304,28 @@ public PublicKey getPublicKey() {
return publicKey;
}
- public byte[] encrypt(ByteBuffer buffer) {
Review Comment:
Reverted this in 741e131ebbc43b7e49fc668e0c76cb6163946b3b
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
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] risdenk commented on a diff in pull request #1279: SOLR-16613: CryptoKeys should handle RSA padding for OpenJ9
Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1279:
URL: https://github.com/apache/solr/pull/1279#discussion_r1063768185
##########
solr/core/src/test/org/apache/solr/cloud/TestRSAKeyPair.java:
##########
@@ -36,26 +38,29 @@ public void testGenKeyPair() throws Exception {
public void testReadKeysFromDisk() throws Exception {
URL privateKey = getClass().getClassLoader().getResource("cryptokeys/priv_key512_pkcs8.pem");
URL publicKey = getClass().getClassLoader().getResource("cryptokeys/pub_key512.der");
-
+ assertNotNull(privateKey);
+ assertNotNull(publicKey);
testRoundTrip(new CryptoKeys.RSAKeyPair(privateKey, publicKey));
}
private void testRoundTrip(CryptoKeys.RSAKeyPair kp) throws Exception {
- final byte[] plaintext = new byte[random().nextInt(64)];
- random().nextBytes(plaintext);
Review Comment:
random().nextBytes() would end up with null bytes and that would cause issues. I switched this to generate a regular string with no null bytes.
##########
solr/core/src/java/org/apache/solr/util/CryptoKeys.java:
##########
@@ -295,15 +304,28 @@ public PublicKey getPublicKey() {
return publicKey;
}
- public byte[] encrypt(ByteBuffer buffer) {
Review Comment:
We only call `ByteBuffer.wrap` in all invocations of this so isn't helpful to have this as a `ByteBuffer`. The decryption also just uses a plain `byte[]` so this makes it consistent.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
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] risdenk merged pull request #1279: SOLR-16613: CryptoKeys should handle RSA padding for OpenJ9
Posted by GitBox <gi...@apache.org>.
risdenk merged PR #1279:
URL: https://github.com/apache/solr/pull/1279
--
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.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
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