TheArchive/zitadel
1
0
Fork 0
mirror of https://github.com/zitadel/zitadel.git synced 2025-08-12 00:17:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(crypto): use passwap for machine and app secrets (#7657)

Browse Source 
* feat(crypto): use passwap for machine and app secrets

* fix command package tests

* add hash generator command test

* naming convention, fix query tests

* rename PasswordHasher and cleanup start commands

* add reducer tests

* fix intergration tests, cleanup old config

* add app secret unit tests

* solve setup panics

* fix push of updated events

* add missing event translations

* update documentation

* solve linter errors

* remove nolint:SA1019 as it doesn't seem to help anyway

* add nolint to deprecated filter usage

* update users migration version

* remove unused ClientSecret from APIConfigChangedEvent

---------

Co-authored-by: Livio Spring <livio.a@gmail.com>
This commit is contained in:
Tim Möhlmann
2024-04-05 12:35:49 +03:00
committed by GitHub
parent 5931fb8f28
commit 2089992d75
135 changed files with 2407 additions and 1779 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
internal/api/grpc/admin/import.go
View File

@@ -292,7 +292,7 @@ func getFileFromGCS(ctx context.Context, input *admin_pb.ImportDataRequest_GCSIn
return ioutil.ReadAll(reader)
}
func importOrg1(ctx context.Context, s *Server, errors *[]*admin_pb.ImportDataError, ctxData authz.CtxData, org *admin_pb.DataOrg, success *admin_pb.ImportDataSuccess, count *counts, initCodeGenerator, emailCodeGenerator, phoneCodeGenerator, passwordlessInitCode, appSecretGenerator crypto.Generator) error {
func importOrg1(ctx context.Context, s *Server, errors *[]*admin_pb.ImportDataError, ctxData authz.CtxData, org *admin_pb.DataOrg, success *admin_pb.ImportDataSuccess, count *counts, initCodeGenerator, emailCodeGenerator, phoneCodeGenerator, passwordlessInitCode crypto.Generator) error {
_, err := s.command.AddOrgWithID(ctx, org.GetOrg().GetName(), ctxData.UserID, ctxData.ResourceOwner, org.GetOrgId(), []string{})
if err != nil {
*errors = append(*errors, &admin_pb.ImportDataError{Type: "org", Id: org.GetOrgId(), Message: err.Error()})
@@ -325,7 +325,7 @@ func importOrg1(ctx context.Context, s *Server, errors *[]*admin_pb.ImportDataEr
*errors = append(*errors, &admin_pb.ImportDataError{Type: "domain_policy", Id: org.GetOrgId(), Message: err.Error()})
}
}
return importResources(ctx, s, errors, successOrg, org, count, initCodeGenerator, emailCodeGenerator, phoneCodeGenerator, passwordlessInitCode, appSecretGenerator)
return importResources(ctx, s, errors, successOrg, org, count, initCodeGenerator, emailCodeGenerator, phoneCodeGenerator, passwordlessInitCode)
}
func importLabelPolicy(ctx context.Context, s *Server, errors *[]*admin_pb.ImportDataError, org *admin_pb.DataOrg) error {
@@ -584,13 +584,13 @@ func importProjects(ctx context.Context, s *Server, errors *[]*admin_pb.ImportDa
return nil
}
func importOIDCApps(ctx context.Context, s *Server, errors *[]*admin_pb.ImportDataError, successOrg *admin_pb.ImportDataSuccessOrg, org *admin_pb.DataOrg, count *counts, appSecretGenerator crypto.Generator) error {
func importOIDCApps(ctx context.Context, s *Server, errors *[]*admin_pb.ImportDataError, successOrg *admin_pb.ImportDataSuccessOrg, org *admin_pb.DataOrg, count *counts) error {
if org.OidcApps == nil {
return nil
}
for _, app := range org.GetOidcApps() {
logging.Debugf("import oidcapplication: %s", app.GetAppId())
_, err := s.command.AddOIDCApplicationWithID(ctx, management.AddOIDCAppRequestToDomain(app.App), org.GetOrgId(), app.GetAppId(), appSecretGenerator)
_, err := s.command.AddOIDCApplicationWithID(ctx, management.AddOIDCAppRequestToDomain(app.App), org.GetOrgId(), app.GetAppId())
if err != nil {
*errors = append(*errors, &admin_pb.ImportDataError{Type: "oidc_app", Id: app.GetAppId(), Message: err.Error()})
if isCtxTimeout(ctx) {
@@ -605,13 +605,13 @@ func importOIDCApps(ctx context.Context, s *Server, errors *[]*admin_pb.ImportDa
return nil
}
func importAPIApps(ctx context.Context, s *Server, errors *[]*admin_pb.ImportDataError, successOrg *admin_pb.ImportDataSuccessOrg, org *admin_pb.DataOrg, count *counts, appSecretGenerator crypto.Generator) error {
func importAPIApps(ctx context.Context, s *Server, errors *[]*admin_pb.ImportDataError, successOrg *admin_pb.ImportDataSuccessOrg, org *admin_pb.DataOrg, count *counts) error {
if org.ApiApps == nil {
return nil
}
for _, app := range org.GetApiApps() {
logging.Debugf("import apiapplication: %s", app.GetAppId())
_, err := s.command.AddAPIApplicationWithID(ctx, management.AddAPIAppRequestToDomain(app.GetApp()), org.GetOrgId(), app.GetAppId(), appSecretGenerator)
_, err := s.command.AddAPIApplicationWithID(ctx, management.AddAPIAppRequestToDomain(app.GetApp()), org.GetOrgId(), app.GetAppId())
if err != nil {
*errors = append(*errors, &admin_pb.ImportDataError{Type: "api_app", Id: app.GetAppId(), Message: err.Error()})
if isCtxTimeout(ctx) {
@@ -700,7 +700,7 @@ func importProjectRoles(ctx context.Context, s *Server, errors *[]*admin_pb.Impo
return nil
}
func importResources(ctx context.Context, s *Server, errors *[]*admin_pb.ImportDataError, successOrg *admin_pb.ImportDataSuccessOrg, org *admin_pb.DataOrg, count *counts, initCodeGenerator, emailCodeGenerator, phoneCodeGenerator, passwordlessInitCode, appSecretGenerator crypto.Generator) error {
func importResources(ctx context.Context, s *Server, errors *[]*admin_pb.ImportDataError, successOrg *admin_pb.ImportDataSuccessOrg, org *admin_pb.DataOrg, count *counts, initCodeGenerator, emailCodeGenerator, phoneCodeGenerator, passwordlessInitCode crypto.Generator) error {
if err := importOrgDomains(ctx, s, errors, successOrg, org); err != nil {
return err
}
@@ -742,10 +742,10 @@ func importResources(ctx context.Context, s *Server, errors *[]*admin_pb.ImportD
if err := importProjects(ctx, s, errors, successOrg, org, count); err != nil {
return err
}
if err := importOIDCApps(ctx, s, errors, successOrg, org, count, appSecretGenerator); err != nil {
if err := importOIDCApps(ctx, s, errors, successOrg, org, count); err != nil {
return err
}
if err := importAPIApps(ctx, s, errors, successOrg, org, count, appSecretGenerator); err != nil {
if err := importAPIApps(ctx, s, errors, successOrg, org, count); err != nil {
return err
}
if err := importAppKeys(ctx, s, errors, successOrg, org, count); err != nil {
@@ -1023,10 +1023,6 @@ func (s *Server) importData(ctx context.Context, orgs []*admin_pb.DataOrg) (*adm
success := &admin_pb.ImportDataSuccess{}
count := &counts{}
appSecretGenerator, err := s.query.InitHashGenerator(ctx, domain.SecretGeneratorTypeAppSecret, s.passwordHashAlg)
if err != nil {
return nil, nil, err
}
initCodeGenerator, err := s.query.InitEncryptionGenerator(ctx, domain.SecretGeneratorTypeInitCode, s.userCodeAlg)
if err != nil {
return nil, nil, err
@@ -1064,7 +1060,7 @@ func (s *Server) importData(ctx context.Context, orgs []*admin_pb.DataOrg) (*adm
count.appKeysCount += len(org.GetAppKeys())
}
for _, org := range orgs {
if err = importOrg1(ctx, s, &errors, ctxData, org, success, count, initCodeGenerator, emailCodeGenerator, phoneCodeGenerator, passwordlessInitCode, appSecretGenerator); err != nil {
if err = importOrg1(ctx, s, &errors, ctxData, org, success, count, initCodeGenerator, emailCodeGenerator, phoneCodeGenerator, passwordlessInitCode); err != nil {
return &admin_pb.ImportDataResponse{Errors: errors, Success: success}, count, err
}
}

2
internal/api/grpc/admin/server.go
View File

@@ -30,7 +30,6 @@ type Server struct {
query *query.Queries
assetsAPIDomain func(context.Context) string
userCodeAlg crypto.EncryptionAlgorithm
passwordHashAlg crypto.HashAlgorithm
auditLogRetention time.Duration
}
@@ -53,7 +52,6 @@ func CreateServer(
query: query,
assetsAPIDomain: assets.AssetAPI(externalSecure),
userCodeAlg: userCodeAlg,
passwordHashAlg: crypto.NewBCrypt(sd.SecretGenerators.PasswordSaltCost),
auditLogRetention: auditLogRetention,
}
}

25
internal/api/grpc/management/project_application.go
View File

@@ -8,7 +8,6 @@ import (
change_grpc "github.com/zitadel/zitadel/internal/api/grpc/change"
object_grpc "github.com/zitadel/zitadel/internal/api/grpc/object"
project_grpc "github.com/zitadel/zitadel/internal/api/grpc/project"
"github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/eventstore"
"github.com/zitadel/zitadel/internal/query"
"github.com/zitadel/zitadel/internal/repository/project"
@@ -81,11 +80,7 @@ func (s *Server) ListAppChanges(ctx context.Context, req *mgmt_pb.ListAppChanges
}
func (s *Server) AddOIDCApp(ctx context.Context, req *mgmt_pb.AddOIDCAppRequest) (*mgmt_pb.AddOIDCAppResponse, error) {
appSecretGenerator, err := s.query.InitHashGenerator(ctx, domain.SecretGeneratorTypeAppSecret, s.passwordHashAlg)
if err != nil {
return nil, err
}
app, err := s.command.AddOIDCApplication(ctx, AddOIDCAppRequestToDomain(req), authz.GetCtxData(ctx).OrgID, appSecretGenerator)
app, err := s.command.AddOIDCApplication(ctx, AddOIDCAppRequestToDomain(req), authz.GetCtxData(ctx).OrgID)
if err != nil {
return nil, err
}
@@ -110,11 +105,7 @@ func (s *Server) AddSAMLApp(ctx context.Context, req *mgmt_pb.AddSAMLAppRequest)
}
func (s *Server) AddAPIApp(ctx context.Context, req *mgmt_pb.AddAPIAppRequest) (*mgmt_pb.AddAPIAppResponse, error) {
appSecretGenerator, err := s.query.InitHashGenerator(ctx, domain.SecretGeneratorTypeAppSecret, s.passwordHashAlg)
if err != nil {
return nil, err
}
app, err := s.command.AddAPIApplication(ctx, AddAPIAppRequestToDomain(req), authz.GetCtxData(ctx).OrgID, appSecretGenerator)
app, err := s.command.AddAPIApplication(ctx, AddAPIAppRequestToDomain(req), authz.GetCtxData(ctx).OrgID)
if err != nil {
return nil, err
}
@@ -209,11 +200,7 @@ func (s *Server) RemoveApp(ctx context.Context, req *mgmt_pb.RemoveAppRequest) (
}
func (s *Server) RegenerateOIDCClientSecret(ctx context.Context, req *mgmt_pb.RegenerateOIDCClientSecretRequest) (*mgmt_pb.RegenerateOIDCClientSecretResponse, error) {
appSecretGenerator, err := s.query.InitHashGenerator(ctx, domain.SecretGeneratorTypeAppSecret, s.passwordHashAlg)
if err != nil {
return nil, err
}
config, err := s.command.ChangeOIDCApplicationSecret(ctx, req.ProjectId, req.AppId, authz.GetCtxData(ctx).OrgID, appSecretGenerator)
config, err := s.command.ChangeOIDCApplicationSecret(ctx, req.ProjectId, req.AppId, authz.GetCtxData(ctx).OrgID)
if err != nil {
return nil, err
}
@@ -228,11 +215,7 @@ func (s *Server) RegenerateOIDCClientSecret(ctx context.Context, req *mgmt_pb.Re
}
func (s *Server) RegenerateAPIClientSecret(ctx context.Context, req *mgmt_pb.RegenerateAPIClientSecretRequest) (*mgmt_pb.RegenerateAPIClientSecretResponse, error) {
appSecretGenerator, err := s.query.InitHashGenerator(ctx, domain.SecretGeneratorTypeAppSecret, s.passwordHashAlg)
if err != nil {
return nil, err
}
config, err := s.command.ChangeAPIApplicationSecret(ctx, req.ProjectId, req.AppId, authz.GetCtxData(ctx).OrgID, appSecretGenerator)
config, err := s.command.ChangeAPIApplicationSecret(ctx, req.ProjectId, req.AppId, authz.GetCtxData(ctx).OrgID)
if err != nil {
return nil, err
}

26
internal/api/grpc/management/server.go
View File

@@ -23,13 +23,12 @@ var _ management.ManagementServiceServer = (*Server)(nil)
type Server struct {
management.UnimplementedManagementServiceServer
command *command.Commands
query *query.Queries
systemDefaults systemdefaults.SystemDefaults
assetAPIPrefix func(context.Context) string
passwordHashAlg crypto.HashAlgorithm
userCodeAlg crypto.EncryptionAlgorithm
externalSecure bool
command *command.Commands
query *query.Queries
systemDefaults systemdefaults.SystemDefaults
assetAPIPrefix func(context.Context) string
userCodeAlg crypto.EncryptionAlgorithm
externalSecure bool
}
func CreateServer(
@@ -40,13 +39,12 @@ func CreateServer(
externalSecure bool,
) *Server {
return &Server{
command: command,
query: query,
systemDefaults: sd,
assetAPIPrefix: assets.AssetAPI(externalSecure),
passwordHashAlg: crypto.NewBCrypt(sd.SecretGenerators.PasswordSaltCost),
userCodeAlg: userCodeAlg,
externalSecure: externalSecure,
command: command,
query: query,
systemDefaults: sd,
assetAPIPrefix: assets.AssetAPI(externalSecure),
userCodeAlg: userCodeAlg,
externalSecure: externalSecure,
}
}

7
internal/api/grpc/management/user.go
View File

@@ -800,18 +800,13 @@ func (s *Server) RemoveMachineKey(ctx context.Context, req *mgmt_pb.RemoveMachin
}
func (s *Server) GenerateMachineSecret(ctx context.Context, req *mgmt_pb.GenerateMachineSecretRequest) (*mgmt_pb.GenerateMachineSecretResponse, error) {
// use SecretGeneratorTypeAppSecret as the secrets will be used in the client_credentials grant like a client secret
secretGenerator, err := s.query.InitHashGenerator(ctx, domain.SecretGeneratorTypeAppSecret, s.passwordHashAlg)
if err != nil {
return nil, err
}
user, err := s.getUserByID(ctx, req.GetUserId())
if err != nil {
return nil, err
}
set := new(command.GenerateMachineSecret)
details, err := s.command.GenerateMachineSecret(ctx, req.UserId, authz.GetCtxData(ctx).OrgID, secretGenerator, set)
details, err := s.command.GenerateMachineSecret(ctx, req.UserId, authz.GetCtxData(ctx).OrgID, set)
if err != nil {
return nil, err
}

2
internal/api/grpc/user/converter.go
View File

@@ -73,7 +73,7 @@ func MachineToPb(view *query.Machine) *user_pb.Machine {
return &user_pb.Machine{
Name: view.Name,
Description: view.Description,
HasSecret: view.Secret != nil,
HasSecret: view.EncodedSecret != "",
AccessTokenType: AccessTokenTypeToPb(view.AccessTokenType),
}
}

2
internal/api/grpc/user/v2/query.go
View File

@@ -109,7 +109,7 @@ func machineToPb(userQ *query.Machine) *user.MachineUser {
return &user.MachineUser{
Name: userQ.Name,
Description: userQ.Description,
HasSecret: userQ.Secret != nil,
HasSecret: userQ.EncodedSecret != "",
AccessTokenType: accessTokenTypeToPb(userQ.AccessTokenType),
}
}

7
internal/api/oidc/client.go
View File

@@ -1050,8 +1050,13 @@ func (s *Server) verifyClientSecret(ctx context.Context, client *query.OIDCClien
if secret == "" {
return oidc.ErrInvalidClient().WithDescription("empty client secret")
}
if err = crypto.CompareHash(client.ClientSecret, []byte(secret), s.hashAlg); err != nil {
ctx, spanPasswordComparison := tracing.NewNamedSpan(ctx, "passwap.Verify")
updated, err := s.hasher.Verify(client.HashedSecret, secret)
spanPasswordComparison.EndWithError(err)
if err != nil {
s.command.OIDCSecretCheckFailed(ctx, client.AppID, client.ProjectID, client.Settings.ResourceOwner)
return oidc.ErrInvalidClient().WithParent(err).WithDescription("invalid secret")
}
s.command.OIDCSecretCheckSucceeded(ctx, client.AppID, client.ProjectID, client.Settings.ResourceOwner, updated)
return nil
}

11
internal/api/oidc/client_credentials.go
View File

@@ -7,8 +7,8 @@ import (
"github.com/zitadel/oidc/v3/pkg/oidc"
"github.com/zitadel/oidc/v3/pkg/op"
"github.com/zitadel/zitadel/internal/crypto"
"github.com/zitadel/zitadel/internal/query"
"github.com/zitadel/zitadel/internal/telemetry/tracing"
"github.com/zitadel/zitadel/internal/zerrors"
)
@@ -41,15 +41,18 @@ func (s *Server) clientCredentialsAuth(ctx context.Context, clientID, clientSecr
if err != nil {
return nil, err // defaults to server error
}
if user.Machine == nil || user.Machine.Secret == nil {
if user.Machine == nil || user.Machine.EncodedSecret == "" {
return nil, zerrors.ThrowPreconditionFailed(nil, "OIDC-pieP8", "Errors.User.Machine.Secret.NotExisting")
}
if err = crypto.CompareHash(user.Machine.Secret, []byte(clientSecret), s.hashAlg); err != nil {
ctx, spanPasswordComparison := tracing.NewNamedSpan(ctx, "passwap.Verify")
updated, err := s.hasher.Verify(user.Machine.EncodedSecret, clientSecret)
spanPasswordComparison.EndWithError(err)
if err != nil {
s.command.MachineSecretCheckFailed(ctx, user.ID, user.ResourceOwner)
return nil, zerrors.ThrowInvalidArgument(err, "OIDC-VoXo6", "Errors.User.Machine.Secret.Invalid")
}
s.command.MachineSecretCheckSucceeded(ctx, user.ID, user.ResourceOwner)
s.command.MachineSecretCheckSucceeded(ctx, user.ID, user.ResourceOwner, updated)
return &clientCredentialsClient{
id: clientID,
user: user,

34
internal/api/oidc/introspect.go
View File

@@ -11,7 +11,6 @@ import (
"github.com/zitadel/oidc/v3/pkg/op"
"github.com/zitadel/zitadel/internal/api/authz"
"github.com/zitadel/zitadel/internal/crypto"
"github.com/zitadel/zitadel/internal/query"
"github.com/zitadel/zitadel/internal/telemetry/tracing"
"github.com/zitadel/zitadel/internal/zerrors"
@@ -149,8 +148,8 @@ func (s *Server) introspectionClientAuth(ctx context.Context, cc *op.ClientCrede
return client.ClientID, client.ProjectID, nil
}
if client.ClientSecret != nil {
if err := crypto.CompareHash(client.ClientSecret, []byte(cc.ClientSecret), s.hashAlg); err != nil {
if client.HashedSecret != "" {
if err := s.introspectionClientSecretAuth(ctx, client, cc.ClientSecret); err != nil {
return "", "", oidc.ErrUnauthorizedClient().WithParent(err)
}
return client.ClientID, client.ProjectID, nil
@@ -167,6 +166,35 @@ func (s *Server) introspectionClientAuth(ctx context.Context, cc *op.ClientCrede
}
}
var errNoAppType = errors.New("introspection client without app type")
func (s *Server) introspectionClientSecretAuth(ctx context.Context, client *query.IntrospectionClient, secret string) error {
var (
successCommand func(ctx context.Context, appID, projectID, resourceOwner, updated string)
failedCommand func(ctx context.Context, appID, projectID, resourceOwner string)
)
switch client.AppType {
case query.AppTypeAPI:
successCommand = s.command.APISecretCheckSucceeded
failedCommand = s.command.APISecretCheckFailed
case query.AppTypeOIDC:
successCommand = s.command.OIDCSecretCheckSucceeded
failedCommand = s.command.OIDCSecretCheckFailed
default:
return zerrors.ThrowInternal(errNoAppType, "OIDC-ooD5Ot", "Errors.Internal")
}
ctx, spanPasswordComparison := tracing.NewNamedSpan(ctx, "passwap.Verify")
updated, err := s.hasher.Verify(client.HashedSecret, secret)
spanPasswordComparison.EndWithError(err)
if err != nil {
failedCommand(ctx, client.AppID, client.ProjectID, client.ResourceOwner)
return err
}
successCommand(ctx, client.AppID, client.ProjectID, client.ResourceOwner, updated)
return nil
}
// clientFromCredentials parses the client ID early,
// and makes a single query for the client for either auth methods.
func (s *Server) clientFromCredentials(ctx context.Context, cc *op.ClientCredentials) (client *query.IntrospectionClient, err error) {

11
internal/api/oidc/op.go
View File

@@ -80,6 +80,7 @@ type OPStorage struct {
}
func NewServer(
ctx context.Context,
config Config,
defaultLogoutRedirectURI string,
externalSecure bool,
@@ -93,13 +94,14 @@ func NewServer(
userAgentCookie, instanceHandler func(http.Handler) http.Handler,
accessHandler *middleware.AccessInterceptor,
fallbackLogger *slog.Logger,
hashConfig crypto.HashConfig,
) (*Server, error) {
opConfig, err := createOPConfig(config, defaultLogoutRedirectURI, cryptoKey)
if err != nil {
return nil, zerrors.ThrowInternal(err, "OIDC-EGrqd", "cannot create op config: %w")
}
storage := newStorage(config, command, query, repo, encryptionAlg, es, projections, externalSecure)
keyCache := newPublicKeyCache(context.TODO(), config.PublicKeyCacheMaxAge, query.GetPublicKeyByID)
keyCache := newPublicKeyCache(ctx, config.PublicKeyCacheMaxAge, query.GetPublicKeyByID)
accessTokenKeySet := newOidcKeySet(keyCache, withKeyExpiryCheck(true))
idTokenHintKeySet := newOidcKeySet(keyCache)
@@ -119,7 +121,10 @@ func NewServer(
if err != nil {
return nil, zerrors.ThrowInternal(err, "OIDC-DAtg3", "cannot create provider")
}
hasher, err := hashConfig.NewHasher()
if err != nil {
return nil, zerrors.ThrowInternal(err, "OIDC-Aij4e", "cannot create secret hasher")
}
server := &Server{
LegacyServer: op.NewLegacyServer(provider, endpoints(config.CustomEndpoints)),
repo: repo,
@@ -133,7 +138,7 @@ func NewServer(
defaultAccessTokenLifetime: config.DefaultAccessTokenLifetime,
defaultIdTokenLifetime: config.DefaultIdTokenLifetime,
fallbackLogger: fallbackLogger,
hashAlg: crypto.NewBCrypt(10), // as we are only verifying in oidc, the cost is already part of the hash string and the config here is irrelevant.
hasher: hasher,
signingKeyAlgorithm: config.SigningKeyAlgorithm,
assetAPIPrefix: assets.AssetAPI(externalSecure),
}

2
internal/api/oidc/server.go
View File

@@ -35,7 +35,7 @@ type Server struct {
defaultIdTokenLifetime time.Duration
fallbackLogger *slog.Logger
hashAlg crypto.HashAlgorithm
hasher *crypto.Hasher
signingKeyAlgorithm string
assetAPIPrefix func(ctx context.Context) string
}