EventManager: always close the connection filesystem

closing the user filesystem is not enough here

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>

internal/common/eventmanager.go

@@ -1370,6 +1370,9 @@ func getHTTPRuleActionBody(c *dataprovider.EventActionHTTPConfig, replacer *stri
 		go func() {
 			defer w.Close()
 			defer user.CloseFs() //nolint:errcheck
+			if conn != nil {
+				defer conn.CloseFS() //nolint:errcheck
+			}
 
 			for _, part := range c.Parts {
 				h := make(textproto.MIMEHeader)
@@ -1587,6 +1590,8 @@ func executeEmailRuleAction(c dataprovider.EventActionEmailConfig, params *Event
 			return fmt.Errorf("error getting email attachments, unable to check root fs for user %q: %w", user.Username, err)
 		}
 		conn := NewBaseConnection(connectionID, protocolEventAction, "", "", user)
+		defer conn.CloseFS() //nolint:errcheck
+
 		res, err := getMailAttachments(conn, fileAttachments, replacer)
 		if err != nil {
 			return err
@@ -1648,6 +1653,8 @@ func executeDeleteFsActionForUser(deletes []string, replacer *strings.Replacer,
 		return fmt.Errorf("delete error, unable to check root fs for user %q: %w", user.Username, err)
 	}
 	conn := NewBaseConnection(connectionID, protocolEventAction, "", "", user)
+	defer conn.CloseFS() //nolint:errcheck
+
 	for _, item := range replacePathsPlaceholders(deletes, replacer) {
 		info, err := conn.DoStat(item, 0, false)
 		if err != nil {
@@ -1716,6 +1723,8 @@ func executeMkDirsFsActionForUser(dirs []string, replacer *strings.Replacer, use
 		return fmt.Errorf("mkdir error, unable to check root fs for user %q: %w", user.Username, err)
 	}
 	conn := NewBaseConnection(connectionID, protocolEventAction, "", "", user)
+	defer conn.CloseFS() //nolint:errcheck
+
 	for _, item := range replacePathsPlaceholders(dirs, replacer) {
 		if err = conn.CheckParentDirs(path.Dir(item)); err != nil {
 			return fmt.Errorf("unable to check parent dirs for %q, user %q: %w", item, user.Username, err)
@@ -1775,6 +1784,8 @@ func executeRenameFsActionForUser(renames []dataprovider.KeyValue, replacer *str
 		return fmt.Errorf("rename error, unable to check root fs for user %q: %w", user.Username, err)
 	}
 	conn := NewBaseConnection(connectionID, protocolEventAction, "", "", user)
+	defer conn.CloseFS() //nolint:errcheck
+
 	for _, item := range renames {
 		source := util.CleanPath(replaceWithReplacer(item.Key, replacer))
 		target := util.CleanPath(replaceWithReplacer(item.Value, replacer))
@@ -1800,6 +1811,8 @@ func executeCopyFsActionForUser(keyVals []dataprovider.KeyValue, replacer *strin
 		return fmt.Errorf("copy error, unable to check root fs for user %q: %w", user.Username, err)
 	}
 	conn := NewBaseConnection(connectionID, protocolEventAction, "", "", user)
+	defer conn.CloseFS() //nolint:errcheck
+
 	for _, item := range keyVals {
 		source := util.CleanPath(replaceWithReplacer(item.Key, replacer))
 		target := util.CleanPath(replaceWithReplacer(item.Value, replacer))
@@ -1831,6 +1844,8 @@ func executeExistFsActionForUser(exist []string, replacer *strings.Replacer,
 		return fmt.Errorf("existence check error, unable to check root fs for user %q: %w", user.Username, err)
 	}
 	conn := NewBaseConnection(connectionID, protocolEventAction, "", "", user)
+	defer conn.CloseFS() //nolint:errcheck
+
 	for _, item := range replacePathsPlaceholders(exist, replacer) {
 		if _, err = conn.DoStat(item, 0, false); err != nil {
 			return fmt.Errorf("error checking existence for path %q, user %q: %w", item, user.Username, err)
@@ -1989,6 +2004,8 @@ func executeCompressFsActionForUser(c dataprovider.EventActionFsCompress, replac
 		return fmt.Errorf("compress error, unable to check root fs for user %q: %w", user.Username, err)
 	}
 	conn := NewBaseConnection(connectionID, protocolEventAction, "", "", user)
+	defer conn.CloseFS() //nolint:errcheck
+
 	name := util.CleanPath(replaceWithReplacer(c.Name, replacer))
 	conn.CheckParentDirs(path.Dir(name)) //nolint:errcheck
 	paths := make([]string, 0, len(c.Paths))

internal/httpd/api_shares.go

@@ -562,6 +562,7 @@ func validateBrowsableShare(share dataprovider.Share, connection *Connection) er
 	basePath := share.Paths[0]
 	info, err := connection.Stat(basePath, 0)
 	if err != nil {
+		connection.CloseFS() //nolint:errcheck
 		return util.NewI18nError(
 			fmt.Errorf("unable to check the share directory: %w", err),
 			util.I18nErrorShareInvalidPath,

internal/httpd/oauth2.go

@@ -15,8 +15,6 @@
 package httpd
 
 import (
-	"crypto/sha256"
-	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"sync"
@@ -53,10 +51,8 @@ type oauth2PendingAuth struct {
 }
 
 func newOAuth2PendingAuth(provider int, redirectURL, clientID string, clientSecret *kms.Secret) oauth2PendingAuth {
-	state := sha256.Sum256(util.GenerateRandomBytes(32))
-
 	return oauth2PendingAuth{
-		State:        hex.EncodeToString(state[:]),
+		State:        util.GenerateOpaqueString(),
 		Provider:     provider,
 		ClientID:     clientID,
 		ClientSecret: clientSecret,

internal/httpd/oidc.go

@@ -16,8 +16,6 @@ package httpd
 
 import (
 	"context"
-	"crypto/sha256"
-	"encoding/hex"
 	"errors"
 	"fmt"
 	"net/http"
@@ -203,12 +201,9 @@ type oidcPendingAuth struct {
 }
 
 func newOIDCPendingAuth(audience tokenAudience) oidcPendingAuth {
-	state := sha256.Sum256(util.GenerateRandomBytes(32))
-	nonce := util.GenerateUniqueID()
-
 	return oidcPendingAuth{
-		State:    hex.EncodeToString(state[:]),
-		Nonce:    nonce,
+		State:    util.GenerateOpaqueString(),
+		Nonce:    util.GenerateOpaqueString(),
 		Audience: audience,
 		IssuedAt: util.GetTimeAsMsSinceEpoch(time.Now()),
 	}
@@ -668,7 +663,7 @@ func (s *httpdServer) handleOIDCRedirect(w http.ResponseWriter, r *http.Request)
 		RefreshToken: oauth2Token.RefreshToken,
 		IDToken:      rawIDToken,
 		Nonce:        idToken.Nonce,
-		Cookie:       xid.New().String(),
+		Cookie:       util.GenerateOpaqueString(),
 	}
 	if !oauth2Token.Expiry.IsZero() {
 		token.ExpiresAt = util.GetTimeAsMsSinceEpoch(oauth2Token.Expiry)

internal/httpd/oidc_test.go

@@ -151,8 +151,8 @@ func TestOIDCLoginLogout(t *testing.T) {
 	assert.Contains(t, rr.Body.String(), util.I18nInvalidAuth)
 
 	expiredAuthReq := oidcPendingAuth{
-		State:    xid.New().String(),
-		Nonce:    xid.New().String(),
+		State:    util.GenerateOpaqueString(),
+		Nonce:    util.GenerateOpaqueString(),
 		Audience: tokenAudienceWebClient,
 		IssuedAt: util.GetTimeAsMsSinceEpoch(time.Now().Add(-10 * time.Minute)),
 	}
@@ -561,7 +561,7 @@ func TestOIDCRefreshToken(t *testing.T) {
 	r, err := http.NewRequest(http.MethodGet, webUsersPath, nil)
 	assert.NoError(t, err)
 	token := oidcToken{
-		Cookie:      xid.New().String(),
+		Cookie:      util.GenerateOpaqueString(),
 		AccessToken: xid.New().String(),
 		TokenType:   "Bearer",
 		ExpiresAt:   util.GetTimeAsMsSinceEpoch(time.Now().Add(-1 * time.Minute)),
@@ -665,7 +665,7 @@ func TestOIDCRefreshToken(t *testing.T) {
 
 func TestOIDCRefreshUser(t *testing.T) {
 	token := oidcToken{
-		Cookie:      xid.New().String(),
+		Cookie:      util.GenerateOpaqueString(),
 		AccessToken: xid.New().String(),
 		TokenType:   "Bearer",
 		ExpiresAt:   util.GetTimeAsMsSinceEpoch(time.Now().Add(1 * time.Minute)),
@@ -779,7 +779,7 @@ func TestValidateOIDCToken(t *testing.T) {
 		},
 	}
 	token := oidcToken{
-		Cookie:      xid.New().String(),
+		Cookie:      util.GenerateOpaqueString(),
 		AccessToken: xid.New().String(),
 		ExpiresAt:   util.GetTimeAsMsSinceEpoch(time.Now().Add(-2 * time.Minute)),
 	}
@@ -795,8 +795,8 @@ func TestValidateOIDCToken(t *testing.T) {
 
 	server.tokenAuth = jwtauth.New("PS256", util.GenerateRandomBytes(32), nil)
 	token = oidcToken{
-		Cookie:      xid.New().String(),
-		AccessToken: xid.New().String(),
+		Cookie:      util.GenerateOpaqueString(),
+		AccessToken: util.GenerateUniqueID(),
 	}
 	oidcMgr.addToken(token)
 	rr = httptest.NewRecorder()
@@ -810,7 +810,7 @@ func TestValidateOIDCToken(t *testing.T) {
 	assert.Len(t, oidcMgr.tokens, 0)
 
 	token = oidcToken{
-		Cookie:      xid.New().String(),
+		Cookie:      util.GenerateOpaqueString(),
 		AccessToken: xid.New().String(),
 		Role:        "admin",
 	}
@@ -1104,7 +1104,7 @@ func TestMemoryOIDCManager(t *testing.T) {
 		AccessToken: xid.New().String(),
 		Nonce:       xid.New().String(),
 		SessionID:   xid.New().String(),
-		Cookie:      xid.New().String(),
+		Cookie:      util.GenerateOpaqueString(),
 		Username:    xid.New().String(),
 		Role:        "admin",
 		Permissions: []string{dataprovider.PermAdminAny},
@@ -1154,7 +1154,7 @@ func TestMemoryOIDCManager(t *testing.T) {
 	token.UsedAt = usedAt
 	oidcMgr.tokens[token.Cookie] = token
 	newToken := oidcToken{
-		Cookie: xid.New().String(),
+		Cookie: util.GenerateOpaqueString(),
 	}
 	oidcMgr.addToken(newToken)
 	oidcMgr.cleanup()
@@ -1663,7 +1663,7 @@ func TestDbOIDCManager(t *testing.T) {
 	}
 
 	token := oidcToken{
-		Cookie:       xid.New().String(),
+		Cookie:       util.GenerateOpaqueString(),
 		AccessToken:  xid.New().String(),
 		TokenType:    "Bearer",
 		RefreshToken: xid.New().String(),

internal/util/util.go

@@ -22,8 +22,10 @@ import (
 	"crypto/elliptic"
 	"crypto/rand"
 	"crypto/rsa"
+	"crypto/sha256"
 	"crypto/tls"
 	"crypto/x509"
+	"encoding/hex"
 	"encoding/json"
 	"encoding/pem"
 	"errors"
@@ -560,7 +562,7 @@ func createDirPathIfMissing(file string, perm os.FileMode) error {
 	return nil
 }
 
-// GenerateRandomBytes generates the secret to use for JWT auth
+// GenerateRandomBytes generates random bytes with the specified length
 func GenerateRandomBytes(length int) []byte {
 	b := make([]byte, length)
 	_, err := io.ReadFull(rand.Reader, b)
@@ -570,6 +572,12 @@ func GenerateRandomBytes(length int) []byte {
 	return b
 }
 
+// GenerateOpaqueString generates a cryptographically secure opaque string
+func GenerateOpaqueString() string {
+	randomBytes := sha256.Sum256(GenerateRandomBytes(32))
+	return hex.EncodeToString(randomBytes[:])
+}
+
 // GenerateUniqueID retuens an unique ID
 func GenerateUniqueID() string {
 	u, err := uuid.NewRandom()

pkgs/build.sh

@@ -1,6 +1,6 @@
 #!/bin/bash
 
-NFPM_VERSION=2.41.0
+NFPM_VERSION=2.41.1
 NFPM_ARCH=${NFPM_ARCH:-amd64}
 if [ -z ${SFTPGO_VERSION} ]
 then