Bug 60230

Summary: Roundtrip test that encrypts XSSFWorkbook and then decrypts it fails with latest code
Product: POI Reporter: PJ Fanning <fanningpj>
Component: XSSFAssignee: POI Developers List <dev>
Status: RESOLVED FIXED    
Severity: regression    
Priority: P2    
Version: 3.14-FINAL   
Target Milestone: ---   
Hardware: PC   
OS: Mac OS X 10.1   
Attachments: test case that hightlights the issue

Description PJ Fanning 2016-10-09 17:52:10 UTC
Created attachment 34349 [details]
test case that hightlights the issue

I have a test case that passes ok in POI 3.15 but that fails in latest 3.16-beta1 codebase.
This is the stacktrace. I will attach a patch file with a test case.

org.apache.poi.EncryptedDocumentException: Input length not multiple of 16 bytes
	at org.apache.poi.poifs.crypt.ChunkedCipherInputStream.read(ChunkedCipherInputStream.java:108)
	at org.apache.poi.poifs.crypt.ChunkedCipherInputStream.read(ChunkedCipherInputStream.java:91)
	at java.io.FilterInputStream.read(FilterInputStream.java:133)
	at java.io.PushbackInputStream.read(PushbackInputStream.java:186)
	at org.apache.poi.util.IOUtils.readFully(IOUtils.java:139)
	at org.apache.poi.util.IOUtils.readFully(IOUtils.java:119)
	at org.apache.poi.openxml4j.opc.internal.ZipHelper.verifyZipHeader(ZipHelper.java:167)
	at org.apache.poi.openxml4j.opc.internal.ZipHelper.openZipStream(ZipHelper.java:229)
	at org.apache.poi.openxml4j.opc.ZipPackage.<init>(ZipPackage.java:97)
	at org.apache.poi.openxml4j.opc.OPCPackage.open(OPCPackage.java:328)
	at org.apache.poi.util.PackageHelper.open(PackageHelper.java:37)
	at org.apache.poi.xssf.usermodel.XSSFWorkbook.<init>(XSSFWorkbook.java:285)
	at org.apache.poi.xssf.TestWorkbookProtection.testAbc(TestWorkbookProtection.java:226)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:497)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)
Caused by: javax.crypto.IllegalBlockSizeException: Input length not multiple of 16 bytes
	at com.sun.crypto.provider.CipherCore.finalNoPadding(CipherCore.java:1016)
	at com.sun.crypto.provider.CipherCore.doFinal(CipherCore.java:960)
	at com.sun.crypto.provider.AESCipher.engineDoFinal(AESCipher.java:479)
	at javax.crypto.Cipher.doFinal(Cipher.java:2297)
	at org.apache.poi.poifs.crypt.ChunkedCipherInputStream.invokeCipher(ChunkedCipherInputStream.java:215)
	at org.apache.poi.poifs.crypt.ChunkedCipherInputStream.nextChunk(ChunkedCipherInputStream.java:202)
	at org.apache.poi.poifs.crypt.ChunkedCipherInputStream.read(ChunkedCipherInputStream.java:105)
	... 35 more
Comment 1 PJ Fanning 2016-10-09 17:56:47 UTC
Possibly related to the changes for https://bz.apache.org/bugzilla/show_bug.cgi?id=59857
I reverted locally to the commit before those changes and the attached test case passed.
Comment 2 Javen O'Neal 2016-10-09 18:49:53 UTC
I can reproduce the regression that you found with attachment 34349 [details].
On the trunk at r1763997, if I revert r1762875 and r1762726 the unit test passes.

Now for the tricky part: which portion of r1762726 is no longer padding the content to 16 bytes?
Comment 3 Javen O'Neal 2016-10-09 19:04:18 UTC
Committed disabled unit test in r1763998.
Comment 4 PJ Fanning 2016-10-09 20:09:52 UTC
I suspect that the issue is with the encryption when outputting the xlsx. Test cases that read pre-saved xlsx files seem unaffected.
Comment 5 Andreas Beeker 2016-10-09 20:35:53 UTC
Patched via r1764008

Left open for further discoveries :)
Comment 6 Andreas Beeker 2016-10-10 14:16:59 UTC
I need to have another look an, as it came to my mind, that depending on the padding there might be more than <chunk-size> bytes in the result, which would be chopped off ... which is not such a good idea for block-based ciphers

The implementation of ChunkedCipherIn/OutputStream is a bit tricky in that regard, as it handles stream-based (XOR/RC4) and block-based (AES/...) encryption modes.
Comment 7 PJ Fanning 2017-06-04 19:31:28 UTC
This xlsx works in poi 3.16