Verify webhook request HMACs

4 files changed, 211 insertions, 7 deletions

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