Switch to APIKey+IP rate limiting, HMAC values

cmd/adminapi/main.go

@@ -87,7 +87,7 @@ func realMain(ctx context.Context) error {
 	// Setup cacher
 	cacher, err := cache.CacherFor(ctx, &config.Cache, cache.MultiKeyFunc(
 		cache.HMACKeyFunc(sha1.New, config.Cache.HMACKey),
-		cache.PrefixKeyFunc("adminapi:"),
+		cache.PrefixKeyFunc("adminapi:cache:"),
 	))
 	if err != nil {
 		return fmt.Errorf("failed to create cacher: %w", err)
@@ -115,7 +115,7 @@ func realMain(ctx context.Context) error {
 	defer limiterStore.Close()
 
 	httplimiter, err := limitware.NewMiddleware(ctx, limiterStore,
-		limitware.APIKeyFunc(ctx, "adminapi", db),
+		limitware.APIKeyFunc(ctx, "adminapi:ratelimit:", config.RateLimit.HMACKey),
 		limitware.AllowOnError(false))
 	if err != nil {
 		return fmt.Errorf("failed to create limiter middleware: %w", err)

cmd/apiserver/main.go

@@ -90,7 +90,7 @@ func realMain(ctx context.Context) error {
 	// Setup cacher
 	cacher, err := cache.CacherFor(ctx, &config.Cache, cache.MultiKeyFunc(
 		cache.HMACKeyFunc(sha1.New, config.Cache.HMACKey),
-		cache.PrefixKeyFunc("apiserver:"),
+		cache.PrefixKeyFunc("apiserver:cache:"),
 	))
 	if err != nil {
 		return fmt.Errorf("failed to create cacher: %w", err)
@@ -128,7 +128,7 @@ func realMain(ctx context.Context) error {
 	defer limiterStore.Close()
 
 	httplimiter, err := limitware.NewMiddleware(ctx, limiterStore,
-		limitware.APIKeyFunc(ctx, "apiserver", db),
+		limitware.APIKeyFunc(ctx, "apiserver:ratelimit:", config.RateLimit.HMACKey),
 		limitware.AllowOnError(false))
 	if err != nil {
 		return fmt.Errorf("failed to create limiter middleware: %w", err)

cmd/server/main.go

@@ -106,7 +106,7 @@ func realMain(ctx context.Context) error {
 	// Setup cacher
 	cacher, err := cache.CacherFor(ctx, &config.Cache, cache.MultiKeyFunc(
 		cache.HMACKeyFunc(sha1.New, config.Cache.HMACKey),
-		cache.PrefixKeyFunc("server:"),
+		cache.PrefixKeyFunc("server:cache:"),
 	))
 	if err != nil {
 		return fmt.Errorf("failed to create cacher: %w", err)
@@ -161,7 +161,7 @@ func realMain(ctx context.Context) error {
 	defer limiterStore.Close()
 
 	httplimiter, err := limitware.NewMiddleware(ctx, limiterStore,
-		limitware.UserEmailKeyFunc(ctx, "server"),
+		limitware.UserIDKeyFunc(ctx, "server:ratelimit:", config.RateLimit.HMACKey),
 		limitware.AllowOnError(false))
 	if err != nil {
 		return fmt.Errorf("failed to create limiter middleware: %w", err)

docs/development.md

@@ -98,6 +98,13 @@ represent best practices.
     export CACHE_TYPE="IN_MEMORY"
     export CACHE_HMAC_KEY="/wC2dki5Z+To9iFwUamINtHIMOH/dME7e5Gy+9h3WTDBhqeeSYkqduZRjcZWwG3kPMdiWAdBxxop5wB+BHTBnSlfVVmy8qKVNv+Wf5ywgxV7SbB8bjNQBHSpn7aC5RxR6nkEsZ2w2fUhTJwD9q+MDo6TQvf+8OXEPrV1SXWNHrs="
 
+    # Configure rate limiter. Create your own values with:
+    #
+    #     openssl rand -base64 128
+    #
+    export RATE_LIMIT_TYPE="MEMORY"
+    export RATE_LIMIT_HMAC_KEY="/wC2dki5Z+To9iFwUamINtHIMOH/dME7e5Gy+9h3WTDBhqeeSYkqduZRjcZWwG3kPMdiWAdBxxop5wB+BHTBnSlfVVmy8qKVNv+Wf5ywgxV7SbB8bjNQBHSpn7aC5RxR6nkEsZ2w2fUhTJwD9q+MDo6TQvf+8OXEPrV1SXWNHrs="
+
     # Configure certificate key management. The CERTIFICATE_SIGNING_KEY should
     # be the value output in the previous step.
     export CERTIFICATE_KEY_MANAGER="FILESYSTEM"

docs/production.md

@@ -326,4 +326,29 @@ small and are automatically re-built on demand, so occasional rotation is likely
 fine for this system.
 
 
+### Rate limit HMAC keys
+
+**Recommended frequency:** 90 days, on breach
+
+This key is used as the HMAC key to named values in the rate limit. For example,
+API keys and IP addresses are rate limited. We do not want the rate limiter to
+have those values in plaintext, so the values are HMACed before being written
+(and HMACed on lookup). This prevents a server operator with access to the rate
+limiter (e.g. Redis) from seeing plaintext data about the system. The data is
+hashed instead of encrypted because we only need a deterministic value to
+lookup.
+
+To generate a new key:
+
+```sh
+openssl rand -base64 128 | tr -d "\n"
+```
+
+Use this value as of the `RATE_LIMIT_HMAC_KEY` environment variable:
+
+```sh
+RATE_LIMIT_HMAC_KEY="43+ViAkv7uHYKjsXhU468NGBZrtlJWtZqTORIiY8V6OMsLAZ+XmUF5He/wIhRlislnteTmChNi+BHveSgkxky81tpZSw45HKdK+XW3X5P7H6092I0u7H31C0NaInrxNxIRAbSw0NxSIKNbfKwucDu1Y36XjJC0pi0wlJHxkdGes="
+```
+
+
 [gcp-kms]: https://cloud.google.com/kms

pkg/ratelimit/config.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/google/exposure-notifications-verification-server/pkg/redis"
 	redigo "github.com/opencensus-integrations/redigo/redis"
+	"github.com/sethvargo/go-envconfig"
 	"github.com/sethvargo/go-limiter"
 	"github.com/sethvargo/go-limiter/memorystore"
 	"github.com/sethvargo/go-limiter/noopstore"
@@ -45,6 +46,10 @@ type Config struct {
 	Tokens   uint64        `env:"RATE_LIMIT_TOKENS, default=60"`
 	Interval time.Duration `env:"RATE_LIMIT_INTERVAL, default=1m"`
 
+	// HMACKey is the key to use when calculating the HMAC of keys before saving
+	// them in the rate limiter.
+	HMACKey envconfig.Base64Bytes `env:"RATE_LIMIT_HMAC_KEY, required"`
+
 	// Redis configuration
 	Redis redis.Config `env:",prefix=RATE_LIMIT_"`
 }

pkg/ratelimit/limitware/middleware.go

@@ -3,16 +3,15 @@ package limitware
 
 import (
 	"context"
+	"crypto/hmac"
 	"crypto/sha1"
 	"fmt"
-	"hash"
 	"net/http"
 	"strconv"
 	"strings"
 	"time"
 
 	"github.com/google/exposure-notifications-verification-server/pkg/controller"
-	"github.com/google/exposure-notifications-verification-server/pkg/database"
 	"github.com/google/exposure-notifications-verification-server/pkg/observability"
 
 	"github.com/google/exposure-notifications-server/pkg/logging"
@@ -197,94 +196,88 @@ func (m *Middleware) Handle(next http.Handler) http.Handler {
 }
 
 // APIKeyFunc returns a default key function for ratelimiting on our API key
-// header. It falls back to rate limiting by the client ip.
-func APIKeyFunc(ctx context.Context, scope string, db *database.Database) httplimit.KeyFunc {
+// header. Since APIKeys are assumed to be "public" at some point, they are
+// rate limited by [api-key,ip].
+func APIKeyFunc(ctx context.Context, scope string, hmacKey []byte) httplimit.KeyFunc {
 	logger := logging.FromContext(ctx).Named(scope + ".ratelimit")
-	ipAddrLimit := IPAddressKeyFunc(ctx, scope)
+	ipAddrLimit := IPAddressKeyFunc(ctx, scope, hmacKey)
 
 	return func(r *http.Request) (string, error) {
 		// Procss the API key
 		v := r.Header.Get("x-api-key")
 		if v != "" {
-			// v2 API keys encode the realm
-			_, realmID, err := db.VerifyAPIKeySignature(v)
-			if err == nil {
-				logger.Debugw("limiting by api key v2 realm", "realm", realmID)
-				dig, err := digest(sha1.New, strconv.FormatUint(uint64(realmID), 10))
-				if err != nil {
-					return "", fmt.Errorf("failed to digest realm id: %w", err)
-				}
-				return fmt.Sprintf("ratelimit:%s:realm:%x", scope, dig), nil
-			}
-
-			// v1 API keys do not, fallback to the database
-			app, err := db.FindAuthorizedAppByAPIKey(v)
-			if err == nil {
-				logger.Debugw("limiting by api key v1 realm", "realm", app.RealmID)
-				dig, err := digest(sha1.New, strconv.FormatUint(uint64(app.RealmID), 10))
-				if err != nil {
-					return "", fmt.Errorf("failed to digest realm id: %w", err)
-				}
-				return fmt.Sprintf("ratelimit:%s:realm:%x", scope, dig), nil
+			logger.Debugw("limiting by apikey")
+			dig, err := digest(fmt.Sprintf("%s:%s", v, remoteIP(r)), hmacKey)
+			if err != nil {
+				return "", fmt.Errorf("failed to digest api key: %w", err)
 			}
+			return fmt.Sprintf("%sapikey:%s", scope, dig), nil
 		}
 
 		return ipAddrLimit(r)
 	}
 }
 
-// UserEmailKeyFunc pulls the user out of the request context and uses that to
+// UserIDKeyFunc pulls the user out of the request context and uses that to
 // ratelimit. It falls back to rate limiting by the client ip.
-func UserEmailKeyFunc(ctx context.Context, scope string) httplimit.KeyFunc {
+func UserIDKeyFunc(ctx context.Context, scope string, hmacKey []byte) httplimit.KeyFunc {
 	logger := logging.FromContext(ctx).Named(scope + ".ratelimit")
-	ipAddrLimit := IPAddressKeyFunc(ctx, scope)
+	ipAddrLimit := IPAddressKeyFunc(ctx, scope, hmacKey)
 
 	return func(r *http.Request) (string, error) {
 		ctx := r.Context()
 
 		// See if a user exists on the context
 		user := controller.UserFromContext(ctx)
-		if user != nil && user.Email != "" {
+		if user != nil {
 			logger.Debugw("limiting by user", "user", user.ID)
-			dig, err := digest(sha1.New, strconv.FormatUint(uint64(user.ID), 10))
+			dig, err := digest(fmt.Sprintf("%d", user.ID), hmacKey)
 			if err != nil {
 				return "", fmt.Errorf("failed to digest user id: %w", err)
 			}
-			return fmt.Sprintf("ratelimit:%s:user:%x", scope, dig), nil
+			return fmt.Sprintf("%suser:%x", scope, dig), nil
 		}
 
 		return ipAddrLimit(r)
 	}
 }
 
 // IPAddressKeyFunc uses the client IP to rate limit.
-func IPAddressKeyFunc(ctx context.Context, scope string) httplimit.KeyFunc {
+func IPAddressKeyFunc(ctx context.Context, scope string, hmacKey []byte) httplimit.KeyFunc {
 	logger := logging.FromContext(ctx).Named(scope + ".ratelimit")
 
 	return func(r *http.Request) (string, error) {
 		// Get the remote addr
-		ip := r.RemoteAddr
-
-		// Check if x-forwarded-for exists, the load balancer sets this, and the
-		// first entry is the real client IP
-		xff := r.Header.Get("x-forwarded-for")
-		if xff != "" {
-			ip = strings.Split(xff, ",")[0]
-		}
+		ip := remoteIP(r)
 
 		logger.Debugw("limiting by ip", "ip", ip)
-		dig, err := digest(sha1.New, ip)
+		dig, err := digest(ip, hmacKey)
 		if err != nil {
 			return "", fmt.Errorf("failed to digest ip: %w", err)
 		}
-		return fmt.Sprintf("ratelimit:%s:ip:%x", scope, dig), nil
+		return fmt.Sprintf("%sip:%x", scope, dig), nil
 	}
 }
 
+// remoteIP returns the "real" remote IP.
+func remoteIP(r *http.Request) string {
+	// Get the remote addr
+	ip := r.RemoteAddr
+
+	// Check if x-forwarded-for exists, the load balancer sets this, and the
+	// first entry is the real client IP
+	xff := r.Header.Get("x-forwarded-for")
+	if xff != "" {
+		ip = strings.Split(xff, ",")[0]
+	}
+
+	return ip
+}
+
 // digest returns the digest of given string as a hex-encoded string, and any
 // errors that occur while hashing.
-func digest(hasher func() hash.Hash, in string) (string, error) {
-	h := hasher()
+func digest(in string, key []byte) (string, error) {
+	h := hmac.New(sha1.New, key)
 	n, err := h.Write([]byte(in))
 	if err != nil {
 		return "", err

terraform/redis.tf

@@ -52,6 +52,28 @@ resource "google_secret_manager_secret_version" "cache-hmac-key" {
   secret_data = random_id.cache-hmac-key.b64_std
 }
 
+# Create secret for the HMAC ratelimit keys
+resource "random_id" "ratelimit-hmac-key" {
+  byte_length = 128
+}
+
+resource "google_secret_manager_secret" "ratelimit-hmac-key" {
+  secret_id = "ratelimit-hmac-key"
+
+  replication {
+    automatic = true
+  }
+
+  depends_on = [
+    google_project_service.services["secretmanager.googleapis.com"],
+  ]
+}
+
+resource "google_secret_manager_secret_version" "ratelimit-hmac-key" {
+  secret      = google_secret_manager_secret.ratelimit-hmac-key.id
+  secret_data = random_id.ratelimit-hmac-key.b64_std
+}
+
 output "redis_host" {
   value = google_redis_instance.cache.host
 }

terraform/service_admin_apiserver.tf

@@ -85,6 +85,12 @@ resource "google_secret_manager_secret_iam_member" "adminapi-cache-hmac-key" {
   member    = "serviceAccount:${google_service_account.adminapi.email}"
 }
 
+resource "google_secret_manager_secret_iam_member" "adminapi-ratelimit-hmac-key" {
+  secret_id = google_secret_manager_secret.ratelimit-hmac-key.id
+  role      = "roles/secretmanager.secretAccessor"
+  member    = "serviceAccount:${google_service_account.adminapi.email}"
+}
+
 resource "google_cloud_run_service" "adminapi" {
   name     = "adminapi"
   location = var.region

terraform/service_apiserver.tf

@@ -91,6 +91,12 @@ resource "google_secret_manager_secret_iam_member" "apiserver-cache-hmac-key" {
   member    = "serviceAccount:${google_service_account.apiserver.email}"
 }
 
+resource "google_secret_manager_secret_iam_member" "apiserver-ratelimit-hmac-key" {
+  secret_id = google_secret_manager_secret.ratelimit-hmac-key.id
+  role      = "roles/secretmanager.secretAccessor"
+  member    = "serviceAccount:${google_service_account.apiserver.email}"
+}
+
 resource "google_cloud_run_service" "apiserver" {
   name     = "apiserver"
   location = var.region

terraform/service_cleanup.tf

@@ -85,6 +85,12 @@ resource "google_secret_manager_secret_iam_member" "cleanup-cache-hmac-key" {
   member    = "serviceAccount:${google_service_account.cleanup.email}"
 }
 
+resource "google_secret_manager_secret_iam_member" "cleanup-ratelimit-hmac-key" {
+  secret_id = google_secret_manager_secret.ratelimit-hmac-key.id
+  role      = "roles/secretmanager.secretAccessor"
+  member    = "serviceAccount:${google_service_account.cleanup.email}"
+}
+
 resource "google_cloud_run_service" "cleanup" {
   name     = "cleanup"
   location = var.region

terraform/service_e2e_runner.tf

@@ -85,6 +85,12 @@ resource "google_secret_manager_secret_iam_member" "e2e-runner-cache-hmac-key" {
   member    = "serviceAccount:${google_service_account.e2e-runner.email}"
 }
 
+resource "google_secret_manager_secret_iam_member" "e2e-runner-ratelimit-hmac-key" {
+  secret_id = google_secret_manager_secret.ratelimit-hmac-key.id
+  role      = "roles/secretmanager.secretAccessor"
+  member    = "serviceAccount:${google_service_account.e2e-runner.email}"
+}
+
 resource "google_cloud_run_service" "e2e-runner" {
   name     = "e2e-runner"
   location = var.region

terraform/service_server.tf

@@ -122,6 +122,12 @@ resource "google_secret_manager_secret_iam_member" "server-cache-hmac-key" {
   member    = "serviceAccount:${google_service_account.server.email}"
 }
 
+resource "google_secret_manager_secret_iam_member" "server-ratelimit-hmac-key" {
+  secret_id = google_secret_manager_secret.ratelimit-hmac-key.id
+  role      = "roles/secretmanager.secretAccessor"
+  member    = "serviceAccount:${google_service_account.server.email}"
+}
+
 resource "google_cloud_run_service" "server" {
   name     = "server"
   location = var.region

terraform/services.tf

@@ -60,6 +60,7 @@ locals {
   }
 
   rate_limit_config = {
+    RATE_LIMIT_HMAC_KEY   = "secret://${google_secret_manager_secret_version.ratelimit-hmac-key.id}"
     RATE_LIMIT_TYPE       = "REDIS"
     RATE_LIMIT_TOKENS     = "60"
     RATE_LIMIT_INTERVAL   = "1m"