offapi/com/sun/star/xml/crypto/CipherID.idl | 2 package/inc/EncryptionData.hxx | 14 ++- package/inc/ZipFile.hxx | 4 - package/inc/ZipPackage.hxx | 4 - package/inc/ZipPackageStream.hxx | 6 + package/source/manifest/ManifestExport.cxx | 31 ++++--- package/source/manifest/ManifestImport.cxx | 6 - package/source/zipapi/XUnbufferedStream.cxx | 1 package/source/zipapi/ZipFile.cxx | 98 +++++++++++++++++++------ package/source/zipapi/ZipOutputEntry.cxx | 13 ++- package/source/zippackage/ZipPackage.cxx | 68 ++++++++++++----- package/source/zippackage/ZipPackageStream.cxx | 37 +++++++-- sfx2/source/doc/objstor.cxx | 2 13 files changed, 209 insertions(+), 77 deletions(-)
New commits: commit 309558858d2b37cbad04b3000391ad9ba570708d Author: Michael Stahl <michael.st...@allotropia.de> AuthorDate: Wed Dec 13 18:36:15 2023 +0100 Commit: Michael Stahl <michael.st...@allotropia.de> CommitDate: Thu Dec 14 09:09:57 2023 +0100 tdf#105844 package,sfx2: remove checksum infoleak when using AEAD 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 f57baefbd3c4c5d8e5ec28e8702c91d60ffc5de2) 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.st...@allotropia.de> (cherry picked from commit 09f23a3dc5cd571df347cba9b003195de35f3ddd) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160694 diff --git a/offapi/com/sun/star/xml/crypto/CipherID.idl b/offapi/com/sun/star/xml/crypto/CipherID.idl index c1ecd297a741..3d9bbc000cfb 100644 --- a/offapi/com/sun/star/xml/crypto/CipherID.idl +++ b/offapi/com/sun/star/xml/crypto/CipherID.idl @@ -46,7 +46,7 @@ constants CipherID @see https://www.w3.org/TR/xmlenc-core1/#sec-AES-GCM - @since LO 24.2 + @since LibreOffice 24.2 */ const long AES_GCM_W3C = 3; }; diff --git a/package/inc/EncryptionData.hxx b/package/inc/EncryptionData.hxx index 73408a090faf..f2d5c0b126b0 100644 --- a/package/inc/EncryptionData.hxx +++ b/package/inc/EncryptionData.hxx @@ -22,6 +22,8 @@ #include <com/sun/star/uno/Sequence.hxx> #include <cppuhelper/weak.hxx> +#include <optional> + class BaseEncryptionData : public cppu::OWeakObject { public: @@ -47,16 +49,20 @@ class EncryptionData final : public BaseEncryptionData public: css::uno::Sequence < sal_Int8 > m_aKey; sal_Int32 m_nEncAlg; - sal_Int32 m_nCheckAlg; + ::std::optional<sal_Int32> m_oCheckAlg; sal_Int32 m_nDerivedKeySize; sal_Int32 m_nStartKeyGenID; bool m_bTryWrongSHA1; - EncryptionData(const BaseEncryptionData& aData, const css::uno::Sequence< sal_Int8 >& aKey, sal_Int32 nEncAlg, sal_Int32 nCheckAlg, sal_Int32 nDerivedKeySize, sal_Int32 nStartKeyGenID, bool const bTryWrongSHA1) + EncryptionData(const BaseEncryptionData& aData, + const css::uno::Sequence<sal_Int8>& aKey, sal_Int32 const nEncAlg, + ::std::optional<sal_Int32> const oCheckAlg, + sal_Int32 const nDerivedKeySize, sal_Int32 const nStartKeyGenID, + bool const bTryWrongSHA1) : BaseEncryptionData( aData ) , m_aKey( aKey ) , m_nEncAlg( nEncAlg ) - , m_nCheckAlg( nCheckAlg ) + , m_oCheckAlg( oCheckAlg ) , m_nDerivedKeySize( nDerivedKeySize ) , m_nStartKeyGenID( nStartKeyGenID ) , m_bTryWrongSHA1(bTryWrongSHA1) @@ -66,7 +72,7 @@ public: : BaseEncryptionData( aData ) , m_aKey( aData.m_aKey ) , m_nEncAlg( aData.m_nEncAlg ) - , m_nCheckAlg( aData.m_nCheckAlg ) + , m_oCheckAlg( aData.m_oCheckAlg ) , m_nDerivedKeySize( aData.m_nDerivedKeySize ) , m_nStartKeyGenID( aData.m_nStartKeyGenID ) , m_bTryWrongSHA1(aData.m_bTryWrongSHA1) diff --git a/package/inc/ZipFile.hxx b/package/inc/ZipFile.hxx index f6b184994729..2d42ed403136 100644 --- a/package/inc/ZipFile.hxx +++ b/package/inc/ZipFile.hxx @@ -70,7 +70,9 @@ class ZipFile const bool bUseBufferedStream = true, const OUString& aMediaType = OUString() ); - bool hasValidPassword ( ZipEntry const & rEntry, const rtl::Reference < EncryptionData > &rData ); + css::uno::Reference<css::io::XInputStream> checkValidPassword( + ZipEntry const& rEntry, rtl::Reference<EncryptionData> const& rData, + rtl::Reference<comphelper::RefCountedMutex> const& rMutexHolder); bool checkSizeAndCRC( const ZipEntry& aEntry ); diff --git a/package/inc/ZipPackage.hxx b/package/inc/ZipPackage.hxx index 48067f39b6f7..5d196fe7cdaa 100644 --- a/package/inc/ZipPackage.hxx +++ b/package/inc/ZipPackage.hxx @@ -78,7 +78,7 @@ class ZipPackage final : public cppu::WeakImplHelper OUString m_aURL; sal_Int32 m_nStartKeyGenerationID; - sal_Int32 m_nChecksumDigestID; + ::std::optional<sal_Int32> m_oChecksumDigestID; sal_Int32 m_nCommonEncryptionID; bool m_bHasEncryptedEntries; bool m_bHasNonEncryptedEntries; @@ -124,7 +124,7 @@ public: sal_Int32 GetStartKeyGenID() const { return m_nStartKeyGenerationID; } sal_Int32 GetEncAlgID() const { return m_nCommonEncryptionID; } - sal_Int32 GetChecksumAlgID() const { return m_nChecksumDigestID; } + ::std::optional<sal_Int32> GetChecksumAlgID() const { return m_oChecksumDigestID; } sal_Int32 GetDefaultDerivedKeySize() const { switch (m_nCommonEncryptionID) { diff --git a/package/inc/ZipPackageStream.hxx b/package/inc/ZipPackageStream.hxx index 91beaf276719..b39c59e633e9 100644 --- a/package/inc/ZipPackageStream.hxx +++ b/package/inc/ZipPackageStream.hxx @@ -29,6 +29,8 @@ #include "EncryptionData.hxx" +#include <optional> + #define PACKAGE_STREAM_NOTSET 0 #define PACKAGE_STREAM_PACKAGEMEMBER 1 #define PACKAGE_STREAM_DETECT 2 @@ -55,7 +57,7 @@ private: sal_Int32 m_nImportedStartKeyAlgorithm; sal_Int32 m_nImportedEncryptionAlgorithm; - sal_Int32 m_nImportedChecksumAlgorithm; + ::std::optional<sal_Int32> m_oImportedChecksumAlgorithm; sal_Int32 m_nImportedDerivedKeySize; sal_uInt8 m_nStreamMode; @@ -95,7 +97,7 @@ public: 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( sal_Int32 nAlgorithm ) { m_nImportedChecksumAlgorithm = nAlgorithm; } + void SetImportedChecksumAlgorithm(::std::optional<sal_Int32> const& roAlgorithm) { m_oImportedChecksumAlgorithm = roAlgorithm; } void SetImportedDerivedKeySize( sal_Int32 nSize ) { m_nImportedDerivedKeySize = nSize; } void SetToBeEncrypted (bool bNewValue) { diff --git a/package/source/manifest/ManifestExport.cxx b/package/source/manifest/ManifestExport.cxx index 9921b1bad8c8..aa9e9aa2a32f 100644 --- a/package/source/manifest/ManifestExport.cxx +++ b/package/source/manifest/ManifestExport.cxx @@ -337,7 +337,7 @@ ManifestExport::ManifestExport( uno::Reference< xml::sax::XDocumentHandler > con xHandler->ignorableWhitespace ( sWhiteSpace ); xHandler->startElement( ELEMENT_FILE_ENTRY , pAttrList); - if ( pVector && pSalt && pIterationCount && pDigest && pDigestAlg && pEncryptAlg && pStartKeyAlg && pDerivedKeySize ) + if (pVector && pSalt && pIterationCount && pEncryptAlg && pStartKeyAlg && pDerivedKeySize) { // ==== Encryption Data rtl::Reference<::comphelper::AttributeList> pNewAttrList = new ::comphelper::AttributeList; @@ -347,20 +347,23 @@ ManifestExport::ManifestExport( uno::Reference< xml::sax::XDocumentHandler > con xHandler->ignorableWhitespace ( sWhiteSpace ); // ==== Digest - OUString sChecksumType; - sal_Int32 nDigestAlgID = 0; - *pDigestAlg >>= nDigestAlgID; - if ( nDigestAlgID == xml::crypto::DigestID::SHA256_1K ) - sChecksumType = sSHA256_1k_URL; - else if ( nDigestAlgID == xml::crypto::DigestID::SHA1_1K ) - sChecksumType = sSHA1_1k_Name; - else - throw uno::RuntimeException( THROW_WHERE "Unexpected digest algorithm is provided!" ); + if (pDigest && pDigestAlg && pDigestAlg->hasValue()) + { + OUString sChecksumType; + sal_Int32 nDigestAlgID = 0; + *pDigestAlg >>= nDigestAlgID; + if ( nDigestAlgID == xml::crypto::DigestID::SHA256_1K ) + sChecksumType = sSHA256_1k_URL; + else if ( nDigestAlgID == xml::crypto::DigestID::SHA1_1K ) + sChecksumType = sSHA1_1k_Name; + else + throw uno::RuntimeException( THROW_WHERE "Unexpected digest algorithm is provided!" ); - pNewAttrList->AddAttribute ( ATTRIBUTE_CHECKSUM_TYPE, sChecksumType ); - *pDigest >>= aSequence; - ::comphelper::Base64::encode(aBuffer, aSequence); - pNewAttrList->AddAttribute ( ATTRIBUTE_CHECKSUM, aBuffer.makeStringAndClear() ); + pNewAttrList->AddAttribute(ATTRIBUTE_CHECKSUM_TYPE, sChecksumType); + *pDigest >>= aSequence; + ::comphelper::Base64::encode(aBuffer, aSequence); + pNewAttrList->AddAttribute(ATTRIBUTE_CHECKSUM, aBuffer.makeStringAndClear()); + } xHandler->startElement( ELEMENT_ENCRYPTION_DATA , pNewAttrList); diff --git a/package/source/manifest/ManifestImport.cxx b/package/source/manifest/ManifestImport.cxx index b7aa57f99ff1..0458eb9c4b8e 100644 --- a/package/source/manifest/ManifestImport.cxx +++ b/package/source/manifest/ManifestImport.cxx @@ -164,10 +164,10 @@ void ManifestImport::doEncryptionData(StringHashMap &rConvertedAttribs) } else if ( aString == SHA256_1K_URL ) { aSequence[PKG_MNFST_DIGESTALG].Name = gsDigestAlgProperty; aSequence[PKG_MNFST_DIGESTALG].Value <<= xml::crypto::DigestID::SHA256_1K; - } else - bIgnoreEncryptData = true; + } + // note: digest is now *optional* - expected not to be used with AEAD - if ( !bIgnoreEncryptData ) { + if (aSequence[PKG_MNFST_DIGESTALG].Value.hasValue()) { aString = rConvertedAttribs[ATTRIBUTE_CHECKSUM]; uno::Sequence < sal_Int8 > aDecodeBuffer; ::comphelper::Base64::decode(aDecodeBuffer, aString); diff --git a/package/source/zipapi/XUnbufferedStream.cxx b/package/source/zipapi/XUnbufferedStream.cxx index bd2cf4d72d72..350f142c25ba 100644 --- a/package/source/zipapi/XUnbufferedStream.cxx +++ b/package/source/zipapi/XUnbufferedStream.cxx @@ -272,6 +272,7 @@ sal_Int32 SAL_CALL XUnbufferedStream::readBytes( Sequence< sal_Int8 >& aData, sa maCompBuffer = m_xCipherContext->convertWithCipherContext( maCompBuffer ); if ( mnZipCurrent == mnZipEnd ) { + // this should throw if AEAD is in use and the tag fails to validate uno::Sequence< sal_Int8 > aSuffix = m_xCipherContext->finalizeCipherContextAndDispose(); if ( aSuffix.hasElements() ) { diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx index 1b4b7ec2e8f8..664cfa7f1e17 100644 --- a/package/source/zipapi/ZipFile.cxx +++ b/package/source/zipapi/ZipFile.cxx @@ -133,8 +133,10 @@ void ZipFile::setInputStream ( const uno::Reference < XInputStream >& xNewStream uno::Reference< xml::crypto::XDigestContext > ZipFile::StaticGetDigestContextForChecksum( const uno::Reference< uno::XComponentContext >& xArgContext, const ::rtl::Reference< EncryptionData >& xEncryptionData ) { + assert(xEncryptionData->m_oCheckAlg); // callers checked it already + uno::Reference< xml::crypto::XDigestContext > xDigestContext; - if ( xEncryptionData->m_nCheckAlg == xml::crypto::DigestID::SHA256_1K ) + if (*xEncryptionData->m_oCheckAlg == xml::crypto::DigestID::SHA256_1K) { uno::Reference< uno::XComponentContext > xContext = xArgContext; if ( !xContext.is() ) @@ -142,9 +144,11 @@ uno::Reference< xml::crypto::XDigestContext > ZipFile::StaticGetDigestContextFor uno::Reference< xml::crypto::XNSSInitializer > xDigestContextSupplier = xml::crypto::NSSInitializer::create( xContext ); - xDigestContext.set( xDigestContextSupplier->getDigestContext( xEncryptionData->m_nCheckAlg, uno::Sequence< beans::NamedValue >() ), uno::UNO_SET_THROW ); + xDigestContext.set(xDigestContextSupplier->getDigestContext( + *xEncryptionData->m_oCheckAlg, uno::Sequence<beans::NamedValue>()), + uno::UNO_SET_THROW); } - else if ( xEncryptionData->m_nCheckAlg == xml::crypto::DigestID::SHA1_1K ) + else if (*xEncryptionData->m_oCheckAlg == xml::crypto::DigestID::SHA1_1K) { if (xEncryptionData->m_bTryWrongSHA1) { @@ -253,7 +257,7 @@ void ZipFile::StaticFillHeader( const ::rtl::Reference< EncryptionData >& rData, *(pHeader++) = static_cast< sal_Int8 >(( nEncAlgID >> 24 ) & 0xFF); // Then the checksum algorithm - sal_Int32 nChecksumAlgID = rData->m_nCheckAlg; + sal_Int32 nChecksumAlgID = rData->m_oCheckAlg ? *rData->m_oCheckAlg : 0; *(pHeader++) = static_cast< sal_Int8 >(( nChecksumAlgID >> 0 ) & 0xFF); *(pHeader++) = static_cast< sal_Int8 >(( nChecksumAlgID >> 8 ) & 0xFF); *(pHeader++) = static_cast< sal_Int8 >(( nChecksumAlgID >> 16 ) & 0xFF); @@ -411,8 +415,7 @@ void CheckSequence( const uno::Sequence< sal_Int8 >& aSequence ) bool ZipFile::StaticHasValidPassword( const uno::Reference< uno::XComponentContext >& rxContext, const Sequence< sal_Int8 > &aReadBuffer, const ::rtl::Reference< EncryptionData > &rData ) { - if (rData->m_nEncAlg == xml::crypto::CipherID::AES_GCM_W3C) - return true; /*TODO fails because of tag*/ + assert(rData->m_nEncAlg != xml::crypto::CipherID::AES_GCM_W3C); // should not be called for AEAD if ( !rData.is() || !rData->m_aKey.hasElements() ) return false; @@ -465,12 +468,31 @@ bool ZipFile::StaticHasValidPassword( const uno::Reference< uno::XComponentConte return bRet; } -bool ZipFile::hasValidPassword ( ZipEntry const & rEntry, const ::rtl::Reference< EncryptionData >& rData ) +uno::Reference<io::XInputStream> ZipFile::checkValidPassword( + ZipEntry const& rEntry, ::rtl::Reference<EncryptionData> const& rData, + rtl::Reference<comphelper::RefCountedMutex> const& rMutex) { ::osl::MutexGuard aGuard( m_aMutexHolder->GetMutex() ); - bool bRet = false; - if ( rData.is() && rData->m_aKey.hasElements() ) + if (rData.is() && rData->m_nEncAlg == xml::crypto::CipherID::AES_GCM_W3C) + { + try // the only way to find out: decrypt the whole stream, which will + { // check the tag + uno::Reference<io::XInputStream> const xRet = + createStreamForZipEntry(rMutex, rEntry, rData, UNBUFF_STREAM_DATA, true); + // currently XBufferedStream reads the whole stream in its ctor (to + // verify the tag) - in case this gets changed, explicitly seek here + uno::Reference<io::XSeekable> const xSeek(xRet, uno::UNO_QUERY_THROW); + xSeek->seek(xSeek->getLength()); + xSeek->seek(0); + return xRet; + } + catch (uno::Exception const&) + { + return {}; + } + } + else if (rData.is() && rData->m_aKey.hasElements()) { css::uno::Reference < css::io::XSeekable > xSeek(xStream, UNO_QUERY_THROW); xSeek->seek( rEntry.nOffset ); @@ -484,10 +506,14 @@ bool ZipFile::hasValidPassword ( ZipEntry const & rEntry, const ::rtl::Reference xStream->readBytes( aReadBuffer, nSize ); - bRet = StaticHasValidPassword( m_xContext, aReadBuffer, rData ); + if (StaticHasValidPassword(m_xContext, aReadBuffer, rData)) + { + return createStreamForZipEntry( + rMutex, rEntry, rData, UNBUFF_STREAM_DATA, true); + } } - return bRet; + return {}; } namespace { @@ -648,8 +674,30 @@ uno::Reference< XInputStream > ZipFile::StaticGetDataFromRawStream( // if we have a digest, then this file is an encrypted one and we should // check if we can decrypt it or not - OSL_ENSURE(rData->m_aDigest.hasElements(), "Can't detect password correctness without digest!"); - if (rData->m_aDigest.hasElements()) + SAL_WARN_IF(rData->m_nEncAlg != xml::crypto::CipherID::AES_GCM_W3C && !rData->m_aDigest.hasElements(), + "package", "Can't detect password correctness without digest!"); + if (rData->m_nEncAlg == xml::crypto::CipherID::AES_GCM_W3C) + { + // skip header + xSeek->seek(n_ConstHeaderSize + rData->m_aInitVector.getLength() + + rData->m_aSalt.getLength() + rData->m_aDigest.getLength()); + + try + { // XUnbufferedStream does not support XSeekable so wrap it + ::rtl::Reference<XBufferedStream> const pRet( + new XBufferedStream(new XUnbufferedStream(rMutexHolder, xStream, rData))); + // currently XBufferedStream reads the whole stream in its ctor (to + // verify the tag) - in case this gets changed, explicitly seek here + pRet->seek(pRet->getLength()); + pRet->seek(0); + return pRet; + } + catch (uno::Exception const&) + { + throw packages::WrongPasswordException(THROW_WHERE); + } + } + else if (rData->m_aDigest.hasElements()) { sal_Int32 nSize = sal::static_int_cast<sal_Int32>(xSeek->getLength()); if (nSize > n_ConstDigestLength + 32) @@ -691,10 +739,15 @@ uno::Reference< XInputStream > ZipFile::getInputStream( ZipEntry& rEntry, bool bNeedRawStream = rEntry.nMethod == STORED; - // if we have a digest, then this file is an encrypted one and we should - // check if we can decrypt it or not - if ( bIsEncrypted && rData.is() && rData->m_aDigest.hasElements() ) - bNeedRawStream = !hasValidPassword ( rEntry, rData ); + if (bIsEncrypted && rData.is()) + { + uno::Reference<XInputStream> const xRet(checkValidPassword(rEntry, rData, aMutexHolder)); + if (xRet.is()) + { + return xRet; + } + bNeedRawStream = true; + } return createStreamForZipEntry ( aMutexHolder, rEntry, @@ -725,9 +778,14 @@ uno::Reference< XInputStream > ZipFile::getDataStream( ZipEntry& rEntry, // if we have a digest, then this file is an encrypted one and we should // check if we can decrypt it or not - OSL_ENSURE( rData->m_aDigest.hasElements(), "Can't detect password correctness without digest!" ); - if ( rData->m_aDigest.hasElements() && !hasValidPassword ( rEntry, rData ) ) - throw packages::WrongPasswordException(THROW_WHERE ); + SAL_WARN_IF(rData->m_nEncAlg != xml::crypto::CipherID::AES_GCM_W3C && !rData->m_aDigest.hasElements(), + "package", "Can't detect password correctness without digest!"); + uno::Reference<XInputStream> const xRet(checkValidPassword(rEntry, rData, aMutexHolder)); + if (!xRet.is()) + { + throw packages::WrongPasswordException(THROW_WHERE); + } + return xRet; } else bNeedRawStream = ( rEntry.nMethod == STORED ); diff --git a/package/source/zipapi/ZipOutputEntry.cxx b/package/source/zipapi/ZipOutputEntry.cxx index 86c8a9782df7..9d9c1e9b6f4c 100644 --- a/package/source/zipapi/ZipOutputEntry.cxx +++ b/package/source/zipapi/ZipOutputEntry.cxx @@ -21,6 +21,7 @@ #include <com/sun/star/io/TempFile.hpp> #include <com/sun/star/packages/zip/ZipConstants.hpp> +#include <com/sun/star/xml/crypto/CipherID.hpp> #include <osl/diagnose.h> @@ -60,7 +61,11 @@ ZipOutputEntryBase::ZipOutputEntryBase( if (m_bEncryptCurrentEntry) { m_xCipherContext = ZipFile::StaticGetCipher( m_xContext, pStream->GetEncryptionData(), true ); - m_xDigestContext = ZipFile::StaticGetDigestContextForChecksum( m_xContext, pStream->GetEncryptionData() ); + if (pStream->GetEncryptionData()->m_oCheckAlg) + { + assert(pStream->GetEncryptionData()->m_nEncAlg != xml::crypto::CipherID::AES_GCM_W3C); + m_xDigestContext = ZipFile::StaticGetDigestContextForChecksum(m_xContext, pStream->GetEncryptionData()); + } } } @@ -118,11 +123,11 @@ void ZipOutputEntryBase::processDeflated( const uno::Sequence< sal_Int8 >& defla if ( nLength > 0 ) { uno::Sequence< sal_Int8 > aTmpBuffer( deflateBuffer.getConstArray(), nLength ); - if ( m_bEncryptCurrentEntry && m_xDigestContext.is() && m_xCipherContext.is() ) + if (m_bEncryptCurrentEntry && m_xCipherContext.is()) { // Need to update our digest before encryption... sal_Int32 nDiff = n_ConstDigestLength - m_nDigested; - if ( nDiff ) + if (m_xDigestContext.is() && nDiff) { sal_Int32 nEat = ::std::min( nLength, nDiff ); uno::Sequence< sal_Int8 > aTmpSeq( aTmpBuffer.getConstArray(), nEat ); @@ -146,7 +151,7 @@ void ZipOutputEntryBase::processDeflated( const uno::Sequence< sal_Int8 >& defla } } - if ( !(isDeflaterFinished() && m_bEncryptCurrentEntry && m_xDigestContext.is() && m_xCipherContext.is()) ) + if (!(isDeflaterFinished() && m_bEncryptCurrentEntry && m_xCipherContext.is())) return; // FIXME64: sequence not 64bit safe. diff --git a/package/source/zippackage/ZipPackage.cxx b/package/source/zippackage/ZipPackage.cxx index aacb5cd22a45..1bae902c177a 100644 --- a/package/source/zippackage/ZipPackage.cxx +++ b/package/source/zippackage/ZipPackage.cxx @@ -138,7 +138,7 @@ class DummyInputStream : public ::cppu::WeakImplHelper< XInputStream > ZipPackage::ZipPackage ( uno::Reference < XComponentContext > xContext ) : m_aMutexHolder( new comphelper::RefCountedMutex ) , m_nStartKeyGenerationID( xml::crypto::DigestID::SHA1 ) -, m_nChecksumDigestID( xml::crypto::DigestID::SHA1_1K ) +, m_oChecksumDigestID( xml::crypto::DigestID::SHA1_1K ) , m_nCommonEncryptionID( xml::crypto::CipherID::BLOWFISH_CFB_8 ) , m_bHasEncryptedEntries ( false ) , m_bHasNonEncryptedEntries ( false ) @@ -251,11 +251,16 @@ void ZipPackage::parseManifest() pStream->SetMediaType ( sMediaType ); pStream->SetFromManifest( true ); - if ( pKeyInfo && pVector && pSize && pDigest && pDigestAlg && pEncryptionAlg ) + if (pKeyInfo + && pVector && pSize && pEncryptionAlg + && ((pEncryptionAlg->has<sal_Int32>() + && pEncryptionAlg->get<sal_Int32>() == xml::crypto::CipherID::AES_GCM_W3C) + || (pDigest && pDigestAlg))) { uno::Sequence < sal_Int8 > aSequence; sal_Int64 nSize = 0; - sal_Int32 nDigestAlg = 0, nEncryptionAlg = 0; + ::std::optional<sal_Int32> oDigestAlg; + sal_Int32 nEncryptionAlg = 0; pStream->SetToBeEncrypted ( true ); @@ -265,11 +270,15 @@ void ZipPackage::parseManifest() *pSize >>= nSize; pStream->setSize ( nSize ); - *pDigest >>= aSequence; - pStream->setDigest ( aSequence ); + if (pDigest && pDigestAlg) + { + *pDigest >>= aSequence; + pStream->setDigest(aSequence); - *pDigestAlg >>= nDigestAlg; - pStream->SetImportedChecksumAlgorithm( nDigestAlg ); + assert(pDigestAlg->has<sal_Int32>()); + oDigestAlg.emplace(pDigestAlg->get<sal_Int32>()); + pStream->SetImportedChecksumAlgorithm(oDigestAlg); + } *pEncryptionAlg >>= nEncryptionAlg; pStream->SetImportedEncryptionAlgorithm( nEncryptionAlg ); @@ -292,16 +301,23 @@ void ZipPackage::parseManifest() || pStream->getName() == "encrypted-package")) { m_bHasEncryptedEntries = true; - m_nChecksumDigestID = nDigestAlg; + m_oChecksumDigestID = oDigestAlg; m_nCommonEncryptionID = nEncryptionAlg; m_nStartKeyGenerationID = nStartKeyAlg; } } - else if ( pSalt && pVector && pCount && pSize && pDigest && pDigestAlg && pEncryptionAlg ) + else if (pSalt && pCount + && pVector && pSize && pEncryptionAlg + && ((pEncryptionAlg->has<sal_Int32>() + && pEncryptionAlg->get<sal_Int32>() == xml::crypto::CipherID::AES_GCM_W3C) + || (pDigest && pDigestAlg))) + { uno::Sequence < sal_Int8 > aSequence; sal_Int64 nSize = 0; - sal_Int32 nCount = 0, nDigestAlg = 0, nEncryptionAlg = 0; + ::std::optional<sal_Int32> oDigestAlg; + sal_Int32 nEncryptionAlg = 0; + sal_Int32 nCount = 0; sal_Int32 nDerivedKeySize = 16, nStartKeyAlg = xml::crypto::DigestID::SHA1; pStream->SetToBeEncrypted ( true ); @@ -318,11 +334,15 @@ void ZipPackage::parseManifest() *pSize >>= nSize; pStream->setSize ( nSize ); - *pDigest >>= aSequence; - pStream->setDigest ( aSequence ); + if (pDigest && pDigestAlg) + { + *pDigest >>= aSequence; + pStream->setDigest(aSequence); - *pDigestAlg >>= nDigestAlg; - pStream->SetImportedChecksumAlgorithm( nDigestAlg ); + assert(pDigestAlg->has<sal_Int32>()); + oDigestAlg.emplace(pDigestAlg->get<sal_Int32>()); + pStream->SetImportedChecksumAlgorithm(oDigestAlg); + } *pEncryptionAlg >>= nEncryptionAlg; pStream->SetImportedEncryptionAlgorithm( nEncryptionAlg ); @@ -344,7 +364,7 @@ void ZipPackage::parseManifest() { m_bHasEncryptedEntries = true; m_nStartKeyGenerationID = nStartKeyAlg; - m_nChecksumDigestID = nDigestAlg; + m_oChecksumDigestID = oDigestAlg; m_nCommonEncryptionID = nEncryptionAlg; } } @@ -1736,13 +1756,18 @@ void SAL_CALL ZipPackage::setPropertyValue( const OUString& aPropertyName, const else if ( rAlgorithm.Name == "ChecksumAlgorithm" ) { sal_Int32 nID = 0; + if (!rAlgorithm.Value.hasValue()) + { + m_oChecksumDigestID.reset(); + continue; + } if ( !( rAlgorithm.Value >>= nID ) || ( nID != xml::crypto::DigestID::SHA1_1K && nID != xml::crypto::DigestID::SHA256_1K ) ) { throw IllegalArgumentException(THROW_WHERE "Unexpected checksum algorithm is provided!", uno::Reference<uno::XInterface>(), 2); } - m_nChecksumDigestID = nID; + m_oChecksumDigestID.emplace(nID); } else { @@ -1765,7 +1790,7 @@ void SAL_CALL ZipPackage::setPropertyValue( const OUString& aPropertyName, const // defaults) with reasonable values m_nStartKeyGenerationID = 0; // this is unused for PGP m_nCommonEncryptionID = xml::crypto::CipherID::AES_CBC_W3C_PADDING; - m_nChecksumDigestID = xml::crypto::DigestID::SHA512_1K; + m_oChecksumDigestID.emplace(xml::crypto::DigestID::SHA512_1K); } else throw UnknownPropertyException(aPropertyName); @@ -1786,7 +1811,14 @@ Any SAL_CALL ZipPackage::getPropertyValue( const OUString& PropertyName ) ::comphelper::SequenceAsHashMap aAlgorithms; aAlgorithms["StartKeyGenerationAlgorithm"] <<= m_nStartKeyGenerationID; aAlgorithms["EncryptionAlgorithm"] <<= m_nCommonEncryptionID; - aAlgorithms["ChecksumAlgorithm"] <<= m_nChecksumDigestID; + if (m_oChecksumDigestID) + { + aAlgorithms["ChecksumAlgorithm"] <<= *m_oChecksumDigestID; + } + else + { + aAlgorithms["ChecksumAlgorithm"]; + } return Any(aAlgorithms.getAsConstNamedValueList()); } if ( PropertyName == STORAGE_ENCRYPTION_KEYS_PROPERTY ) diff --git a/package/source/zippackage/ZipPackageStream.cxx b/package/source/zippackage/ZipPackageStream.cxx index 494d8aaa6efc..6ef2241d464a 100644 --- a/package/source/zippackage/ZipPackageStream.cxx +++ b/package/source/zippackage/ZipPackageStream.cxx @@ -85,7 +85,6 @@ ZipPackageStream::ZipPackageStream ( ZipPackage & rNewPackage, , m_bIsEncrypted ( false ) , m_nImportedStartKeyAlgorithm( 0 ) , m_nImportedEncryptionAlgorithm( 0 ) -, m_nImportedChecksumAlgorithm( 0 ) , m_nImportedDerivedKeySize( 0 ) , m_nStreamMode( PACKAGE_STREAM_NOTSET ) , m_nMagicalHackPos( 0 ) @@ -208,7 +207,7 @@ sal_Int32 ZipPackageStream::GetIVSize() const *m_xBaseEncryptionData, GetEncryptionKey(bugs), GetEncryptionAlgorithm(), - m_nImportedChecksumAlgorithm ? m_nImportedChecksumAlgorithm : m_rZipPackage.GetChecksumAlgID(), + m_oImportedChecksumAlgorithm ? m_oImportedChecksumAlgorithm : m_rZipPackage.GetChecksumAlgID(), m_nImportedDerivedKeySize ? m_nImportedDerivedKeySize : m_rZipPackage.GetDefaultDerivedKeySize(), GetStartKeyGenID(), bugs != Bugs::None); @@ -354,6 +353,9 @@ uno::Reference< io::XInputStream > ZipPackageStream::TryToGetRawFromDataStream( throw io::IOException(THROW_WHERE ); } +// presumably the purpose of this is to transfer encrypted streams between +// storages, needed for password-protected macros in documents, which is +// tragically a feature that exists bool ZipPackageStream::ParsePackageRawStream() { OSL_ENSURE( GetOwnSeekStream().is(), "A stream must be provided!" ); @@ -395,7 +397,14 @@ bool ZipPackageStream::ParsePackageRawStream() + xTempEncrData->m_aDigest.getLength() + aMediaType.getLength() * sizeof( sal_Unicode ); m_nImportedEncryptionAlgorithm = nEncAlgorithm; - m_nImportedChecksumAlgorithm = nChecksumAlgorithm; + if (nChecksumAlgorithm == 0) + { + m_oImportedChecksumAlgorithm.reset(); + } + else + { + m_oImportedChecksumAlgorithm.emplace(nChecksumAlgorithm); + } m_nImportedDerivedKeySize = nDerivedKeySize; m_nImportedStartKeyAlgorithm = nStartKeyGenID; m_nMagicalHackSize = nMagHackSize; @@ -617,13 +626,20 @@ bool ZipPackageStream::saveChild( throw uno::RuntimeException(); pPropSet[PKG_MNFST_DIGEST].Name = sDigestProperty; - pPropSet[PKG_MNFST_DIGEST].Value <<= m_xBaseEncryptionData->m_aDigest; + if (xEncData->m_oCheckAlg) + { + pPropSet[PKG_MNFST_DIGEST].Value <<= m_xBaseEncryptionData->m_aDigest; + } pPropSet[PKG_MNFST_ENCALG].Name = sEncryptionAlgProperty; pPropSet[PKG_MNFST_ENCALG].Value <<= xEncData->m_nEncAlg; pPropSet[PKG_MNFST_STARTALG].Name = sStartKeyAlgProperty; pPropSet[PKG_MNFST_STARTALG].Value <<= xEncData->m_nStartKeyGenID; pPropSet[PKG_MNFST_DIGESTALG].Name = sDigestAlgProperty; - pPropSet[PKG_MNFST_DIGESTALG].Value <<= xEncData->m_nCheckAlg; + if (xEncData->m_oCheckAlg) + { + assert(xEncData->m_nEncAlg != xml::crypto::CipherID::AES_GCM_W3C); + pPropSet[PKG_MNFST_DIGESTALG].Value <<= *xEncData->m_oCheckAlg; + } pPropSet[PKG_MNFST_DERKEYSIZE].Name = sDerivedKeySizeProperty; pPropSet[PKG_MNFST_DERKEYSIZE].Value <<= xEncData->m_nDerivedKeySize; } @@ -808,13 +824,20 @@ bool ZipPackageStream::saveChild( throw uno::RuntimeException(); pPropSet[PKG_MNFST_DIGEST].Name = sDigestProperty; - pPropSet[PKG_MNFST_DIGEST].Value <<= m_xBaseEncryptionData->m_aDigest; + if (xEncData->m_oCheckAlg) + { + pPropSet[PKG_MNFST_DIGEST].Value <<= m_xBaseEncryptionData->m_aDigest; + } pPropSet[PKG_MNFST_ENCALG].Name = sEncryptionAlgProperty; pPropSet[PKG_MNFST_ENCALG].Value <<= xEncData->m_nEncAlg; pPropSet[PKG_MNFST_STARTALG].Name = sStartKeyAlgProperty; pPropSet[PKG_MNFST_STARTALG].Value <<= xEncData->m_nStartKeyGenID; pPropSet[PKG_MNFST_DIGESTALG].Name = sDigestAlgProperty; - pPropSet[PKG_MNFST_DIGESTALG].Value <<= xEncData->m_nCheckAlg; + if (xEncData->m_oCheckAlg) + { + assert(xEncData->m_nEncAlg != xml::crypto::CipherID::AES_GCM_W3C); + pPropSet[PKG_MNFST_DIGESTALG].Value <<= *xEncData->m_oCheckAlg; + } pPropSet[PKG_MNFST_DERKEYSIZE].Name = sDerivedKeySizeProperty; pPropSet[PKG_MNFST_DERKEYSIZE].Value <<= xEncData->m_nDerivedKeySize; diff --git a/sfx2/source/doc/objstor.cxx b/sfx2/source/doc/objstor.cxx index 8a303d1c331f..40449b004713 100644 --- a/sfx2/source/doc/objstor.cxx +++ b/sfx2/source/doc/objstor.cxx @@ -366,7 +366,7 @@ void SfxObjectShell::SetupStorage( const uno::Reference< embed::XStorage >& xSto if (UseODFWholesomeEncryption(nDefVersion)) { pEncryptionAlgs[1].Value <<= xml::crypto::CipherID::AES_GCM_W3C; - pEncryptionAlgs[2].Value <<= xml::crypto::DigestID::SHA256_1K; + pEncryptionAlgs[2].Value.clear(); } else {