Fix ASiC-E/S CAdES signature save

metsma · kristelmerilain · commit e4bcf869cc12 · 2025-12-05T20:51:15.000+02:00
IB-8309

Signed-off-by: Raul Metsma &lt;raul@metsma.ee&gt;
diff --git a/src/ASiC_E.cpp b/src/ASiC_E.cpp
@@ -182,7 +182,7 @@ void ASiC_E::parseManifestAndLoadFiles(const ZipSerialize &z)
             auto media_type = file[{"media-type", MANIFEST_NS}];
             DEBUG("full_path = '%s', media_type = '%s'", full_path.data(), media_type.data());
 
-            if(manifestFiles.find(full_path) != manifestFiles.end())
+            if(manifestFiles.contains(full_path))
                 THROW("Manifest multiple entries defined for file '%s'.", full_path.data());
 
             // ODF does not specify that mimetype should be first in manifest
@@ -198,11 +198,11 @@ void ASiC_E::parseManifestAndLoadFiles(const ZipSerialize &z)
 
             manifestFiles.insert(full_path);
             if(mediaType() == MIMETYPE_ADOC &&
-               (full_path.compare(0, 9, "META-INF/") == 0 ||
-                full_path.compare(0, 9, "metadata/") == 0))
-                d->metadata.push_back(new DataFilePrivate(dataStream(full_path, z), string(full_path), string(media_type)));
+               (full_path.starts_with("META-INF/") ||
+                full_path.starts_with("metadata/")))
+                d->metadata.push_back(new DataFilePrivate(z, string(full_path), string(media_type)));
             else
-                addDataFilePrivate(dataStream(full_path, z), string(full_path), string(media_type));
+                addDataFilePrivate(new DataFilePrivate(z, string(full_path), string(media_type)));
         }
         if(!mimeFound)
             THROW("Manifest is missing mediatype file entry.");
@@ -214,7 +214,7 @@ void ASiC_E::parseManifestAndLoadFiles(const ZipSerialize &z)
              * 6.2.2 Contents of Container
              * 3) The root element of each "*signatures*.xml" content shall be either:
              */
-            if(file.compare(0, 9, "META-INF/") == 0 &&
+            if(file.starts_with("META-INF/") &&
                file.find("signatures") != string::npos)
             {
                 try
@@ -229,9 +229,9 @@ void ASiC_E::parseManifestAndLoadFiles(const ZipSerialize &z)
                 continue;
             }
 
-            if(file == "mimetype" || file.compare(0, 8,"META-INF") == 0)
+            if(file == "mimetype" || file.starts_with("META-INF"))
                 continue;
-            if(manifestFiles.count(file) == 0)
+            if(!manifestFiles.contains(file))
                 THROW("File '%s' found in container is not described in manifest.", file.c_str());
         }
     }
diff --git a/src/ASiC_S.cpp b/src/ASiC_S.cpp
@@ -19,6 +19,7 @@
 
 #include "ASiC_S.h"
 
+#include "DataFile_p.h"
 #include "SignatureTST.h"
 #include "SignatureXAdES_LTA.h"
 #include "crypto/Signer.h"
@@ -78,7 +79,10 @@ ASiC_S::ASiC_S(const string &path)
         else if(!dataFiles().empty())
             THROW("Can not add document to ASiC-S container which already contains a document.");
         else
