Skip to content

Commit

Permalink
Refact bouncer auth (#2456)
Browse files Browse the repository at this point in the history 
Co-authored-by: blotus <sebastien@crowdsec.net>
  • Loading branch information
@mmetc @blotus
mmetc and blotus authored Dec 4, 2023
1 parent a5ab73d commit 23968e4
Show file tree
Hide file tree
Showing 7 changed files with 112 additions and 145 deletions.
4 changes: 1 addition & 3 deletions cmd/crowdsec-cli/machines.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ import (
"github.com/crowdsecurity/crowdsec/cmd/crowdsec-cli/require"
)

var (
passwordLength = 64
)
const passwordLength = 64

func generatePassword(length int) string {
upper := "ABCDEFGHIJKLMNOPQRSTUVWXY"
Expand Down
12 changes: 6 additions & 6 deletions pkg/apiserver/apiserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,13 @@ func NewServer(config *csconfig.LocalApiServerCfg) (*APIServer, error) {
var flushScheduler *gocron.Scheduler
dbClient, err := database.NewClient(config.DbConfig)
if err != nil {
return &APIServer{}, fmt.Errorf("unable to init database client: %w", err)
return nil, fmt.Errorf("unable to init database client: %w", err)
}

if config.DbConfig.Flush != nil {
flushScheduler, err = dbClient.StartFlushScheduler(config.DbConfig.Flush)
if err != nil {
return &APIServer{}, err
return nil, err
}
}

Expand All @@ -129,7 +129,7 @@ func NewServer(config *csconfig.LocalApiServerCfg) (*APIServer, error) {

if config.TrustedProxies != nil && config.UseForwardedForHeaders {
if err := router.SetTrustedProxies(*config.TrustedProxies); err != nil {
return &APIServer{}, fmt.Errorf("while setting trusted_proxies: %w", err)
return nil, fmt.Errorf("while setting trusted_proxies: %w", err)
}
router.ForwardedByClientIP = true
} else {
Expand Down Expand Up @@ -215,7 +215,7 @@ func NewServer(config *csconfig.LocalApiServerCfg) (*APIServer, error) {
log.Printf("Loading CAPI manager")
apiClient, err = NewAPIC(config.OnlineClient, dbClient, config.ConsoleConfig, config.CapiWhitelists)
if err != nil {
return &APIServer{}, err
return nil, err
}
log.Infof("CAPI manager configured successfully")
isMachineEnrolled = isEnrolled(apiClient.apiClient)
Expand All @@ -225,7 +225,7 @@ func NewServer(config *csconfig.LocalApiServerCfg) (*APIServer, error) {
log.Infof("Machine is enrolled in the console, Loading PAPI Client")
papiClient, err = NewPAPI(apiClient, dbClient, config.ConsoleConfig, *config.PapiLogLevel)
if err != nil {
return &APIServer{}, err
return nil, err
}
controller.DecisionDeleteChan = papiClient.Channels.DeleteDecisionChannel
} else {
Expand All @@ -241,7 +241,7 @@ func NewServer(config *csconfig.LocalApiServerCfg) (*APIServer, error) {
if trustedIPs, err := config.GetTrustedIPs(); err == nil {
controller.TrustedIPs = trustedIPs
} else {
return &APIServer{}, err
return nil, err
}

return &APIServer{
Expand Down
9 changes: 5 additions & 4 deletions pkg/apiserver/apiserver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"testing"
"time"

"github.com/crowdsecurity/go-cs-lib/cstest"
"github.com/crowdsecurity/go-cs-lib/version"

middlewares "github.com/crowdsecurity/crowdsec/pkg/apiserver/middlewares/v1"
Expand Down Expand Up @@ -295,8 +296,8 @@ func TestWithWrongDBConfig(t *testing.T) {
config.API.Server.DbConfig.Type = "test"
apiServer, err := NewServer(config.API.Server)

assert.Equal(t, apiServer, &APIServer{})
assert.Equal(t, "unable to init database client: unknown database type 'test'", err.Error())
cstest.RequireErrorContains(t, err, "unable to init database client: unknown database type 'test'")
assert.Nil(t, apiServer)
}

func TestWithWrongFlushConfig(t *testing.T) {
Expand All @@ -305,8 +306,8 @@ func TestWithWrongFlushConfig(t *testing.T) {
config.API.Server.DbConfig.Flush.MaxItems = &maxItems
apiServer, err := NewServer(config.API.Server)

assert.Equal(t, apiServer, &APIServer{})
assert.Equal(t, "max_items can't be zero or negative number", err.Error())
cstest.RequireErrorContains(t, err, "max_items can't be zero or negative number")
assert.Nil(t, apiServer)
}

func TestUnknownPath(t *testing.T) {
Expand Down
213 changes: 90 additions & 123 deletions pkg/apiserver/middlewares/v1/api_key.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,132 +55,110 @@ func HashSHA512(str string) string {
return hashStr
}

func (a *APIKey) authTLS(c *gin.Context, logger *log.Entry) *ent.Bouncer {
if a.TlsAuth == nil {
logger.Error("TLS Auth is not configured but client presented a certificate")
return nil
}

validCert, extractedCN, err := a.TlsAuth.ValidateCert(c)
if !validCert {
logger.Errorf("invalid client certificate: %s", err)
return nil
}
if err != nil {
logger.Error(err)
return nil
}

logger = logger.WithFields(log.Fields{
"cn": extractedCN,
})

bouncerName := fmt.Sprintf("%s@%s", extractedCN, c.ClientIP())
bouncer, err := a.DbClient.SelectBouncerByName(bouncerName)

//This is likely not the proper way, but isNotFound does not seem to work
if err != nil && strings.Contains(err.Error(), "bouncer not found") {
//Because we have a valid cert, automatically create the bouncer in the database if it does not exist
//Set a random API key, but it will never be used
apiKey, err := GenerateAPIKey(dummyAPIKeySize)
if err != nil {
logger.Errorf("error generating mock api key: %s", err)
return nil
}
logger.Infof("Creating bouncer %s", bouncerName)
bouncer, err = a.DbClient.CreateBouncer(bouncerName, c.ClientIP(), HashSHA512(apiKey), types.TlsAuthType)
if err != nil {
logger.Errorf("while creating bouncer db entry: %s", err)
return nil
}
} else if err != nil {
//error while selecting bouncer
logger.Errorf("while selecting bouncers: %s", err)
return nil
} else if bouncer.AuthType != types.TlsAuthType {
//bouncer was found in DB
logger.Errorf("bouncer isn't allowed to auth by TLS")
return nil
}
return bouncer
}

func (a *APIKey) authPlain(c *gin.Context, logger *log.Entry) *ent.Bouncer {
val, ok := c.Request.Header[APIKeyHeader]
if !ok {
logger.Errorf("API key not found")
return nil
}
hashStr := HashSHA512(val[0])

bouncer, err := a.DbClient.SelectBouncer(hashStr)
if err != nil {
logger.Errorf("while fetching bouncer info: %s", err)
return nil
}

if bouncer.AuthType != types.ApiKeyAuthType {
logger.Errorf("bouncer %s attempted to login using an API key but it is configured to auth with %s", bouncer.Name, bouncer.AuthType)
return nil
}

return bouncer
}

func (a *APIKey) MiddlewareFunc() gin.HandlerFunc {
return func(c *gin.Context) {
var bouncer *ent.Bouncer
var err error

logger := log.WithFields(log.Fields{
"ip": c.ClientIP(),
})

if c.Request.TLS != nil && len(c.Request.TLS.PeerCertificates) > 0 {
if a.TlsAuth == nil {
log.WithField("ip", c.ClientIP()).Error("TLS Auth is not configured but client presented a certificate")
c.JSON(http.StatusForbidden, gin.H{"message": "access forbidden"})
c.Abort()
return
}
validCert, extractedCN, err := a.TlsAuth.ValidateCert(c)
if !validCert {
log.WithField("ip", c.ClientIP()).Errorf("invalid client certificate: %s", err)
c.JSON(http.StatusForbidden, gin.H{"message": "access forbidden"})
c.Abort()
return
}
if err != nil {
log.WithField("ip", c.ClientIP()).Error(err)
c.JSON(http.StatusForbidden, gin.H{"message": "access forbidden"})
c.Abort()
return
}
bouncerName := fmt.Sprintf("%s@%s", extractedCN, c.ClientIP())
bouncer, err = a.DbClient.SelectBouncerByName(bouncerName)
//This is likely not the proper way, but isNotFound does not seem to work
if err != nil && strings.Contains(err.Error(), "bouncer not found") {
//Because we have a valid cert, automatically create the bouncer in the database if it does not exist
//Set a random API key, but it will never be used
apiKey, err := GenerateAPIKey(dummyAPIKeySize)
if err != nil {
log.WithFields(log.Fields{
"ip": c.ClientIP(),
"cn": extractedCN,
}).Errorf("error generating mock api key: %s", err)
c.JSON(http.StatusForbidden, gin.H{"message": "access forbidden"})
c.Abort()
return
}
log.WithFields(log.Fields{
"ip": c.ClientIP(),
"cn": extractedCN,
}).Infof("Creating bouncer %s", bouncerName)
bouncer, err = a.DbClient.CreateBouncer(bouncerName, c.ClientIP(), HashSHA512(apiKey), types.TlsAuthType)
if err != nil {
log.WithFields(log.Fields{
"ip": c.ClientIP(),
"cn": extractedCN,
}).Errorf("creating bouncer db entry : %s", err)
c.JSON(http.StatusForbidden, gin.H{"message": "access forbidden"})
c.Abort()
return
}
} else if err != nil {
//error while selecting bouncer
log.WithFields(log.Fields{
"ip": c.ClientIP(),
"cn": extractedCN,
}).Errorf("while selecting bouncers: %s", err)
c.JSON(http.StatusForbidden, gin.H{"message": "access forbidden"})
c.Abort()
return
} else if bouncer.AuthType != types.TlsAuthType {
//bouncer was found in DB
log.WithFields(log.Fields{
"ip": c.ClientIP(),
"cn": extractedCN,
}).Errorf("bouncer isn't allowed to auth by TLS")
c.JSON(http.StatusForbidden, gin.H{"message": "access forbidden"})
c.Abort()
return
}
bouncer = a.authTLS(c, logger)
} else {
//API Key Authentication
val, ok := c.Request.Header[APIKeyHeader]
if !ok {
log.WithFields(log.Fields{
"ip": c.ClientIP(),
}).Errorf("API key not found")
c.JSON(http.StatusForbidden, gin.H{"message": "access forbidden"})
c.Abort()
return
}
hashStr := HashSHA512(val[0])
bouncer, err = a.DbClient.SelectBouncer(hashStr)
if err != nil {
log.WithFields(log.Fields{
"ip": c.ClientIP(),
}).Errorf("while fetching bouncer info: %s", err)
c.JSON(http.StatusForbidden, gin.H{"message": "access forbidden"})
c.Abort()
return
}
if bouncer.AuthType != types.ApiKeyAuthType {
log.WithFields(log.Fields{
"ip": c.ClientIP(),
}).Errorf("bouncer %s attempted to login using an API key but it is configured to auth with %s", bouncer.Name, bouncer.AuthType)
c.JSON(http.StatusForbidden, gin.H{"message": "access forbidden"})
c.Abort()
return
}
bouncer = a.authPlain(c, logger)
}

if bouncer == nil {
log.WithFields(log.Fields{
"ip": c.ClientIP(),
}).Errorf("bouncer not found")
c.JSON(http.StatusForbidden, gin.H{"message": "access forbidden"})
c.Abort()
return
}

//maybe we want to store the whole bouncer object in the context instead, this would avoid another db query
//in StreamDecision
logger = logger.WithFields(log.Fields{
"name": bouncer.Name,
})

// maybe we want to store the whole bouncer object in the context instead, this would avoid another db query
// in StreamDecision
c.Set("BOUNCER_NAME", bouncer.Name)
c.Set("BOUNCER_HASHED_KEY", bouncer.APIKey)

if bouncer.IPAddress == "" {
err = a.DbClient.UpdateBouncerIP(c.ClientIP(), bouncer.ID)
if err != nil {
log.WithFields(log.Fields{
"ip": c.ClientIP(),
"name": bouncer.Name,
}).Errorf("Failed to update ip address for '%s': %s\n", bouncer.Name, err)
if err := a.DbClient.UpdateBouncerIP(c.ClientIP(), bouncer.ID); err != nil {
logger.Errorf("Failed to update ip address for '%s': %s\n", bouncer.Name, err)
c.JSON(http.StatusForbidden, gin.H{"message": "access forbidden"})
c.Abort()
return
Expand All @@ -189,34 +167,23 @@ func (a *APIKey) MiddlewareFunc() gin.HandlerFunc {

if bouncer.IPAddress != c.ClientIP() && bouncer.IPAddress != "" {
log.Warningf("new IP address detected for bouncer '%s': %s (old: %s)", bouncer.Name, c.ClientIP(), bouncer.IPAddress)
err = a.DbClient.UpdateBouncerIP(c.ClientIP(), bouncer.ID)
if err != nil {
log.WithFields(log.Fields{
"ip": c.ClientIP(),
"name": bouncer.Name,
}).Errorf("Failed to update ip address for '%s': %s\n", bouncer.Name, err)
if err := a.DbClient.UpdateBouncerIP(c.ClientIP(), bouncer.ID); err != nil {
logger.Errorf("Failed to update ip address for '%s': %s\n", bouncer.Name, err)
c.JSON(http.StatusForbidden, gin.H{"message": "access forbidden"})
c.Abort()
return
}
}

useragent := strings.Split(c.Request.UserAgent(), "/")

if len(useragent) != 2 {
log.WithFields(log.Fields{
"ip": c.ClientIP(),
"name": bouncer.Name,
}).Warningf("bad user agent '%s'", c.Request.UserAgent())
logger.Warningf("bad user agent '%s'", c.Request.UserAgent())
useragent = []string{c.Request.UserAgent(), "N/A"}
}

if bouncer.Version != useragent[1] || bouncer.Type != useragent[0] {
if err := a.DbClient.UpdateBouncerTypeAndVersion(useragent[0], useragent[1], bouncer.ID); err != nil {
log.WithFields(log.Fields{
"ip": c.ClientIP(),
"name": bouncer.Name,
}).Errorf("failed to update bouncer version and type: %s", err)
logger.Errorf("failed to update bouncer version and type: %s", err)
c.JSON(http.StatusForbidden, gin.H{"message": "bad user agent"})
c.Abort()
return
Expand Down
2 changes: 1 addition & 1 deletion pkg/apiserver/middlewares/v1/middlewares.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ func NewMiddlewares(dbClient *database.Client) (*Middlewares, error) {

ret.JWT, err = NewJWT(dbClient)
if err != nil {
return &Middlewares{}, err
return nil, err
}

ret.APIKey = NewAPIKey(dbClient)
Expand Down
6 changes: 3 additions & 3 deletions pkg/database/bouncers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
func (c *Client) SelectBouncer(apiKeyHash string) (*ent.Bouncer, error) {
result, err := c.Ent.Bouncer.Query().Where(bouncer.APIKeyEQ(apiKeyHash)).First(c.CTX)
if err != nil {
return &ent.Bouncer{}, errors.Wrapf(QueryFail, "select bouncer: %s", err)
return nil, err
}

return result, nil
Expand All @@ -22,7 +22,7 @@ func (c *Client) SelectBouncer(apiKeyHash string) (*ent.Bouncer, error) {
func (c *Client) SelectBouncerByName(bouncerName string) (*ent.Bouncer, error) {
result, err := c.Ent.Bouncer.Query().Where(bouncer.NameEQ(bouncerName)).First(c.CTX)
if err != nil {
return &ent.Bouncer{}, errors.Wrapf(QueryFail, "select bouncer: %s", err)
return nil, err
}

return result, nil
Expand All @@ -31,7 +31,7 @@ func (c *Client) SelectBouncerByName(bouncerName string) (*ent.Bouncer, error) {
func (c *Client) ListBouncers() ([]*ent.Bouncer, error) {
result, err := c.Ent.Bouncer.Query().All(c.CTX)
if err != nil {
return []*ent.Bouncer{}, errors.Wrapf(QueryFail, "listing bouncer: %s", err)
return nil, errors.Wrapf(QueryFail, "listing bouncers: %s", err)
}
return result, nil
}
Expand Down
Loading

0 comments on commit 23968e4

Please sign in to comment.