From bef880fc4964a69ff808e17626c2be4babf1a9ea Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Wed, 24 Apr 2024 15:20:28 +0300 Subject: Generate challenge out of database transaction in build task handler --- mod/mod-build-task.cxx | 139 +++++++++++++++++++++++++------------------------ 1 file changed, 71 insertions(+), 68 deletions(-) (limited to 'mod') diff --git a/mod/mod-build-task.cxx b/mod/mod-build-task.cxx index c4968a7..07aff8d 100644 --- a/mod/mod-build-task.cxx +++ b/mod/mod-build-task.cxx @@ -44,6 +44,28 @@ using namespace odb::core; static thread_local mt19937 rand_gen (random_device {} ()); +// The challenge (nonce) is randomly generated for every build task if brep is +// configured to authenticate bbot agents. +// +// 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. +// +// Note that since generating a challenge is not exactly cheap/fast, we will +// generate it in advance for every task request, out of the database +// transaction, and will cache it if it turns out that it wasn't used (no +// package configuration to (re-)build, etc). +// +static thread_local optional challenge; + // Generate a random number in the specified range (max value is included). // static inline size_t @@ -617,63 +639,6 @@ handle (request& rq, response& rs) : optional ()), options_->build_hard_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 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, - process_env (options_->openssl (), - options_->openssl_envvar ()), - "rand", - options_->openssl_option (), 64); - - vector 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 ( - 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 ()); @@ -1373,6 +1338,39 @@ handle (request& rq, response& rs) move (tms), move (bms), move (tests)}; }; + if (agent_fp && !challenge) + 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, + process_env (options_->openssl (), + options_->openssl_envvar ()), + "rand", + options_->openssl_option (), 64); + + vector 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 ( + now.time_since_epoch ()).count ()); + + sha256 cs (nonce.data (), nonce.size ()); + cs.append (&t, sizeof (t)); + challenge = cs.string (); + } + catch (const system_error& e) + { + fail << "unable to generate nonce: " << e; + } + // While at it, collect the aborted for various reasons builds // (interactive builds in multiple configurations, builds with too many // auxiliary machines, etc) to send the notification emails at the end @@ -1758,7 +1756,6 @@ handle (request& rq, response& rs) toolchain_version); shared_ptr b (build_db_->find (bid)); - optional cl (challenge ()); // Move the interactive build login information into the build // object, if the package to be built interactively. @@ -1783,13 +1780,15 @@ handle (request& rq, response& rs) move (toolchain_version), move (login), move (agent_fp), - move (cl), + move (challenge), build_machine { mh.name, move (mh.summary)}, move (aux->build_auxiliary_machines), controller_checksum (*cm->config), machine_checksum (*cm->machine)); + challenge = nullopt; + build_db_->persist (b); } else @@ -1825,7 +1824,10 @@ handle (request& rq, response& rs) } b->agent_fingerprint = move (agent_fp); - b->agent_challenge = move (cl); + + b->agent_challenge = move (challenge); + challenge = nullopt; + b->machine = build_machine {mh.name, move (mh.summary)}; // Mark the section as loaded, so auxiliary_machines are @@ -1994,8 +1996,6 @@ handle (request& rq, response& rs) sort (rebuilds.begin (), rebuilds.end (), cmp); - optional cl (challenge ()); - // Pick the first build configuration from the ordered list. // // Note that the configurations and packages may not match the @@ -2086,10 +2086,10 @@ handle (request& rq, response& rs) unforced = (b->force == force_state::unforced); - // Can't move from, as may need them on the next iteration. - // - b->agent_fingerprint = agent_fp; - b->agent_challenge = cl; + b->agent_fingerprint = move (agent_fp); + + b->agent_challenge = move (challenge); + challenge = nullopt; const machine_header_manifest& mh (*cm.machine); b->machine = build_machine {mh.name, mh.summary}; @@ -2182,9 +2182,12 @@ handle (request& rq, response& rs) } catch (const odb::deadlock&) { - // Just try with the next rebuild. But first, reset the task - // manifest and the session that we may have prepared. + // Just try with the next rebuild. But first, restore the agent's + // fingerprint and challenge and reset the task manifest and the + // session that we may have prepared. // + agent_fp = move (b->agent_fingerprint); + challenge = move (b->agent_challenge); task_response = task_response_manifest (); } -- cgit v1.1