diff options
| -rw-r--r-- | bpkg/archive.cxx | 3 | ||||
| -rw-r--r-- | bpkg/auth.cxx | 463 | ||||
| -rw-r--r-- | bpkg/buildfile | 1 | ||||
| -rw-r--r-- | bpkg/diagnostics.cxx | 12 | ||||
| -rw-r--r-- | bpkg/diagnostics.hxx | 12 | ||||
| -rw-r--r-- | bpkg/openssl.cxx | 60 | ||||
| -rw-r--r-- | bpkg/openssl.hxx | 31 | ||||
| -rw-r--r-- | tests/rep-auth.test | 2 |
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 |