From 1f268737e9e47b8580cc3c93028c8616fbd16a55 Mon Sep 17 00:00:00 2001 From: Madhava Jay Date: Tue, 12 May 2020 22:01:40 +1000 Subject: [PATCH] Added deploy script and fixed package namespace issue (#46) * Convert to HTTP handlers inside outside of the main package Previously, the top-level package exported functions with the signature `func(*util.Context) util.StatusError`, and the `main` package was responsible for wrapping these and producing normal HTTP handlers. However, the Google Cloud Functions execution environment expects to be able to invoke HTTP handlers which are exported directly from the top-level package. We move the logic of converting these inner handlers into HTTP handlers to the `internal/util` package, allowing us to expose HTTP handlers from the top-level package * Added deploy script and fixed package namespace issue - functions package renamed to upload-token.functions - added CheckHTTPS function to detect HTTP calls from headers - added HSTS header - updated README.md - updated Postman JSON file Co-authored-by: Joshua Liebow-Feeser --- .firebaserc | 5 - .gcloudignore | 20 +++ COVID-Watch.postman_collection.json | 247 ---------------------------- Covid Watch.postman_collection.json | 135 +++++++++++++++ README.md | 52 +++++- deploy.sh | 2 + functions/challenge.go | 6 +- functions/cmd/main.go | 2 +- functions/go.mod | 2 +- functions/go.sum | 1 + functions/internal/pow/pow.go | 2 +- functions/internal/report/token.go | 2 +- functions/internal/util/handler.go | 9 + functions/internal/util/util.go | 94 +++++++++++ 14 files changed, 311 insertions(+), 268 deletions(-) delete mode 100644 .firebaserc create mode 100644 .gcloudignore delete mode 100644 COVID-Watch.postman_collection.json create mode 100644 Covid Watch.postman_collection.json create mode 100755 deploy.sh diff --git a/.firebaserc b/.firebaserc deleted file mode 100644 index 30c7fe5..0000000 --- a/.firebaserc +++ /dev/null @@ -1,5 +0,0 @@ -{ - "projects": { - "default": "covidwatch-354ce" - } -} \ No newline at end of file diff --git a/.gcloudignore b/.gcloudignore new file mode 100644 index 0000000..d6a9396 --- /dev/null +++ b/.gcloudignore @@ -0,0 +1,20 @@ +# This file specifies files that are *not* uploaded to Google Cloud Platform +# using gcloud. It follows the same syntax as .gitignore, with the addition of +# "#!include" directives (which insert the entries of the given .gitignore-style +# file at that point). +# +# For more information, run: +# $ gcloud topic gcloudignore +# +.gcloudignore +# If you would like to upload your .git directory, .gitignore file or files +# from your .gitignore file, remove the corresponding line +# below: +.git +.gitignore + +#!include:.gitignore +.firebase.json +.firestore.indexes.json +firestore.rules +.github diff --git a/COVID-Watch.postman_collection.json b/COVID-Watch.postman_collection.json deleted file mode 100644 index f7f1b90..0000000 --- a/COVID-Watch.postman_collection.json +++ /dev/null @@ -1,247 +0,0 @@ -{ - "info": { - "_postman_id": "3b1ece4f-159d-4b6b-8788-7937b7f96c34", - "name": "COVID-Watch", - "schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json" - }, - "item": [ - { - "name": "Emulator DELETE ALL", - "request": { - "method": "DELETE", - "header": [], - "url": { - "raw": "http://localhost:8080/emulator/v1/projects/covidwatch-354ce/databases/(default)/documents", - "protocol": "http", - "host": [ - "localhost" - ], - "port": "8080", - "path": [ - "emulator", - "v1", - "projects", - "covidwatch-354ce", - "databases", - "(default)", - "documents" - ] - } - }, - "response": [] - }, - { - "name": "v1 TCN GET ALL Signed Reports", - "request": { - "method": "GET", - "header": [], - "url": { - "raw": "http://localhost:8080/v1/projects/covidwatch-354ce/databases/(default)/documents/signed_reports", - "protocol": "http", - "host": [ - "localhost" - ], - "port": "8080", - "path": [ - "v1", - "projects", - "covidwatch-354ce", - "databases", - "(default)", - "documents", - "signed_reports" - ] - } - }, - "response": [] - }, - { - "name": "v1 TCN POST New Signed Report", - "request": { - "method": "POST", - "header": [], - "body": { - "mode": "raw", - "raw": "{\n \"temporary_contact_key_bytes\": \"PvLGpfQZgGqnoQRtSr0AHd8J5/WdKwaJNLRCkhGlgHU=\",\n \"memo_data\": \"SGVsbG8sIFdvcmxkIQ==\",\n \"memo_type\": 1,\n \"start_index\": 1,\n \"end_index\": 8,\n \"signature_bytes\": \"+k7HDsVZPY5Pxcz0cpwVBvDOHrrQ0+AyDVL/MbGkXBYG2WAyoqLaNxFuXiB9rSzkdCesDv1NSSk06hrjx2YABA==\",\n \"report_verification_public_key_bytes\": \"v78liBBYQrFXqOH6YydUD1aGpXLMgruKATAjFZ0ycLk=\"\n}", - "options": { - "raw": { - "language": "json" - } - } - }, - "url": { - "raw": "http://localhost:5001/covidwatch-354ce/us-central1/submitReport", - "protocol": "http", - "host": [ - "localhost" - ], - "port": "5001", - "path": [ - "covidwatch-354ce", - "us-central1", - "submitReport" - ] - } - }, - "response": [] - }, - { - "name": "v1 LIVE GET ALL Signed Reports", - "request": { - "method": "GET", - "header": [], - "url": { - "raw": "https://firestore.googleapis.com/v1/projects/covidwatch-354ce/databases/(default)/documents/signed_reports", - "protocol": "https", - "host": [ - "firestore", - "googleapis", - "com" - ], - "path": [ - "v1", - "projects", - "covidwatch-354ce", - "databases", - "(default)", - "documents", - "signed_reports" - ] - } - }, - "response": [] - }, - { - "name": "v2 AG GET ALL Diagnosis", - "protocolProfileBehavior": { - "disableBodyPruning": true - }, - "request": { - "method": "GET", - "header": [], - "body": { - "mode": "raw", - "raw": "{\n\t\"timestamp\": \"2020-04-30T03:11:54.097Z\"\n}", - "options": { - "raw": { - "language": "json" - } - } - }, - "url": { - "raw": "http://localhost:5001/covidwatch-354ce/us-central1/fetchDiagnosis", - "protocol": "http", - "host": [ - "localhost" - ], - "port": "5001", - "path": [ - "covidwatch-354ce", - "us-central1", - "fetchDiagnosis" - ] - } - }, - "response": [] - }, - { - "name": "v2 AG POST New Diagnosis", - "request": { - "method": "POST", - "header": [], - "body": { - "mode": "raw", - "raw": "{\n \"diagnosis_keys\": [\n {\n \"key_data\": \"PvLGpfQZgGqnoQRtSr0AHd8J5/WdKwaJNLRCkhGlgHU=\",\n \"rolling_start_number\": 1,\n \"transmission_risk_level\": 0\n },\n {\n \"key_data\": \"AvLGpfQZgGqnoQRtSr0AHd8J5/WdKwaJNLRCkhGlgHU=\",\n \"rolling_start_number\": 2,\n \"transmission_risk_level\": 255\n }\n ],\n \"public_health_authority_permission_number\": \"osghr\"\n}\n", - "options": { - "raw": { - "language": "json" - } - } - }, - "url": { - "raw": "http://localhost:5001/covidwatch-354ce/us-central1/submitDiagnosis", - "protocol": "http", - "host": [ - "localhost" - ], - "port": "5001", - "path": [ - "covidwatch-354ce", - "us-central1", - "submitDiagnosis" - ] - } - }, - "response": [] - }, - { - "name": "v2 SECURE GET Generate Permission Number", - "protocolProfileBehavior": { - "disableBodyPruning": true - }, - "request": { - "method": "GET", - "header": [], - "body": { - "mode": "raw", - "raw": "", - "options": { - "raw": { - "language": "json" - } - } - }, - "url": { - "raw": "http://localhost:5001/covidwatch-354ce/us-central1/createPermissionNumber", - "protocol": "http", - "host": [ - "localhost" - ], - "port": "5001", - "path": [ - "covidwatch-354ce", - "us-central1", - "createPermissionNumber" - ] - } - }, - "response": [] - }, - { - "name": "v2 SECURE GET ALL Permission Numbers", - "request": { - "auth": { - "type": "bearer", - "bearer": [ - { - "key": "token", - "value": "owner", - "type": "string" - } - ] - }, - "method": "GET", - "header": [], - "url": { - "raw": "http://localhost:8080/v1/projects/covidwatch-354ce/databases/(default)/documents/diagnosis_permission_number", - "protocol": "http", - "host": [ - "localhost" - ], - "port": "8080", - "path": [ - "v1", - "projects", - "covidwatch-354ce", - "databases", - "(default)", - "documents", - "diagnosis_permission_number" - ] - } - }, - "response": [] - } - ], - "protocolProfileBehavior": {} -} \ No newline at end of file diff --git a/Covid Watch.postman_collection.json b/Covid Watch.postman_collection.json new file mode 100644 index 0000000..da9ade6 --- /dev/null +++ b/Covid Watch.postman_collection.json @@ -0,0 +1,135 @@ +{ + "info": { + "_postman_id": "82f0dd05-50ae-4e45-b1d4-9c77824286e2", + "name": "Covid Watch", + "schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json" + }, + "item": [ + { + "name": "Emulator DELETE ALL", + "request": { + "method": "DELETE", + "header": [], + "url": { + "raw": "http://localhost:8081/emulator/v1/projects/test/databases/(default)/documents", + "protocol": "http", + "host": [ + "localhost" + ], + "port": "8081", + "path": [ + "emulator", + "v1", + "projects", + "test", + "databases", + "(default)", + "documents" + ] + } + }, + "response": [] + }, + { + "name": "Emulator GET ALL challenges", + "request": { + "auth": { + "type": "bearer", + "bearer": [ + { + "key": "token", + "value": "owner", + "type": "string" + } + ] + }, + "method": "GET", + "header": [], + "url": { + "raw": "http://localhost:8081/v1/projects/test/databases/(default)/documents/challenges", + "protocol": "http", + "host": [ + "localhost" + ], + "port": "8081", + "path": [ + "v1", + "projects", + "test", + "databases", + "(default)", + "documents", + "challenges" + ] + } + }, + "response": [] + }, + { + "name": "v1 GET /challenge POW", + "request": { + "auth": { + "type": "bearer", + "bearer": [ + { + "key": "token", + "value": "owner", + "type": "string" + } + ] + }, + "method": "GET", + "header": [ + { + "key": "X-Forwarded-Proto", + "value": "https", + "description": "Bypass HTTP check (only 1 required)", + "type": "text" + }, + { + "key": "Forwarded", + "value": "for=\\\"localhost\\\";proto=https", + "description": "Bypass HTTP check (only 1 required)", + "type": "text", + "disabled": true + } + ], + "url": { + "raw": "http://localhost:8080/challenge", + "protocol": "http", + "host": [ + "localhost" + ], + "port": "8080", + "path": [ + "challenge" + ] + } + }, + "response": [] + } + ], + "event": [ + { + "listen": "prerequest", + "script": { + "id": "7e3831e8-b404-4dbe-830c-4290d06b7715", + "type": "text/javascript", + "exec": [ + "" + ] + } + }, + { + "listen": "test", + "script": { + "id": "4ddc2ad9-9394-4392-97b5-65317ca81e6e", + "type": "text/javascript", + "exec": [ + "" + ] + } + } + ], + "protocolProfileBehavior": {} +} \ No newline at end of file diff --git a/README.md b/README.md index 1f04f8f..6f8b784 100644 --- a/README.md +++ b/README.md @@ -59,8 +59,7 @@ the new one and cause confusion). If the `--host-port` flag is omitted, the emulator will choose a random port, which has a high likelihood of not being in use. The emulator will output which -address to use by displaying a line like `export -FIRESTORE_EMULATOR_HOST=::1:8195`. +address to use by displaying a line like `export FIRESTORE_EMULATOR_HOST=::1:8195`. Note that, if the local IP address is an IPv6 address (like `::1`), then you will need to put square brackets around the address for compatibility with Go's @@ -71,16 +70,51 @@ FIRESTORE_EMULATOR_HOST=[::1]:8195`. You can hit the endpoints with curl or Postman. +Try: + +``` +curl --request GET 'http://localhost:8080/challenge' \ + --header 'X-Forwarded-Proto: https' +``` + +### HTTPS + +The local dev server uses HTTP so you must send a fake HTTPS header to prevent the +request from being rejected. + +If you see an error message like this with HTTP Code 426: + +``` +{"Error":"Please use HTTPS"} +``` + +You should add either: + +``` +X-Forwarded-Proto: https +``` + +or + +``` +Forwarded: for=\"localhost\";proto=https +``` + ## Firebase Security -Unauthenticated Firestore access is disabled. If you want to access the emulator -you can send the following header to override the security with the bearer token -"owner". +Unauthenticated Firestore access is disabled. If you want to bypass the firestore.rules +on the emulator REST interface just supply the auth bearer header value "owner": -```text -curl --location \ - --request GET 'http://localhost:8080/v1/projects/covidwatch-354ce/databases/(default)/documents/challenges' \ - --header 'Authorization: Bearer owner' +``` +Authorization: Bearer owner +``` + +## Deployment + +You can deploy the functions with: + +``` +$ ./deploy.sh ``` ### Postman Collection diff --git a/deploy.sh b/deploy.sh new file mode 100755 index 0000000..1ce8538 --- /dev/null +++ b/deploy.sh @@ -0,0 +1,2 @@ +#!/bin/sh +cd functions && gcloud functions deploy challenge --runtime go113 --trigger-http --entry-point ChallengeHandler --allow-unauthenticated diff --git a/functions/challenge.go b/functions/challenge.go index e9d3dc0..5170b41 100644 --- a/functions/challenge.go +++ b/functions/challenge.go @@ -3,8 +3,8 @@ package functions import ( "encoding/json" - "functions/internal/pow" - "functions/internal/util" + "upload-token.functions/internal/pow" + "upload-token.functions/internal/util" ) // ChallengeHandler is a handler for the /challenge endpoint. @@ -22,4 +22,4 @@ func challengeHandler(ctx *util.Context) util.StatusError { json.NewEncoder(ctx.HTTPResponseWriter()).Encode(c) return nil -} +} \ No newline at end of file diff --git a/functions/cmd/main.go b/functions/cmd/main.go index e9c689d..08ef01c 100644 --- a/functions/cmd/main.go +++ b/functions/cmd/main.go @@ -5,7 +5,7 @@ import ( "log" "os" - "functions" + "upload-token.functions" "github.com/GoogleCloudPlatform/functions-framework-go/funcframework" ) diff --git a/functions/go.mod b/functions/go.mod index e1c07b4..7e2f11c 100644 --- a/functions/go.mod +++ b/functions/go.mod @@ -1,4 +1,4 @@ -module functions +module upload-token.functions // As of this writing, Google Cloud runtime is on 1.13.8 in beta. go 1.13 diff --git a/functions/go.sum b/functions/go.sum index 312c32b..476321c 100644 --- a/functions/go.sum +++ b/functions/go.sum @@ -66,6 +66,7 @@ github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDk github.com/cloudevents/sdk-go v1.0.0 h1:gS5I0s2qPmdc4GBPlUmzZU7RH30BaiOdcRJ1RkXnPrc= github.com/cloudevents/sdk-go v1.0.0/go.mod h1:3TkmM0cFqkhCHOq5JzzRU/RxRkwzoS8TZ+G448qVTog= github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= +upload-token.functions v0.0.0-20200510234420-82addb73d71b h1:oQR/KUGwEJe/mU1diDgu4iFz2viloj0bvsMKkL5zQQw= github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= diff --git a/functions/internal/pow/pow.go b/functions/internal/pow/pow.go index cc735e1..1b15510 100644 --- a/functions/internal/pow/pow.go +++ b/functions/internal/pow/pow.go @@ -14,7 +14,7 @@ import ( "golang.org/x/crypto/argon2" - "functions/internal/util" + "upload-token.functions/internal/util" ) const ( diff --git a/functions/internal/report/token.go b/functions/internal/report/token.go index f3a3131..1f72ac4 100644 --- a/functions/internal/report/token.go +++ b/functions/internal/report/token.go @@ -7,7 +7,7 @@ import ( "strconv" "strings" - "functions/internal/util" + "upload-token.functions/internal/util" ) // On the design of the upload token (see [1] for more details): diff --git a/functions/internal/util/handler.go b/functions/internal/util/handler.go index 080f8a8..3129945 100644 --- a/functions/internal/util/handler.go +++ b/functions/internal/util/handler.go @@ -17,6 +17,15 @@ type Handler = func(ctx *Context) StatusError // - Converting any errors into an HTTP response func MakeHTTPHandler(handler func(ctx *Context) StatusError) func(http.ResponseWriter, *http.Request) { return func(w http.ResponseWriter, r *http.Request) { + // Add HSTS header. + addHSTS(w) + + // Reject insecure HTTP requests. + if err := checkHTTPS(r); err != nil { + writeStatusError(w, r, err) + return + } + ctx, err := NewContext(w, r) if err != nil { writeStatusError(w, r, err) diff --git a/functions/internal/util/util.go b/functions/internal/util/util.go index 120c233..5e454fc 100644 --- a/functions/internal/util/util.go +++ b/functions/internal/util/util.go @@ -9,6 +9,8 @@ import ( "io" "net/http" "os" + "strings" + "regexp" "cloud.google.com/go/firestore" "google.golang.org/grpc/codes" @@ -29,6 +31,7 @@ type Context struct { // *http.Request. func NewContext(w http.ResponseWriter, r *http.Request) (Context, StatusError) { ctx := r.Context() + // In production, automatically detect credentials from the environment. projectID := firestore.DetectProjectID if os.Getenv("FIRESTORE_EMULATOR_HOST") != "" { @@ -130,6 +133,7 @@ func NewBadRequestError(err error) StatusError { // NewMethodNotAllowedError wraps err in a StatusError whose HTTPStatusCode // method returns http.StatusMethodNotAllowed and whose Message method returns // "unsupported method: " followed by the given method string. + func NewMethodNotAllowedError(method string) StatusError { return statusError{ code: http.StatusMethodNotAllowed, @@ -176,3 +180,93 @@ func ReadCryptoRandBytes(b []byte) { panic(fmt.Errorf("could not read random bytes: %v", err)) } } + +// newStatusError constructs a new statusError with the given code and error. +// The given error will be used as the message returned by StatusError.Message. +func newStatusError(code int, err error) statusError { + return statusError { + code: code, + error: err, + // Leave empty so that error.Error() will be used as the return value + // from Message. + message: "", + } +} + +// checkHTTPS retrieves the scheme from the X-Forwarded-Proto or RFC7239 + +// We do this because in the function running in a cloud container the TLS termination +// has happened upstream so we need to check the headers to reject HTTP only. +// Requests on GCE contain both of these headers and anything supplied by the client is +// overwritten. Locally in development mode we don't use HTTPS so the client should send +// one of these headers. + +var ( + // De-facto standard header keys. + xForwardedProto = http.CanonicalHeaderKey("X-Forwarded-Proto") + forwarded = http.CanonicalHeaderKey("Forwarded") // RFC7239 + + protoRegex = regexp.MustCompile(`(?i)(?:proto=)(https|http)`) +) + +func checkHTTPS(r *http.Request) StatusError { + var scheme string + + // Retrieve the scheme from X-Forwarded-Proto. + if proto := r.Header.Get(xForwardedProto); proto != "" { + scheme = strings.ToLower(proto) + } else if proto = r.Header.Get(forwarded); proto != "" { + // match should contain at least two elements if the protocol was + // specified in the Forwarded header. The first element will always be + // the 'proto=' capture, which we ignore. In the case of multiple proto + // parameters (invalid) we only extract the first. + if match := protoRegex.FindStringSubmatch(proto); len(match) == 2 { + scheme = strings.ToLower(match[1]) + } else if len(match) > 2 { + return NewInternalServerError( + fmt.Errorf("Header 'forward' has more than 2 elements")) + } + } + + // We want to ensure that clients always use HTTPS. Even if we don't + // serve our API over HTTP, if clients use HTTP, they are vulnerable + // to man-in-the-middle attacks in which the attacker communicates + // with our service over HTTPS. In order to prevent this, it is not + // sufficient to simply auto-upgrade to HTTPS (e.g., via a redirect + // status code in the 300s). If we do this, then code which + // erroneously uses HTTP will continue to work, and so it might get + // deployed. Instead, we have to ensure that such code breaks + // completely, alerting the code's developers to the issue, and + // ensuring that they will change the code to use HTTPS directly. + // Thus, we want an error code with the following properties: + // - Guaranteed that smart clients (such as web browsers) will not + // attempt to automatically upgrade to HTTPS + // - Doesn't have another meaning which might cause developers to + // overlook the error or lead them down the wrong path (e.g., if + // we chose 400 - bad request - they might go down the path of + // debugging their request format) + // + // For these reasons, we choose error code 418 - server is a teapot. + // It is as unlikely as any other error code to cause the client to + // automatically upgrade to HTTPS, and it is guaranteed to get a + // developer's attention, hopefully getting them to look at the + // response body, which will contain the relevant information. + if scheme != "https" { + return newStatusError(http.StatusTeapot, + errors.New("unsupported protocol HTTP; only HTTPS is supported")) + } + return nil +} + +// Add HSTS to force HTTPS usage. +// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Strict-Transport-Security +// In the following example, max-age is set to 2 years, raised from what was a former +// limit max-age of 1 year. Note that 1 year is acceptable for a domain to be included +// in browsers' HSTS preload lists. 2 years is, however, the recommended goal as a +// website's final HSTS configuration as explained on https://hstspreload.org. +// It also suffixed with preload which is necessary for inclusion in most major web +// browsers' HSTS preload lists, e.g. Chromium, Edge, & Firefox. +var headerHSTS = http.CanonicalHeaderKey("Strict-Transport-Security") +func addHSTS(w http.ResponseWriter) { + w.Header().Set(headerHSTS, "max-age=63072000; includeSubDomains; preload") +}