From f0d13f961fedfe93f3114778608edca8533cd717 Mon Sep 17 00:00:00 2001 From: Daniel Santos <39691264+Dougniel@users.noreply.github.com> Date: Sun, 11 Dec 2022 02:55:55 +1100 Subject: [PATCH] [COMPRESS-633] Add encryption support for SevenZ (#332) * feat: Encyrption support for Seven7 Implementation of password-based encryption for 7z compressor COMPRESS-633 * feat: Encyrption support for Seven7 without `AES/CBC/PKCS5Padding` As `AES/CBC/PKCS5Padding` is raised as weak of security, a manual implementation to fill cither block size is done COMPRESS-633 * feat: Encyrption support for SevenZ - implementation without storing password in a clear way - several corrections suggeested by reviewers COMPRESS-633 * feat: Encyrption support for SevenZ typo COMPRESS-633 * feat: Encyrption support for SevenZ Avoid incrasing the public API surface with uneccessary method COMPRESS-633 * feat: Encyrption support for SevenZ no IDE specifi config files COMPRESS-633 * Fix spelling * Update super class from master * AES256Options does not need to be public * Fix spelling in Javadoc Co-authored-by: Gary Gregory --- .gitignore | 1 + .../archivers/sevenz/AES256Options.java | 100 +++++++++++ .../archivers/sevenz/AES256SHA256Decoder.java | 165 +++++++++++++++--- .../compress/archivers/sevenz/SevenZFile.java | 18 +- .../archivers/sevenz/SevenZOutputFile.java | 78 +++++++-- .../sevenz/SevenZOutputFileTest.java | 72 +++++++- 6 files changed, 379 insertions(+), 55 deletions(-) create mode 100644 src/main/java/org/apache/commons/compress/archivers/sevenz/AES256Options.java diff --git a/.gitignore b/.gitignore index aae2af58b3d..c57d236b9dd 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ target .classpath .settings .idea +.vscode *.iml *~ /.externalToolBuilders/ diff --git a/src/main/java/org/apache/commons/compress/archivers/sevenz/AES256Options.java b/src/main/java/org/apache/commons/compress/archivers/sevenz/AES256Options.java new file mode 100644 index 00000000000..d6bb17a825f --- /dev/null +++ b/src/main/java/org/apache/commons/compress/archivers/sevenz/AES256Options.java @@ -0,0 +1,100 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ +package org.apache.commons.compress.archivers.sevenz; + +import java.security.GeneralSecurityException; +import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; +import javax.crypto.Cipher; +import javax.crypto.SecretKey; +import javax.crypto.spec.IvParameterSpec; +import javax.crypto.spec.SecretKeySpec; + +/** + * Options for {@link SevenZMethod#AES256SHA256} encoder + * + * @since 1.23 + * @see AES256SHA256Decoder + */ +class AES256Options { + + private final byte[] salt; + private final byte[] iv; + private final int numCyclesPower; + private final Cipher cipher; + + /** + * @param password password used for encryption + */ + public AES256Options(char[] password) { + this(password, new byte[0], randomBytes(16), 19); + } + + /** + * @param password password used for encryption + * @param salt for password hash salting (enforce password security) + * @param iv Initialization Vector (IV) used by cipher algorithm + * @param numCyclesPower another password security enforcer parameter that controls the cycles of password hashing. More the + * this number is high, more security you'll have but also high CPU usage + */ + public AES256Options(char[] password, byte[] salt, byte[] iv, int numCyclesPower) { + this.salt = salt; + this.iv = iv; + this.numCyclesPower = numCyclesPower; + + // NOTE: for security purposes, password is wrapped in a Cipher as soon as possible to not stay in memory + final byte[] aesKeyBytes = AES256SHA256Decoder.sha256Password(password, numCyclesPower, salt); + final SecretKey aesKey = new SecretKeySpec(aesKeyBytes, "AES"); + + try { + cipher = Cipher.getInstance("AES/CBC/NoPadding"); + cipher.init(Cipher.ENCRYPT_MODE, aesKey, new IvParameterSpec(iv)); + } catch (final GeneralSecurityException generalSecurityException) { + throw new IllegalStateException( + "Encryption error (do you have the JCE Unlimited Strength Jurisdiction Policy Files installed?)", + generalSecurityException + ); + } + } + + byte[] getIv() { + return iv; + } + + int getNumCyclesPower() { + return numCyclesPower; + } + + byte[] getSalt() { + return salt; + } + + Cipher getCipher() { + return cipher; + } + + private static byte[] randomBytes(int size) { + byte[] bytes = new byte[size]; + try { + SecureRandom.getInstanceStrong().nextBytes(bytes); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("No strong secure random available to generate strong AES key", e); + } + return bytes; + } +} diff --git a/src/main/java/org/apache/commons/compress/archivers/sevenz/AES256SHA256Decoder.java b/src/main/java/org/apache/commons/compress/archivers/sevenz/AES256SHA256Decoder.java index 8fb5a778715..19d43443ff1 100644 --- a/src/main/java/org/apache/commons/compress/archivers/sevenz/AES256SHA256Decoder.java +++ b/src/main/java/org/apache/commons/compress/archivers/sevenz/AES256SHA256Decoder.java @@ -17,14 +17,21 @@ */ package org.apache.commons.compress.archivers.sevenz; +import static java.nio.charset.StandardCharsets.UTF_16LE; + import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; +import java.nio.ByteBuffer; +import java.nio.CharBuffer; import java.security.GeneralSecurityException; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; +import java.util.Arrays; import javax.crypto.Cipher; import javax.crypto.CipherInputStream; +import javax.crypto.CipherOutputStream; import javax.crypto.SecretKey; import javax.crypto.spec.IvParameterSpec; import javax.crypto.spec.SecretKeySpec; @@ -32,6 +39,10 @@ import org.apache.commons.compress.PasswordRequiredException; class AES256SHA256Decoder extends AbstractCoder { + + AES256SHA256Decoder() { + super(AES256Options.class); + } @Override InputStream decode(final String archiveName, final InputStream in, final long uncompressedLength, @@ -73,26 +84,7 @@ private CipherInputStream init() throws IOException { System.arraycopy(passwordBytes, 0, aesKeyBytes, saltSize, Math.min(passwordBytes.length, aesKeyBytes.length - saltSize)); } else { - final MessageDigest digest; - try { - digest = MessageDigest.getInstance("SHA-256"); - } catch (final NoSuchAlgorithmException noSuchAlgorithmException) { - throw new IOException("SHA-256 is unsupported by your Java implementation", - noSuchAlgorithmException); - } - final byte[] extra = new byte[8]; - for (long j = 0; j < (1L << numCyclesPower); j++) { - digest.update(salt); - digest.update(passwordBytes); - digest.update(extra); - for (int k = 0; k < extra.length; k++) { - ++extra[k]; - if (extra[k] != 0) { - break; - } - } - } - aesKeyBytes = digest.digest(); + aesKeyBytes = sha256Password(passwordBytes, numCyclesPower, salt); } final SecretKey aesKey = new SecretKeySpec(aesKeyBytes, "AES"); @@ -103,8 +95,8 @@ private CipherInputStream init() throws IOException { isInitialized = true; return cipherInputStream; } catch (final GeneralSecurityException generalSecurityException) { - throw new IOException("Decryption error " + - "(do you have the JCE Unlimited Strength Jurisdiction Policy Files installed?)", + throw new IllegalStateException( + "Decryption error (do you have the JCE Unlimited Strength Jurisdiction Policy Files installed?)", generalSecurityException); } } @@ -127,4 +119,133 @@ public void close() throws IOException { } }; } + + @Override + OutputStream encode(OutputStream out, Object options) throws IOException { + final AES256Options opts = (AES256Options) options; + + return new OutputStream() { + private final CipherOutputStream cipherOutputStream = new CipherOutputStream(out, opts.getCipher()); + + // Ensures that data are encrypt in respect of cipher block size and pad with '0' if smaller + // NOTE: As "AES/CBC/PKCS5Padding" is weak and should not be used, we use "AES/CBC/NoPadding" with this + // manual implementation for padding possible thanks to the size of the file stored separately + private final int cipherBlockSize = opts.getCipher().getBlockSize(); + private final byte[] cipherBlockBuffer = new byte[cipherBlockSize]; + private int count = 0; + + @Override + public void write(int b) throws IOException { + cipherBlockBuffer[count++] = (byte) b; + if (count == cipherBlockSize) { + flushBuffer(); + } + } + + @Override + public void write(byte[] b, int off, int len) throws IOException { + int gap = len + count > cipherBlockSize ? cipherBlockSize - count : len; + System.arraycopy(b, off, cipherBlockBuffer, count, gap); + count += gap; + + if (count == cipherBlockSize) { + flushBuffer(); + + if (len - gap >= cipherBlockSize) { + // skip buffer to encrypt data chunks big enought to fit cipher block size + int multipleCipherBlockSizeLen = (len - gap) / cipherBlockSize * cipherBlockSize; + cipherOutputStream.write(b, off + gap, multipleCipherBlockSizeLen); + gap += multipleCipherBlockSizeLen; + } + System.arraycopy(b, off + gap, cipherBlockBuffer, 0, len - gap); + count = len - gap; + } + } + + private void flushBuffer() throws IOException { + cipherOutputStream.write(cipherBlockBuffer); + count = 0; + Arrays.fill(cipherBlockBuffer, (byte) 0); + } + + @Override + public void flush() throws IOException { + cipherOutputStream.flush(); + } + + @Override + public void close() throws IOException { + if (count > 0) { + cipherOutputStream.write(cipherBlockBuffer); + } + cipherOutputStream.close(); + } + }; + } + + @Override + byte[] getOptionsAsProperties(Object options) throws IOException { + final AES256Options opts = (AES256Options) options; + final byte[] props = new byte[2 + opts.getSalt().length + opts.getIv().length]; + + // First byte : control (numCyclesPower + flags of salt or iv presence) + props[0] = (byte) (opts.getNumCyclesPower() | (opts.getSalt().length == 0 ? 0 : (1 << 7)) | (opts.getIv().length == 0 ? 0 : (1 << 6))); + + if (opts.getSalt().length != 0 || opts.getIv().length != 0) { + // second byte : size of salt/iv data + props[1] = (byte) (((opts.getSalt().length == 0 ? 0 : opts.getSalt().length - 1) << 4) | (opts.getIv().length == 0 ? 0 : opts.getIv().length - 1)); + + // remain bytes : salt/iv data + System.arraycopy(opts.getSalt(), 0, props, 2, opts.getSalt().length); + System.arraycopy(opts.getIv(), 0, props, 2 + opts.getSalt().length, opts.getIv().length); + } + + return props; + } + + static byte[] sha256Password(final char[] password, final int numCyclesPower, final byte[] salt) { + return sha256Password(utf16Decode(password), numCyclesPower, salt); + } + + static byte[] sha256Password(final byte[] password, final int numCyclesPower, final byte[] salt) { + final MessageDigest digest; + try { + digest = MessageDigest.getInstance("SHA-256"); + } catch (final NoSuchAlgorithmException noSuchAlgorithmException) { + throw new IllegalStateException("SHA-256 is unsupported by your Java implementation", noSuchAlgorithmException); + } + final byte[] extra = new byte[8]; + for (long j = 0; j < (1L << numCyclesPower); j++) { + digest.update(salt); + digest.update(password); + digest.update(extra); + for (int k = 0; k < extra.length; k++) { + ++extra[k]; + if (extra[k] != 0) { + break; + } + } + } + return digest.digest(); + } + + /** + * Convenience method that encodes Unicode characters into bytes in UTF-16 (ittle-endian byte order) charset + * + * @param chars characters to encode + * @return encoded characters + * @since 1.23 + */ + static byte[] utf16Decode(final char[] chars) { + if (chars == null) { + return null; + } + final ByteBuffer encoded = UTF_16LE.encode(CharBuffer.wrap(chars)); + if (encoded.hasArray()) { + return encoded.array(); + } + final byte[] e = new byte[encoded.remaining()]; + encoded.get(e); + return e; + } } diff --git a/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java b/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java index 002a5da0162..a7d938d55b6 100644 --- a/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java +++ b/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java @@ -30,7 +30,6 @@ import java.io.InputStream; import java.nio.ByteBuffer; import java.nio.ByteOrder; -import java.nio.CharBuffer; import java.nio.channels.Channels; import java.nio.channels.SeekableByteChannel; import java.nio.file.Files; @@ -137,7 +136,7 @@ public SevenZFile(final File fileName, final char[] password) throws IOException */ public SevenZFile(final File fileName, final char[] password, final SevenZFileOptions options) throws IOException { this(Files.newByteChannel(fileName.toPath(), EnumSet.of(StandardOpenOption.READ)), // NOSONAR - fileName.getAbsolutePath(), utf16Decode(password), true, options); + fileName.getAbsolutePath(), AES256SHA256Decoder.utf16Decode(password), true, options); } /** @@ -256,7 +255,7 @@ public SevenZFile(final SeekableByteChannel channel, final String fileName, */ public SevenZFile(final SeekableByteChannel channel, final String fileName, final char[] password, final SevenZFileOptions options) throws IOException { - this(channel, fileName, utf16Decode(password), false, options); + this(channel, fileName, AES256SHA256Decoder.utf16Decode(password), false, options); } /** @@ -2056,19 +2055,6 @@ public String getDefaultName() { return lastSegment + "~"; } - private static byte[] utf16Decode(final char[] chars) { - if (chars == null) { - return null; - } - final ByteBuffer encoded = UTF_16LE.encode(CharBuffer.wrap(chars)); - if (encoded.hasArray()) { - return encoded.array(); - } - final byte[] e = new byte[encoded.remaining()]; - encoded.get(e); - return e; - } - private static int assertFitsIntoNonNegativeInt(final String what, final long value) throws IOException { if (value > Integer.MAX_VALUE || value < 0) { throw new IOException("Cannot handle " + what + " " + value); diff --git a/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFile.java b/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFile.java index 238dfb5c759..14080640d77 100644 --- a/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFile.java +++ b/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFile.java @@ -47,8 +47,10 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; import java.util.zip.CRC32; - import org.apache.commons.compress.archivers.ArchiveEntry; import org.apache.commons.compress.utils.CountingOutputStream; import org.apache.commons.compress.utils.TimeUtils; @@ -70,6 +72,7 @@ public class SevenZOutputFile implements Closeable { private Iterable contentMethods = Collections.singletonList(new SevenZMethodConfiguration(SevenZMethod.LZMA2)); private final Map additionalSizes = new HashMap<>(); + private AES256Options aes256Options; /** * Opens file to write a 7z archive to. @@ -78,9 +81,25 @@ public class SevenZOutputFile implements Closeable { * @throws IOException if opening the file fails */ public SevenZOutputFile(final File fileName) throws IOException { - this(Files.newByteChannel(fileName.toPath(), - EnumSet.of(StandardOpenOption.CREATE, StandardOpenOption.WRITE, - StandardOpenOption.TRUNCATE_EXISTING))); + this(fileName, null); + } + + /** + * Opens file to write a 7z archive to. + * + * @param fileName the file to write to + * @param password optional password if the archive has to be encrypted + * @throws IOException if opening the file fails + * @since 1.23 + */ + public SevenZOutputFile(final File fileName, char[] password) throws IOException { + this( + Files.newByteChannel( + fileName.toPath(), + EnumSet.of(StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING) + ), + password + ); } /** @@ -95,8 +114,27 @@ public SevenZOutputFile(final File fileName) throws IOException { * @since 1.13 */ public SevenZOutputFile(final SeekableByteChannel channel) throws IOException { + this(channel, null); + } + + /** + * Prepares channel to write a 7z archive to. + * + *

{@link + * org.apache.commons.compress.utils.SeekableInMemoryByteChannel} + * allows you to write to an in-memory archive.

+ * + * @param channel the channel to write to + * @param password optional password if the archive has to be encrypted + * @throws IOException if the channel cannot be positioned properly + * @since 1.23 + */ + public SevenZOutputFile(final SeekableByteChannel channel, char[] password) throws IOException { this.channel = channel; channel.position(SevenZFile.SIGNATURE_HEADER_SIZE); + if (password != null) { + this.aes256Options = new AES256Options(password); + } } /** @@ -413,7 +451,19 @@ public void write(final byte[] b, final int off, final int len) private Iterable getContentMethods(final SevenZArchiveEntry entry) { final Iterable ms = entry.getContentMethods(); - return ms == null ? contentMethods : ms; + Iterable iter = ms == null ? contentMethods : ms; + + if (aes256Options != null) { + // prepend encryption + iter = + Stream + .concat( + Stream.of(new SevenZMethodConfiguration(SevenZMethod.AES256SHA256, aes256Options)), + StreamSupport.stream(iter.spliterator(), false) + ) + .collect(Collectors.toList()); + } + return iter; } private void writeHeader(final DataOutput header) throws IOException { @@ -532,15 +582,15 @@ private void writeSingleCodec(final SevenZMethodConfiguration m, final OutputStr private void writeSubStreamsInfo(final DataOutput header) throws IOException { header.write(NID.kSubStreamsInfo); -// -// header.write(NID.kCRC); -// header.write(1); -// for (final SevenZArchiveEntry entry : files) { -// if (entry.getHasCrc()) { -// header.writeInt(Integer.reverseBytes(entry.getCrc())); -// } -// } -// + // + // header.write(NID.kCRC); + // header.write(1); + // for (final SevenZArchiveEntry entry : files) { + // if (entry.getHasCrc()) { + // header.writeInt(Integer.reverseBytes(entry.getCrc())); + // } + // } + // header.write(NID.kEnd); } diff --git a/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFileTest.java b/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFileTest.java index 908951fdd7a..53b165435a9 100644 --- a/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFileTest.java +++ b/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFileTest.java @@ -20,8 +20,11 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import org.apache.commons.compress.utils.TimeUtils; import org.junit.jupiter.api.Test; @@ -41,6 +44,7 @@ import java.util.Iterator; import org.apache.commons.compress.AbstractTestCase; +import org.apache.commons.compress.PasswordRequiredException; import org.apache.commons.compress.utils.ByteUtils; import org.apache.commons.compress.utils.SeekableInMemoryByteChannel; import org.tukaani.xz.LZMA2Options; @@ -488,6 +492,42 @@ public void testArchiveWithMixedMethods() throws Exception { } } + /** + * Test password-based encryption + * + *

+ * As AES/CBC Cipher requires a minimum of 16 bytes file data to be encrypted, some padding logic has been implemented. + * This test checks different file sizes (1, 16..) to ensure code coverage + *

+ */ + @Test + public void testEncrypt() throws Exception { + output = new File(dir, "encrypted.7z"); + try (SevenZOutputFile outArchive = new SevenZOutputFile(output, "foo".toCharArray())) { + addFile(outArchive, 0, 1, null); + addFile(outArchive, 1, 16, null); + addFile(outArchive, 2, 32, null); + addFile(outArchive, 3, 33, null); + addFile(outArchive, 4, 10000, null); + } + + // Is archive really password-based encrypted ? + try (SevenZFile archive = new SevenZFile(output)) { + assertThrows( + "A password should be needed", + PasswordRequiredException.class, + () -> verifyFile(archive, 0)); + } + + try (SevenZFile archive = new SevenZFile(output, "foo".toCharArray())) { + assertEquals(Boolean.TRUE, verifyFile(archive, 0, 1, null)); + assertEquals(Boolean.TRUE, verifyFile(archive, 1, 16, null)); + assertEquals(Boolean.TRUE, verifyFile(archive, 2, 32, null)); + assertEquals(Boolean.TRUE, verifyFile(archive, 3, 33, null)); + assertEquals(Boolean.TRUE, verifyFile(archive, 4, 10000, null)); + } + } + private void testCompress252(final int numberOfFiles, final int numberOfNonEmptyFiles) throws Exception { final int nonEmptyModulus = numberOfNonEmptyFiles != 0 @@ -542,21 +582,39 @@ private void addFile(final SevenZOutputFile archive, final int index, final bool } private void addFile(final SevenZOutputFile archive, final int index, final boolean nonEmpty, final Iterable methods) + throws Exception { + addFile(archive, index, nonEmpty ? 1 : 0, methods); + } + + private void addFile(final SevenZOutputFile archive, final int index, final int size, final Iterable methods) throws Exception { final SevenZArchiveEntry entry = new SevenZArchiveEntry(); entry.setName("foo/" + index + ".txt"); entry.setContentMethods(methods); archive.putArchiveEntry(entry); - archive.write(nonEmpty ? new byte[] { 'A' } : new byte[0]); + archive.write(generateFileData(size)); archive.closeArchiveEntry(); } + private byte[] generateFileData(int size) { + byte[] data = new byte[size]; + for (int i = 0; i < size; i++) { + data[i] = (byte) ('A' + (i % 26)); + } + return data; + } + private Boolean verifyFile(final SevenZFile archive, final int index) throws Exception { return verifyFile(archive, index, null); } private Boolean verifyFile(final SevenZFile archive, final int index, final Iterable methods) throws Exception { + return verifyFile(archive, index, 1, methods); + } + + private Boolean verifyFile(final SevenZFile archive, final int index, final int size, + final Iterable methods) throws Exception { final SevenZArchiveEntry entry = archive.getNextEntry(); if (entry == null) { return null; @@ -566,8 +624,16 @@ private Boolean verifyFile(final SevenZFile archive, final int index, if (entry.getSize() == 0) { return Boolean.FALSE; } - assertEquals(1, entry.getSize()); - assertEquals('A', archive.read()); + assertEquals(size, entry.getSize()); + + byte[] actual = new byte[size]; + int count = 0; + while (count < size) { + int read = archive.read(actual, count, actual.length - count); + assertNotEquals(-1, read, "EOF reached before reading all expected data"); + count += read; + } + assertArrayEquals(generateFileData(size), actual); assertEquals(-1, archive.read()); if (methods != null) { assertContentMethodsEquals(methods, entry.getContentMethods());