diff options
| -rw-r--r-- | etc/brep-module.conf | 23 | ||||
| -rw-r--r-- | mod/hmac.cxx | 95 | ||||
| -rw-r--r-- | mod/hmac.hxx | 29 | ||||
| -rw-r--r-- | mod/mod-ci-github.cxx | 85 | ||||
| -rw-r--r-- | mod/module.cli | 9 | ||||
| -rw-r--r-- | web/server/module.hxx | 5 |
6 files changed, 239 insertions, 7 deletions
diff --git a/etc/brep-module.conf b/etc/brep-module.conf index 0ba085f..37220a3 100644 --- a/etc/brep-module.conf +++ b/etc/brep-module.conf @@ -453,6 +453,29 @@ menu About=?about # ci-handler-timeout +# The GitHub App ID. Found in the app's settings on GitHub. +# +# ci-github-app-id + + +# The GitHub App's configured webhook secret. If not set, then the GitHub CI +# service is disabled. +# +# ci-github-app-webhook-secret + + +# The private key used during GitHub API authentication. Created in the GitHub +# App's settings. +# +# ci-github-app-private-key + + +# The number of seconds a JWT (authentication token) should be valid for. The +# maximum allowed by GitHub is 10 minutes. +# +# ci-github-jwt-validity-period 600 + + # The directory to save upload data to for the specified upload type. If # unspecified, the build artifacts upload functionality will be disabled for # this type. diff --git a/mod/hmac.cxx b/mod/hmac.cxx new file mode 100644 index 0000000..1a78b4c --- /dev/null +++ b/mod/hmac.cxx @@ -0,0 +1,95 @@ +#include <mod/hmac.hxx> + +#include <libbutl/openssl.hxx> + +using namespace std; +using namespace butl; + +string brep:: +compute_hmac (const options::openssl_options& o, + const void* m, size_t l, + const char* k) +{ + try + { + fdpipe errp (fdopen_pipe ()); // stderr pipe. + + // To compute an HMAC over stdin with the key <secret>: + // + // openssl mac -digest SHA256 -macopt "key:<secret>" HMAC + // + // Note that here we assume both output and diagnostics will fit into pipe + // buffers and don't poll both with fdselect(). + // + openssl os (path ("-"), // Read message from openssl::out. + path ("-"), // Write output to openssl::in. + process::pipe (errp.in.get (), move (errp.out)), + process_env (o.openssl (), o.openssl_envvar ()), + "mac", o.openssl_option (), + "-digest", "SHA256", + "-macopt", string ("key:") + k, + "HMAC"); + + ifdstream err (move (errp.in)); + + string h; // The HMAC value. + try + { + // In case of an exception, skip and close input after output. + // + // Note: re-open in/out so that they get automatically closed on + // an exception. + // + ifdstream in (os.in.release (), fdstream_mode::skip); + ofdstream out (os.out.release ()); + + // Write the message to openssl's input. + // + out.write (static_cast<const char*> (m), l); + out.close (); + + // Read the HMAC value from openssl's output. + // + h = in.read_text (); + in.close (); + } + catch (const io_error& e) + { + // If the process exits with non-zero status, assume the IO error is due + // to that and fall through. + // + if (os.wait ()) + { + throw_generic_error ( + e.code ().value (), + (string ("unable to read/write openssl stdout/stdin: ") + + e.what ()).c_str ()); + } + } + + if (!os.wait ()) + { + string et (err.read_text ()); + throw_generic_error (EINVAL, + ("non-zero openssl exit status: " + et).c_str ()); + } + + err.close (); + + return h; + } + catch (const process_error& e) + { + throw_generic_error ( + e.code ().value (), + (string ("unable to execute openssl: ") + e.what ()).c_str ()); + } + catch (const io_error& e) + { + // Unable to read diagnostics from stderr. + // + throw_generic_error ( + e.code ().value (), + (string ("unable to read openssl stderr : ") + e.what ()).c_str ()); + } +} diff --git a/mod/hmac.hxx b/mod/hmac.hxx new file mode 100644 index 0000000..586d0e8 --- /dev/null +++ b/mod/hmac.hxx @@ -0,0 +1,29 @@ +#ifndef MOD_HMAC_HXX +#define MOD_HMAC_HXX + +#include <libbrep/types.hxx> +#include <libbrep/utility.hxx> + +#include <mod/module-options.hxx> + +namespace brep +{ + // Compute the HMAC-SHA256 message authentication code over a message using + // the given key (alpha-numeric string, not encoded). + // + // Return the HMAC value or throw std::system_error in case of an error. + // + // Example output: + // + // 5e822587094c68e646db8b916da1db2056d92f1dea4252136a533b4147a30cb7 + // + // Note that although any cryptographic hash function can be used to compute + // an HMAC, this implementation supports only SHA-256. + // + string + compute_hmac (const options::openssl_options&, + const void* message, size_t len, + const char* key); +} + +#endif diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx index e19a41b..c4aaec1 100644 --- a/mod/mod-ci-github.cxx +++ b/mod/mod-ci-github.cxx @@ -7,6 +7,7 @@ #include <libbutl/json/parser.hxx> #include <mod/jwt.hxx> +#include <mod/hmac.hxx> #include <mod/module-options.hxx> #include <stdexcept> @@ -64,10 +65,8 @@ namespace brep HANDLER_DIAG; - // @@ TODO: disable service if HMAC is not specified in config. - // - if (false) - throw invalid_request (404, "CI request submission disabled"); + if (!options_->ci_github_app_webhook_secret_specified ()) + throw invalid_request (404, "GitHub CI request submission disabled"); // Process headers. // @@ -81,14 +80,35 @@ namespace brep // more context would be useful? // string event; // Webhook event. + string hmac; // Received HMAC. { bool content_type (false); for (const name_value& h: rq.headers ()) { + // HMAC authenticating this request. Note that it won't be present + // unless a webhook secret has been set in the GitHub app's settings. + // + if (icasecmp (h.name, "x-hub-signature-256") == 0) + { + if (!h.value) + throw invalid_request (400, "missing x-hub-signature-256 value"); + + // Parse the x-hub-signature-256 header value. For example: + // + // sha256=5e82258... + // + // Check for the presence of the "sha256=" prefix and then strip it + // to leave only the HMAC value. + // + if (h.value->find ("sha256=", 0, 7) == string::npos) + throw invalid_request (400, "invalid x-hub-signature-256 value"); + + hmac = h.value->substr (7); + } // This event's UUID. // - if (icasecmp (h.name, "x-github-delivery") == 0) + else if (icasecmp (h.name, "x-github-delivery") == 0) { // @@ TODO Check that delivery UUID has not been received before // (replay attack). @@ -123,6 +143,59 @@ namespace brep if (event.empty ()) throw invalid_request (400, "missing x-github-event header"); + + if (hmac.empty ()) + throw invalid_request (400, "missing x-hub-signature-256 header"); + } + + // Read the entire request body into a buffer because we need to compute + // an HMAC over it and then parse it as JSON. The alternative of reading + // from the stream twice works out to be more complicated (see also @@ + // TODO item in web/server/module.hxx). + // + string body; + { + // Note that even though we may not need caching right now, we may later + // (e.g., to support cancel) so let's just enable it right away. + // + size_t limit (128 * 1024); + + istream& is (rq.content (limit, limit)); + + try + { + getline (is, body, '\0'); + } + catch (const io_error& e) + { + fail << "unable to read request body: " << e; + } + } + + // Verify the received HMAC. + // + // Compute the HMAC value over the request body using the configured + // webhook secret as key and compare it to the received HMAC. + // + try + { + string h ( + compute_hmac (*options_, + body.data (), body.size (), + options_->ci_github_app_webhook_secret ().c_str ())); + + if (!icasecmp (h, hmac)) + { + string m ("computed HMAC does not match received HMAC"); + + error << m; + + throw invalid_request (400, move (m)); + } + } + catch (const system_error& e) + { + fail << "unable to compute request HMAC: " << e; } // There is a webhook event (specified in the x-github-event header) and @@ -140,7 +213,7 @@ namespace brep check_suite_event cs; try { - json::parser p (rq.content (64 * 1024), "check_suite event"); + json::parser p (body.data (), body.size (), "check_suite event"); cs = check_suite_event (p); } diff --git a/mod/module.cli b/mod/module.cli index 7c07dbc..104337b 100644 --- a/mod/module.cli +++ b/mod/module.cli @@ -862,11 +862,18 @@ namespace brep "The GitHub App ID. Found in the app's settings on GitHub." } + string ci-github-app-webhook-secret + { + "<secret>", + "The GitHub App's configured webhook secret. If not set, then the + GitHub CI service is disabled." + } + path ci-github-app-private-key { "<path>", "The private key used during GitHub API authentication. Created in - the GitHub app's settings." + the GitHub App's settings." } uint16_t ci-github-jwt-validity-period = 600 diff --git a/web/server/module.hxx b/web/server/module.hxx index 20f6217..1b067f5 100644 --- a/web/server/module.hxx +++ b/web/server/module.hxx @@ -82,6 +82,11 @@ namespace web using name_values = std::vector<name_value>; using butl::path; + // @@ TODO: Expose the request input stream rewinding support provided by + // web::apache::request::rewind(). Might come in useful in cases + // where more than one thing needs to be done with the request + // body, e.g., compute its MAC and then parse its contents. + // class request { public: |