You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by bu...@apache.org on 2016/09/20 12:40:18 UTC

[Bug 60153] New: Use ZipEntrySource in SXSSF module

https://bz.apache.org/bugzilla/show_bug.cgi?id=60153

            Bug ID: 60153
           Summary: Use ZipEntrySource in SXSSF module
           Product: POI
           Version: 3.14-FINAL
          Hardware: PC
                OS: Mac OS X 10.1
            Status: NEW
          Severity: normal
          Priority: P2
         Component: SXSSF
          Assignee: dev@poi.apache.org
          Reporter: fanningpj@yahoo.com

The current SXSSF implementation uses temp files and this is problematic for
companies who want all their data at rest to be encrypted.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59841 added support for
developers to use their own implementations of ZipEntrySource when reading inro
XSSF workbook. I have my own subclass of this that encrypts the temp file data.
The ZipEntrySource interface is read only but it would be feasible to have
support for a writer to create ZipEntrySources too (eg something like
org.apache.poi.poifs.crypt.TestSecureTempZip).
org.apache.poi.xssf.streaming.SheetDataWriter exposes the temp file via its
public API. If we were able to deprecate this and just provide Readers and
Writers, then this could also be subclassed to have the data in the temp file
encrypted. We could also allow a pattern where the readers and writers could be
decorated with encryptors/decryptors.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 60153] [PATCH] Use ZipEntrySource in SXSSF module

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60153

--- Comment #11 from PJ Fanning <fa...@yahoo.com> ---
Hi Javen,
I can look at putting together some doc and related sample code.
Could take a few days.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 60153] Use ZipEntrySource in SXSSF module

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60153

--- Comment #2 from PJ Fanning <fa...@yahoo.com> ---
Created attachment 34284
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34284&action=edit
[PATCH] open up SXSSF classes so that they can be subclassed

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 60153] [PATCH] Use ZipEntrySource in SXSSF module

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60153

PJ Fanning <fa...@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Use ZipEntrySource in SXSSF |[PATCH] Use ZipEntrySource
                   |module                      |in SXSSF module

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 60153] [PATCH] Use ZipEntrySource in SXSSF module

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60153

--- Comment #4 from PJ Fanning <fa...@yahoo.com> ---
Created attachment 34286
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34286&action=edit
[PATCH] open up SXSSF classes so that they can be subclassed

updated patch based on Javen's commit and tidied up some issues

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 60153] [PATCH] Use ZipEntrySource in SXSSF module

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60153

--- Comment #7 from PJ Fanning <fa...@yahoo.com> ---
(In reply to Javen O'Neal from comment #6)
> 3. Can you write a unit test that demonstrates that temporary files created
> by SXSSFWorkbook are encrypted?

Thanks Javen. I can look at the 3 topics you highlighted and I can attach a new
patch file over the coming days.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 60153] [PATCH] Use ZipEntrySource in SXSSF module

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60153

Javen O'Neal <on...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34284|0                           |1
        is obsolete|                            |
  Attachment #34284|0                           |1
           is patch|                            |

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 60153] [PATCH] Use ZipEntrySource in SXSSF module

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60153

Javen O'Neal <on...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |NEEDINFO

--- Comment #5 from Javen O'Neal <on...@apache.org> ---
I am assuming a lazy consensus that there are no security issues with this
implementation. Most of the crypto code has been around in TestSecureTempZip
for a while anyway. This patch just makes that code available to developers by
moving it into the main library.

I applied your patch from comment 4 with a one minor change in r1763943.
1. provide protected or public accessor methods rather than elevating
visibility of private variables. This gives us more freedom in the future to
consolidate code between HSSF, XSSF, and SXSSF.

Remaining questions:

1. In AesZipFileZipEntrySource, should the close method do nothing if the
object has already been closed (guard the code with an if (closed)?
2. TestSecureTempZip demonstrates how to read an AES-encrypted XSSFWorkbook and
TestSXSSFWorkbookWithCustomZipEntrySource demonstrates how to write an
SXSSFWorkbook using encrypted temporary files, but we don't have an example for
writing an SXSSFWorkbook where both temporary files and the saved workbook are
AES-encrypted. Would you be willing to provide a code example or unit test for
this?

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 60153] [PATCH] Use ZipEntrySource in SXSSF module

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60153

Javen O'Neal <on...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34346|0                           |1
           is patch|                            |

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 60153] [PATCH] Use ZipEntrySource in SXSSF module

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60153

--- Comment #3 from Javen O'Neal <on...@apache.org> ---
The patch from comment 2 looks good to me, including the relevant unit tests :)
I committed SXSSFWorkbook#flushSheets() to get the ball rolling in r1761668.

Can someone else review this patch to make sure this implementation doesn't
write unencrypted data to disk and is unlikely to be the source of a security
vulnerability?

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 60153] [PATCH] Use ZipEntrySource in SXSSF module

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60153

Javen O'Neal <on...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEEDINFO                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from Javen O'Neal <on...@apache.org> ---
Thanks for the prompt patches! Applied in r1763969.

If you have any other unit tests you would like to add, please reopen this bug.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 60153] Use ZipEntrySource in SXSSF module

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60153

--- Comment #1 from PJ Fanning <fa...@yahoo.com> ---
I have a sample of what I'd like to change in
https://github.com/pjfanning/poi/commit/72f0ada28f17b0079d296fa6dbc98d3b76b058a3

I can add test cases to demonstrate the ability to plug in custom temp file
implementations to override the use of simple temp files.
By subclassing this version of SXSSFWorkbook, developers can replace the temp
file behaviour in write(OutputStream stream) with a custom ZipEntrySource (see
org.apache.poi.poifs.crypt.TestSecureTempZip) before calling injectData.
createSheetDataWriter() can be overriden to return a subclass of
SheetDataWriter that overrides the decorateInputStream snd decorateOutputStream
methods I've added.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 60153] [PATCH] Use ZipEntrySource in SXSSF module

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60153

--- Comment #10 from Javen O'Neal <on...@apache.org> ---
Would you be interested in writing a few sentences to add to the Encryption
documentation https://poi.apache.org/encryption.html

The documentation source lives here:
https://svn.apache.org/viewvc/poi/site/src/documentation/content/xdocs/encryption.xml?view=log

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 60153] [PATCH] Use ZipEntrySource in SXSSF module

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60153

Javen O'Neal <on...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34286|0                           |1
           is patch|                            |
  Attachment #34286|application/mbox            |text/plain
          mime type|                            |

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 60153] [PATCH] Use ZipEntrySource in SXSSF module

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60153

--- Comment #8 from PJ Fanning <fa...@yahoo.com> ---
Created attachment 34346
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34346&action=edit
Extra test cases for encrypted temp data

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 60153] [PATCH] Use ZipEntrySource in SXSSF module

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60153

--- Comment #6 from Javen O'Neal <on...@apache.org> ---
3. Can you write a unit test that demonstrates that temporary files created by
SXSSFWorkbook are encrypted?

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org