Add .gitignore

The code has even turned out simpler.

.clang-format

@@ -0,0 +1,8 @@
+BasedOnStyle: Chromium
+ColumnLimit: 100
+IndentCaseLabels: false
+AccessModifierOffset: -2
+ContinuationIndentWidth: 2
+SpaceAfterTemplateKeyword: false
+SpaceAfterCStyleCast: true
+SpacesBeforeTrailingComments: 2

.gitignore

@@ -0,0 +1,8 @@
+/builddir
+/pdf-simple-sign.cflags
+/pdf-simple-sign.config
+/pdf-simple-sign.creator
+/pdf-simple-sign.creator.user
+/pdf-simple-sign.cxxflags
+/pdf-simple-sign.files
+/pdf-simple-sign.includes

NEWS

@@ -1,3 +1,12 @@
+1.1.1 (2020-09-06)
+
+ * Fix a dysfunctional example in the manual
+
+ * Go: write the xref table in a deterministic order
+
+ * Add a trivial test suite, based on pdfsig from poppler-utils
+
+
 1.1 (2020-09-05)
 
  * Make it possible to change the signature reservation with an option

README.adoc

@@ -12,6 +12,8 @@ Documentation
 See the link:pdf-simple-sign.adoc[man page] for information about usage.
 The rest of this README will concern itself with externalities.
 
+image:https://pkg.go.dev/badge/janouch.name/pdf-simple-sign@master/pdf["PkgGoDev", link="https://pkg.go.dev/janouch.name/pdf-simple-sign@master/pdf"]
+
 Building
 --------
 Build dependencies: Meson, Asciidoctor, a C++11 compiler, pkg-config +

go.mod

@@ -1,6 +1,6 @@
 module janouch.name/pdf-simple-sign
 
-go 1.14
+go 1.17
 
 require (
 	go.mozilla.org/pkcs7 v0.0.0-20200128120323-432b2356ecb1

go.sum

@@ -1,9 +1,5 @@
-go.mozilla.org/pkcs7 v0.0.0-20181029144607-24857c352dd8 h1:W3oGFPlHBLgXdsbPVixWFMYsuPhm81/Qww3XAgBbn/0=
-go.mozilla.org/pkcs7 v0.0.0-20181029144607-24857c352dd8/go.mod h1:5fWP3IVYEMc04wC+lMJAfkmNmKAl2P1swVv8VS+URZ8=
 go.mozilla.org/pkcs7 v0.0.0-20200128120323-432b2356ecb1 h1:A/5uWzF44DlIgdm/PQFwfMkW0JX+cIcQi/SwLAmZP5M=
 go.mozilla.org/pkcs7 v0.0.0-20200128120323-432b2356ecb1/go.mod h1:SNgMg+EgDFwmvSmLRTNKC5fegJjB7v23qTQ0XLGUNHk=
-golang.org/x/crypto v0.0.0-20181127143415-eb0de9b17e85 h1:et7+NAX3lLIk5qUCTA9QelBjGE/NkhzYw/mhnr0s7nI=
-golang.org/x/crypto v0.0.0-20181127143415-eb0de9b17e85/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20200728195943-123391ffb6de h1:ikNHVSjEfnvz6sxdSPCaPt572qowuyMDMJLLm3Db3ig=
 golang.org/x/crypto v0.0.0-20200728195943-123391ffb6de/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=

meson.build

@@ -1,9 +1,9 @@
 project('pdf-simple-sign', 'cpp', default_options : ['cpp_std=c++11'],
-	version : '1.1')
+	version : '1.1.1')
 
 conf = configuration_data()
-conf.set('PROJECT_NAME', '"' + meson.project_name() + '"')
-conf.set('PROJECT_VERSION', '"' + meson.project_version() + '"')
+conf.set_quoted('PROJECT_NAME', meson.project_name())
+conf.set_quoted('PROJECT_VERSION', meson.project_version())
 configure_file(output : 'config.h', configuration : conf)
 
 cryptodep = dependency('libcrypto')

pdf-simple-sign.adoc

@@ -50,10 +50,11 @@ Examples
 Create a self-signed certificate, make a document containing the current date,
 sign it and verify the attached signature:
 
