aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKaren Arutyunov <karen@codesynthesis.com>2017-05-30 14:44:49 +0300
committerKaren Arutyunov <karen@codesynthesis.com>2017-05-31 00:55:22 +0300
commitbe8ddf26165f25d323657c1e6553af9b42b6d6bf (patch)
tree759584203412294b7b43cf4e05b9f0431c3ec40b
parent52c0019692491173b934111c31c2a0bba84c6d71 (diff)
Make use of butl::openssl, ifdstream::read_text() and ifdstream::read_binary()
-rw-r--r--bpkg/archive.cxx3
-rw-r--r--bpkg/auth.cxx463
-rw-r--r--bpkg/buildfile1
-rw-r--r--bpkg/diagnostics.cxx12
-rw-r--r--bpkg/diagnostics.hxx12
-rw-r--r--bpkg/openssl.cxx60
-rw-r--r--bpkg/openssl.hxx31
-rw-r--r--tests/rep-auth.test2
8 files changed, 257 insertions, 327 deletions
diff --git a/bpkg/archive.cxx b/bpkg/archive.cxx
index ce62fab..f3c41f8 100644
--- a/bpkg/archive.cxx
+++ b/bpkg/archive.cxx
@@ -144,8 +144,7 @@ namespace bpkg
//
ifdstream is (move (pr.second.in_ofd), ifdstream::badbit);
- string s;
- getline (is, s, '\0');
+ string s (is.read_text ());
is.close ();
if (pr.second.wait () && pr.first.wait ())
diff --git a/bpkg/auth.cxx b/bpkg/auth.cxx
index ceacabe..4faf9ed 100644
--- a/bpkg/auth.cxx
+++ b/bpkg/auth.cxx
@@ -7,15 +7,15 @@
#include <ratio>
#include <limits> // numeric_limits
#include <cstring> // strlen(), strcmp()
-#include <iterator> // ostreambuf_iterator, istreambuf_iterator
+#include <iterator> // ostreambuf_iterator
#include <libbutl/sha256.hxx>
#include <libbutl/base64.hxx>
#include <libbutl/process.hxx>
+#include <libbutl/openssl.hxx>
#include <libbutl/fdstream.hxx>
#include <libbutl/filesystem.hxx>
-#include <bpkg/openssl.hxx>
#include <bpkg/package.hxx>
#include <bpkg/package-odb.hxx>
#include <bpkg/database.hxx>
@@ -26,6 +26,15 @@ using namespace butl;
namespace bpkg
{
+ // Print process command line.
+ //
+ static void
+ print_command (const char* const args[], size_t n)
+ {
+ if (verb >= 2)
+ print_process (args, n);
+ }
+
// Find the repository location prefix that ends with the version component.
// We consider all repositories under this location to be related.
//
@@ -112,60 +121,58 @@ namespace bpkg
{
tracer trace ("real_fingerprint");
- try
+ auto calc_failed = [&rl] (const exception* e = nullptr)
{
- process pr (start_openssl (
- co, "x509", {"-sha256", "-noout", "-fingerprint"}, true, true));
+ diag_record dr (error);
+ dr << "unable to calculate certificate fingerprint for "
+ << rl.canonical_name ();
- try
- {
- ifdstream is (move (pr.in_ofd), fdstream_mode::skip);
- ofdstream os (move (pr.out_fd));
- os << pem;
- os.close ();
+ if (e != nullptr)
+ dr << ": " << *e;
+ };
- string s;
- getline (is, s);
- is.close ();
+ try
+ {
+ openssl os (print_command,
+ fdstream_mode::text, fdstream_mode::text, 2,
+ co.openssl (), "x509",
+ co.openssl_option (), "-sha256", "-noout", "-fingerprint");
- const size_t n (19);
- if (!(s.size () > n && s.compare (0, n, "SHA256 Fingerprint=") == 0))
- throw io_error ("");
+ os.out << pem;
+ os.out.close ();
- string fp;
+ string s;
+ getline (os.in, s);
+ os.in.close ();
- try
- {
- fp = fingerprint_to_sha256 (string (s, n));
- }
- catch (const invalid_argument&)
- {
- throw io_error ("");
- }
-
- if (pr.wait ())
- return fp;
+ try
+ {
+ const size_t n (19);
- // Fall through.
- //
+ if (os.wait () &&
+ s.size () > n && s.compare (0, n, "SHA256 Fingerprint=") == 0)
+ return fingerprint_to_sha256 (string (s, n));
}
- catch (const io_error&)
+ catch (const invalid_argument&)
{
- // Child exit status doesn't matter. Just wait for the process
- // completion and fall through.
- //
- pr.wait (); // Check throw.
}
- error << "unable to calculate certificate fingerprint for "
- << rl.canonical_name ();
+ calc_failed ();
+
+ // Fall through.
+ }
+ // For old versions of g++ (as of 4.9) ios_base::failure is not derived
+ // from system_error.
+ //
+ catch (const io_error& e)
+ {
+ calc_failed (&e);
// Fall through.
}
- catch (const process_error& e)
+ catch (const system_error& e)
{
- error << "unable to calculate certificate fingerprint for "
- << rl.canonical_name () << ": " << e;
+ calc_failed (&e);
// Fall through.
}
@@ -196,6 +203,15 @@ namespace bpkg
{
tracer trace ("parse_cert");
+ auto parse_failed = [&repo] (const exception* e = nullptr)
+ {
+ diag_record dr (error);
+ dr << "unable to parse certificate for " << repo;
+
+ if (e != nullptr)
+ dr << ": " << *e;
+ };
+
try
{
// The order of the options we pass to openssl determines the order in
@@ -217,76 +233,72 @@ namespace bpkg
// The final line should be the email but will be silently missing if
// the cert has no email.
//
- process pr (start_openssl (
- co,
- "x509",
- {
- "-noout",
- "-subject",
- "-dates",
- "-email",
-
- // Previously we have used "RFC2253,sep_multiline" format to display
- // the requested fields, but that resulted in some undesirable
- // behavior like escaping commas (\,) while dispaying only one field
- // per line. The reason for that is RFC2253 specifier which get
- // expanded into:
- //
- // esc_2253,esc_ctrl,esc_msb,utf8,dump_nostr,dump_unknown,dump_der,
- // sep_comma_plus,dn_rev,sname.
- //
- // Now we filtered them and leave just those specifiers that we
- // really need:
- //
- // utf8 - use UTF8 encoding for strings;
- //
- // esc_ctrl - display control characters in \XX notation (we
- // don't expect them in properly created
- // certificates, but it's better to print this way if
- // they appear);
- //
- // sname - use short form for field names (like
- // "O=Code Synthesis" vs
- // "organizationName=Code Synthesis");
- //
- // dump_nostr - do not print any binary data in the binary form;
- // dump_der
- //
- // sep_multiline - display field per line.
- //
- "-nameopt", "utf8,esc_ctrl,dump_nostr,dump_der,sname,sep_multiline"
- },
- true,
- true));
-
- try
- {
- // We unset failbit to provide the detailed error description (which
- // certificate field is missed) on failure.
+ openssl os (
+ print_command,
+ fdstream_mode::text, fdstream_mode::text, 2,
+ co.openssl (), "x509",
+ co.openssl_option (),
+ "-noout",
+ "-subject",
+ "-dates",
+ "-email",
+
+ // Previously we have used "RFC2253,sep_multiline" format to display
+ // the requested fields, but that resulted in some undesirable
+ // behavior like escaping commas (\,) while dispaying only one field
+ // per line. The reason for that is RFC2253 specifier which get
+ // expanded into:
//
- ifdstream is (
- move (pr.in_ofd), fdstream_mode::skip, ifdstream::badbit);
-
- ofdstream os (move (pr.out_fd));
-
- // Reading from and writing to the child process standard streams from
- // the same thread is generally a bad idea. Depending on the program
- // implementation we can block on writing if the process input pipe
- // buffer get filled. That can happen if the process do not read
- // anymore, being blocked on writing to the filled output pipe, which
- // get filled not being read on the other end.
+ // esc_2253,esc_ctrl,esc_msb,utf8,dump_nostr,dump_unknown,dump_der,
+ // sep_comma_plus,dn_rev,sname.
+ //
+ // Now we filtered them and leave just those specifiers that we
+ // really need:
+ //
+ // utf8 - use UTF8 encoding for strings;
+ //
+ // esc_ctrl - display control characters in \XX notation (we
+ // don't expect them in properly created
+ // certificates, but it's better to print this way if
+ // they appear);
+ //
+ // sname - use short form for field names (like
+ // "O=Code Synthesis" vs
+ // "organizationName=Code Synthesis");
//
- // Fortunatelly openssl reads the certificate before performing any
- // output.
+ // dump_nostr - do not print any binary data in the binary form;
+ // dump_der
//
- os << pem;
- os.close ();
+ // sep_multiline - display field per line.
+ //
+ "-nameopt", "utf8,esc_ctrl,dump_nostr,dump_der,sname,sep_multiline"
+ );
+ // We unset failbit to provide the detailed error description (which
+ // certificate field is missed) on failure.
+ //
+ os.in.exceptions (ifdstream::badbit);
+
+ // Reading from and writing to the child process standard streams from
+ // the same thread is generally a bad idea. Depending on the program
+ // implementation we can block on writing if the process input pipe
+ // buffer get filled. That can happen if the process do not read
+ // anymore, being blocked on writing to the filled output pipe, which
+ // get filled not being read on the other end.
+ //
+ // Fortunatelly openssl reads the certificate before performing any
+ // output.
+ //
+ os.out << pem;
+ os.out.close ();
+
+ try
+ {
auto bad_cert ([](const string& d) {throw invalid_argument (d);});
- auto get = [&is, &trace] (string& s) -> bool
+ auto get = [&os, &trace] (string& s) -> bool
{
- bool r (getline (is, s));
+ bool r (getline (os.in, s));
l6 ([&]{trace << s;});
return r;
};
@@ -307,41 +319,41 @@ namespace bpkg
};
auto parse_date = [&s](size_t o, const char* name) -> timestamp
+ {
+ // Certificate validity dates are internally represented as ASN.1
+ // GeneralizedTime and UTCTime
+ // (https://www.ietf.org/rfc/rfc4517.txt). While GeneralizedTime
+ // format allows fraction of a second to be specified, the x.509
+ // Certificate specification (https://www.ietf.org/rfc/rfc5280.txt)
+ // do not permit them to be included into the validity dates. These
+ // dates are printed by openssl in the 'MON DD HH:MM:SS[ GMT]'
+ // format. MON is a month abbreviated name (C locale), timezone is
+ // either GMT or absent (means local time). Examples:
+ //
+ // Apr 11 10:20:02 2016 GMT
+ // Apr 11 10:20:02 2016
+ //
+ // We will require the date to be in GMT, as generally can not
+ // interpret the certificate origin local time. Note:
+ // openssl-generated certificate dates are always in GMT.
+ //
+ try
{
- // Certificate validity dates are internally represented as ASN.1
- // GeneralizedTime and UTCTime
- // (https://www.ietf.org/rfc/rfc4517.txt). While GeneralizedTime
- // format allows fraction of a second to be specified, the x.509
- // Certificate specification (https://www.ietf.org/rfc/rfc5280.txt)
- // do not permit them to be included into the validity dates. These
- // dates are printed by openssl in the 'MON DD HH:MM:SS[ GMT]'
- // format. MON is a month abbreviated name (C locale), timezone is
- // either GMT or absent (means local time). Examples:
+ // Assume the global locale is not changed, and still "C".
//
- // Apr 11 10:20:02 2016 GMT
- // Apr 11 10:20:02 2016
- //
- // We will require the date to be in GMT, as generally can not
- // interpret the certificate origin local time. Note:
- // openssl-generated certificate dates are always in GMT.
- //
- try
- {
- // Assume the global locale is not changed, and still "C".
- //
- const char* end;
- timestamp t (from_string (
- s.c_str () + o, "%b %d %H:%M:%S %Y", false, &end));
-
- if (strcmp (end, " GMT") == 0)
- return t;
- }
- catch (const system_error&)
- {
- }
-
- throw invalid_argument ("invalid " + string (name) + " date");
- };
+ const char* end;
+ timestamp t (from_string (
+ s.c_str () + o, "%b %d %H:%M:%S %Y", false, &end));
+
+ if (strcmp (end, " GMT") == 0)
+ return t;
+ }
+ catch (const system_error&)
+ {
+ }
+
+ throw invalid_argument ("invalid " + string (name) + " date");
+ };
string name;
string org;
@@ -368,7 +380,7 @@ namespace bpkg
if (org.empty ())
bad_cert ("no organization name (O)");
- if (!is || s.compare (0, 10, "notBefore=") != 0)
+ if (!os.in || s.compare (0, 10, "notBefore=") != 0)
bad_cert ("no start date");
timestamp not_before (parse_date (10, "start"));
@@ -387,10 +399,10 @@ namespace bpkg
// Ensure no data left in the stream.
//
- if (is.peek () != ifdstream::traits_type::eof ())
+ if (os.in.peek () != ifdstream::traits_type::eof ())
bad_cert ("unexpected data");
- is.close ();
+ os.in.close ();
shared_ptr<certificate> cert (
make_shared<certificate> (
@@ -401,37 +413,39 @@ namespace bpkg
move (not_before),
move (not_after)));
- if (pr.wait ())
+ if (os.wait ())
return cert;
// Fall through.
//
}
- catch (const io_error&)
- {
- // Child exit status doesn't matter. Just wait for the process
- // completion and fall through.
- //
- pr.wait (); // Check throw.
- }
catch (const invalid_argument& e)
{
// If the child exited with an error status, then omit any output
// parsing diagnostics since we were probably parsing garbage.
//
- if (pr.wait ())
+ if (os.wait ())
fail << "invalid certificate for " << repo << ": " << e << endf;
// Fall through.
}
- error << "unable to parse certificate for " << repo;
+ parse_failed ();
// Fall through.
}
- catch (const process_error& e)
+ // For old versions of g++ (as of 4.9) ios_base::failure is not derived
+ // from system_error.
+ //
+ catch (const io_error& e)
{
- error << "unable to parse certificate for " << repo << ": " << e;
+ parse_failed (&e);
+
+ // Fall through.
+ }
+ catch (const system_error& e)
+ {
+ parse_failed (&e);
// Fall through.
}
@@ -749,66 +763,58 @@ namespace bpkg
<< rl.canonical_name () <<
info << "certificate name is " << cert.name;
- try
+ auto auth_failed = [&rl] (const exception* e = nullptr)
{
- process pr (start_openssl (
- co, "rsautl",
- {
- "-verify",
- "-certin",
- "-inkey",
- f.string ().c_str ()
- },
- true,
- true));
-
- try
- {
- ifdstream is (move (pr.in_ofd), fdstream_mode::skip);
+ diag_record dr (error);
+ dr << "unable to authenticate repository " << rl.canonical_name ();
- // Write the signature to the openssl process input in the binary mode.
- //
- ofdstream os (move (pr.out_fd), fdstream_mode::binary);
-
- for (const auto& c: sm.signature)
- os.put (c); // Sets badbit on failure.
+ if (e != nullptr)
+ dr << ": " << *e;
+ };
- os.close ();
+ try
+ {
+ openssl os (print_command,
+ path ("-"), fdstream_mode::text, 2,
+ co.openssl (), "rsautl",
+ co.openssl_option (), "-verify", "-certin", "-inkey", f);
- string s;
- getline (is, s);
+ for (const auto& c: sm.signature)
+ os.out.put (c); // Sets badbit on failure.
- bool v (is.eof ());
- is.close ();
+ os.out.close ();
- if (pr.wait () && v)
- {
- if (s != sm.sha256sum)
- fail << "packages manifest file signature mismatch for "
- << rl.canonical_name ();
+ string s;
+ getline (os.in, s);
- return; // All good.
- }
+ bool v (os.in.eof ());
+ os.in.close ();
- // Fall through.
- //
- }
- catch (const io_error&)
+ if (os.wait () && v)
{
- // Child exit status doesn't matter. Just wait for the process
- // completion and fall through.
- //
- pr.wait (); // Check throw.
+ if (s != sm.sha256sum)
+ fail << "packages manifest file signature mismatch for "
+ << rl.canonical_name ();
+
+ return; // All good.
}
- error << "unable to authenticate repository " << rl.canonical_name ();
+ auth_failed ();
// Fall through.
}
- catch (const process_error& e)
+ // For old versions of g++ (as of 4.9) ios_base::failure is not derived
+ // from system_error.
+ //
+ catch (const io_error& e)
{
- error << "unable to authenticate repository "
- << rl.canonical_name () << ": " << e;
+ auth_failed (&e);
+
+ // Fall through.
+ }
+ catch (const system_error& e)
+ {
+ auth_failed (&e);
// Fall through.
}
@@ -844,52 +850,47 @@ namespace bpkg
warn << "certificate for repository " << r
<< " expires in less than " << left.count () + 1 << " day(s)";
- try
+ auto sign_failed = [&r] (const exception* e = nullptr)
{
- process pr (start_openssl (
- co, "rsautl", {"-sign", "-inkey", key_name.c_str ()}, true, true));
+ diag_record dr (error);
+ dr << "unable to sign repository " << r;
- try
- {
- // Read the signature from the openssl process output in the binary
- // mode.
- //
- ifdstream is (
- move (pr.in_ofd), fdstream_mode::binary | fdstream_mode::skip);
+ if (e != nullptr)
+ dr << ": " << *e;
+ };
- ofdstream os (move (pr.out_fd));
- os << sha256sum;
- os.close ();
+ try
+ {
+ openssl os (print_command,
+ fdstream_mode::text, path ("-"), 2,
+ co.openssl (), "rsautl",
+ co.openssl_option (), "-sign", "-inkey", key_name);
- // Additional parentheses required to make compiler to distinguish
- // the variable definition from a function declaration.
- //
- vector<char> signature
- ((istreambuf_iterator<char> (is)), istreambuf_iterator<char> ());
+ os.out << sha256sum;
+ os.out.close ();
- is.close ();
+ vector<char> signature (os.in.read_binary ());
+ os.in.close ();
- if (pr.wait ())
- return signature;
+ if (os.wait ())
+ return signature;
- // Fall through.
- //
- }
- catch (const io_error&)
- {
- // Child exit status doesn't matter. Just wait for the process
- // completion and fall through.
- //
- pr.wait (); // Check throw.
- }
+ sign_failed ();
- error << "unable to sign repository " << r;
+ // Fall through.
+ }
+ // For old versions of g++ (as of 4.9) ios_base::failure is not derived
+ // from system_error.
+ //
+ catch (const io_error& e)
+ {
+ sign_failed (&e);
// Fall through.
}
- catch (const process_error& e)
+ catch (const system_error& e)
{
- error << "unable to sign repository " << r << ": " << e;
+ sign_failed (&e);
// Fall through.
}
diff --git a/bpkg/buildfile b/bpkg/buildfile
index e555178..e214bc9 100644
--- a/bpkg/buildfile
+++ b/bpkg/buildfile
@@ -21,7 +21,6 @@ exe{bpkg}: \
{hxx }{ forward } \
{hxx cxx}{ help } {hxx ixx cxx}{ help-options } \
{hxx cxx}{ manifest-utility } \
-{hxx cxx}{ openssl } \
{hxx }{ options-types } \
{hxx ixx cxx}{ package } \
{hxx ixx cxx}{ package-odb } file{ package.xml } \
diff --git a/bpkg/diagnostics.cxx b/bpkg/diagnostics.cxx
index 16dcc46..7fafbb9 100644
--- a/bpkg/diagnostics.cxx
+++ b/bpkg/diagnostics.cxx
@@ -96,6 +96,18 @@ namespace bpkg
static_cast<trace_mark&> (*this) << "DEALLOCATE " << s.text ();
}
+ // tracer
+ //
+ void tracer::
+ operator() (const char* const args[], size_t n) const
+ {
+ if (verb >= 3)
+ {
+ diag_record dr (*this);
+ process::print (dr.os, args, n);
+ }
+ }
+
const basic_mark error ("error");
const basic_mark warn ("warning");
const basic_mark info ("info");
diff --git a/bpkg/diagnostics.hxx b/bpkg/diagnostics.hxx
index 8edf606..ecff515 100644
--- a/bpkg/diagnostics.hxx
+++ b/bpkg/diagnostics.hxx
@@ -198,7 +198,17 @@ namespace bpkg
deallocate (odb::connection&, const odb::statement&);
};
using trace_mark = butl::diag_mark<trace_mark_base>;
- using tracer = trace_mark;
+
+ class tracer: public trace_mark
+ {
+ public:
+ using trace_mark::trace_mark;
+
+ // process_run() command tracer interface.
+ //
+ void
+ operator() (const char* const [], std::size_t) const;
+ };
// fail
//
diff --git a/bpkg/openssl.cxx b/bpkg/openssl.cxx
deleted file mode 100644
index 5a1e29c..0000000
--- a/bpkg/openssl.cxx
+++ /dev/null
@@ -1,60 +0,0 @@
-// file : bpkg/openssl.cxx -*- C++ -*-
-// copyright : Copyright (c) 2014-2017 Code Synthesis Ltd
-// license : MIT; see accompanying LICENSE file
-
-#include <bpkg/openssl.hxx>
-
-#include <libbutl/process.hxx>
-
-#include <bpkg/diagnostics.hxx>
-
-using namespace std;
-using namespace butl;
-
-namespace bpkg
-{
- process
- start_openssl (const common_options& co,
- const char* command,
- const cstrings& options,
- bool in,
- bool out,
- bool err)
- {
- cstrings args {co.openssl ().string ().c_str (), command};
-
- // Add extra options. Normally the order of options is not important
- // (unless they override each other). However, openssl 1.0.1 seems to have
- // bugs in that department (that were apparently fixed in 1.0.2). To work
- // around these bugs we pass user-supplied options first.
- //
- for (const string& o: co.openssl_option ())
- args.push_back (o.c_str ());
-
- args.insert (args.end (), options.begin (), options.end ());
- args.push_back (nullptr);
-
- try
- {
- process_path pp (process::path_search (args[0]));
-
- if (verb >= 2)
- print_process (args);
-
- // If the caller is interested in reading STDOUT and STDERR, then
- // redirect STDERR to STDOUT, so both can be read from the same stream.
- //
- return process (
- pp, args.data (), in ? -1 : 0, out ? -1 : 1, err ? (out ? 1 : -1): 2);
- }
- catch (const process_error& e)
- {
- error << "unable to execute " << args[0] << ": " << e;
-
- if (e.child)
- exit (1);
-
- throw failed ();
- }
- }
-}
diff --git a/bpkg/openssl.hxx b/bpkg/openssl.hxx
deleted file mode 100644
index 8023db6..0000000
--- a/bpkg/openssl.hxx
+++ /dev/null
@@ -1,31 +0,0 @@
-// file : bpkg/openssl.hxx -*- C++ -*-
-// copyright : Copyright (c) 2014-2017 Code Synthesis Ltd
-// license : MIT; see accompanying LICENSE file
-
-#ifndef BPKG_OPENSSL_HXX
-#define BPKG_OPENSSL_HXX
-
-#include <libbutl/process.hxx>
-
-#include <bpkg/types.hxx>
-#include <bpkg/utility.hxx>
-
-#include <bpkg/common-options.hxx>
-
-namespace bpkg
-{
- // Start the openssl process. Parameters in, out, err flags if the caller
- // wish to write to, or read from the process STDIN, STDOUT, STDERR streams.
- // If out and err are both true, then STDERR is redirected to STDOUT, and
- // they both can be read from in_ofd descriptor.
- //
- butl::process
- start_openssl (const common_options&,
- const char* command,
- const cstrings& options,
- bool in = false,
- bool out = false,
- bool err = false);
-}
-
-#endif // BPKG_OPENSSL_HXX
diff --git a/tests/rep-auth.test b/tests/rep-auth.test
index 12815f9..c67bd75 100644
--- a/tests/rep-auth.test
+++ b/tests/rep-auth.test
@@ -582,7 +582,7 @@ sc = " " # Space character to append to here-document line when required.
:
$rep_info $rep/signature-mismatch 2>>~%EOE% != 0
%.*
- %error: unable to authenticate repository build2.org/rep-auth/signature-mismatch%
+ %error: unable to authenticate repository build2.org/rep-auth/signature-mismatch: .*%
EOE
: create-rep