aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--etc/brep-module.conf28
-rw-r--r--libbrep/build.cxx2
-rw-r--r--libbrep/build.hxx10
-rw-r--r--libbrep/build.xml2
-rw-r--r--libbrep/utility.hxx3
-rw-r--r--mod/build-config.cxx77
-rw-r--r--mod/build-config.hxx15
-rw-r--r--mod/database-module.cxx6
-rw-r--r--mod/database-module.hxx3
-rw-r--r--mod/diagnostics.hxx2
-rw-r--r--mod/mod-build-result.cxx123
-rw-r--r--mod/mod-build-task.cxx89
-rw-r--r--mod/options.cli42
13 files changed, 371 insertions, 31 deletions
diff --git a/etc/brep-module.conf b/etc/brep-module.conf
index 53678bb..6499d4b 100644
--- a/etc/brep-module.conf
+++ b/etc/brep-module.conf
@@ -76,6 +76,20 @@ menu About=?about
# build-config
+# Directory containing build bot agent public keys. If specified, then brep
+# will perform agent authentication and will reject build results from
+# unauthenticated ones. If not specified, then build results are accepted from
+# all agents (which will be a security risk if the brep instance is publicly
+# accessible).
+#
+# The directory is expected to contain one PEM-encoded public key per file with
+# the .pem extension. All other files and subdirectories are ignored. The brep
+# instance needs to be restarted after adding new key files for the changes to
+# take effect.
+#
+# build-bot-agent-keys
+
+
# Number of packages build configurations per page.
#
# build-configurations 10
@@ -144,6 +158,20 @@ menu About=?about
# build-db-retry 10
+# The openssl program to be used for crypto operations. You can also specify
+# additional options that should be passed to the openssl program with
+# openssl-option. If the openssl program is not explicitly specified, then brep
+# will use openssl by default.
+#
+# openssl openssl
+
+
+# Additional option to be passed to the openssl program (see openssl for
+# details). Repeat this option to specify multiple openssl options.
+#
+# openssl-option
+
+
# Trace verbosity. Disabled by default.
#
# verbosity 0
diff --git a/libbrep/build.cxx b/libbrep/build.cxx
index 2391165..0941c5f 100644
--- a/libbrep/build.cxx
+++ b/libbrep/build.cxx
@@ -60,6 +60,7 @@ namespace brep
build (string pnm, version pvr,
string cfg,
string tnm, version tvr,
+ optional<string> afp, optional<string> ach,
string mnm, string msm,
optional<butl::target_triplet> trg)
: id (package_id (move (pnm), pvr), move (cfg), tvr),
@@ -71,6 +72,7 @@ namespace brep
state (build_state::building),
timestamp (timestamp_type::clock::now ()),
force (force_state::unforced),
+ agent_fingerprint (move (afp)), agent_challenge (move (ach)),
machine (move (mnm)),
machine_summary (move (msm)),
target (move (trg))
diff --git a/libbrep/build.hxx b/libbrep/build.hxx
index afa96ed..6b58402 100644
--- a/libbrep/build.hxx
+++ b/libbrep/build.hxx
@@ -145,6 +145,8 @@ namespace brep
build (string package_name, version package_version,
string configuration,
string toolchain_name, version toolchain_version,
+ optional<string> agent_fingerprint,
+ optional<string> agent_challenge,
string machine, string machine_summary,
optional<butl::target_triplet> target);
@@ -164,10 +166,16 @@ namespace brep
force_state force;
- // Must present for the built state, may present for the building state.
+ // Must be present for the built state, may be present for the building
+ // state.
//
optional<result_status> status;
+ // May be present only for the building state.
+ //
+ optional<string> agent_fingerprint;
+ optional<string> agent_challenge;
+
// Present only for building and built states.
//
optional<string> machine;
diff --git a/libbrep/build.xml b/libbrep/build.xml
index 7466c97..cee65ac 100644
--- a/libbrep/build.xml
+++ b/libbrep/build.xml
@@ -20,6 +20,8 @@
<column name="timestamp" type="BIGINT" null="false"/>
<column name="force" type="TEXT" null="false"/>
<column name="status" type="TEXT" null="true"/>
+ <column name="agent_fingerprint" type="TEXT" null="true"/>
+ <column name="agent_challenge" type="TEXT" null="true"/>
<column name="machine" type="TEXT" null="true"/>
<column name="machine_summary" type="TEXT" null="true"/>
<column name="target" type="TEXT" null="true"/>
diff --git a/libbrep/utility.hxx b/libbrep/utility.hxx
index c81413c..1900bc4 100644
--- a/libbrep/utility.hxx
+++ b/libbrep/utility.hxx
@@ -11,7 +11,8 @@
#include <cassert> // assert()
#include <iterator> // make_move_iterator()
-#include <libbutl/utility.hxx> // reverse_iterate(), operator<<(ostream, exception)
+#include <libbutl/utility.hxx> // reverse_iterate(),
+ // operator<<(ostream, exception)
namespace brep
{
diff --git a/mod/build-config.cxx b/mod/build-config.cxx
index 9eb40ce..9e30b64 100644
--- a/mod/build-config.cxx
+++ b/mod/build-config.cxx
@@ -5,18 +5,26 @@
#include <mod/build-config.hxx>
#include <map>
+#include <sstream>
+
+#include <libbutl/sha256.hxx>
+#include <libbutl/utility.hxx> // throw_generic_error()
+#include <libbutl/openssl.hxx>
+#include <libbutl/filesystem.hxx>
#include <web/mime-url-encoding.hxx>
namespace brep
{
+ using namespace std;
using namespace web;
+ using namespace butl;
using namespace bbot;
shared_ptr<const build_configs>
shared_build_config (const path& p)
{
- static std::map<path, weak_ptr<build_configs>> configs;
+ static map<path, weak_ptr<build_configs>> configs;
auto i (configs.find (p));
if (i != configs.end ())
@@ -32,6 +40,73 @@ namespace brep
return c;
}
+ shared_ptr<const bot_agent_keys>
+ shared_bot_agent_keys (const options::openssl_options& o, const dir_path& d)
+ {
+ static map<dir_path, weak_ptr<bot_agent_keys>> keys;
+
+ auto i (keys.find (d));
+ if (i != keys.end ())
+ {
+ if (shared_ptr<bot_agent_keys> k = i->second.lock ())
+ return k;
+ }
+
+ shared_ptr<bot_agent_keys> ak (make_shared<bot_agent_keys> ());
+
+ // Intercept exception handling to make error descriptions more
+ // informative.
+ //
+ // Path of the key being converted. Used for diagnostics.
+ //
+ path p;
+
+ try
+ {
+ for (const dir_entry& de: dir_iterator (d))
+ {
+ if (de.path ().extension () == "pem" &&
+ de.type () == entry_type::regular)
+ {
+ p = d / de.path ();
+
+ openssl os (p, path ("-"), 2,
+ o.openssl (), "pkey",
+ o.openssl_option (), "-pubin", "-outform", "DER");
+
+ vector<char> k (os.in.read_binary ());
+ os.in.close ();
+
+ if (!os.wait ())
+ throw io_error ("");
+
+ ak->emplace (sha256 (k.data (), k.size ()).string (), move (p));
+ }
+ }
+ }
+ catch (const io_error&)
+ {
+ ostringstream os;
+ os << "unable to convert bbot agent pubkey " << p;
+ throw_generic_error (EIO, os.str ().c_str ());
+ }
+ catch (const process_error& e)
+ {
+ ostringstream os;
+ os << "unable to convert bbot agent pubkey " << p;
+ throw_generic_error (e.code ().value (), os.str ().c_str ());
+ }
+ catch (const system_error& e)
+ {
+ ostringstream os;
+ os<< "unable to iterate over agents keys directory '" << d << "'";
+ throw_generic_error (e.code ().value (), os.str ().c_str ());
+ }
+
+ keys[d] = ak;
+ return ak;
+ }
+
string
build_log_url (const string& host, const dir_path& root,
const build& b,
diff --git a/mod/build-config.hxx b/mod/build-config.hxx
index 6058774..b49819d 100644
--- a/mod/build-config.hxx
+++ b/mod/build-config.hxx
@@ -5,6 +5,8 @@
#ifndef MOD_BUILD_CONFIG_HXX
#define MOD_BUILD_CONFIG_HXX
+#include <map>
+
#include <libbbot/build-config.hxx>
#include <libbrep/types.hxx>
@@ -12,6 +14,8 @@
#include <libbrep/build.hxx>
+#include <mod/options.hxx>
+
namespace brep
{
// Return pointer to the shared build configurations instance, creating one
@@ -21,6 +25,17 @@ namespace brep
shared_ptr<const bbot::build_configs>
shared_build_config (const path&);
+ // Map of build bot agent public keys fingerprints to the key file paths.
+ //
+ using bot_agent_keys = std::map<string, path>;
+
+ // Return pointer to the shared build bot agent public keys map, creating
+ // one on the first call. Throw system_error on the underlying openssl or OS
+ // error. Not thread-safe.
+ //
+ shared_ptr<const bot_agent_keys>
+ shared_bot_agent_keys (const options::openssl_options&, const dir_path&);
+
// Return the package configuration build log url. By default the url is to
// the operations combined log.
//
diff --git a/mod/database-module.cxx b/mod/database-module.cxx
index 9daff53..67e4c9d 100644
--- a/mod/database-module.cxx
+++ b/mod/database-module.cxx
@@ -34,7 +34,8 @@ namespace brep
build_db_ (r.initialized_ ? r.build_db_ : nullptr),
build_conf_ (r.initialized_ ? r.build_conf_ : nullptr),
build_conf_names_ (r.initialized_ ? r.build_conf_names_ : nullptr),
- build_conf_map_ (r.initialized_ ? r.build_conf_map_ : nullptr)
+ build_conf_map_ (r.initialized_ ? r.build_conf_map_ : nullptr),
+ bot_agent_keys_ (r.initialized_ ? r.bot_agent_keys_ : nullptr)
{
}
@@ -67,6 +68,9 @@ namespace brep
throw_generic_error (EIO, os.str ().c_str ());
}
+ if (bo.build_bot_agent_keys_specified ())
+ bot_agent_keys_ = shared_bot_agent_keys (bo, bo.build_bot_agent_keys ());
+
cstrings conf_names;
using conf_map_type = map<const char*,
diff --git a/mod/database-module.hxx b/mod/database-module.hxx
index a61e2e4..ec8f37e 100644
--- a/mod/database-module.hxx
+++ b/mod/database-module.hxx
@@ -18,6 +18,7 @@
#include <mod/module.hxx>
#include <mod/options.hxx>
+#include <mod/build-config.hxx>
namespace brep
{
@@ -74,6 +75,8 @@ namespace brep
butl::compare_c_string>>
build_conf_map_;
+ shared_ptr<const bot_agent_keys> bot_agent_keys_;
+
private:
virtual bool
handle (request&, response&, log&);
diff --git a/mod/diagnostics.hxx b/mod/diagnostics.hxx
index e05d56a..46b9e17 100644
--- a/mod/diagnostics.hxx
+++ b/mod/diagnostics.hxx
@@ -261,7 +261,7 @@ namespace brep
const char* name_;
const void* data_;
};
- typedef diag_mark<basic_mark_base> basic_mark;
+ using basic_mark = diag_mark<basic_mark_base>;
template <typename E>
struct fail_mark_base
diff --git a/mod/mod-build-result.cxx b/mod/mod-build-result.cxx
index ae6c5b1..41bfb2b 100644
--- a/mod/mod-build-result.cxx
+++ b/mod/mod-build-result.cxx
@@ -7,7 +7,9 @@
#include <odb/database.hxx>
#include <odb/transaction.hxx>
+#include <libbutl/openssl.hxx>
#include <libbutl/sendmail.hxx>
+#include <libbutl/fdstream.hxx>
#include <libbutl/process-io.hxx>
#include <libbutl/manifest-parser.hxx>
#include <libbutl/manifest-serializer.hxx>
@@ -226,6 +228,11 @@ handle (request& rq, response&)
return true;
}
+ auto print_args = [&trace, this] (const char* args[], size_t n)
+ {
+ l2 ([&]{trace << process_args {args, n};});
+ };
+
// Load and update the package build configuration (if present).
//
shared_ptr<build> b;
@@ -245,28 +252,109 @@ handle (request& rq, response&)
warn_expired ("non-matching timestamp");
else
{
- unforced = b->force == force_state::unforced;
+ // Check the challenge.
+ //
+ // If the challenge doesn't match expectations (probably due to the
+ // authentication settings change), then we log this case with the
+ // warning severity and respond with the 200 HTTP code as if the
+ // challenge is valid. The thinking is that we shouldn't alarm a
+ // law-abaiding agent and shouldn't provide any information to a
+ // malicious one.
+ //
+ auto warn_auth = [&rqm, &warn] (const string& d)
+ {
+ warn << "session '" << rqm.session << "' authentication failed: " << d;
+ };
+
+ bool auth (false);
- // Don's send email for the success-to-success status change, unless the
- // build was forced.
+ // Must both be present or absent.
//
- notify = !(rqm.result.status == result_status::success &&
- b->status && *b->status == rqm.result.status && unforced);
+ if (!b->agent_challenge != !rqm.challenge)
+ warn_auth (rqm.challenge
+ ? "unexpected challenge"
+ : "challenge is expected");
+ else if (bot_agent_keys_ == nullptr) // Authentication is disabled.
+ auth = true;
+ else if (!b->agent_challenge) // Authentication is recently enabled.
+ warn_auth ("challenge is required now");
+ else
+ {
+ assert (b->agent_fingerprint && rqm.challenge);
+ auto i (bot_agent_keys_->find (*b->agent_fingerprint));
+
+ // The agent's key is recently replaced.
+ //
+ if (i == bot_agent_keys_->end ())
+ warn_auth ("agent's public key not found");
+ else
+ {
+ try
+ {
+ openssl os (print_args,
+ path ("-"), fdstream_mode::text, 2,
+ options_->openssl (), "rsautl",
+ options_->openssl_option (),
+ "-verify", "-pubin", "-inkey", i->second);
+
+ for (const auto& c: *rqm.challenge)
+ os.out.put (c); // Sets badbit on failure.
+
+ os.out.close ();
+
+ string s;
+ getline (os.in, s);
+
+ bool v (os.in.eof ());
+ os.in.close ();
+
+ if (os.wait () && v)
+ {
+ auth = s == *b->agent_challenge;
+
+ if (!auth)
+ warn_auth ("challenge mismatched");
+ }
+ else // The signature is presumably meaningless.
+ warn_auth ("unable to verify challenge");
+ }
+ catch (const system_error& e)
+ {
+ fail << "unable to verify challenge: " << e;
+ }
+ }
+ }
- prev_status = move (b->status);
+ if (auth)
+ {
+ unforced = b->force == force_state::unforced;
- b->state = build_state::built;
- b->status = rqm.result.status;
- b->force = force_state::unforced;
+ // Don's send email for the success-to-success status change, unless
+ // the build was forced.
+ //
+ notify = !(rqm.result.status == result_status::success &&
+ b->status && *b->status == rqm.result.status && unforced);
- // Mark the section as loaded, so results are updated.
- //
- b->results_section.load ();
- b->results = move (rqm.result.results);
+ prev_status = move (b->status);
+
+ b->state = build_state::built;
+ b->status = rqm.result.status;
+ b->force = force_state::unforced;
+
+ // Cleanup the authentication data.
+ //
+ b->agent_fingerprint = nullopt;
+ b->agent_challenge = nullopt;
+
+ // Mark the section as loaded, so results are updated.
+ //
+ b->results_section.load ();
+ b->results = move (rqm.result.results);
- b->timestamp = timestamp::clock::now ();
+ b->timestamp = timestamp::clock::now ();
- build_db_->update (b);
+ build_db_->update (b);
+ }
}
t.commit ();
@@ -299,11 +387,6 @@ handle (request& rq, response&)
? *p->package_email
: p->email);
- auto print_args = [&trace, this] (const char* args[], size_t n)
- {
- l2 ([&]{trace << process_args {args, n};});
- };
-
// Redirect the diagnostics to webserver error log.
//
// Note: if using this somewhere else, then need to factor out all this
diff --git a/mod/mod-build-task.cxx b/mod/mod-build-task.cxx
index e47ae60..c018b65 100644
--- a/mod/mod-build-task.cxx
+++ b/mod/mod-build-task.cxx
@@ -11,8 +11,12 @@
#include <odb/transaction.hxx>
#include <odb/schema-catalog.hxx>
+#include <libbutl/sha256.hxx>
#include <libbutl/utility.hxx> // compare_c_string
+#include <libbutl/openssl.hxx>
+#include <libbutl/fdstream.hxx> // nullfd
#include <libbutl/filesystem.hxx> // path_match()
+#include <libbutl/process-io.hxx>
#include <libbutl/manifest-parser.hxx>
#include <libbutl/manifest-serializer.hxx>
@@ -110,6 +114,21 @@ handle (request& rq, response& rs)
throw invalid_request (400, e.what ());
}
+ // Obtain the agent's public key fingerprint if requested. If the fingerprint
+ // is requested but is not present in the request or is unknown, then respond
+ // with 401 HTTP code (unauthorized).
+ //
+ optional<string> agent_fp;
+
+ if (bot_agent_keys_ != nullptr)
+ {
+ if (!tqm.fingerprint ||
+ bot_agent_keys_->find (*tqm.fingerprint) == bot_agent_keys_->end ())
+ throw invalid_request (401, "unauthorized");
+
+ agent_fp = move (tqm.fingerprint);
+ }
+
task_response_manifest tsm;
// Map build configurations to machines that are capable of building them.
@@ -188,10 +207,8 @@ handle (request& rq, response& rs)
cm.config->target,
cm.config->vars);
- // @@ We don't support challenge at the moment.
- //
return task_response_manifest (move (session),
- nullopt,
+ move (b->agent_challenge),
move (result_url),
move (task));
};
@@ -224,6 +241,61 @@ handle (request& rq, response& rs)
timestamp forced_rebuild_expiration (
expiration (options_->build_forced_rebuild_timeout ()));
+ // Return the challenge (nonce) if brep is configured to authenticate bbot
+ // agents. Return nullopt otherwise.
+ //
+ // Nonce generator must guarantee a probabilistically insignificant chance
+ // of repeating a previously generated value. The common approach is to use
+ // counters or random number generators (alone or in combination), that
+ // produce values of the sufficient length. 64-bit non-repeating and
+ // 512-bit random numbers are considered to be more than sufficient for
+ // most practical purposes.
+ //
+ // We will produce the challenge as the sha256sum of the 512-bit random
+ // number and the 64-bit current timestamp combination. The latter is
+ // not really a non-repeating counter and can't be used alone. However
+ // adding it is a good and cheap uniqueness improvement.
+ //
+ auto challenge = [&agent_fp, &now, &fail, &trace, this] ()
+ {
+ optional<string> r;
+
+ if (agent_fp)
+ {
+ try
+ {
+ auto print_args = [&trace, this] (const char* args[], size_t n)
+ {
+ l2 ([&]{trace << process_args {args, n};});
+ };
+
+ openssl os (print_args,
+ nullfd, path ("-"), 2,
+ options_->openssl (), "rand",
+ options_->openssl_option (), 64);
+
+ vector<char> nonce (os.in.read_binary ());
+ os.in.close ();
+
+ if (!os.wait () || nonce.size () != 64)
+ fail << "unable to generate nonce";
+
+ uint64_t t (chrono::duration_cast<std::chrono::nanoseconds> (
+ now.time_since_epoch ()).count ());
+
+ sha256 cs (nonce.data (), nonce.size ());
+ cs.append (&t, sizeof (t));
+ r = cs.string ();
+ }
+ catch (const system_error& e)
+ {
+ fail << "unable to generate nonce: " << e;
+ }
+ }
+
+ return r;
+ };
+
// Convert butl::standard_version type to brep::version.
//
brep::version toolchain_version (tqm.toolchain_version.string ());
@@ -393,6 +465,7 @@ handle (request& rq, response& rs)
machine_header_manifest& mh (*cm.machine);
build_id bid (move (id), cm.config->name, toolchain_version);
shared_ptr<build> b (build_db_->find<build> (bid));
+ optional<string> cl (challenge ());
// If build configuration doesn't exist then create the new one
// and persist. Otherwise put it into the building state, refresh
@@ -405,6 +478,8 @@ handle (request& rq, response& rs)
move (bid.configuration),
move (tqm.toolchain_name),
move (toolchain_version),
+ move (agent_fp),
+ move (cl),
mh.name,
move (mh.summary),
cm.config->target);
@@ -438,6 +513,8 @@ handle (request& rq, response& rs)
b->force = force_state::forced;
b->toolchain_name = move (tqm.toolchain_name);
+ b->agent_fingerprint = move (agent_fp);
+ b->agent_challenge = move (cl);
b->machine = mh.name;
b->machine_summary = move (mh.summary);
b->target = cm.config->target;
@@ -504,6 +581,8 @@ handle (request& rq, response& rs)
sort (rebuilds.begin (), rebuilds.end (), cmp);
+ optional<string> cl (challenge ());
+
// Pick the first package configuration from the ordered list.
//
// Note that the configurations may not match the required criteria
@@ -557,8 +636,10 @@ handle (request& rq, response& rs)
b->state = build_state::building;
b->machine = mh.name;
- // Can't move from, as may need it on the next iteration.
+ // Can't move from, as may need them on the next iteration.
//
+ b->agent_fingerprint = agent_fp;
+ b->agent_challenge = cl;
b->toolchain_name = tqm.toolchain_name;
b->machine_summary = mh.summary;
diff --git a/mod/options.cli b/mod/options.cli
index 65f5549..e6beb6e 100644
--- a/mod/options.cli
+++ b/mod/options.cli
@@ -52,6 +52,27 @@ namespace brep
}
};
+ class openssl_options
+ {
+ path openssl = "openssl"
+ {
+ "<path>",
+ "The openssl program to be used for crypto operations. You can also
+ specify additional options that should be passed to the openssl
+ program with \cb{openssl-option}. If the openssl program is not
+ explicitly specified, then \cb{brep} will use \cb{openssl} by
+ default."
+ }
+
+ strings openssl-option
+ {
+ "<opt>",
+ "Additional option to be passed to the openssl program (see
+ \cb{openssl} for details). Repeat this option to specify multiple
+ openssl options."
+ }
+ };
+
class package_db
{
string package-db-user
@@ -107,14 +128,31 @@ namespace brep
}
};
- class build
+ class build: openssl_options
{
path build-config
{
"<buildtab>",
"Build configuration file. If not specified, then the package building
functionality will be disabled. If specified, then the build database
- must be configured (see \cb{build-db-*})."
+ must be configured (see \cb{build-db-*}). The \cb{brep} instance
+ needs to be restarted after modifying <buildtab> for the changes to
+ take effect."
+ }
+
+ dir_path build-bot-agent-keys
+ {
+ "<dir>",
+ "Directory containing build bot agent public keys. If specified, then
+ \cb{brep} will perform agent authentication and will reject build
+ results from unauthenticated ones. If not specified, then build
+ results are accepted from all agents (which will be a security
+ risk if the \cb{brep} instance is publicly accessible).
+
+ The directory is expected to contain one PEM-encoded public key
+ per file with the \cb{.pem} extension. All other files and
+ subdirectories are ignored. The \cb{brep} instance needs to be
+ restarted after adding new key files for the changes to take effect."
}
size_t build-forced-rebuild-timeout = 600