From 6c709b97a19ca16963161ae95fa92539a1682251 Mon Sep 17 00:00:00 2001 From: grumbulon Date: Fri, 20 Jan 2023 18:29:33 -0500 Subject: [PATCH 1/4] add rate limiting --- go.mod | 2 ++ go.sum | 4 ++++ internal/api/api.go | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/go.mod b/go.mod index 63c98c7..d9192c0 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.19 require ( github.com/glebarez/sqlite v1.6.0 github.com/go-chi/chi/v5 v5.0.8 + github.com/go-chi/httprate v0.7.1 github.com/go-chi/jwtauth/v5 v5.1.0 github.com/miekg/dns v1.1.50 github.com/swaggo/swag v1.8.9 @@ -14,6 +15,7 @@ require ( require ( github.com/KyleBanks/depth v1.2.1 // indirect + github.com/cespare/xxhash/v2 v2.2.0 // indirect github.com/go-openapi/jsonpointer v0.19.5 // indirect github.com/go-openapi/jsonreference v0.20.0 // indirect github.com/go-openapi/spec v0.20.6 // indirect diff --git a/go.sum b/go.sum index a62485a..fc28eb2 100644 --- a/go.sum +++ b/go.sum @@ -2,6 +2,8 @@ github.com/KyleBanks/depth v1.2.1 h1:5h8fQADFrWtarTdtDudMmGsC7GPbOAu6RVB3ffsVFHc github.com/KyleBanks/depth v1.2.1/go.mod h1:jzSb9d0L43HxTQfT+oSA1EEp2q+ne2uh6XgeJcm8brE= github.com/adrg/xdg v0.4.0 h1:RzRqFcjH4nE5C6oTAxhBtoE2IRyjBSa62SCbyPidvls= github.com/adrg/xdg v0.4.0/go.mod h1:N6ag73EX4wyxeaoeHctc1mas01KZgsj5tYiAIwqJE/E= +github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44= +github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/chzyer/logex v1.2.0/go.mod h1:9+9sk7u7pGNWYMkh0hdiL++6OeibzJccyQU4p4MedaY= github.com/chzyer/readline v1.5.0/go.mod h1:x22KAscuvRqlLoK9CsoYsmxoXZMMFVyOl86cAH8qUic= github.com/chzyer/test v0.0.0-20210722231415-061457976a23/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU= @@ -23,6 +25,8 @@ github.com/go-chi/chi/v5 v5.0.8 h1:lD+NLqFcAi1ovnVZpsnObHGW4xb4J8lNmoYVfECH1Y0= github.com/go-chi/chi/v5 v5.0.8/go.mod h1:DslCQbL2OYiznFReuXYUmQ2hGd1aDpCnlMNITLSKoi8= github.com/go-chi/httplog v0.2.5 h1:S02eG9NTrB/9kk3Q3RA3F6CR2b+v8WzB8IxK+zq3dBo= github.com/go-chi/httplog v0.2.5/go.mod h1:/pIXuFSrOdc5heKIJRA5Q2mW7cZCI2RySqFZNFoZjKg= +github.com/go-chi/httprate v0.7.1 h1:d5kXARdms2PREQfU4pHvq44S6hJ1hPu4OXLeBKmCKWs= +github.com/go-chi/httprate v0.7.1/go.mod h1:6GOYBSwnpra4CQfAKXu8sQZg+nZ0M1g9QnyFvxrAB8A= github.com/go-chi/jwtauth/v5 v5.1.0 h1:wJyf2YZ/ohPvNJBwPOzZaQbyzwgMZZceE1m8FOzXLeA= github.com/go-chi/jwtauth/v5 v5.1.0/go.mod h1:MA93hc1au3tAQwCKry+fI4LqJ5MIVN4XSsglOo+lSc8= github.com/go-openapi/jsonpointer v0.19.3/go.mod h1:Pl9vOtqEWErmShwVjC8pYs9cog34VGT37dQOVbmoatg= diff --git a/internal/api/api.go b/internal/api/api.go index afa57bd..d1068f6 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -2,13 +2,16 @@ package api import ( "context" + "encoding/json" "fmt" "net/http" "time" + "git.freecumextremist.com/grumbulon/pomme/internal" "git.freecumextremist.com/grumbulon/pomme/internal/db" "github.com/go-chi/chi/v5" "github.com/go-chi/httplog" + "github.com/go-chi/httprate" "github.com/go-chi/jwtauth/v5" ) @@ -56,6 +59,25 @@ func API() http.Handler { // Protected routes api.Group(func(api chi.Router) { + api.Use(httprate.Limit( + 10, // requests + 10*time.Second, // per duration + httprate.WithKeyFuncs(httprate.KeyByIP, httprate.KeyByEndpoint), + httprate.WithLimitHandler(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusTooManyRequests) + err := json.NewEncoder(w).Encode( + internal.Response{ + HTTPResponse: http.StatusTooManyRequests, + Message: "API rate limit exceded", + }) + if err != nil { + internalServerError(w, "internal server error") + + return + } + }), + )) api.Use(jwtauth.Verifier(tokenAuth)) api.Use(jwtauth.Authenticator) @@ -65,6 +87,25 @@ func API() http.Handler { // Open routes api.Group(func(api chi.Router) { + api.Use(httprate.Limit( + 5, // requests + 5*time.Second, // per duration + httprate.WithKeyFuncs(httprate.KeyByIP, httprate.KeyByEndpoint), + httprate.WithLimitHandler(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusTooManyRequests) + err := json.NewEncoder(w).Encode( + internal.Response{ + HTTPResponse: http.StatusTooManyRequests, + Message: "API rate limit exceded", + }) + if err != nil { + internalServerError(w, "internal server error") + + return + } + }), + )) api.Use(setDBMiddleware) api.With(setDBMiddleware).Post("/create", NewUser) api.With(setDBMiddleware).Post("/login", Login) From 27fd45a1f9ab0d697b29af17a75f207d1dce94ae Mon Sep 17 00:00:00 2001 From: grumbulon Date: Fri, 20 Jan 2023 20:33:56 -0500 Subject: [PATCH 2/4] documented rate limiting, added check if file already exists and error out if it does, and small swagger edits --- docs/docs.go | 8 ++++---- docs/swagger.json | 8 ++++---- docs/swagger.yaml | 18 ++++++++++++------ internal/api/auth.go | 7 +++++-- internal/api/zone.go | 14 +++++++++++--- internal/util/fs.go | 4 ++++ 6 files changed, 40 insertions(+), 19 deletions(-) diff --git a/docs/docs.go b/docs/docs.go index 2b132ad..3e42963 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -19,7 +19,7 @@ const docTemplate = `{ "paths": { "/api/login": { "post": { - "description": "login", + "description": "login to Pomme\nRate limited: 5 requests every 5 second", "consumes": [ "application/json" ], @@ -29,7 +29,7 @@ const docTemplate = `{ "tags": [ "accounts" ], - "summary": "auth a regular user", + "summary": "authenticate as a regular user", "parameters": [ { "type": "string", @@ -69,7 +69,7 @@ const docTemplate = `{ "Bearer": [] } ], - "description": "parse your zonefile -- you must specify \"Bearer\" before entering your token", + "description": "parse your zonefile\nRate limited: 10 requests every 10 second\nyou must specify \"Bearer\" before entering your token", "consumes": [ "multipart/form-data" ], @@ -119,7 +119,7 @@ const docTemplate = `{ "Bearer": [] } ], - "description": "upload a file -- you must specify \"Bearer\" before entering your token", + "description": "upload takes files from the user and stores it locally to be parsed. Uploads are associated with a specific user.\nRate limited: 10 requests every 10 second\nyou must specify \"Bearer\" before entering your token", "consumes": [ "multipart/form-data" ], diff --git a/docs/swagger.json b/docs/swagger.json index 0b8a0e3..ad189e9 100644 --- a/docs/swagger.json +++ b/docs/swagger.json @@ -10,7 +10,7 @@ "paths": { "/api/login": { "post": { - "description": "login", + "description": "login to Pomme\nRate limited: 5 requests every 5 second", "consumes": [ "application/json" ], @@ -20,7 +20,7 @@ "tags": [ "accounts" ], - "summary": "auth a regular user", + "summary": "authenticate as a regular user", "parameters": [ { "type": "string", @@ -60,7 +60,7 @@ "Bearer": [] } ], - "description": "parse your zonefile -- you must specify \"Bearer\" before entering your token", + "description": "parse your zonefile\nRate limited: 10 requests every 10 second\nyou must specify \"Bearer\" before entering your token", "consumes": [ "multipart/form-data" ], @@ -110,7 +110,7 @@ "Bearer": [] } ], - "description": "upload a file -- you must specify \"Bearer\" before entering your token", + "description": "upload takes files from the user and stores it locally to be parsed. Uploads are associated with a specific user.\nRate limited: 10 requests every 10 second\nyou must specify \"Bearer\" before entering your token", "consumes": [ "multipart/form-data" ], diff --git a/docs/swagger.yaml b/docs/swagger.yaml index f8f707e..fa70452 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -37,7 +37,9 @@ paths: post: consumes: - application/json - description: login + description: |- + login to Pomme + Rate limited: 5 requests every 5 second parameters: - description: Username in: query @@ -60,15 +62,17 @@ paths: description: Unauthorized schema: $ref: '#/definitions/api.httpError' - summary: auth a regular user + summary: authenticate as a regular user tags: - accounts /api/parse: post: consumes: - multipart/form-data - description: parse your zonefile -- you must specify "Bearer" before entering - your token + description: |- + parse your zonefile + Rate limited: 10 requests every 10 second + you must specify "Bearer" before entering your token parameters: - description: Zonefile name in: query @@ -100,8 +104,10 @@ paths: post: consumes: - multipart/form-data - description: upload a file -- you must specify "Bearer" before entering your - token + description: |- + upload takes files from the user and stores it locally to be parsed. Uploads are associated with a specific user. + Rate limited: 10 requests every 10 second + you must specify "Bearer" before entering your token parameters: - description: Zonefile to upload in: formData diff --git a/internal/api/auth.go b/internal/api/auth.go index f3e7c4e..39aa347 100644 --- a/internal/api/auth.go +++ b/internal/api/auth.go @@ -31,8 +31,11 @@ type httpInternalServerError struct { // Auth godoc // -// @Summary auth a regular user -// @Description login +// @Summary authenticate as a regular user +// @Description login to Pomme +// +// @Description Rate limited: 5 requests every 5 second +// // @Tags accounts // @Accept json // @Produce json diff --git a/internal/api/zone.go b/internal/api/zone.go index 753b3ae..259d6a6 100644 --- a/internal/api/zone.go +++ b/internal/api/zone.go @@ -34,7 +34,11 @@ type Zone struct { // Upload godoc // // @Summary upload a zonefile -// @Description upload a file -- you must specify "Bearer" before entering your token +// @Description upload takes files from the user and stores it locally to be parsed. Uploads are associated with a specific user. +// +// @Description Rate limited: 10 requests every 10 second +// @Description you must specify "Bearer" before entering your token +// // @Tags DNS // @Accept mpfd // @Produce json @@ -71,7 +75,7 @@ func ReceiveFile(w http.ResponseWriter, r *http.Request) { } if err = util.MakeLocal(name[0], claims["username"].(string), buf); err != nil { - internalServerError(w, "internal server error") + internalServerError(w, err.Error()) return } @@ -98,7 +102,11 @@ func ReceiveFile(w http.ResponseWriter, r *http.Request) { // Parse godoc // // @Summary parse your zonefile -// @Description parse your zonefile -- you must specify "Bearer" before entering your token +// @Description parse your zonefile +// +// @Description Rate limited: 10 requests every 10 second +// @Description you must specify "Bearer" before entering your token +// // @Tags DNS // @Accept mpfd // @Produce json diff --git a/internal/util/fs.go b/internal/util/fs.go index 4358573..c46b092 100644 --- a/internal/util/fs.go +++ b/internal/util/fs.go @@ -7,6 +7,10 @@ import ( ) func MakeLocal(filename, username string, buf bytes.Buffer) error { + if _, err := os.Stat(fmt.Sprintf("/tmp/tmpfile-%s-%s", filename, username)); !os.IsNotExist(err) { + return fmt.Errorf("file %s already exists: %w", filename, err) + } + defer buf.Reset() f, err := os.Create("/tmp/tmpfile-" + filename + "-" + username) //nolint: gosec From 320f757917ffb83c209aa074d35f702d4170d3dd Mon Sep 17 00:00:00 2001 From: grumbulon Date: Sat, 21 Jan 2023 11:19:29 -0500 Subject: [PATCH 3/4] add confirmation of zonefile upload and add mimetype validation to only allow users to upload text/plain --- internal/api/zone.go | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/internal/api/zone.go b/internal/api/zone.go index 259d6a6..1a152a0 100644 --- a/internal/api/zone.go +++ b/internal/api/zone.go @@ -2,6 +2,7 @@ package api import ( "bytes" + "encoding/json" "fmt" "io" "log" @@ -64,6 +65,12 @@ func ReceiveFile(w http.ResponseWriter, r *http.Request) { return } + ok := validateContentType(file) + if !ok { + http.Error(w, "file must be text/plain", http.StatusUnsupportedMediaType) + + return + } defer file.Close() //nolint: errcheck name := strings.Split(header.Filename, ".") @@ -97,6 +104,20 @@ func ReceiveFile(w http.ResponseWriter, r *http.Request) { }) buf.Reset() + + w.WriteHeader(http.StatusCreated) + w.Header().Set("Content-Type", "application/json") + err = json.NewEncoder(w).Encode( + internal.Response{ + HTTPResponse: http.StatusCreated, + Message: "Successfully uploaded zonefile", + }) + + if err != nil { + internalServerError(w, "internal server error") + + return + } } // Parse godoc @@ -197,3 +218,20 @@ func (zone *ZoneRequest) Parse() error { return nil } + +func validateContentType(file io.Reader) bool { + bytes, err := io.ReadAll(file) + if err != nil { + return false + } + + mimeType := http.DetectContentType(bytes) + mime := strings.Contains(mimeType, "text/plain") + + switch mime { + case true: + return true + default: + return false + } +} From 7e6594f3c7ba2ba19e5ca682d478b5d1f8a9072d Mon Sep 17 00:00:00 2001 From: grumbulon Date: Sat, 21 Jan 2023 11:21:41 -0500 Subject: [PATCH 4/4] move defer file.Close() to before validateContentType because that could cause problems --- internal/api/zone.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/api/zone.go b/internal/api/zone.go index 1a152a0..e1512d4 100644 --- a/internal/api/zone.go +++ b/internal/api/zone.go @@ -65,13 +65,14 @@ func ReceiveFile(w http.ResponseWriter, r *http.Request) { return } + defer file.Close() //nolint: errcheck + ok := validateContentType(file) if !ok { http.Error(w, "file must be text/plain", http.StatusUnsupportedMediaType) return } - defer file.Close() //nolint: errcheck name := strings.Split(header.Filename, ".")