diff options
| -rw-r--r-- | mod/jwt.cxx | 64 | ||||
| -rw-r--r-- | mod/jwt.hxx | 12 | ||||
| -rw-r--r-- | mod/mod-ci-github.cxx | 9 |
3 files changed, 54 insertions, 31 deletions
diff --git a/mod/jwt.cxx b/mod/jwt.cxx index 11b76ec..43b8629 100644 --- a/mod/jwt.cxx +++ b/mod/jwt.cxx @@ -52,18 +52,18 @@ using namespace butl; // // base64url($header) + '.' + base64url($payload) + '.' + base64url($signature) // -string -brep::gen_jwt (const options::openssl_options& o, - const path& pk, - const string& iss, - const std::chrono::minutes& vp) +string brep:: +gen_jwt (const options::openssl_options& o, + const path& pk, + const string& iss, + const std::chrono::minutes& vp) { // Create the header. // string h; // Header (base64url-encoded). { vector<char> b; - json::buffer_serializer s (b); + json::buffer_serializer s (b, 0 /* indentation */); s.begin_object (); s.member ("typ", "JWT"); @@ -81,11 +81,6 @@ brep::gen_jwt (const options::openssl_options& o, // "Issued at" time. // - // @@ TODO GitHub recommends setting this time to 60 seconds in the past - // to combat clock drift. Seems likely to be a general problem - // with client/server authentication schemes so perhaps passing - // the expected drift/skew as an argument might make sense? - // seconds iat ( duration_cast<seconds> (system_clock::now ().time_since_epoch ())); @@ -109,9 +104,10 @@ brep::gen_jwt (const options::openssl_options& o, // // The signature (base64url-encoded). Will be left empty if openssl exits - // with a non-zero status. + // with a non-zero status. @@ // string s; + try { // Sign the concatenated header and payload using openssl. // @@ -119,27 +115,49 @@ brep::gen_jwt (const options::openssl_options& o, // // Note that RSA is indicated by the contents of the private key. // + // Note that here we assume both output and diagnostics will fit into pipe + // buffers and don't both with fdselect(). + // openssl os (path ("-"), // Read message from openssl::out. path ("-"), // Write output to openssl::in. 2, // Diagnostics to stderr. process_env (o.openssl (), o.openssl_envvar ()), "dgst", o.openssl_option (), "-sha256", "-sign", pk); - // Write the concatenated header and payload to openssl's input. - // - os.out << h << '.' << p; - os.out.close (); + // @@ TODO redirect stderr to pipe/ofdstream. - // Read the binary signature from openssl's output. - // - vector<char> bs (os.in.read_binary ()); - os.in.close (); + try + { + // Write the concatenated header and payload to openssl's input. + // + os.out << h << '.' << p; + os.out.close (); - if (os.wait ()) - s = base64url_encode (bs); + // Read the binary signature from openssl's output. + // + vector<char> bs (os.in.read_binary ()); + os.in.close (); + } + catch (const io_error&) + { + if (!os.wait ()) + throw system_error (); // @@ + + // Fall through. + } + + if (!os.wait ()) + throw system_error (); // @@ + + s = base64url_encode (bs); + } + catch () + { + // @@ TODO: catch all possible errors and translate to suitable + // system_error. } - // Return the token, or empty if openssl exited with a non-zero status. + // Return the token, or empty if openssl exited with a non-zero status. @@ // return !s.empty () ? h + '.' + p + '.' + s diff --git a/mod/jwt.hxx b/mod/jwt.hxx index 65ad5c5..25e9c21 100644 --- a/mod/jwt.hxx +++ b/mod/jwt.hxx @@ -10,7 +10,7 @@ namespace brep { - // Generate a JSON Web Token (JWT), defined in RFC 7519. + // Generate a JSON Web Token (JWT), defined in RFC7519. // // A JWT is essentially the token issuer's name along with a number of // claims, signed with a private key. @@ -20,16 +20,18 @@ namespace brep // // The token expires when the validity period has elapsed. // - // Return the token or empty if openssl exited with a non-zero status. + // The backdate argument specifies the number of seconds to subtract from + // the "issued at" time in order to combat potential clock drift (which can + // casue the token to be not valid yet). // - // Throw process_error or io_error (both derived from std::system_error) if - // openssl could not be executed or communication with its process failed. + // Return the token or std::system_error in case if an error. // string gen_jwt (const options::openssl_options&, const path& private_key, const string& issuer, - const std::chrono::minutes& validity_period); + const std::chrono::minutes& validity_period, + const std::chrono::seconds& backdate = 60); } #endif diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx index a82ce43..86f52b9 100644 --- a/mod/mod-ci-github.cxx +++ b/mod/mod-ci-github.cxx @@ -114,7 +114,8 @@ struct repository string full_name; string default_branch; - explicit repository (json::parser&); + explicit + repository (json::parser&); repository () = default; }; @@ -125,7 +126,8 @@ struct check_suite_event ::check_suite check_suite; ::repository repository; - explicit check_suite_event (json::parser&); + explicit + check_suite_event (json::parser&); check_suite_event () = default; }; @@ -234,6 +236,7 @@ handle (request& rq, response& rs) try { // Use the maximum validity period allowed by GitHub (10 minutes). + // @@ Let's make configurable. // string jwt (gen_jwt (*options_, options_->ci_github_app_private_key (), @@ -270,7 +273,7 @@ handle (request& rq, response& rs) // @@ TODO: should we write more detailed diagnostics to log? Maybe we // should do this for all unsuccessful calls to respond(). // - // @@ TMP These exceptions end up in the apache error log. + // Note: these exceptions end up in the apache error log. // throw invalid_request (400, "malformed JSON in request body"); } |