aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--mod/hmac.cxx92
-rw-r--r--mod/hmac.hxx29
-rw-r--r--mod/mod-ci-github.cxx110
-rw-r--r--mod/module.cli6
-rw-r--r--web/server/module.hxx5
5 files changed, 236 insertions, 6 deletions
diff --git a/mod/hmac.cxx b/mod/hmac.cxx
new file mode 100644
index 0000000..dc9eca0
--- /dev/null
+++ b/mod/hmac.cxx
@@ -0,0 +1,92 @@
+#include <mod/hmac.hxx>
+
+#include <libbutl/openssl.hxx>
+
+using namespace std;
+using namespace butl;
+
+string brep::
+compute_hmac (const options::openssl_options& o,
+ const vector<char>& m,
+ 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
+ //
+ 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.
+ try
+ {
+ // In case of exception, skip and close input after output.
+ //
+ // Note: re-open in/out so that they get automatically closed on
+ // exception.
+ //
+ ifdstream in (os.in.release (), fdstream_mode::skip);
+ ofdstream out (os.out.release ());
+
+ // Write the message to openssl's input.
+ //
+ out.write (m.data (), m.size ());
+ out.close ();
+
+ // Read the HMAC 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..104b629
--- /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.
+ //
+ // Return the HMAC 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 vector<char>& message,
+ const char* key);
+}
+
+#endif
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);
}
diff --git a/mod/module.cli b/mod/module.cli
index c635689..bec4c2d 100644
--- a/mod/module.cli
+++ b/mod/module.cli
@@ -832,6 +832,12 @@ 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."
+ }
+
path ci-github-app-private-key
{
"<path>",
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: