Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22557#discussion_r221067787
--- Diff:
common/network-common/src/main/java/org/apache/spark/network/crypto/AuthEngine.java
---
@@ -241,29 +249,52 @@ private SecretKeySpec generateKey(String kdf, int
iterations, byte[] salt, int k
return new SecretKeySpec(key.getEncoded(), conf.keyAlgorithm());
}
- private byte[] doCipherOp(CryptoCipher cipher, byte[] in, boolean
isFinal)
+ private byte[] doCipherOp(int mode, byte[] in, boolean isFinal)
throws GeneralSecurityException {
+ CryptoCipher cipher;
+ switch (mode) {
+ case Cipher.ENCRYPT_MODE:
+ cipher = encryptor;
+ break;
+ case Cipher.DECRYPT_MODE:
+ cipher = decryptor;
+ break;
+ default:
+ throw new IllegalArgumentException(String.valueOf(mode));
+ }
+
Preconditions.checkState(cipher != null);
- int scale = 1;
- while (true) {
- int size = in.length * scale;
- byte[] buffer = new byte[size];
- try {
- int outSize = isFinal ? cipher.doFinal(in, 0, in.length, buffer, 0)
- : cipher.update(in, 0, in.length, buffer, 0);
- if (outSize != buffer.length) {
- byte[] output = new byte[outSize];
- System.arraycopy(buffer, 0, output, 0, output.length);
- return output;
- } else {
- return buffer;
+ try {
+ int scale = 1;
+ while (true) {
+ int size = in.length * scale;
+ byte[] buffer = new byte[size];
+ try {
+ int outSize = isFinal ? cipher.doFinal(in, 0, in.length, buffer,
0)
+ : cipher.update(in, 0, in.length, buffer, 0);
+ if (outSize != buffer.length) {
+ byte[] output = new byte[outSize];
+ System.arraycopy(buffer, 0, output, 0, output.length);
+ return output;
+ } else {
+ return buffer;
+ }
+ } catch (ShortBufferException e) {
+ // Try again with a bigger buffer.
+ scale *= 2;
}
- } catch (ShortBufferException e) {
- // Try again with a bigger buffer.
- scale *= 2;
}
+ } catch (InternalError ie) {
+ // SPARK-25535. The commons-cryto library will throw InternalError
if something goes wrong,
+ // and leave bad state behind in the Java wrappers, so it's not safe
to use them afterwards.
+ if (mode == Cipher.ENCRYPT_MODE) {
+ this.encryptor = null;
+ } else {
+ this.decryptor = null;
--- End diff --
I can add a check that the ciphers are not null, but no need to have a
separate flag. These need to be set to null so that `close` knows not to try to
mess with them.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
