Author: centic
Date: Sun Feb 5 21:11:03 2023
New Revision: 1907444
URL: http://svn.apache.org/viewvc?rev=1907444&view=rev
Log:
Bug 66436: Fix invalid handling of padded encrypted bytes
The encrypted data is padded to 16 bytes, but these additional bytes
were not read from the stream
Make the reading of the additional bytes "lenient" to not introduce
breaking change if some existing functionality did produce non-aligned
data for some reason.
Added:
poi/trunk/poi/src/test/java/org/apache/poi/poifs/crypt/agile/
poi/trunk/poi/src/test/java/org/apache/poi/poifs/crypt/agile/TestAgileDecryptor.java
Modified:
poi/trunk/poi-ooxml/src/test/java/org/apache/poi/poifs/crypt/tests/TestAgileEncryptionParameters.java
poi/trunk/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
poi/trunk/poi/src/main/java/org/apache/poi/poifs/crypt/ChunkedCipherInputStream.java
Modified:
poi/trunk/poi-ooxml/src/test/java/org/apache/poi/poifs/crypt/tests/TestAgileEncryptionParameters.java
URL:
http://svn.apache.org/viewvc/poi/trunk/poi-ooxml/src/test/java/org/apache/poi/poifs/crypt/tests/TestAgileEncryptionParameters.java?rev=1907444&r1=1907443&r2=1907444&view=diff
==============================================================================
---
poi/trunk/poi-ooxml/src/test/java/org/apache/poi/poifs/crypt/tests/TestAgileEncryptionParameters.java
(original)
+++
poi/trunk/poi-ooxml/src/test/java/org/apache/poi/poifs/crypt/tests/TestAgileEncryptionParameters.java
Sun Feb 5 21:11:03 2023
@@ -18,16 +18,14 @@ package org.apache.poi.poifs.crypt.tests
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assumptions.assumeTrue;
+import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Stream;
-import javax.crypto.Cipher;
-
import org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream;
import org.apache.poi.POIDataSamples;
import org.apache.poi.poifs.crypt.ChainingMode;
@@ -39,67 +37,127 @@ import org.apache.poi.poifs.crypt.Encryp
import org.apache.poi.poifs.crypt.HashAlgorithm;
import org.apache.poi.poifs.filesystem.POIFSFileSystem;
import org.apache.poi.util.IOUtils;
-import org.junit.jupiter.api.BeforeAll;
+import org.apache.poi.xssf.usermodel.XSSFCell;
+import org.apache.poi.xssf.usermodel.XSSFRow;
+import org.apache.poi.xssf.usermodel.XSSFSheet;
+import org.apache.poi.xssf.usermodel.XSSFWorkbook;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
class TestAgileEncryptionParameters {
-
- static byte[] testData;
-
- public static Stream<Arguments> data() {
+ public static Stream<Arguments> data() throws Exception {
CipherAlgorithm[] caList = {CipherAlgorithm.aes128,
CipherAlgorithm.aes192, CipherAlgorithm.aes256, CipherAlgorithm.rc2,
CipherAlgorithm.des, CipherAlgorithm.des3};
HashAlgorithm[] haList = {HashAlgorithm.sha1, HashAlgorithm.sha256,
HashAlgorithm.sha384, HashAlgorithm.sha512, HashAlgorithm.md5};
ChainingMode[] cmList = {ChainingMode.cbc, ChainingMode.cfb};
+ List<byte[]> byteList = initTestData();
+
List<Arguments> data = new ArrayList<>();
for (CipherAlgorithm ca : caList) {
for (HashAlgorithm ha : haList) {
for (ChainingMode cm : cmList) {
- data.add(Arguments.of(ca,ha,cm));
+ // do not iterate all byte-arrays here to keep runtime of
test at bay
+ data.add(Arguments.of(byteList.get(0),
EncryptionMode.agile, ca, ha, cm));
+ data.add(Arguments.of(byteList.get(1),
EncryptionMode.agile, ca, ha, cm));
}
}
}
+ // iterate all byte-array for each encryption-mode
+ // they usually only support certain algorithms, so keep them fixed
+ // also to not have a very long test-runtime
+ for (byte[] bytes : byteList) {
+ data.add(Arguments.of(bytes, EncryptionMode.agile,
CipherAlgorithm.aes192, HashAlgorithm.sha256, ChainingMode.cbc));
+
+ data.add(Arguments.of(bytes, EncryptionMode.standard,
CipherAlgorithm.aes128, HashAlgorithm.sha1, ChainingMode.ecb));
+
+ // CryptoAPI does not support getDataStream()
+ // data.add(Arguments.of(bytes, EncryptionMode.cryptoAPI,
CipherAlgorithm.rc4, HashAlgorithm.sha1, ChainingMode.cfb));
+
+ // xor does not support createEncryptionInfoEntry()
+ // data.add(Arguments.of(bytes, EncryptionMode.xor,
CipherAlgorithm.des3, HashAlgorithm.md5, ChainingMode.cfb));
+
+ data.add(Arguments.of(bytes, EncryptionMode.binaryRC4,
CipherAlgorithm.rc4, HashAlgorithm.sha512, ChainingMode.ecb));
+ }
+
return data.stream();
}
- @BeforeAll
- public static void initTestData() throws Exception {
- InputStream testFile =
POIDataSamples.getDocumentInstance().openResourceAsStream("SampleDoc.docx");
- testData = IOUtils.toByteArray(testFile);
- testFile.close();
+ public static List<byte[]> initTestData() throws Exception {
+ List<byte[]> data = new ArrayList<>();
+
+ // read a sample file for encrypting
+ try (InputStream testFile =
POIDataSamples.getDocumentInstance().openResourceAsStream("SampleDoc.docx")) {
+ data.add(IOUtils.toByteArray(testFile));
+ }
+
+ // create a small sample workbook for encrypting
+ UnsynchronizedByteArrayOutputStream bosOrig = new
UnsynchronizedByteArrayOutputStream();
+ try (XSSFWorkbook workbook = new XSSFWorkbook()) {
+ XSSFSheet sheet = workbook.createSheet();
+ XSSFRow row = sheet.createRow(0);
+ XSSFCell cell = row.createCell(0);
+ cell.setCellValue("Hello Apache POI");
+ workbook.write(bosOrig);
+ }
+ bosOrig.close();
+
+ data.add(IOUtils.toByteArray(new
ByteArrayInputStream(bosOrig.toByteArray())));
+
+ // test with a dummy-block of data that is a multiple of 16
+ byte[] testData = new byte[4000];
+ for (int i = 0; i < testData.length; i++) {
+ testData[i] = (byte)i;
+ }
+ data.add(testData);
+
+ // test with a dummy-block of data that is not a multiple of 16
+ testData = new byte[3292];
+ for (int i = 0; i < testData.length; i++) {
+ testData[i] = (byte)i;
+ }
+ data.add(testData);
+
+ return data;
}
@ParameterizedTest
@MethodSource("data")
- void testAgileEncryptionModes(CipherAlgorithm ca, HashAlgorithm ha,
ChainingMode cm) throws Exception {
- int maxKeyLen = Cipher.getMaxAllowedKeyLength(ca.jceId);
- assumeTrue(maxKeyLen >= ca.defaultKeySize, "Please install JCE
Unlimited Strength Jurisdiction Policy files");
-
- UnsynchronizedByteArrayOutputStream bos = new
UnsynchronizedByteArrayOutputStream();
-
- POIFSFileSystem fsEnc = new POIFSFileSystem();
- EncryptionInfo infoEnc = new EncryptionInfo(EncryptionMode.agile, ca,
ha, -1, -1, cm);
+ void testAgileEncryptionModes(byte[] testData, EncryptionMode mode,
CipherAlgorithm ca, HashAlgorithm ha, ChainingMode cm)
+ throws Exception {
+ EncryptionInfo infoEnc = new EncryptionInfo(mode, ca, ha, -1, -1, cm);
Encryptor enc = infoEnc.getEncryptor();
enc.confirmPassword("foobaa");
- OutputStream os = enc.getDataStream(fsEnc);
- os.write(testData);
- os.close();
- bos.reset();
- fsEnc.writeFilesystem(bos);
- fsEnc.close();
-
- POIFSFileSystem fsDec = new POIFSFileSystem(bos.toInputStream());
- EncryptionInfo infoDec = new EncryptionInfo(fsDec);
- Decryptor dec = infoDec.getDecryptor();
- boolean passed = dec.verifyPassword("foobaa");
- assertTrue(passed);
- InputStream is = dec.getDataStream(fsDec);
- byte[] actualData = IOUtils.toByteArray(is);
- is.close();
- fsDec.close();
- assertArrayEquals(testData, actualData, "Failed roundtrip -
"+ca+"-"+ha+"-"+cm);
+
+ byte[] inputData;
+ try (POIFSFileSystem fsEnc = new POIFSFileSystem()) {
+ try (OutputStream os = enc.getDataStream(fsEnc)) {
+ os.write(testData);
+ }
+
+ try (UnsynchronizedByteArrayOutputStream bos = new
UnsynchronizedByteArrayOutputStream()) {
+ fsEnc.writeFilesystem(bos);
+
+ bos.close();
+ inputData = bos.toByteArray();
+ }
+ }
+
+ byte[] actualData;
+ try (POIFSFileSystem fsDec = new POIFSFileSystem(new
ByteArrayInputStream(inputData))) {
+ EncryptionInfo infoDec = new EncryptionInfo(fsDec);
+ Decryptor dec = infoDec.getDecryptor();
+ boolean passed = dec.verifyPassword("foobaa");
+ assertTrue(passed);
+ InputStream is = dec.getDataStream(fsDec);
+
+ actualData = IOUtils.toByteArray(is);
+ is.close();
+ }
+
+ // input-data and resulting decrypted data should be equal
+ assertArrayEquals(testData, actualData,
+ "Having " + testData.length + " bytes and parameters - " + ca
+ "-" + ha + "-" + cm);
}
}
Modified:
poi/trunk/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
URL:
http://svn.apache.org/viewvc/poi/trunk/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFBugs.java?rev=1907444&r1=1907443&r2=1907444&view=diff
==============================================================================
---
poi/trunk/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
(original)
+++
poi/trunk/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
Sun Feb 5 21:11:03 2023
@@ -30,12 +30,17 @@ import static org.apache.poi.xssf.XSSFTe
import static org.junit.jupiter.api.Assertions.*;
import static org.junit.jupiter.api.Assumptions.assumeFalse;
+import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStream;
import java.nio.charset.StandardCharsets;
+import java.security.GeneralSecurityException;
import java.time.Duration;
import java.time.Instant;
import java.time.LocalDateTime;
@@ -65,6 +70,10 @@ import org.apache.poi.openxml4j.opc.Pack
import org.apache.poi.openxml4j.opc.PackageRelationship;
import org.apache.poi.openxml4j.opc.PackagingURIHelper;
import org.apache.poi.openxml4j.util.ZipSecureFile;
+import org.apache.poi.poifs.crypt.Decryptor;
+import org.apache.poi.poifs.crypt.EncryptionInfo;
+import org.apache.poi.poifs.crypt.EncryptionMode;
+import org.apache.poi.poifs.crypt.Encryptor;
import org.apache.poi.poifs.filesystem.DirectoryNode;
import org.apache.poi.poifs.filesystem.DocumentEntry;
import org.apache.poi.poifs.filesystem.DocumentInputStream;
@@ -90,6 +99,7 @@ import org.apache.poi.ss.formula.functio
import org.apache.poi.ss.formula.ptg.Ptg;
import org.apache.poi.ss.usermodel.*;
import org.apache.poi.ss.util.*;
+import org.apache.poi.util.IOUtils;
import org.apache.poi.util.LocaleUtil;
import org.apache.poi.util.TempFile;
import org.apache.poi.util.XMLHelper;
@@ -3747,4 +3757,104 @@ public final class TestXSSFBugs extends
assertEquals(23, stylesTable.getFonts().size());
}
}
+
+ private static final String secretKey = "foobaa";
+
+ @Test
+ void testBug66436() throws IOException, InvalidFormatException,
GeneralSecurityException {
+ final File temp_excel_poi = TempFile.createTempFile("temp_excel_poi",
".xlsx");
+ final File temp_excel_poi_encrypt =
TempFile.createTempFile("temp_excel_poi_encrypt", ".xlsx");
+ final File temp_excel_poi_decrypt =
TempFile.createTempFile("temp_excel_poi_decrypt", ".xlsx");
+
+ /* create new excel by poi */
+ try (XSSFWorkbook workbook = new XSSFWorkbook();
+ FileOutputStream foss = new FileOutputStream(temp_excel_poi)) {
+ XSSFSheet sheet = workbook.createSheet();
+ XSSFRow row = sheet.createRow(0);
+ XSSFCell cell = row.createCell(0);
+ cell.setCellValue("Hello Apache POI");
+ workbook.write(foss);
+ }
+
+ // read bytes of workbook before
+ UnsynchronizedByteArrayOutputStream bosOrig = new
UnsynchronizedByteArrayOutputStream();
+ try (FileInputStream fis = new FileInputStream(temp_excel_poi)) {
+ IOUtils.copy(fis, bosOrig);
+ }
+
+ // for the encrypted bytes
+ UnsynchronizedByteArrayOutputStream bosEnc = new
UnsynchronizedByteArrayOutputStream();
+
+ /* encrypt excel by poi */
+ try (POIFSFileSystem fs = new POIFSFileSystem()) {
+ EncryptionInfo info = new EncryptionInfo(EncryptionMode.agile);
+ Encryptor enc = info.getEncryptor();
+ enc.confirmPassword(secretKey);
+
+ // Read in an existing OOXML file and write to encrypted output
stream
+ // don't forget to close the output stream otherwise the padding
bytes aren't added
+ try (OPCPackage opc = OPCPackage.open(new
FileInputStream(temp_excel_poi));
+ OutputStream os = enc.getDataStream(fs)) {
+ opc.save(os);
+ }
+
+ fs.writeFilesystem(bosEnc);
+
+ bosEnc.close();
+
+ // Write out the encrypted version
+ try (FileOutputStream fos = new
FileOutputStream(temp_excel_poi_encrypt)) {
+ IOUtils.copy(new ByteArrayInputStream(bosEnc.toByteArray()),
fos);
+ }
+ }
+
+ // for the decrytped bytes
+ UnsynchronizedByteArrayOutputStream bosDec = new
UnsynchronizedByteArrayOutputStream();
+
+ /* decrypt excel by poi */
+ try (POIFSFileSystem fileSystem = new
POIFSFileSystem(temp_excel_poi_encrypt)) {
+ EncryptionInfo info = new EncryptionInfo(fileSystem);
+ Decryptor d = Decryptor.getInstance(info);
+ if (!d.verifyPassword(secretKey)) {
+ throw new RuntimeException("Unable to process: document is
encrypted");
+ }
+
+ // parse dataStream
+ try (InputStream dataStream = d.getDataStream(fileSystem)) {
+ IOUtils.copy(dataStream, bosDec);
+ }
+
+ try (FileOutputStream fos = new
FileOutputStream(temp_excel_poi_decrypt)) {
+ IOUtils.copy(new ByteArrayInputStream(bosDec.toByteArray()),
fos);
+ }
+ }
+
+ // input-data and resulting decrypted data should be equal
+ /* This is a flaky assertion, maybe there is a timestamp in the files
which can differ
+ in the two byte-arrays. Other tests verify this for
encryption/decryption anyway
+ assertArrayEquals(bosOrig.toByteArray(), bosDec.toByteArray(),
+ "Having " + bosOrig.size() + " bytes");*/
+
+ // also make sure the original and the resulting decrypted
+ // file can be read, i.e. is a valid Zip
+ readByCommonsCompress(temp_excel_poi);
+ readByCommonsCompress(temp_excel_poi_decrypt);
+ }
+
+ private static void readByCommonsCompress(File temp_excel_poi) throws
IOException {
+ /* read by commons-compress*/
+ try (ZipFile zipFile = new ZipFile(temp_excel_poi)) {
+ ZipArchiveEntry entry = zipFile.getEntry("xl/workbook.xml");
+ InputStream inputStream = zipFile.getInputStream(entry);
+
+ BufferedReader bufferedReader = new BufferedReader(new
InputStreamReader(inputStream, StandardCharsets.UTF_8));
+ while (true) {
+ String line = bufferedReader.readLine();
+ if (line == null) {
+ break;
+ }
+ //System.out.println(line);
+ }
+ }
+ }
}
\ No newline at end of file
Modified:
poi/trunk/poi/src/main/java/org/apache/poi/poifs/crypt/ChunkedCipherInputStream.java
URL:
http://svn.apache.org/viewvc/poi/trunk/poi/src/main/java/org/apache/poi/poifs/crypt/ChunkedCipherInputStream.java?rev=1907444&r1=1907443&r2=1907444&view=diff
==============================================================================
---
poi/trunk/poi/src/main/java/org/apache/poi/poifs/crypt/ChunkedCipherInputStream.java
(original)
+++
poi/trunk/poi/src/main/java/org/apache/poi/poifs/crypt/ChunkedCipherInputStream.java
Sun Feb 5 21:11:03 2023
@@ -196,6 +196,17 @@ public abstract class ChunkedCipherInput
throw new EOFException("buffer underrun");
}
+ // encrypted data is processed in chunks of 16 bytes,
+ // so try to read some more data if the current data is not a
+ // multiple of 16 bytes
+ if (totalBytes % 16 != 0) {
+ int toRead = 16 - totalBytes % 16;
+ int read = super.read(plain, totalBytes, toRead);
+ if (read > 0) {
+ totalBytes += read;
+ }
+ }
+
System.arraycopy(plain, 0, chunk, 0, totalBytes);
invokeCipher(totalBytes, totalBytes == chunkSize);
Added:
poi/trunk/poi/src/test/java/org/apache/poi/poifs/crypt/agile/TestAgileDecryptor.java
URL:
http://svn.apache.org/viewvc/poi/trunk/poi/src/test/java/org/apache/poi/poifs/crypt/agile/TestAgileDecryptor.java?rev=1907444&view=auto
==============================================================================
---
poi/trunk/poi/src/test/java/org/apache/poi/poifs/crypt/agile/TestAgileDecryptor.java
(added)
+++
poi/trunk/poi/src/test/java/org/apache/poi/poifs/crypt/agile/TestAgileDecryptor.java
Sun Feb 5 21:11:03 2023
@@ -0,0 +1,102 @@
+/* ====================================================================
+ 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.poi.poifs.crypt.agile;
+
+import static org.apache.poi.poifs.crypt.Decryptor.DEFAULT_POIFS_ENTRY;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Stream;
+
+import org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream;
+import org.apache.poi.poifs.crypt.Decryptor;
+import org.apache.poi.poifs.crypt.EncryptionInfo;
+import org.apache.poi.poifs.crypt.EncryptionMode;
+import org.apache.poi.poifs.crypt.Encryptor;
+import org.apache.poi.poifs.filesystem.DocumentInputStream;
+import org.apache.poi.poifs.filesystem.POIFSFileSystem;
+import org.apache.poi.util.HexDump;
+import org.apache.poi.util.IOUtils;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+class TestAgileDecryptor {
+ @SuppressWarnings("PrimitiveArrayArgumentToVarargsMethod")
+ public static Stream<Arguments> data() {
+ List<Arguments> data = new ArrayList<>();
+ data.add(Arguments.of(new byte[15]));
+ data.add(Arguments.of(new byte[16]));
+ data.add(Arguments.of(new byte[17]));
+ data.add(Arguments.of(new byte[3292]));
+ data.add(Arguments.of(new byte[3293]));
+ data.add(Arguments.of(new byte[4000]));
+
+ return data.stream();
+ }
+
+ @ParameterizedTest
+ @MethodSource("data")
+ void testAgileDecryptor(byte[] testData) throws Exception {
+ EncryptionInfo infoEnc = new EncryptionInfo(EncryptionMode.agile);
+ Encryptor enc = infoEnc.getEncryptor();
+ enc.confirmPassword("f");
+
+ byte[] encData;
+ byte[] encDocument;
+ try (POIFSFileSystem fsEnc = new POIFSFileSystem()) {
+ try (OutputStream os = enc.getDataStream(fsEnc)) {
+ os.write(testData);
+ }
+
+ UnsynchronizedByteArrayOutputStream bos = new
UnsynchronizedByteArrayOutputStream();
+ fsEnc.writeFilesystem(bos);
+
+ bos.close();
+ encData = bos.toByteArray();
+
+ DocumentInputStream dis =
fsEnc.getRoot().createDocumentInputStream(DEFAULT_POIFS_ENTRY);
+ /*long _length =*/ dis.readLong();
+ encDocument = IOUtils.toByteArray(dis);
+ }
+
+ byte[] actualData;
+ try (POIFSFileSystem fsDec = new POIFSFileSystem(new
ByteArrayInputStream(encData))) {
+ EncryptionInfo infoDec = new EncryptionInfo(fsDec);
+ Decryptor dec = infoDec.getDecryptor();
+ assertTrue(dec.verifyPassword("f"));
+ InputStream is = dec.getDataStream(fsDec);
+
+ actualData = IOUtils.toByteArray(is);
+ is.close();
+ }
+
+ // input-data and resulting decrypted data should be equal
+ assertArrayEquals(testData, actualData,
+ "Having " + testData.length + " bytes, had expected \n" +
+ HexDump.dump(testData, 0, 0) + " and actual \n" +
+ HexDump.dump(actualData, 0, 0) + " encrypted \n" +
+ HexDump.dump(encDocument, 0, 0) + " full encrypted \n" +
+ HexDump.dump(encData, 0, 0));
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
