You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/09/16 18:15:16 UTC
[GitHub] [accumulo] milleruntime opened a new pull request, #2935: Modify LogReader to print just the crypto params
milleruntime opened a new pull request, #2935:
URL: https://github.com/apache/accumulo/pull/2935
* Added the -e option to LogReader to not read the whole WAL and just print the crypto params
* Make the LogReader options extend ConfigOpts to allow passing in properties and changed the -p option to --regex to differentiate from the properties option
* Added checkWALEncryption() to PerTableCryptoIT that calls the LogReader
* Rename a test in CryptoTest
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a diff in pull request #2935: Modify LogReader to print just the crypto params
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2935:
URL: https://github.com/apache/accumulo/pull/2935#discussion_r975546130
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java:
##########
@@ -172,6 +187,45 @@ public void execute(String[] args) throws Exception {
}
}
+ private void printCryptoParams(FSDataInputStream input, Path path) {
+ byte[] magic4 = DfsLogger.LOG_FILE_HEADER_V4.getBytes(UTF_8);
+ byte[] magic3 = DfsLogger.LOG_FILE_HEADER_V3.getBytes(UTF_8);
+ byte[] noCryptoBytes = new NoFileEncrypter().getDecryptionParameters();
+
+ if (magic4.length != magic3.length)
+ throw new AssertionError("Always expect log file headers to be same length : " + magic4.length
+ + " != " + magic3.length);
+
+ byte[] magicBuffer = new byte[magic4.length];
+ try {
+ input.readFully(magicBuffer);
+ if (Arrays.equals(magicBuffer, magic4)) {
+ byte[] cryptoParams = CryptoUtils.readParams(input);
Review Comment:
It would be nice to throw a CryptoException when we are expecting a file to be encrypted and get an IOException. I am not sure if this is the best place though.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a diff in pull request #2935: Modify LogReader to print just the crypto params
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2935:
URL: https://github.com/apache/accumulo/pull/2935#discussion_r975558043
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java:
##########
@@ -172,6 +187,45 @@ public void execute(String[] args) throws Exception {
}
}
+ private void printCryptoParams(FSDataInputStream input, Path path) {
+ byte[] magic4 = DfsLogger.LOG_FILE_HEADER_V4.getBytes(UTF_8);
+ byte[] magic3 = DfsLogger.LOG_FILE_HEADER_V3.getBytes(UTF_8);
+ byte[] noCryptoBytes = new NoFileEncrypter().getDecryptionParameters();
+
+ if (magic4.length != magic3.length)
+ throw new AssertionError("Always expect log file headers to be same length : " + magic4.length
+ + " != " + magic3.length);
+
+ byte[] magicBuffer = new byte[magic4.length];
+ try {
+ input.readFully(magicBuffer);
+ if (Arrays.equals(magicBuffer, magic4)) {
+ byte[] cryptoParams = CryptoUtils.readParams(input);
Review Comment:
I think improvements to the exception handling would be good to look at with the follow on task "Make CryptoException extend GeneralSecurityException" I mentioned in #2930
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime merged pull request #2935: Modify LogReader to print just the crypto params
Posted by GitBox <gi...@apache.org>.
milleruntime merged PR #2935:
URL: https://github.com/apache/accumulo/pull/2935
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on pull request #2935: Modify LogReader to print just the crypto params
Posted by GitBox <gi...@apache.org>.
milleruntime commented on PR #2935:
URL: https://github.com/apache/accumulo/pull/2935#issuecomment-1255238435
Full ITs passed in my fork.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #2935: Modify LogReader to print just the crypto params
Posted by GitBox <gi...@apache.org>.
dlmarion commented on code in PR #2935:
URL: https://github.com/apache/accumulo/pull/2935#discussion_r975454287
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java:
##########
@@ -172,6 +187,45 @@ public void execute(String[] args) throws Exception {
}
}
+ private void printCryptoParams(FSDataInputStream input, Path path) {
+ byte[] magic4 = DfsLogger.LOG_FILE_HEADER_V4.getBytes(UTF_8);
+ byte[] magic3 = DfsLogger.LOG_FILE_HEADER_V3.getBytes(UTF_8);
+ byte[] noCryptoBytes = new NoFileEncrypter().getDecryptionParameters();
+
+ if (magic4.length != magic3.length)
+ throw new AssertionError("Always expect log file headers to be same length : " + magic4.length
+ + " != " + magic3.length);
+
+ byte[] magicBuffer = new byte[magic4.length];
+ try {
+ input.readFully(magicBuffer);
+ if (Arrays.equals(magicBuffer, magic4)) {
+ byte[] cryptoParams = CryptoUtils.readParams(input);
Review Comment:
I wonder if you should throw a CryptoException here instead of letting this fall through to a RuntimeException. If you caught the CryptoException in the call to this method, then you could continue processing the next file after logging the error.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org