Merge pull request #1075 from grandinj/1003_decrypt

Issue #1003: Decrypting database with incorrect password renders the database corrupt 

h2/src/docsrc/html/changelog.html

@@ -21,6 +21,8 @@ Change Log
 
 <h2>Next Version (unreleased)</h2>
 <ul>
+<li>Issue #1003: Decrypting database with incorrect password renders the database corrupt 
+</li>
 <li>Issue #873: No error when `=` in equal condition when column is not of array type 
 </li>
 <li>Issue #1069: Failed to add DATETIME(3) column since 1.4.197

h2/src/main/org/h2/tools/ChangeFileEncryption.java

@@ -12,9 +12,9 @@ import java.nio.channels.FileChannel;
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.concurrent.TimeUnit;
-
 import org.h2.engine.Constants;
 import org.h2.message.DbException;
+import org.h2.mvstore.MVStore;
 import org.h2.security.SHA256;
 import org.h2.store.FileLister;
 import org.h2.store.FileStore;
@@ -63,8 +63,13 @@ public class ChangeFileEncryption extends Tool {
      *
      * @param args the command line arguments
      */
-    public static void main(String... args) throws SQLException {
-        new ChangeFileEncryption().runTool(args);
+    public static void main(String... args) {
+        try {
+            new ChangeFileEncryption().runTool(args);
+        } catch (SQLException ex) {
+            ex.printStackTrace(System.err);
+            System.exit(1);
+        }
     }
 
     @Override
@@ -109,8 +114,7 @@ public class ChangeFileEncryption extends Tool {
     }
 
     /**
-     * Get the file encryption key for a given password. The password must be
-     * supplied as char arrays and is cleaned in this method.
+     * Get the file encryption key for a given password.
      *
      * @param password the password as a char array
      * @return the encryption key
@@ -119,7 +123,8 @@ public class ChangeFileEncryption extends Tool {
         if (password == null) {
             return null;
         }
-        return SHA256.getKeyPasswordHash("file", password);
+        // the clone is to avoid the unhelpful array cleaning
+        return SHA256.getKeyPasswordHash("file", password.clone());
     }
 
     /**
@@ -187,22 +192,22 @@ public class ChangeFileEncryption extends Tool {
         for (String fileName : files) {
             // don't process a lob directory, just the files in the directory.
             if (!FileUtils.isDirectory(fileName)) {
-                change.process(fileName, quiet);
+                change.process(fileName, quiet, decryptPassword);
             }
         }
     }
 
-    private void process(String fileName, boolean quiet) {
+    private void process(String fileName, boolean quiet, char[] decryptPassword) throws SQLException {
         if (fileName.endsWith(Constants.SUFFIX_MV_FILE)) {
             try {
-                copy(fileName, quiet);
+                copyMvStore(fileName, quiet, decryptPassword);
             } catch (IOException e) {
                 throw DbException.convertIOException(e,
                         "Error encrypting / decrypting file " + fileName);
             }
             return;
         }
-        FileStore in;
+        final FileStore in;
         if (decrypt == null) {
             in = FileStore.open(null, fileName, "r");
         } else {
@@ -210,23 +215,35 @@ public class ChangeFileEncryption extends Tool {
         }
         try {
             in.init();
-            copy(fileName, in, encrypt, quiet);
+            copyPageStore(fileName, in, encrypt, quiet);
         } finally {
             in.closeSilently();
         }
     }
 
-    private void copy(String fileName, boolean quiet) throws IOException {
+    private void copyMvStore(String fileName, boolean quiet, char[] decryptPassword) throws IOException, SQLException {
         if (FileUtils.isDirectory(fileName)) {
             return;
         }
+        // check that we have the right encryption key
+        try {
+            final MVStore source = new MVStore.Builder().
+                    fileName(fileName).
+                    readOnly().
+                    encryptionKey(decryptPassword).
+                    open();
+            source.close();
+        } catch (IllegalStateException ex) {
+            throw new SQLException("error decrypting file " + fileName, ex);
+        }
+
         String temp = directory + "/temp.db";
         try (FileChannel fileIn = getFileChannel(fileName, "r", decryptKey)){
             try(InputStream inStream = new FileChannelInputStream(fileIn, true)) {
                 FileUtils.delete(temp);
                 try (OutputStream outStream = new FileChannelOutputStream(getFileChannel(temp, "rw", encryptKey),
                         true)) {
-                    byte[] buffer = new byte[4 * 1024];
+                    final byte[] buffer = new byte[4 * 1024];
                     long remaining = fileIn.size();
                     long total = remaining;
                     long time = System.nanoTime();
@@ -247,19 +264,21 @@ public class ChangeFileEncryption extends Tool {
         FileUtils.move(temp, fileName);
     }
 
-    private FileChannel getFileChannel(String fileName, String r, byte[] decryptKey) throws IOException {
+    private static FileChannel getFileChannel(String fileName, String r,
+            byte[] decryptKey) throws IOException {
         FileChannel fileIn = FilePath.get(fileName).open(r);
         if (decryptKey != null) {
-            fileIn = new FilePathEncrypt.FileEncrypt(fileName, decryptKey, fileIn);
+            fileIn = new FilePathEncrypt.FileEncrypt(fileName, decryptKey,
+                    fileIn);
         }
         return fileIn;
     }
 
-    private void copy(String fileName, FileStore in, byte[] key, boolean quiet) {
+    private void copyPageStore(String fileName, FileStore in, byte[] key, boolean quiet) {
         if (FileUtils.isDirectory(fileName)) {
             return;
         }
-        String temp = directory + "/temp.db";
+        final String temp = directory + "/temp.db";
         FileUtils.delete(temp);
         FileStore fileOut;
         if (key == null) {
@@ -267,8 +286,8 @@ public class ChangeFileEncryption extends Tool {
         } else {
             fileOut = FileStore.open(null, temp, "rw", cipherType, key);
         }
+        final byte[] buffer = new byte[4 * 1024];
         fileOut.init();
-        byte[] buffer = new byte[4 * 1024];
         long remaining = in.length() - FileStore.HEADER_LENGTH;
         long total = remaining;
         in.seek(FileStore.HEADER_LENGTH);

h2/src/test/org/h2/test/unit/TestSampleApps.java

@@ -13,7 +13,6 @@ import java.io.PrintStream;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.nio.charset.StandardCharsets;
-
 import org.h2.store.fs.FileUtils;
 import org.h2.test.TestBase;
 import org.h2.tools.DeleteDbFiles;
@@ -106,8 +105,6 @@ public class TestSampleApps extends TestBase {
         // tools
         testApp("Allows changing the database file encryption password or algorithm*",
                 org.h2.tools.ChangeFileEncryption.class, "-help");
-        testApp("Allows changing the database file encryption password or algorithm*",
-                org.h2.tools.ChangeFileEncryption.class);
         testApp("Deletes all files belonging to a database.*",
                 org.h2.tools.DeleteDbFiles.class, "-help");
         FileUtils.delete(getBaseDir() + "/optimizations.sql");

h2/src/test/org/h2/test/unit/TestTools.java

@@ -38,7 +38,6 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Random;
 import java.util.UUID;
-
 import org.h2.api.ErrorCode;
 import org.h2.engine.SysProperties;
 import org.h2.store.FileLister;
@@ -1033,18 +1032,18 @@ public class TestTools extends TestBase {
         conn.close();
         String[] args = { "-dir", dir, "-db", "testChangeFileEncryption",
                 "-cipher", "AES", "-decrypt", "abc", "-quiet" };
-        ChangeFileEncryption.main(args);
+        new ChangeFileEncryption().runTool(args);
         args = new String[] { "-dir", dir, "-db", "testChangeFileEncryption",
                 "-cipher", "AES", "-encrypt", "def", "-quiet" };
-        ChangeFileEncryption.main(args);
+        new ChangeFileEncryption().runTool(args);
         conn = getConnection(url, "sa", "def 123");
         stat = conn.createStatement();
         stat.execute("SELECT * FROM TEST");
         new AssertThrows(ErrorCode.CANNOT_CHANGE_SETTING_WHEN_OPEN_1) {
             @Override
             public void test() throws SQLException {
-                ChangeFileEncryption.main(new String[] { "-dir", dir, "-db",
-                        "testChangeFileEncryption", "-cipher", "AES",
+                new ChangeFileEncryption().runTool(new String[] { "-dir", dir,
+                        "-db", "testChangeFileEncryption", "-cipher", "AES",
                         "-decrypt", "def", "-quiet" });
             }
         };
@@ -1055,14 +1054,8 @@ public class TestTools extends TestBase {
     }
 
     private void testChangeFileEncryptionWithWrongPassword() throws SQLException {
-        if (config.mvStore) {
-            // the file system encryption abstraction used by the MVStore
-            // doesn't detect wrong passwords
-            return;
-        }
         org.h2.Driver.load();
         final String dir = getBaseDir();
-        // TODO: this doesn't seem to work in MVSTORE mode yet
         String url = "jdbc:h2:" + dir + "/testChangeFileEncryption;CIPHER=AES";
         DeleteDbFiles.execute(dir, "testChangeFileEncryption", true);
         Connection conn = getConnection(url, "sa", "abc 123");