The problem in the bugdoc was the directory entries. These entries
are valid in ZIP packages (even if not common); they may be useful
to e.g. define per-directory permissions (ACLs).
In normal mode, ZipFile reads central directory; there we can read
if the entry has FAT file attributes; and then, if the entry is a
directory. Then it is OK to skip it.
In repair mode, central directory is not used, local file headers
don't contain a "directory" flag. A workaround is used, checking
if there are entries that represent directories of other entries.
Also this change fixes some places that didn't pass the recovery
flag correctly.
Change-Id: I324671841a2c4d0f279b03801d95c8f2eeb99b46
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162888
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
https://www.rfc-editor.org/rfc/rfc9106.html
* add css::xml::crypto::KDFID constant group
* add "KeyDerivationFunction" to setEncryptionAlgorithms sequence
* Argon2 is used by default for wholesome ODF encryption, but
$LO_ARGON2_DISABLE can be set to use PBKDF2
* extend various structs in package
* use 3 new ODF attributes "loext:argon2-iterations" "loext:argon2-memory"
"loext:argon2-lanes" to store the arguments
* use this URL for now:
"urn:org:documentfoundation:names:experimental:office:manifest:argon2id"
* use default arguments according to second recommendation from "7.4.
Recommendations" of RFC9106; 64 MiB RAM should hopefully not be too
much even for 32 bit builds
Change-Id: I683118cc5e0706bd6544db6fb909096768ac9920
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161009
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
... for ODF wholesome encryption.
4.3 <manifest:file-entry>
For directories, the manifest file should contain a <manifest:file-entry> element only if a directory contains a document or a sub document.
Because the "encrypted-package" is not a document but a package, we
should probably omit the file-entry for the root document.
ZipPackage::writeTempFile() always generates the root document becuase
it's needed for GPG properties, and ManifestExport filters it out.
A bit tricky to implement, because there isn't a clean distinction
between the package and the root document/storage in the package module,
in particular there's no other place than the root storage to store the
MediaType property.
Change-Id: Id7e72a64e2faa074dce80cd5fefb2fa189e2e3ee
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160717
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
AEAD provides the verification of the password automatically, by reading
the entire stream the tag at the end will be verified.
The existing attributes manifest:checksum-type/manifest:checksum leak
information about the plain text.
This was mitigated with the addChaffWhenEncryptedStorage() functions
(see commit f57baefbd3) but a better
solution that also works for non-XML streams is to simply omit the
attributes; authenticated encryption provides better verification
without any leak.
* "ChecksumAlgorithm" property can be set to void now to remove the
checksum
* change a bunch of members in EncryptionData, ZipPackage,
ZipPackageStream to optional
* change ZipFile::checkValidPassword() to open the stream and return it
Change-Id: Id95288d0c238c4f9940fc5a185df814e8edcbad3
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160711
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
... and use it in the new experimental ODF encryption mode.
https://www.w3.org/TR/xmlenc-core1/#sec-AES-GCM
Unfortunately it turned out that NSS PK11_CipherOp() does not work with
CKM_AES_GCM because it is initialized with "context->multi = PR_FALSE"
in sftk_CryptInit(), so the one-step functions PK11_Encrypt() and
PK11_Decrypt() have to be used.
NSS 3.52 also changed a parameter struct definition - see
https://fedoraproject.org/wiki/Changes/NssGCMParams - which is not a
problem for RHEL or SUSE system NSS since those are rebased, but it
is likely a problem for less well maintained Ubuntu LTS, so use
the old struct definition which evidently still works with NSS 3.94.
NSS 3.52 also added a new PK11_AEADOp() API but it looks like this
doesn't support incremental encryption either.
Change-Id: Ibd4a672db74b65b1218926ba35ff8d2f70444c7e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160505
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
==2515797== Conditional jump or move depends on uninitialised value(s)
==2515797== at 0x33FAB399: ZipFile::recover() (ZipFile.cxx:1090)
==2515797== by 0x33FA4D32: ZipFile::ZipFile(rtl::Reference<comphelper::RefCountedMutex>, com::sun::star::uno::Reference<com::sun::star::io::XInputStream> const&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext>, bool, bool) (ZipFile.cxx:111)
==2515797== by 0x33FEF134: void std::_Construct<ZipFile, rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool, bool&>(ZipFile*, rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool&&, bool&) (stl_construct.h:119)
==2515797== by 0x33FED528: void std::_Optional_payload_base<ZipFile>::_M_construct<rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool, bool&>(rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool&&, bool&) (optional:278)
==2515797== by 0x33FEB73B: void std::_Optional_base_impl<ZipFile, std::_Optional_base<ZipFile, false, false> >::_M_construct<rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool, bool&>(rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool&&, bool&) (optional:457)
==2515797== by 0x33FE77DB: std::enable_if<is_constructible_v<ZipFile, rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool, bool&>, ZipFile&>::type std::optional<ZipFile>::emplace<rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool, bool&>(rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool&&, bool&) (optional:918)
==2515797== by 0x33FD61FD: ZipPackage::initialize(com::sun::star::uno::Sequence<com::sun::star::uno::Any> const&) (ZipPackage.cxx:760)
==2515797== by 0x64DE1EC: cppuhelper::ServiceManager::Data::Implementation::doCreateInstanceWithArguments(com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, com::sun::star::uno::Sequence<com::sun::star::uno::Any> const&) (servicemanager.cxx:732)
==2515797== by 0x64DDF19: cppuhelper::ServiceManager::Data::Implementation::createInstanceWithArguments(com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool, com::sun::star::uno::Sequence<com::sun::star::uno::Any> const&) (servicemanager.cxx:694)
==2515797== by 0x64E0E8F: cppuhelper::ServiceManager::createInstanceWithArgumentsAndContext(rtl::OUString const&, com::sun::star::uno::Sequence<com::sun::star::uno::Any> const&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&) (servicemanager.cxx:1018)
==2515797== by 0x34A3B25F: OStorage_Impl::OpenOwnPackage() (xstorage.cxx:435)
==2515797== by 0x34A3C16C: OStorage_Impl::ReadContents() (xstorage.cxx:541)
==2515797== Uninitialised value was created by a stack allocation
==2515797== at 0x33FAB02C: ZipFile::recover() (ZipFile.cxx:1034)
since:
commit abda72eeac
Date: Mon Feb 20 00:32:22 2023 +0100
tdf#82984 tdf#94915 zip64 support (import + export)
where
- aEntry.nCompressedSize = nCompressedSize;
- aEntry.nSize = nSize;
was removed before the subsequent use of aEntry.nCompressedSize/aEntry.nSize
change things (git show -w for clarity) to check bounds first just for
the range the extra fields might be in and read those, and then follow
up with the original check that the newly discovered
aEntry.nCompressedSize/aEntry.nSize are within the bounds of the overall
file
Change-Id: Iad4ce8297109b06bc5baf77df4f3e86659cbb4cf
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/150969
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Implemented import + export for "Zip64 Extended Information Extra Field",
(in "Local file header" and "Central directory file header")
and for Data descriptor.
Focused only to be able to handle files with over 4GB uncompressed size,
in the zip archive.
The 64k filecount, and the 4GB compressed size limit is probably still present
Tried to follow pkware .ZIP File Format Specification,
Some cases were not clear to me and/or some zip compressing tool may not
perfectly follow the standard, like 'extra field' should be 28 bytes long,
but its reader now can read shorter (or longer) 'extra field'.
Replaced some 32bit codes with 64bit codes, in stream handling, in deflater.
Tested with an ods file that contained a content.xml that bigger then 4BG+
(import + export + reimport) on windows.
I think 4GB+ files import/export would be too slow fot unittest.
So, for unit test, used the small but zip64 format files,
that was attached to the bugzilla tickets
Note: It helps with Bug 128244 too (1 of the unittest tests it),
but that ods file missing manifest.xml, so LO won't be able to import it.
Change-Id: Idfeb90594388fd34ae719677f5d268ca9a484fb1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147306
Tested-by: Jenkins
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
to use in places where we know we can use the TempFileFast mechanism.
Speeds up export to EPUB from 43s to 17s on my windows machine.
This is essentially a copy of OTempFileService, but it does not
implement XTempFile (because these temp files have no name on Windows).
Also remove a couple of calls to Flush() (which is not necessary now),
and TellEnd(), which is a little slow on Windows.
Change-Id: Iced300bf8121eae08c7a011d62ec68b93330d84c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/141598
Tested-by: Noel Grandin <noel.grandin@collabora.co.uk>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
...so that its TOOLS_WARN_EXCEPTION can be used in
comphelper/source/misc/logging.cxx in a follow-up commit. (And while at it,
rename from diangose_ex.h to the more appropriate diagnose_ex.hxx. The
comphelper module is sufficiently low-level for this immediate use case, so use
that at least for now; o3tl might be even more suitable but doesn't have a
Library until now. Also, for the immediate use case it would have sufficed to
only break DbgGetCaughtException, exceptionToString, TOOLS_WARN_EXCEPTION,
TOOLS_WARN_EXCEPTION_IF, and TOOLS_INFO_EXCEPTION out of
include/tools/diagnose_ex.h into an additional new
include/comphelper/diagnose_ex.hxx, but its probably easier overall to just move
the complete include file as is.)
Change-Id: I9f3222d4ccf1a9ac29d7eb9ba1530d53e2affaee
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/138451
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
All call sites already effectively asked for an unsigned return type, including:
* The ThreadPool ctor took an nWorkers argument of type sal_Int32, but
internally stores that as std::size_t mnMaxWorkers.
* ZipOutputStream::reduceScheduledThreadTasksToGivenNumberOrLess apparently
benefits from an unsigned nThreadTasks parameter, getting rid of various casts
in its implementation that were necessary to silence signed vs. unsigned
comparison warnings.
The only drawback is that
comphelper::ThreadPool::getPreferredConcurrency() * 4
in package/source/zippackage/ZipPackageStream.cxx would now silently wrap around
instead of causing UB on overflow (which could be detected with appropriate
tools). Ideally, it would use some o3tl::saturating_mul if we had that, so add
a TODO comment for now.
While std::thread::hardware_concurrency returns unsigned, it looked more natural
to go with std::size_t here, as some call sites already used that (see above),
so the implementation of ThreadPool::getPreferredConcurrency is a natural place
to hide clamping std::thread::hardware_concurrency() to std::size_t (in the
unlikely case that std::size_t is of smaller rank than unsigned).
This required addition of o3tl::clamp_to_unsigned in o3tl/safeint.hxx.
Change-Id: I0a04a8b32e63ebfeb39f924c4b38520455a6fb38
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/135309
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
...where a signed and an unsigned value are compared, and the signed value has
just been proven to be non-negative here
Change-Id: I9665e6c2c4c5557f2d4cf1bb646f9fffc7bd7d30
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/133442
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
That these are only flagged when building against libc++ and not against
libstdc++ is a consequence of the plugin's dependence on implementation details
of the relevant classes, but so be it.
Change-Id: I6b96f81939edab2b8e618aa53c9fe23492edfbbd
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/131562
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
by just using the tempfile service as intended, we can stay
on the "happy path" which means that, on Windows, if there is
sufficient system RAM, the temporary file never even hits
the disk.
Change-Id: I3b27cf09bd40a4cfee01c23273af05860708a16f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128258
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
... to avoid hidden cost of multiple COW checks, because they
call getArray() internally.
This obsoletes [loplugin:sequenceloop].
Also rename toNonConstRange to asNonConstRange, to reflect that
the result is a view of the sequence, not an independent object.
TODO: also drop non-const operator[], but introduce operator[]
in SequenceRange.
Change-Id: Idd5fd7a3400fe65274d2a6343025e2ef8911635d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123518
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Mike Kaganski