-            addDataFile(dataStream(file, z), file, "application/octet-stream");
+        {
+            addDataFileChecks(file, "application/octet-stream");
+            addDataFilePrivate(new DataFilePrivate(z, file, "application/octet-stream"));
+        }
     }
     if(foundTimestamp && !foundManifest)
     {
diff --git a/src/ASiContainer.cpp b/src/ASiContainer.cpp
@@ -36,8 +36,6 @@ using namespace digidoc;
 using namespace digidoc::util;
 using namespace std;
 
-constexpr unsigned long MAX_MEM_FILE = 500UL*1024UL*1024UL;
-
 class ASiContainer::Private
 {
 public:
@@ -139,23 +137,6 @@ vector<Signature *> ASiContainer::signatures() const
     return d->signatures;
 }
 
-/**
- * <p>
- * Read a datafile from container.
- * </p>
- * If expected size of the data is too big, then stream is written to temp file.
- *
- * @param path name of the file in zip container stream is used to read from.
- * @param z Zip container.
- * @return returns data as a stream.
- */
-unique_ptr<iostream> ASiContainer::dataStream(string_view path, const ZipSerialize &z) const
-{
-    if(auto i = d->properties.find(path); i != d->properties.cend() && i->second.size > MAX_MEM_FILE)
-        return make_unique<fstream>(z.extract<fstream>(path));
-    return make_unique<stringstream>(z.extract<stringstream>(path));
-}
-
 /**
  * Adds document to the container. Documents can be removed from container only
  * after all signatures are removed.
@@ -208,9 +189,9 @@ void ASiContainer::addDataFileChecks(const string &fileName, const string &media
         THROW("MediaType does not meet format requirements (RFC2045, section 5.1) '%s'.", mediaType.c_str());
 }
 
-void ASiContainer::addDataFilePrivate(unique_ptr<istream> is, string fileName, string mediaType)
+void ASiContainer::addDataFilePrivate(DataFile *dataFile)
 {
-    d->documents.push_back(new DataFilePrivate(std::move(is), std::move(fileName), std::move(mediaType)));
+    d->documents.push_back(dataFile);
 }
 
 /**
diff --git a/src/ASiContainer.h b/src/ASiContainer.h
@@ -64,11 +64,10 @@ namespace digidoc
           ASiContainer(std::string_view mimetype);
 
           virtual void addDataFileChecks(const std::string &path, const std::string &mediaType);
-          void addDataFilePrivate(std::unique_ptr<std::istream> is, std::string fileName, std::string mediaType);
+          void addDataFilePrivate(DataFile *dataFile);
           Signature* addSignature(std::unique_ptr<Signature> &&signature);
           virtual void canSave() = 0;
           XMLDocument createManifest() const;
-          std::unique_ptr<std::iostream> dataStream(std::string_view path, const ZipSerialize &z) const;
           ZipSerialize load(const std::string &path, bool requireMimetype, const std::set<std::string_view> &supported);
           virtual void save(const ZipSerialize &s) = 0;
           void deleteSignature(Signature* s);
diff --git a/src/DataFile.cpp b/src/DataFile.cpp
@@ -22,14 +22,15 @@
 #include "crypto/Digest.h"
 #include "util/File.h"
 #include "util/log.h"
+#include "util/ZipSerialize.h"
 
 #include <fstream>
+#include <sstream>
 
 using namespace digidoc;
 using namespace digidoc::util;
 using namespace std;
 
-
 /**
  * @class digidoc::DataFile
  *
@@ -96,6 +97,19 @@ DataFilePrivate::DataFilePrivate(unique_ptr<istream> &&is, string filename, stri
 {
 }
 
+DataFilePrivate::DataFilePrivate(const ZipSerialize &z, string filename, string mediatype)
+    : d(make_unique<Private>())
+    , m_filename(std::move(filename))
+    , m_mediatype(std::move(mediatype))
+{
+    auto prop = z.properties(m_filename);
+    d->size.emplace(prop.size);
+    if(d->size.value() > MAX_MEM_FILE)
+        m_is = make_unique<fstream>(z.extract<fstream>(m_filename));
+    else
+        m_is = make_unique<stringstream>(z.extract<stringstream>(m_filename));
+}
+
 void DataFilePrivate::digest(const Digest &digest) const
 {
     m_is->clear();
diff --git a/src/DataFile_p.h b/src/DataFile_p.h
@@ -27,12 +27,17 @@
 
 namespace digidoc
 {
+
+constexpr unsigned long MAX_MEM_FILE = 500UL*1024UL*1024UL;
+
 class Digest;
+class ZipSerialize;
 
 class DataFilePrivate final: public DataFile
 {
 public:
     DataFilePrivate(std::unique_ptr<std::istream> &&is, std::string filename, std::string mediatype, std::string id = {});
+    DataFilePrivate(const ZipSerialize &z, std::string filename, std::string mediatype);
 
     std::string id() const final { return m_id.empty() ? m_filename : m_id; }
     std::string fileName() const final { return m_filename; }
diff --git a/src/SiVaContainer.cpp b/src/SiVaContainer.cpp
@@ -59,12 +59,14 @@ static auto base64_decode(string_view data) {
         0x29, 0x2A, 0x2B, 0x2C, 0x2D, 0x2E, 0x2F, 0x30, 0x31, 0x32, 0x33, 0x64, 0x64, 0x64, 0x64, 0x64
     };
 
-    auto out = make_unique<stringstream>();
+    string buf;
+    buf.reserve(base64_enc_size(data.size()));
+    auto out = make_unique<stringstream>(std::move(buf));
     int value = 0;
     int bits = -8;
     for(auto c: data)
     {
-        if(c == '\r' || c == '\n' || c == ' ')
+        if(c == '\r' || c == '\n' || c == ' ' || static_cast<uint8_t>(c) > 128)
             continue;
         uint8_t check = T[c];
         if(check == 0x64)
@@ -83,8 +85,8 @@ static auto base64_decode(string_view data) {
 class SiVaContainer::Private
 {
 public:
-    string path, mediaType;
-    unique_ptr<istream> ddoc;
+    std::filesystem::path path;
+    string  mediaType;
     vector<DataFile*> dataFiles;
     vector<Signature*> signatures;
 };
@@ -109,26 +111,32 @@ void SignatureSiVa::validate(const string &policy) const
     static const set<string_view> QES = { "QESIG", "QES", "QESEAL",
         "ADESEAL_QC", "ADESEAL" }; // Special treamtent for E-Seals
     Exception e(EXCEPTION_PARAMS("Signature validation"));
-    for(const Exception &exception: _exceptions)
-        e.addCause(exception);
-    if(!Exception::hasWarningIgnore(Exception::SignatureDigestWeak) &&
-        Digest::isWeakDigest(_signatureMethod))
-    {
-        Exception ex(EXCEPTION_PARAMS("Signature digest weak"));
-        ex.setCode(Exception::SignatureDigestWeak);
-        e.addCause(ex);
-    }
-    if(_indication == "TOTAL-PASSED")
-    {
-        if(QES.count(_signatureLevel) || _signatureLevel.empty() || policy == POLv1)
+    try {
+        for(const Exception &exception: _exceptions)
+            e.addCause(exception);
+        if(!Exception::hasWarningIgnore(Exception::SignatureDigestWeak) &&
+            Digest::isWeakDigest(_signatureMethod))
+        {
+            Exception ex(EXCEPTION_PARAMS("Signature digest weak"));
+            ex.setCode(Exception::SignatureDigestWeak);
+            e.addCause(ex);
+        }
+        if(_indication == "TOTAL-PASSED")
         {
-            if(!e.causes().empty())
-                throw e;
-            return;
+            if(QES.contains(_signatureLevel) || _signatureLevel.empty() || policy == POLv1)
+            {
+                if(!e.causes().empty())
+                    throw e;
+                return;
+            }
+            Exception ex(EXCEPTION_PARAMS("Signing certificate does not meet Qualification requirements"));
+            ex.setCode(Exception::CertificateIssuerMissing);
+            e.addCause(ex);
         }
-        Exception ex(EXCEPTION_PARAMS("Signing certificate does not meet Qualification requirements"));
-        ex.setCode(Exception::CertificateIssuerMissing);
+    } catch(const Exception &ex) {
         e.addCause(ex);
+    } catch(...) {
+        EXCEPTION_ADD(e, "Failed to validate signature");
     }
     if(!e.causes().empty())
         throw e;
@@ -139,14 +147,13 @@ SiVaContainer::SiVaContainer(const string &path, ContainerOpenCB *cb, bool useHa
     : d(make_unique<Private>())
 {
     DEBUG("SiVaContainer::SiVaContainer(%s, %d)", path.c_str(), useHashCode);
-    unique_ptr<istream> ifs = make_unique<ifstream>(File::encodeName(d->path = path), ifstream::binary);
+    unique_ptr<istream> ifs = make_unique<ifstream>(d->path = File::encodeName(path), ifstream::binary);
     auto fileName = File::fileName(path);
     istream *is = ifs.get();
     if(File::fileExtension(path, {"ddoc"}))
     {
         d->mediaType = "application/x-ddoc";
-        d->ddoc = std::move(ifs);
-        ifs = parseDDoc(useHashCode);
+        ifs = parseDDoc(ifs, useHashCode);
         is = ifs.get();
     }
     else if(File::fileExtension(path, {"pdf"}))
@@ -156,7 +163,6 @@ SiVaContainer::SiVaContainer(const string &path, ContainerOpenCB *cb, bool useHa
     }
     else if(File::fileExtension(path, {"asice", "sce", "asics", "scs"}))
     {
-        static const string_view metaInf = "META-INF/";
         ZipSerialize z(path, false);
         vector<string> list = z.list();
         if(list.front() != "mimetype")
@@ -165,17 +171,17 @@ SiVaContainer::SiVaContainer(const string &path, ContainerOpenCB *cb, bool useHa
             d->mediaType != ASiContainer::MIMETYPE_ASIC_E && d->mediaType != ASiContainer::MIMETYPE_ASIC_S)
             THROW("Unknown file");
         if(none_of(list.cbegin(), list.cend(), [](const string &file) {
-                return file.rfind(metaInf, 0) == 0 && util::File::fileExtension(file, {"p7s"});
+                return file.starts_with("META-INF/") && util::File::fileExtension(file, {"p7s"});
             }))
             THROW("Unknown file");
 
         for(const string &file: list)
         {
-            if(file == "mimetype" || file.rfind(metaInf, 0) == 0)
+            if(file == "mimetype" || file.starts_with("META-INF/"))
                 continue;
             if(const auto directory = File::directory(file);
                 directory.empty() || directory == "/" || directory == "./")
-                d->dataFiles.push_back(new DataFilePrivate(make_unique<stringstream>(z.extract<stringstream>(file)), file, "application/octet-stream"));
+                d->dataFiles.push_back(new DataFilePrivate(z, file, "application/octet-stream"));
         }
     }
     else
@@ -339,11 +345,11 @@ unique_ptr<Container> SiVaContainer::openInternal(const string &path, ContainerO
     }
 }
 
-unique_ptr<istream> SiVaContainer::parseDDoc(bool useHashCode)
+unique_ptr<istream> SiVaContainer::parseDDoc(const unique_ptr<istream> &ddoc, bool useHashCode)
 {
     try
     {
-        auto doc = XMLDocument::openStream(*d->ddoc, {}, true);
+        auto doc = XMLDocument::openStream(*ddoc, {}, true);
         for(auto dataFile = doc/"DataFile"; dataFile; dataFile++)
         {
             auto contentType = dataFile["ContentType"];
@@ -401,16 +407,13 @@ void SiVaContainer::removeSignature(unsigned int /*index*/)
 
 void SiVaContainer::save(const string &path)
 {
-    string to = path.empty() ? d->path : path;
-    if(d->ddoc)
-    {
-        d->ddoc->clear();
-        d->ddoc->seekg(0);
-        if(ofstream out{File::encodeName(to), ofstream::binary})
-            out << d->ddoc->rdbuf();
-    }
-    else
-        d->dataFiles[0]->saveAs(to);
+    if(d->path.empty() || path.empty())
+        return; // This is readonly container
+    auto dest = File::encodeName(path);
+    if(std::error_code ec;
+        !std::filesystem::copy_file(d->path, dest, ec))
+        THROW("Failed to save container");
+    d->path = std::move(dest);
 }
 
 Signature *SiVaContainer::sign(Signer * /*signer*/)
