Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security #5

Open
wants to merge 3 commits into
base: db2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ require (

require (
github.com/KyleBanks/depth v1.2.1 // indirect
github.com/aymerick/douceur v0.2.0 // indirect
github.com/cespare/xxhash/v2 v2.2.0 // indirect
github.com/cockroachdb/apd v1.1.0 // indirect
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
Expand All @@ -24,6 +25,7 @@ require (
github.com/go-openapi/spec v0.21.0 // indirect
github.com/go-openapi/swag v0.23.0 // indirect
github.com/gofrs/uuid v3.2.0+incompatible // indirect
github.com/gorilla/css v1.0.1 // indirect
github.com/jackc/chunkreader/v2 v2.0.1 // indirect
github.com/jackc/fake v0.0.0-20150926172116-812a484cc733 // indirect
github.com/jackc/pgio v1.0.0 // indirect
Expand All @@ -34,6 +36,7 @@ require (
github.com/josharian/intern v1.0.0 // indirect
github.com/mailru/easyjson v0.7.7 // indirect
github.com/mfridman/interpolate v0.0.2 // indirect
github.com/microcosm-cc/bluemonday v1.0.27 // indirect
github.com/pashagolub/pgxmock/v4 v4.3.0 // indirect
github.com/sethvargo/go-retry v0.3.0 // indirect
github.com/swaggo/files v1.0.1 // indirect
Expand Down
6 changes: 6 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,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/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3dyBCFEj5IhUbnKptjxatkF07cF2ak3yi77so=
github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw=
github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk=
github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4=
github.com/bsm/ginkgo/v2 v2.12.0 h1:Ny8MWAHyOepLGlLKYmXG4IEkioBysk6GpaRTLC8zwWs=
github.com/bsm/ginkgo/v2 v2.12.0/go.mod h1:SwYbGRRDovPVboqFv0tPTcG1sN61LM1Z4ARdbAV9g4c=
github.com/bsm/gomega v1.27.10 h1:yeMWxP2pV2fG3FgAODIY8EiRE3dy0aeFYt4l7wh6yKA=
Expand Down Expand Up @@ -33,6 +35,8 @@ github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc=
github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs=
github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/gorilla/css v1.0.1 h1:ntNaBIghp6JmvWnxbZKANoLyuXTPZ4cAMlo6RyhlbO8=
github.com/gorilla/css v1.0.1/go.mod h1:BvnYkspnSzMmwRK+b8/xgNPLiIuNZr6vbZBTPQ2A3b0=
github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY=
github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ=
github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k=
Expand Down Expand Up @@ -75,6 +79,8 @@ github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWE
github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
github.com/mfridman/interpolate v0.0.2 h1:pnuTK7MQIxxFz1Gr+rjSIx9u7qVjf5VOoM/u6BbAxPY=
github.com/mfridman/interpolate v0.0.2/go.mod h1:p+7uk6oE07mpE/Ik1b8EckO0O4ZXiGAfshKBWLUM9Xg=
github.com/microcosm-cc/bluemonday v1.0.27 h1:MpEUotklkwCSLeH+Qdx1VJgNqLlpY2KXwXFM08ygZfk=
github.com/microcosm-cc/bluemonday v1.0.27/go.mod h1:jFi9vgW+H7c3V0lb6nR74Ib/DIB5OBs92Dimizgw2cA=
github.com/ncruces/go-strftime v0.1.9 h1:bY0MQC28UADQmHmaF5dgpLmImcShSi2kHU9XLdhx/f4=
github.com/ncruces/go-strftime v0.1.9/go.mod h1:Fwc5htZGVVkseilnfgOVb9mKy6w1naJmn9CehxcKcls=
github.com/pashagolub/pgxmock/v4 v4.3.0 h1:DqT7fk0OCK6H0GvqtcMsLpv8cIwWqdxWgfZNLeHCb/s=
Expand Down
2 changes: 1 addition & 1 deletion internal/db/migrations/00001_create_table_users.sql
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ CREATE TABLE "USER" (
id SERIAL PRIMARY KEY,
username TEXT UNIQUE NOT NULL,
email TEXT UNIQUE NOT NULL,
password_hash TEXT NOT NULL,
password_hash BYTEA NOT NULL,
URL_to_avatar TEXT,
created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
modified_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP
Expand Down
7 changes: 7 additions & 0 deletions internal/http/auth/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ func (h *AuthHandler) Register(w http.ResponseWriter, r *http.Request) {
return
}

err = utils.SanitizeStruct(&req)
if err != nil {
h.logger.Error(r.Context(), "sanitize error", err)
utils.WriteResponse(w, http.StatusInternalServerError, httpErrors.ErrInternal)
return
}

_, err = govalidator.ValidateStruct(&req)
if err != nil {
h.logger.Error(r.Context(), "register", err)
Expand Down
7 changes: 7 additions & 0 deletions internal/http/auth/update_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ func (h *AuthHandler) UpdateUser(w http.ResponseWriter, r *http.Request) {
return
}

err = utils.SanitizeStruct(&req)
if err != nil {
h.logger.Error(r.Context(), "sanitize error", err)
utils.WriteResponse(w, http.StatusInternalServerError, httpErrors.ErrInternal)
return
}

_, err = govalidator.ValidateStruct(&req)
Comment on lines +49 to 56
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Если у вас всегда идет сначала санитайзинг, потом валидация, мб вынести в вспомогательную функцию? А то хэндлеры разрастаются

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

у ValidateStruct своя логика возврата ошибки и записи в ответ именно того поля, которое не прошло валидацию, если городить проверки откуда какую ошибку отправлять не очень читаемо выходит.

if err != nil {
utils.ProcessValidationErrors(w, err)
Expand Down
7 changes: 7 additions & 0 deletions internal/http/events/add_event.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ func (h EventHandler) AddEvent(w http.ResponseWriter, r *http.Request) {
return
}

err = utils.SanitizeStruct(&req)
if err != nil {
h.logger.Error(r.Context(), "sanitize error", err)
utils.WriteResponse(w, http.StatusInternalServerError, httpErrors.ErrInternal)
return
}

reqErr = checkNewEventRequest(req)
if reqErr != nil {
utils.WriteResponse(w, http.StatusBadRequest, reqErr)
Expand Down
7 changes: 7 additions & 0 deletions internal/http/events/update_event.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ func (h EventHandler) UpdateEvent(w http.ResponseWriter, r *http.Request) {
return
}

err = utils.SanitizeStruct(&req)
if err != nil {
h.logger.Error(r.Context(), "sanitize error", err)
utils.WriteResponse(w, http.StatusInternalServerError, httpErrors.ErrInternal)
return
}

reqErr = checkNewEventRequest(req)
if reqErr != nil {
utils.WriteResponse(w, http.StatusBadRequest, reqErr)
Expand Down
23 changes: 23 additions & 0 deletions internal/http/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/microcosm-cc/bluemonday"
"mime/multipart"
"net/http"
"path/filepath"
"reflect"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -175,3 +177,24 @@ func HandleImageUpload(r *http.Request) (models.MediaFile, error) {
File: file,
}, nil
}

// SanitizeStruct чистит всех строковых полей структуры от XSS
func SanitizeStruct(input interface{}) error {
v := reflect.ValueOf(input)
if v.Kind() == reflect.Ptr {
v = v.Elem()
}
if v.Kind() != reflect.Struct {
return fmt.Errorf("input is not a struct")
}

p := bluemonday.UGCPolicy()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В доке bluemonday говорится, что политику надо создавать 1 раз, а не на каждый санитайзинг

Кстати, по идее можно было обойтись без санитайзинга на стороне бэка, у вас handlebars должен на фронте санитайзить. Но, конечно, лишним не будет

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

поправил

for i := 0; i < v.NumField(); i++ {
field := v.Field(i)
if field.Kind() == reflect.String && field.CanSet() {
original := field.String()
field.SetString(p.Sanitize(original))
}
}
return nil
}
4 changes: 4 additions & 0 deletions internal/models/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ var (
Field: "username",
Message: "user already exists",
}
ErrInvalidPassword = &AuthError{
Field: "password",
Message: "wrong password or login",
}
)

func (e AuthError) Error() string {
Expand Down
9 changes: 8 additions & 1 deletion internal/repository/postgres/users/add_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package userRepository

import (
"context"
"crypto/rand"
"fmt"

"kudago/internal/models"
Expand All @@ -13,11 +14,17 @@ const addUserQuery = `
RETURNING id, created_at`

func (d *UserDB) AddUser(ctx context.Context, user models.User) (models.User, error) {
salt := make([]byte, 8)
if _, err := rand.Read(salt); err != nil {
return models.User{}, fmt.Errorf("failed to generate salt: %w", err)
}
passwordHash := hashPass(salt, user.Password)

var userInfo UserInfo
err := d.pool.QueryRow(ctx, addUserQuery,
user.Username,
user.Email,
user.Password,
passwordHash,
user.ImageURL,
).Scan(
&userInfo.ID,
Expand Down
18 changes: 18 additions & 0 deletions internal/repository/postgres/users/argon2_hash_pass.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package userRepository

import (
"bytes"
"golang.org/x/crypto/argon2"
)

func hashPass(salt []byte, plainPassword string) []byte {
hashedPass := argon2.IDKey([]byte(plainPassword), []byte(salt), 1, 64*1024, 4, 32)
return append(salt, hashedPass...)
}

func checkPass(passHash []byte, plainPassword string) bool {
salt := make([]byte, 8)
copy(salt, passHash[:8])
userPassHash := hashPass(salt, plainPassword)
return bytes.Equal(userPassHash, passHash)
}
13 changes: 10 additions & 3 deletions internal/repository/postgres/users/check_credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,32 @@ import (
)

const checkCredentialsQuery = `
SELECT id, username, email, created_at, url_to_avatar
SELECT id, username, email, created_at, url_to_avatar, password_hash
FROM "USER"
WHERE username = $1 AND password_hash = $2`
WHERE username = $1`

func (d UserDB) CheckCredentials(ctx context.Context, username, password string) (models.User, error) {
var userInfo UserInfo
err := d.pool.QueryRow(ctx, checkCredentialsQuery, username, password).Scan(
var storedPassHash []byte

err := d.pool.QueryRow(ctx, checkCredentialsQuery, username).Scan(
&userInfo.ID,
&userInfo.Username,
&userInfo.Email,
&userInfo.CreatedAt,
&userInfo.ImageURL,
&storedPassHash,
)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
return models.User{}, fmt.Errorf("%s: %w", models.LevelDB, models.ErrUserNotFound)
}
return models.User{}, fmt.Errorf("%s: %w", models.LevelDB, err)
}
if !checkPass(storedPassHash, password) {
return models.User{}, fmt.Errorf("%s: %w", models.LevelDB, models.ErrInvalidPassword)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это должно происходить на уровне бизнес-логики

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

поправил


user := toDomainUser(userInfo)
return user, nil
}