diff options
Diffstat (limited to 'mod/mod-ci-github.cxx')
| -rw-r--r-- | mod/mod-ci-github.cxx | 110 |
1 files changed, 104 insertions, 6 deletions
diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx index e19a41b..7cdc01d 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,12 @@ namespace brep HANDLER_DIAG; - // @@ TODO: disable service if HMAC is not specified in config. - // - if (false) + if (!options_->ci_github_app_webhook_secret_specified ()) + { + error << "webhook secret not configured"; + throw invalid_request (404, "CI request submission disabled"); + } // Process headers. // @@ -80,15 +83,26 @@ namespace brep // we already do for the most part), and "webhook event" only when // more context would be useful? // - string event; // Webhook event. + string event; // Webhook event. + string r_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"); + + r_hmac = *h.value; + } // 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). @@ -118,6 +132,21 @@ namespace brep } } + // Parse the x-hub-signature-256 header value. For example: + // + // sha256=5e822587094c68e646db8b916da1db2056d92f1dea4252136a533b4147a30cb7 + // + // Check for the presence of the "sha256=" prefix and then strip it to + // leave only the HMAC. + // + if (r_hmac.find ("sha256=", 0, 7) == string::npos) + { + throw invalid_request (400, + "invalid x-hub-signature-256 value: '" + + r_hmac + '\''); + } + r_hmac = r_hmac.substr (7); + if (!content_type) throw invalid_request (400, "missing content-type header"); @@ -125,6 +154,75 @@ namespace brep throw invalid_request (400, "missing x-github-event 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 and expensive. + // + // @@ TMP Although it is possible (with a bit of work) to rewind the + // request's input stream, in that case we'd have to read from it + // twice (once for the HMAC and once for the JSON parsing) instead + // of just this once, temp buffer(s) would be necessary, and we'd + // still be storing the entire request body in memory anyway (just + // not here). + // + vector<char> body; + { + // Note: buffer=0 disables caching. + // + istream& is (rq.content (64 * 1024, 0 /* buffer */)); + + // Note that istream::read() sets failbit if unable to read the + // requested number of bytes. + // + is.exceptions (istream::badbit); + + try + { + size_t n (0); // Total bytes read. + + while (!eof (is)) + { + body.resize (n + 8192); + is.read (body.data () + n, 8192); + n += is.gcount (); + } + + body.resize (n); + } + catch (const io_error& e) + { + fail << "unable to read request body: " << e; + } + } + + // Verify the received HMAC. + // + // Compute the HMAC over the request body using the configured webhook + // secret as key and compare it to the received HMAC. + // + try + { + string hmac ( + compute_hmac (*options_, + body, + options_->ci_github_app_webhook_secret ().c_str ())); + + // @@ TODO GitHub recommends constant-time comparison (timing attack). + // + if (!icasecmp (hmac, r_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 // each event contains a bunch of actions (specified in the JSON request // body). @@ -140,7 +238,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); } |