package/inc/ZipPackage.hxx | 13 +------
package/inc/ZipPackageStream.hxx | 25 +++++++++-----
package/source/zippackage/ZipPackage.cxx | 36 ++++++++++++++++----
package/source/zippackage/ZipPackageStream.cxx | 43 ++++++++++++-------------
4 files changed, 70 insertions(+), 47 deletions(-)
New commits:
commit c6d1f16c2d4ea405c18c96ed5ce71f901f9f7fa5
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Tue Sep 17 11:47:05 2024 +0200
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Wed Sep 18 15:06:53 2024 +0200
package: tweak password fallback in ZipPackageStream::getDataStream()
Fallbacks are only useful if an existing file is imported.
It looks like it might have been possible to hit this case by storing a
new document as ODF 1.1, and that sets m_bIsEncrypted but not
m_oImportedAlgorithms (as Mike Kaganski pointed out), haven't tried it.
Change-Id: Ia82c2e43372f50aa4e7ad9c2c62878986295c815
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/173551
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
(cherry picked from commit e22f1a2993d1c675ed8017090d1557d74d8b2917)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/173504
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
diff --git a/package/source/zippackage/ZipPackageStream.cxx
b/package/source/zippackage/ZipPackageStream.cxx
index 4576053bb636..a7b7160586f8 100644
--- a/package/source/zippackage/ZipPackageStream.cxx
+++ b/package/source/zippackage/ZipPackageStream.cxx
@@ -1030,7 +1030,8 @@ uno::Reference< io::XInputStream > SAL_CALL
ZipPackageStream::getDataStream()
// note: due to SHA1 check this fallback is only done for
// * ODF 1.2 files written by OOo < 3.4beta / LO < 3.5
// * ODF 1.1/OOoXML files written by any version
- if ( m_rZipPackage.GetStartKeyGenID() ==
xml::crypto::DigestID::SHA1 )
+ if (m_oImportedAlgorithms
+ && m_oImportedAlgorithms->nImportedStartKeyAlgorithm ==
xml::crypto::DigestID::SHA1)
{
SAL_WARN("package", "ZipPackageStream::getDataStream(): SHA1
mismatch, trying fallbacks...");
try
commit a4900a512ca66932423af32af250deb8b5f4f258
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Fri Sep 13 16:26:29 2024 +0200
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Wed Sep 18 15:06:46 2024 +0200
tdf#162841 package: fix loading AES-GCM encrypted macros from ODF
The problem is that ZipPackageStream::GetEncryptionData() doesn't handle
the checksum correctly; what is required here is *no checksum* but the
check of m_oImportedChecksumAlgorithm results in calling
m_rZipPackage.GetChecksumAlgID() instead, so it ends up in invalid
situation and assert:
package/source/zippackage/ZipPackageStream.cxx:656: virtual bool
ZipPackageStream::saveChild(): Assertion `xEncData->m_nEncAlg !=
xml::crypto::CipherID::AES_GCM_W3C' failed.
Refactor this so all the imported algorithm identifiers are in a struct
in a std::optional member.
(regression from commit 09f23a3dc5cd571df347cba9b003195de35f3ddd)
Change-Id: I4b705520cd9bc800ce3c8611f8ad01a1e1008929
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/173342
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
Tested-by: Jenkins
(cherry picked from commit 6e76e8a210e51e7c79e0e845c7a4c0db9fb55abc)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/173396
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
diff --git a/package/inc/ZipPackage.hxx b/package/inc/ZipPackage.hxx
index 9848396dcf11..293e0849a39e 100644
--- a/package/inc/ZipPackage.hxx
+++ b/package/inc/ZipPackage.hxx
@@ -58,6 +58,8 @@ enum InitialisationMode
e_IMode_XStream
};
+sal_Int32 GetDefaultDerivedKeySize(sal_Int32 nCipherID);
+
class ZipPackage final : public cppu::WeakImplHelper
<
css::lang::XInitialization,
@@ -128,16 +130,7 @@ public:
sal_Int32 GetEncAlgID() const { return m_nCommonEncryptionID; }
::std::optional<sal_Int32> GetChecksumAlgID() const { return
m_oChecksumDigestID; }
sal_Int32 GetDefaultDerivedKeySize() const {
- switch (m_nCommonEncryptionID)
- {
- case css::xml::crypto::CipherID::BLOWFISH_CFB_8:
- return 16;
- case css::xml::crypto::CipherID::AES_CBC_W3C_PADDING:
- case css::xml::crypto::CipherID::AES_GCM_W3C:
- return 32;
- default:
- O3TL_UNREACHABLE;
- }
+ return ::GetDefaultDerivedKeySize(m_nCommonEncryptionID);
}
rtl::Reference<comphelper::RefCountedMutex>& GetSharedMutexRef() { return
m_aMutexHolder; }
diff --git a/package/inc/ZipPackageStream.hxx b/package/inc/ZipPackageStream.hxx
index 0cb52e88c892..ca56fa1817d1 100644
--- a/package/inc/ZipPackageStream.hxx
+++ b/package/inc/ZipPackageStream.hxx
@@ -36,6 +36,18 @@
#define PACKAGE_STREAM_DATA 3
#define PACKAGE_STREAM_RAW 4
+struct ImportedAlgorithms
+{
+ sal_Int32 nImportedStartKeyAlgorithm;
+ sal_Int32 nImportedEncryptionAlgorithm;
+ // optional because it is not used with AEAD
+ ::std::optional<sal_Int32> oImportedChecksumAlgorithm;
+ // GPG encrypted ODF does not have this in the file, but don't use optional
+ // here because it depends on the nImportedEncryptionAlgorithm of the same
+ // entry, so theoretically it could be different for different entries.
+ sal_Int32 nImportedDerivedKeySize;
+};
+
class ZipPackage;
struct ZipEntry;
class ZipPackageStream final : public cppu::ImplInheritanceHelper
@@ -54,10 +66,7 @@ private:
css::uno::Sequence< css::beans::NamedValue > m_aStorageEncryptionKeys;
css::uno::Sequence< sal_Int8 > m_aEncryptionKey;
- sal_Int32 m_nImportedStartKeyAlgorithm;
- sal_Int32 m_nImportedEncryptionAlgorithm;
- ::std::optional<sal_Int32> m_oImportedChecksumAlgorithm;
- sal_Int32 m_nImportedDerivedKeySize;
+ ::std::optional<ImportedAlgorithms> m_oImportedAlgorithms;
sal_uInt8 m_nStreamMode;
sal_uInt32 m_nMagicalHackPos;
@@ -94,10 +103,10 @@ public:
void SetToBeCompressed (bool bNewValue) { m_bToBeCompressed = bNewValue;}
void SetIsEncrypted (bool bNewValue) { m_bIsEncrypted = bNewValue;}
- void SetImportedStartKeyAlgorithm( sal_Int32 nAlgorithm ) {
m_nImportedStartKeyAlgorithm = nAlgorithm; }
- void SetImportedEncryptionAlgorithm( sal_Int32 nAlgorithm ) {
m_nImportedEncryptionAlgorithm = nAlgorithm; }
- void SetImportedChecksumAlgorithm(::std::optional<sal_Int32> const&
roAlgorithm) { m_oImportedChecksumAlgorithm = roAlgorithm; }
- void SetImportedDerivedKeySize( sal_Int32 nSize ) {
m_nImportedDerivedKeySize = nSize; }
+ void SetImportedAlgorithms(ImportedAlgorithms const algorithms)
+ {
+ m_oImportedAlgorithms.emplace(algorithms);
+ }
void SetToBeEncrypted (bool bNewValue)
{
m_bToBeEncrypted = bNewValue;
diff --git a/package/source/zippackage/ZipPackage.cxx
b/package/source/zippackage/ZipPackage.cxx
index 5a29ae7f09cd..89891784ff8e 100644
--- a/package/source/zippackage/ZipPackage.cxx
+++ b/package/source/zippackage/ZipPackage.cxx
@@ -135,6 +135,20 @@ class DummyInputStream : public ::cppu::WeakImplHelper<
XInputStream >
{}
};
+} // namespace
+
+sal_Int32 GetDefaultDerivedKeySize(sal_Int32 const nCipherID)
+{
+ switch (nCipherID)
+ {
+ case css::xml::crypto::CipherID::BLOWFISH_CFB_8:
+ return 16;
+ case css::xml::crypto::CipherID::AES_CBC_W3C_PADDING:
+ case css::xml::crypto::CipherID::AES_GCM_W3C:
+ return 32;
+ default:
+ O3TL_UNREACHABLE;
+ }
}
ZipPackage::ZipPackage ( uno::Reference < XComponentContext > xContext )
@@ -320,11 +334,9 @@ void ZipPackage::parseManifest()
assert(pDigestAlg->has<sal_Int32>());
oDigestAlg.emplace(pDigestAlg->get<sal_Int32>());
-
pStream->SetImportedChecksumAlgorithm(oDigestAlg);
}
*pEncryptionAlg >>= nEncryptionAlg;
- pStream->SetImportedEncryptionAlgorithm(
nEncryptionAlg );
*pKeyInfo >>= m_aGpgProps;
@@ -338,7 +350,14 @@ void ZipPackage::parseManifest()
// c.f.
ZipPackageStream::GetEncryptionKey()
// trying to get key value from properties
const sal_Int32 nStartKeyAlg =
xml::crypto::DigestID::SHA256;
- pStream->SetImportedStartKeyAlgorithm(
nStartKeyAlg );
+
+ pStream->SetImportedAlgorithms({
+ .nImportedStartKeyAlgorithm =
nStartKeyAlg,
+ .nImportedEncryptionAlgorithm =
nEncryptionAlg,
+ .oImportedChecksumAlgorithm =
oDigestAlg,
+ // note m_nCommonEncryptionID is not
inited yet here
+ .nImportedDerivedKeySize =
::GetDefaultDerivedKeySize(nEncryptionAlg),
+ });
if (!m_bHasEncryptedEntries
&& (pStream->getName() == "content.xml"
@@ -405,19 +424,22 @@ void ZipPackage::parseManifest()
assert(pDigestAlg->has<sal_Int32>());
oDigestAlg.emplace(pDigestAlg->get<sal_Int32>());
-
pStream->SetImportedChecksumAlgorithm(oDigestAlg);
}
*pEncryptionAlg >>= nEncryptionAlg;
- pStream->SetImportedEncryptionAlgorithm(
nEncryptionAlg );
if ( pDerivedKeySize )
*pDerivedKeySize >>= nDerivedKeySize;
- pStream->SetImportedDerivedKeySize(
nDerivedKeySize );
if ( pStartKeyAlg )
*pStartKeyAlg >>= nStartKeyAlg;
- pStream->SetImportedStartKeyAlgorithm(
nStartKeyAlg );
+
+ pStream->SetImportedAlgorithms({
+ .nImportedStartKeyAlgorithm =
nStartKeyAlg,
+ .nImportedEncryptionAlgorithm =
nEncryptionAlg,
+ .oImportedChecksumAlgorithm =
oDigestAlg,
+ .nImportedDerivedKeySize =
nDerivedKeySize,
+ });
pStream->SetToBeCompressed ( true );
pStream->SetToBeEncrypted ( true );
diff --git a/package/source/zippackage/ZipPackageStream.cxx
b/package/source/zippackage/ZipPackageStream.cxx
index 48aec1bba5e1..4576053bb636 100644
--- a/package/source/zippackage/ZipPackageStream.cxx
+++ b/package/source/zippackage/ZipPackageStream.cxx
@@ -84,9 +84,6 @@ ZipPackageStream::ZipPackageStream ( ZipPackage & rNewPackage,
, m_bToBeEncrypted ( false )
, m_bHaveOwnKey ( false )
, m_bIsEncrypted ( false )
-, m_nImportedStartKeyAlgorithm( 0 )
-, m_nImportedEncryptionAlgorithm( 0 )
-, m_nImportedDerivedKeySize( 0 )
, m_nStreamMode( PACKAGE_STREAM_NOTSET )
, m_nMagicalHackPos( 0 )
, m_nMagicalHackSize( 0 )
@@ -186,7 +183,9 @@ uno::Reference< io::XInputStream >
ZipPackageStream::GetRawEncrStreamNoHeaderCop
sal_Int32 ZipPackageStream::GetEncryptionAlgorithm() const
{
- return m_nImportedEncryptionAlgorithm ? m_nImportedEncryptionAlgorithm :
m_rZipPackage.GetEncAlgID();
+ return m_oImportedAlgorithms
+ ? m_oImportedAlgorithms->nImportedEncryptionAlgorithm
+ : m_rZipPackage.GetEncAlgID();
}
sal_Int32 ZipPackageStream::GetIVSize() const
@@ -212,8 +211,8 @@ sal_Int32 ZipPackageStream::GetIVSize() const
*m_xBaseEncryptionData,
GetEncryptionKey(bugs),
GetEncryptionAlgorithm(),
- m_oImportedChecksumAlgorithm ? m_oImportedChecksumAlgorithm :
m_rZipPackage.GetChecksumAlgID(),
- m_nImportedDerivedKeySize ? m_nImportedDerivedKeySize :
m_rZipPackage.GetDefaultDerivedKeySize(),
+ m_oImportedAlgorithms ?
m_oImportedAlgorithms->oImportedChecksumAlgorithm :
m_rZipPackage.GetChecksumAlgID(),
+ m_oImportedAlgorithms ?
m_oImportedAlgorithms->nImportedDerivedKeySize :
m_rZipPackage.GetDefaultDerivedKeySize(),
GetStartKeyGenID(),
bugs != Bugs::None);
@@ -264,7 +263,9 @@ sal_Int32 ZipPackageStream::GetStartKeyGenID() const
{
// generally should all the streams use the same Start Key
// but if raw copy without password takes place, we should preserve the
imported algorithm
- return m_nImportedStartKeyAlgorithm ? m_nImportedStartKeyAlgorithm :
m_rZipPackage.GetStartKeyGenID();
+ return m_oImportedAlgorithms
+ ? m_oImportedAlgorithms->nImportedStartKeyAlgorithm
+ : m_rZipPackage.GetStartKeyGenID();
}
uno::Reference< io::XInputStream >
ZipPackageStream::TryToGetRawFromDataStream( bool bAddHeaderForEncr )
@@ -401,17 +402,14 @@ bool ZipPackageStream::ParsePackageRawStream()
+
xTempEncrData->m_aInitVector.getLength()
+
xTempEncrData->m_aDigest.getLength()
+
aMediaType.getLength() * sizeof( sal_Unicode );
- m_nImportedEncryptionAlgorithm = nEncAlgorithm;
- if (nChecksumAlgorithm == 0)
- {
- m_oImportedChecksumAlgorithm.reset();
- }
- else
- {
-
m_oImportedChecksumAlgorithm.emplace(nChecksumAlgorithm);
- }
- m_nImportedDerivedKeySize = nDerivedKeySize;
- m_nImportedStartKeyAlgorithm = nStartKeyGenID;
+ m_oImportedAlgorithms.emplace(ImportedAlgorithms{
+ .nImportedStartKeyAlgorithm = nStartKeyGenID,
+ .nImportedEncryptionAlgorithm = nEncAlgorithm,
+ .oImportedChecksumAlgorithm = nChecksumAlgorithm == 0
+ ? ::std::optional<sal_Int32>{}
+ : ::std::optional<sal_Int32>{nChecksumAlgorithm},
+ .nImportedDerivedKeySize = nDerivedKeySize,
+ });
m_nMagicalHackSize = nMagHackSize;
msMediaType = aMediaType;
@@ -929,7 +927,7 @@ void SAL_CALL ZipPackageStream::setInputStream( const
uno::Reference< io::XInput
{
// if seekable access is required the wrapping will be done on demand
m_xStream = aStream;
- m_nImportedEncryptionAlgorithm = 0;
+ m_oImportedAlgorithms.reset();
m_bHasSeekable = false;
SetPackageMember ( false );
aEntry.nTime = -1;
@@ -1055,7 +1053,7 @@ uno::Reference< io::XInputStream > SAL_CALL
ZipPackageStream::getDataStream()
// missing a specified startkey of SHA256
// force SHA256 and see if that works
- m_nImportedStartKeyAlgorithm =
xml::crypto::DigestID::SHA256;
+ m_oImportedAlgorithms->nImportedStartKeyAlgorithm =
xml::crypto::DigestID::SHA256;
xResult = m_rZipPackage.getZipFile().getDataStream(aEntry,
GetEncryptionData(), oDecryptedSize,
m_rZipPackage.GetSharedMutexRef());
@@ -1065,7 +1063,7 @@ uno::Reference< io::XInputStream > SAL_CALL
ZipPackageStream::getDataStream()
{
// if that didn't work, restore to SHA1 and trundle
through the *other* earlier
// bug fix
- m_nImportedStartKeyAlgorithm = xml::crypto::DigestID::SHA1;
+ m_oImportedAlgorithms->nImportedStartKeyAlgorithm =
xml::crypto::DigestID::SHA1;
}
// workaround for the encrypted documents generated with the
old OOo1.x bug.