- $ openssl req -newkey rsa:2048 -subj "/CN=Test" -nodes
-   -keyout key.pem -x509 -out cert.pem 2>/dev/null
+ $ openssl req -newkey rsa:2048 -subj /CN=Test -nodes \
+   -keyout key.pem -x509 -addext keyUsage=digitalSignature \
+   -out cert.pem 2>/dev/null
  $ openssl pkcs12 -inkey key.pem -in cert.pem \
-   -export -passout pass:test -out key-cert.p12
+   -export -passout pass: -out key-pair.p12
  $ date | groff -T pdf > test.pdf
  $ pdf-simple-sign test.pdf test.signed.pdf key-pair.p12 ""
  $ pdfsig test.signed.pdf

pdf-simple-sign.cpp

@@ -16,26 +16,26 @@
 // CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 //
 
-#include <cstdio>
 #include <cmath>
+#include <cstdio>
 #undef NDEBUG
 #include <cassert>
 
-#include <vector>
 #include <map>
-#include <regex>
 #include <memory>
+#include <regex>
 #include <set>
+#include <vector>
 
 #if defined __GLIBCXX__ && __GLIBCXX__ < 20140422
 #error Need libstdc++ >= 4.9 for <regex>
 #endif
 
-#include <unistd.h>
 #include <getopt.h>
 #include <openssl/err.h>
-#include <openssl/x509v3.h>
 #include <openssl/pkcs12.h>
+#include <openssl/x509v3.h>
+#include <unistd.h>
 
 #include "config.h"
 
