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.stahl@allotropia.de>
Tested-by: Jenkins

4 files changed, 68 insertions, 46 deletions

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 7c6431b19961..d3c5158e7e17 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;
@@ -93,10 +102,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 906acf4d7860..a5dcc5b76e6b 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 f18e0b26532f..bd6bdf4cd74d 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_nOwnStreamOrigSize( 0 )
@@ -185,7 +182,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
@@ -211,8 +210,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);
 
@@ -263,7 +262,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 )
@@ -395,17 +396,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_nOwnStreamOrigSize = nMagHackSize;
                     msMediaType = aMediaType;
 
@@ -915,7 +913,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;
@@ -1041,7 +1039,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());
@@ -1051,7 +1049,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.