diff --git a/src/SiVaContainer.h b/src/SiVaContainer.h
@@ -105,7 +105,7 @@ class SiVaContainer final: public Container
     SiVaContainer(const std::string &path, ContainerOpenCB *cb, bool useHashCode);
     DISABLE_COPY(SiVaContainer);
 
-    std::unique_ptr<std::istream> parseDDoc(bool useHashCode);
+    std::unique_ptr<std::istream> parseDDoc(const std::unique_ptr<std::istream> &ddoc, bool useHashCode);
 
     class Private;
     std::unique_ptr<Private> d;
diff --git a/src/util/ZipSerialize.cpp b/src/util/ZipSerialize.cpp
@@ -198,7 +198,7 @@ ZipSerialize::Properties ZipSerialize::properties(const string &file) const
 {
     int unzResult = unzLocateFile(d.get(), file.c_str(), 1);
     if(unzResult != UNZ_OK)
-        THROW("Failed to open file inside ZIP container. ZLib error: %d", unzResult);
+        THROW("Failed to locate '%s' inside ZIP container. ZLib error: %d", file.c_str(), unzResult);
 
     unz_file_info info;
     unzResult = unzGetCurrentFileInfo(d.get(), &info, nullptr, 0, nullptr, 0, nullptr, 0);
diff --git a/test/libdigidocpp_boost.cpp b/test/libdigidocpp_boost.cpp
@@ -335,6 +335,8 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(signature, Doc, DocTypes)
         // Reload from file and validate
         d = Container::openPtr(Doc::EXT + ".tmp");
         BOOST_CHECK_EQUAL(d->signatures().size(), 2U);
+        if(d->signatures().empty())
+            return;
         if(s3 = d->signatures().back(); s3)
         {
             BOOST_CHECK_EQUAL(s3->signingCertificate(), signer3.cert());
@@ -401,6 +403,8 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(files, Doc, DocTypes)
         d->save(data + Doc::EXT + ".tmp");
         d = Container::openPtr(data + Doc::EXT + ".tmp");
         BOOST_CHECK_EQUAL(d->signatures().size(), 1U);
+        if(d->signatures().empty())
+            return;
         s1 = d->signatures().front();
         s1->validate();
     }

Original file line number	Diff line number	Diff line change
`@@ -198,7 +198,7 @@ ZipSerialize::Properties ZipSerialize::properties(const string &file) const`
`198`	`198`	`{`
`199`	`199`	`int unzResult = unzLocateFile(d.get(), file.c_str(), 1);`
`200`	`200`	`if(unzResult != UNZ_OK)`
`201`		`- THROW("Failed to open file inside ZIP container. ZLib error: %d", unzResult);`
	`201`	`+ THROW("Failed to locate '%s' inside ZIP container. ZLib error: %d", file.c_str(), unzResult);`
`202`	`202`
`203`	`203`	`unz_file_info info;`
`204`	`204`	`unzResult = unzGetCurrentFileInfo(d.get(), &info, nullptr, 0, nullptr, 0, nullptr, 0);`