@@ -55,7 +55,7 @@ static std::string concatenate(const std::vector<std::string>& v, const std::str
 
 template<typename... Args>
 std::string ssprintf(const std::string& format, Args... args) {
-  size_t size = std::snprintf(nullptr, 0, format.c_str(), args... ) + 1;
+  size_t size = std::snprintf(nullptr, 0, format.c_str(), args...) + 1;
   std::unique_ptr<char[]> buf(new char[size]);
   std::snprintf(buf.get(), size, format.c_str(), args...);
   return std::string(buf.get(), buf.get() + size - 1);
@@ -262,14 +262,12 @@ static std::string pdf_serialize(const pdf_object& o) {
   case pdf_object::NL:      return "\n";
   case pdf_object::NIL:     return "null";
   case pdf_object::BOOL:    return o.number ? "true" : "false";
-  case pdf_object::NUMERIC:
-  {
+  case pdf_object::NUMERIC: {
     if (o.is_integer()) return std::to_string((long long) o.number);
     return std::to_string(o.number);
   }
   case pdf_object::KEYWORD: return o.string;
-  case pdf_object::NAME:
-  {
+  case pdf_object::NAME: {
     std::string escaped = "/";
     for (char c : o.string) {
       if (c == '#' || strchr(pdf_lexer::delimiters, c) || strchr(pdf_lexer::whitespace, c))
@@ -279,8 +277,7 @@ static std::string pdf_serialize(const pdf_object& o) {
     }
     return escaped;
   }
-  case pdf_object::STRING:
-  {
+  case pdf_object::STRING: {
     std::string escaped;
     for (char c : o.string) {
       if (c == '\\' || c == '(' || c == ')')
@@ -293,15 +290,13 @@ static std::string pdf_serialize(const pdf_object& o) {
   case pdf_object::E_ARRAY: return "]";
   case pdf_object::B_DICT:  return "<<";
   case pdf_object::E_DICT:  return ">>";
-  case pdf_object::ARRAY:
-  {
+  case pdf_object::ARRAY: {
     std::vector<std::string> v;
     for (const auto& i : o.array)
       v.push_back(pdf_serialize(i));
     return "[ " + concatenate(v, " ") + " ]";
   }
-  case pdf_object::DICT:
-  {
+  case pdf_object::DICT: {
     std::string s;
     for (const auto i : o.dict)
       // FIXME the key is also supposed to be escaped by pdf_serialize()
@@ -372,8 +367,8 @@ pdf_object pdf_updater::parse_obj(pdf_lexer& lex, std::vector<pdf_object>& stack
 
   auto g = stack.back(); stack.pop_back();
   auto n = stack.back(); stack.pop_back();
-  if (!g.is_integer() || g.number < 0 || g.number > UINT_MAX
-   || !n.is_integer() || n.number < 0 || n.number > UINT_MAX)
+  if (!g.is_integer() || g.number < 0 || g.number > UINT_MAX ||
+      !n.is_integer() || n.number < 0 || n.number > UINT_MAX)
     return {pdf_object::END, "invalid object ID pair"};
 
   pdf_object obj{pdf_object::OBJECT};
@@ -397,8 +392,8 @@ pdf_object pdf_updater::parse_R(std::vector<pdf_object>& stack) const {
 
   auto g = stack.back(); stack.pop_back();
   auto n = stack.back(); stack.pop_back();
-  if (!g.is_integer() || g.number < 0 || g.number > UINT_MAX
-   || !n.is_integer() || n.number < 0 || n.number > UINT_MAX)
+  if (!g.is_integer() || g.number < 0 || g.number > UINT_MAX ||
+      !n.is_integer() || n.number < 0 || n.number > UINT_MAX)
     return {pdf_object::END, "invalid reference ID pair"};
 
   pdf_object ref{pdf_object::REFERENCE};
@@ -415,8 +410,7 @@ pdf_object pdf_updater::parse(pdf_lexer& lex, std::vector<pdf_object>& stack) co
   case pdf_object::COMMENT:
     // These are not important to parsing, not even for this procedure's needs
     return parse(lex, stack);
-  case pdf_object::B_ARRAY:
-  {
+  case pdf_object::B_ARRAY: {
     std::vector<pdf_object> array;
     while (1) {
       auto object = parse(lex, array);
@@ -428,8 +422,7 @@ pdf_object pdf_updater::parse(pdf_lexer& lex, std::vector<pdf_object>& stack) co
     }
     return array;
   }
-  case pdf_object::B_DICT:
-  {
+  case pdf_object::B_DICT: {
     std::vector<pdf_object> array;
     while (1) {
       auto object = parse(lex, array);
@@ -477,8 +470,8 @@ std::string pdf_updater::load_xref(pdf_lexer& lex, std::set<uint>& loaded_entrie
       break;
 
     auto second = parse(lex, throwaway_stack);
-    if (!object.is_integer() || object.number < 0 || object.number > UINT_MAX
-     || !second.is_integer() || second.number < 0 || second.number > UINT_MAX)
+    if (!object.is_integer() || object.number < 0 || object.number > UINT_MAX ||
+        !second.is_integer() || second.number < 0 || second.number > UINT_MAX)
       return "invalid xref section header";
 
     const size_t start = object.number;
@@ -487,9 +480,9 @@ std::string pdf_updater::load_xref(pdf_lexer& lex, std::set<uint>& loaded_entrie
       auto off = parse(lex, throwaway_stack);
       auto gen = parse(lex, throwaway_stack);
       auto key = parse(lex, throwaway_stack);
-      if (!off.is_integer() || off.number < 0 || off.number > document.length()
-       || !gen.is_integer() || gen.number < 0 || gen.number > 65535
-       || key.type != pdf_object::KEYWORD)
+      if (!off.is_integer() || off.number < 0 || off.number > document.length() ||
+          !gen.is_integer() || gen.number < 0 || gen.number > 65535 ||
+          key.type != pdf_object::KEYWORD)
         return "invalid xref entry";
 
       bool free = true;
@@ -657,8 +650,8 @@ void pdf_updater::flush_updates() {
   }
 
   trailer["Size"] = {pdf_object::NUMERIC, double(xref_size)};
-  document += "trailer\n" + pdf_serialize(trailer)
-    + ssprintf("\nstartxref\n%zu\n%%%%EOF\n", startxref);
+  document +=
+    "trailer\n" + pdf_serialize(trailer) + ssprintf("\nstartxref\n%zu\n%%%%EOF\n", startxref);
 }
 
 // -------------------------------------------------------------------------------------------------
@@ -700,9 +693,9 @@ static pdf_object pdf_get_first_page(pdf_updater& pdf, uint node_n, uint node_ge
   // XXX technically speaking, this may be an indirect reference.  The correct way to solve this
   //   seems to be having "pdf_updater" include a wrapper around "obj.dict.find"
   auto kids = obj.dict.find("Kids");
-  if (kids == obj.dict.end() || kids->second.type != pdf_object::ARRAY
-   || kids->second.array.empty()
-   || kids->second.array.at(0).type != pdf_object::REFERENCE)
+  if (kids == obj.dict.end() || kids->second.type != pdf_object::ARRAY ||
+      kids->second.array.empty() ||
+      kids->second.array.at(0).type != pdf_object::REFERENCE)
     return {pdf_object::NIL};
 
   // XXX nothing prevents us from recursing in an evil circular graph
@@ -740,8 +733,8 @@ static std::string pdf_fill_in_signature(std::string& document, size_t sign_off,
   // OpenSSL error reasons will usually be of more value than any distinction I can come up with
   std::string err = "OpenSSL failure";
 
-  if (!(p12 = d2i_PKCS12_fp(pkcs12_fp, nullptr))
-   || !PKCS12_parse(p12, pkcs12_pass.c_str(), &private_key, &certificate, &chain)) {
+  if (!(p12 = d2i_PKCS12_fp(pkcs12_fp, nullptr)) ||
+      !PKCS12_parse(p12, pkcs12_pass.c_str(), &private_key, &certificate, &chain)) {
     err = pkcs12_path + ": parse failure";
     goto error;
   }
@@ -766,8 +759,8 @@ static std::string pdf_fill_in_signature(std::string& document, size_t sign_off,
 #endif
 
   // The default digest is SHA1, which is mildly insecure now -- hence using PKCS7_sign_add_signer
-  if (!(p7 = PKCS7_sign(nullptr, nullptr, nullptr, nullptr, sign_flags))
-   || !PKCS7_sign_add_signer(p7, certificate, private_key, EVP_sha256(), sign_flags))
+  if (!(p7 = PKCS7_sign(nullptr, nullptr, nullptr, nullptr, sign_flags)) ||
+      !PKCS7_sign_add_signer(p7, certificate, private_key, EVP_sha256(), sign_flags))
     goto error;
   // For RFC 3161, this is roughly how a timestamp token would be attached (see Appendix A):
   //   PKCS7_add_attribute(signer_info, NID_id_smime_aa_timeStampToken, V_ASN1_SEQUENCE, value)
@@ -777,10 +770,10 @@ static std::string pdf_fill_in_signature(std::string& document, size_t sign_off,
 
   // Adaptation of the innards of the undocumented PKCS7_final() -- I didn't feel like making
   // a copy of the whole document.  Hopefully this writes directly into a digest BIO.
-  if (!(p7bio = PKCS7_dataInit(p7, nullptr))
-   || (ssize_t) sign_off != BIO_write(p7bio, document.data(), sign_off)
-   || (ssize_t) tail_len != BIO_write(p7bio, document.data() + tail_off, tail_len)
-   || BIO_flush(p7bio) != 1 || !PKCS7_dataFinal(p7, p7bio))
+  if (!(p7bio = PKCS7_dataInit(p7, nullptr)) ||
+      (ssize_t) sign_off != BIO_write(p7bio, document.data(), sign_off) ||
+      (ssize_t) tail_len != BIO_write(p7bio, document.data() + tail_off, tail_len) ||
+      BIO_flush(p7bio) != 1 || !PKCS7_dataFinal(p7, p7bio))
     goto error;
 
 #if 0
@@ -850,7 +843,7 @@ static std::string pdf_sign(std::string& document, ushort reservation) {
   // 8.7 Digital Signatures - /signature dictionary/
   auto sigdict_n = pdf.allocate();
   size_t byterange_off = 0, byterange_len = 0, sign_off = 0, sign_len = 0;
-  pdf.update(sigdict_n, [&]{
+  pdf.update(sigdict_n, [&] {
     // The timestamp is important for Adobe Acrobat Reader DC.  The ideal would be to use RFC 3161.
     pdf.document.append("<< /Type/Sig /Filter/Adobe.PPKLite /SubFilter/adbe.pkcs7.detached\n"
                         "   /M" + pdf_serialize(pdf_date(time(nullptr))) + " /ByteRange ");
@@ -883,7 +876,7 @@ static std::string pdf_sign(std::string& document, ushort reservation) {
   }}});
 
   auto sigfield_n = pdf.allocate();
-  pdf.update(sigfield_n, [&]{ pdf.document += pdf_serialize(sigfield); });
+  pdf.update(sigfield_n, [&] { pdf.document += pdf_serialize(sigfield); });
 
   auto pages_ref = root.dict.find("Pages");
   if (pages_ref == root.dict.end() || pages_ref->second.type != pdf_object::REFERENCE)
@@ -901,7 +894,7 @@ static std::string pdf_sign(std::string& document, ushort reservation) {
     annots = {pdf_object::ARRAY};
   }
   annots.array.emplace_back(pdf_object::REFERENCE, sigfield_n, 0);
-  pdf.update(page.n, [&]{ pdf.document += pdf_serialize(page); });
+  pdf.update(page.n, [&] { pdf.document += pdf_serialize(page); });
 
   // 8.6.1 Interactive Form Dictionary
   if (root.dict.count("AcroForm"))
@@ -918,7 +911,7 @@ static std::string pdf_sign(std::string& document, ushort reservation) {
   if (pdf.version(root) < 16)
     root.dict["Version"] = {pdf_object::NAME, "1.6"};
 
-  pdf.update(root_ref->second.n, [&]{ pdf.document += pdf_serialize(root); });
+  pdf.update(root_ref->second.n, [&] { pdf.document += pdf_serialize(root); });
   pdf.flush_updates();
 
   // Now that we know the length of everything, store byte ranges of what we're about to sign,
@@ -947,9 +940,9 @@ static void die(int status, const char* format, ...) {
 
 int main(int argc, char* argv[]) {
   auto invocation_name = argv[0];
-  auto usage = [=]{
+  auto usage = [=] {
     die(1, "Usage: %s [-h] [-r RESERVATION] INPUT-FILENAME OUTPUT-FILENAME PKCS12-PATH PKCS12-PASS",
-            invocation_name);
+        invocation_name);
   };
 
   static struct option opts[] = {
@@ -963,8 +956,7 @@ int main(int argc, char* argv[]) {
   long reservation = 4096;
   while (1) {
     int option_index = 0;
-    auto c = getopt_long(argc, const_cast<char* const*>(argv),
-                         "hVr:", opts, &option_index);
+    auto c = getopt_long(argc, const_cast<char* const*>(argv), "hVr:", opts, &option_index);
     if (c == -1)
       break;
 

pdf/pdf.go

@@ -853,30 +853,19 @@ func (u *Updater) FlushUpdates() {
 		return updated[i] < updated[j]
 	})
 
-	groups := make(map[uint]uint)
-	for i := 0; i < len(updated); {
-		start, count := updated[i], uint(1)
-		for i++; i != len(updated) && updated[i] == start+count; i++ {
-			count++
-		}
-		groups[start] = count
-	}
-
-	// Taking literally "Each cross-reference section begins with a line
-	// containing the keyword xref. Following this line are one or more
-	// cross-reference subsections." from 3.4.3 in PDF Reference.
-	if len(groups) == 0 {
-		groups[0] = 0
-	}
-
 	buf := bytes.NewBuffer(u.Document)
 	startXref := buf.Len() + 1
 	buf.WriteString("\nxref\n")
 
-	for start, count := range groups {
-		fmt.Fprintf(buf, "%d %d\n", start, count)
-		for i := uint(0); i < count; i++ {
-			ref := u.xref[start+uint(i)]
+	for i := 0; i < len(updated); {
+		start, stop := updated[i], updated[i]+1
+		for i++; i < len(updated) && updated[i] == stop; i++ {
+			stop++
+		}
+
+		fmt.Fprintf(buf, "%d %d\n", start, stop-start)
+		for ; start < stop; start++ {
+			ref := u.xref[start]
 			if ref.nonfree {
 				fmt.Fprintf(buf, "%010d %05d n \n", ref.offset, ref.generation)
 			} else {
@@ -885,6 +874,13 @@ func (u *Updater) FlushUpdates() {
 		}
 	}
 
+	// Taking literally "Each cross-reference section begins with a line
+	// containing the keyword xref. Following this line are one or more
+	// cross-reference subsections." from 3.4.3 in PDF Reference.
+	if len(updated) == 0 {
+		fmt.Fprintf(buf, "%d %d\n", 0, 0)
+	}
+
 	u.Trailer["Size"] = NewNumeric(float64(u.xrefSize))
 	trailer := NewDict(u.Trailer)
 

test.sh

@@ -0,0 +1,77 @@
+#!/bin/sh -e
+# Test basic functionality of both versions
+# Usage: ./test.sh builddir/pdf-simple-sign cmd/pdf-simple-sign/pdf-simple-sign
+
+log() { echo "`tput sitm`-- $1`tput sgr0`"; }
+die() { echo "`tput bold`-- $1`tput sgr0`"; exit 1; }
+
+# Get rid of old test files
+rm -rf tmp
+mkdir tmp
+
+# Create documents in various tools
+log "Creating source documents"
+inkscape --pipe --export-filename=tmp/cairo.pdf <<'EOF' 2>/dev/null || :
+<svg xmlns="http://www.w3.org/2000/svg"><text x="5" y="10">Hello</text></svg>
+EOF
+
+date | tee tmp/lowriter.txt | groff -T pdf > tmp/groff.pdf || :
+lowriter --convert-to pdf tmp/lowriter.txt --outdir tmp >/dev/null || :
+convert rose: tmp/imagemagick.pdf || :
+
+# Create a root CA certificate pair
+log "Creating certificates"
+openssl req -newkey rsa:2048 -subj "/CN=Test CA" -nodes \
+	-keyout tmp/ca.key.pem -x509 -out tmp/ca.cert.pem 2>/dev/null
+
+# Create a private NSS database and insert our test CA there
+rm -rf tmp/nssdir
+mkdir tmp/nssdir
+certutil -N --empty-password -d sql:tmp/nssdir
+certutil -d sql:tmp/nssdir -A -n root -t ,C, -a -i tmp/ca.cert.pem
+
+# Create a leaf certificate pair
+cat > tmp/cert.cfg <<'EOF'
+[smime]
+basicConstraints = CA:FALSE
+keyUsage = digitalSignature
+extendedKeyUsage = emailProtection
+nsCertType = email
+EOF
+
+openssl req -newkey rsa:2048 -subj "/CN=Test Leaf" -nodes \
+	-keyout tmp/key.pem -out tmp/cert.csr 2>/dev/null
+openssl x509 -req -in tmp/cert.csr -out tmp/cert.pem \
+	-CA tmp/ca.cert.pem -CAkey tmp/ca.key.pem -set_serial 1 \
+	-extensions smime -extfile tmp/cert.cfg 2>/dev/null
+openssl verify -CAfile tmp/ca.cert.pem tmp/cert.pem >/dev/null
+openssl pkcs12 -inkey tmp/key.pem -in tmp/cert.pem \
+	-export -passout pass: -out tmp/key-pair.p12
+
+for tool in "$@"; do
+	rm -f tmp/*.signed.pdf
+	for source in tmp/*.pdf; do
+		log "Testing $tool with $source"
+		result=${source%.pdf}.signed.pdf
+		$tool "$source" "$result" tmp/key-pair.p12 ""
+		pdfsig -nssdir sql:tmp/nssdir "$result" | grep Validation
+	done
+
+	log "Testing $tool for expected failures"
+	$tool "$result" "$source.fail.pdf" tmp/key-pair.p12 "" \
+		&& die "Double signing shouldn't succeed"
+	$tool -r 1 "$source" "$source.fail.pdf" tmp/key-pair.p12 "" \
+		&& die "Too low reservations shouldn't succeed"
+
+	# Our generators do not use PDF versions higher than 1.5
+	log "Testing $tool for version detection"
+	grep -q "/Version /1.6" "$result" \
+		|| die "Version detection seems to misbehave (no upgrade)"
+
+	sed '1s/%PDF-1../%PDF-1.7/' "$source" > "$source.alt"
+	$tool "$source.alt" "$result.alt" tmp/key-pair.p12 ""
+	grep -q "/Version /1.6" "$result.alt" \
+		&& die "Version detection seems to misbehave (downgraded)"
+done
+
+log "OK"