Fix: Prevent ExtSvcTokens from containing nil characters (#88243)

* Fix: Prevent ExtSvcTokens from containing nil characters

* Rebase

* Add more logs

* Nit. nil -> NUL

* Nit. Part -> Parts

* Back to const

* Account for comments

Co-authored-by: Claudiu Dragalina-Paraipan <claudiu.dragalina@grafana.com>

---------

Co-authored-by: Claudiu Dragalina-Paraipan <claudiu.dragalina@grafana.com>

pkg/services/pluginsintegration/pluginconfig/envvars.go

@@ -179,7 +179,7 @@ func (p *EnvVarsProvider) pluginSettingsEnvVars(pluginID string) []string {
 
 func (p *EnvVarsProvider) envVar(key, value string) string {
 	if strings.Contains(value, "\x00") {
-		p.logger.Error("Variable with key '%s' contains a nil character", key)
+		p.logger.Error("Variable with key '%s' contains NUL", key)
 	}
 	return fmt.Sprintf("%s=%s", key, value)
 }

pkg/services/serviceaccounts/extsvcaccounts/models.go

@@ -15,16 +15,19 @@ const (
 	kvStoreType = "extsvc-token"
 	// #nosec G101 - this is not a hardcoded secret
 	tokenNamePrefix = "extsvc-token"
+
+	maxTokenGenRetries = 10
 )
 
 var (
-	ErrCannotBeDeleted     = errutil.BadRequest("extsvcaccounts.ErrCannotBeDeleted", errutil.WithPublicMessage("external service account cannot be deleted"))
-	ErrCannotBeUpdated     = errutil.BadRequest("extsvcaccounts.ErrCannotBeUpdated", errutil.WithPublicMessage("external service account cannot be updated"))
-	ErrCannotCreateToken   = errutil.BadRequest("extsvcaccounts.ErrCannotCreateToken", errutil.WithPublicMessage("cannot add external service account token"))
-	ErrCannotDeleteToken   = errutil.BadRequest("extsvcaccounts.ErrCannotDeleteToken", errutil.WithPublicMessage("cannot delete external service account token"))
-	ErrCannotListTokens    = errutil.BadRequest("extsvcaccounts.ErrCannotListTokens", errutil.WithPublicMessage("cannot list external service account tokens"))
-	ErrCredentialsNotFound = errutil.NotFound("extsvcaccounts.credentials-not-found")
-	ErrInvalidName         = errutil.BadRequest("extsvcaccounts.ErrInvalidName", errutil.WithPublicMessage("only external service account names can be prefixed with 'extsvc-'"))
+	ErrCannotBeDeleted      = errutil.BadRequest("extsvcaccounts.ErrCannotBeDeleted", errutil.WithPublicMessage("external service account cannot be deleted"))
+	ErrCannotBeUpdated      = errutil.BadRequest("extsvcaccounts.ErrCannotBeUpdated", errutil.WithPublicMessage("external service account cannot be updated"))
+	ErrCannotCreateToken    = errutil.BadRequest("extsvcaccounts.ErrCannotCreateToken", errutil.WithPublicMessage("cannot add external service account token"))
+	ErrCannotDeleteToken    = errutil.BadRequest("extsvcaccounts.ErrCannotDeleteToken", errutil.WithPublicMessage("cannot delete external service account token"))
+	ErrCannotListTokens     = errutil.BadRequest("extsvcaccounts.ErrCannotListTokens", errutil.WithPublicMessage("cannot list external service account tokens"))
+	ErrCredentialsGenFailed = errutil.Internal("extsvcaccounts.ErrCredentialsGenFailed")
+	ErrCredentialsNotFound  = errutil.NotFound("extsvcaccounts.ErrCredentialsNotFound")
+	ErrInvalidName          = errutil.BadRequest("extsvcaccounts.ErrInvalidName", errutil.WithPublicMessage("only external service account names can be prefixed with 'extsvc-'"))
 
 	extsvcuser = &user.SignedInUser{
 		OrgID: extsvcauth.TmpOrgID,

pkg/services/serviceaccounts/extsvcaccounts/service.go

@@ -355,7 +355,7 @@ func (esa *ExtSvcAccountsService) getExtSvcAccountToken(ctx context.Context, org
 
 	// Generate token
 	ctxLogger.Info("Generate new service account token", "service", extSvcSlug, "orgID", orgID)
-	newKeyInfo, err := satokengen.New(extSvcSlug)
+	newKeyInfo, err := genTokenWithRetries(ctxLogger, extSvcSlug)
 	if err != nil {
 		return "", err
 	}
@@ -380,6 +380,52 @@ func (esa *ExtSvcAccountsService) getExtSvcAccountToken(ctx context.Context, org
 	return newKeyInfo.ClientSecret, nil
 }
 
+func genTokenWithRetries(ctxLogger log.Logger, extSvcSlug string) (satokengen.KeyGenResult, error) {
+	var newKeyInfo satokengen.KeyGenResult
+	var err error
+	retry := 0
+	for retry < maxTokenGenRetries {
+		newKeyInfo, err = satokengen.New(extSvcSlug)
+		if err != nil {
+			return satokengen.KeyGenResult{}, err
+		}
+
+		if !strings.Contains(newKeyInfo.ClientSecret, "\x00") {
+			return newKeyInfo, nil
+		}
+
+		retry++
+
+		ctxLogger.Warn("Generated a token containing NUL, retrying",
+			"service", extSvcSlug,
+			"retry", retry,
+		)
+		// On first retry, log the token parts that contain a nil byte
+		if retry == 1 {
+			logTokenNULParts(ctxLogger, extSvcSlug, newKeyInfo.ClientSecret)
+		}
+	}
+
+	return satokengen.KeyGenResult{}, ErrCredentialsGenFailed.Errorf("Failed to generate a token for %s", extSvcSlug)
+}
+
+// logTokenNULParts logs a warning if the external service token contains a nil byte
+// Tokens normally have 3 parts "gl+serviceID_secret_checksum"
+// Log the part of the generated token that contains a nil byte
+func logTokenNULParts(ctxLogger log.Logger, extSvcSlug string, token string) {
+	parts := strings.Split(token, "_")
+	for i := range parts {
+		if strings.Contains(parts[i], "\x00") {
+			ctxLogger.Warn("Token contains NUL",
+				"service", extSvcSlug,
+				"part", i,
+				"part_len", len(parts[i]),
+				"parts_count", len(parts),
+			)
+		}
+	}
+}
+
 // GetExtSvcCredentials get the credentials of an External Service from an encrypted storage
 func (esa *ExtSvcAccountsService) GetExtSvcCredentials(ctx context.Context, orgID int64, extSvcSlug string) (*Credentials, error) {
 	ctxLogger := esa.logger.FromContext(ctx)
@@ -391,6 +437,10 @@ func (esa *ExtSvcAccountsService) GetExtSvcCredentials(ctx context.Context, orgI
 	if !ok {
 		return nil, ErrCredentialsNotFound.Errorf("No credential found for in store %v", extSvcSlug)
 	}
+	if strings.Contains(token, "\x00") {
+		ctxLogger.Warn("Loaded token from store containing NUL", "service", extSvcSlug)
+		logTokenNULParts(ctxLogger, extSvcSlug, token)
+	}
 	return &Credentials{Secret: token}, nil
 }