From 9d4c4d6a814ac1ece9135676f2c28c611d906d65 Mon Sep 17 00:00:00 2001 From: Keegan Witt Date: Wed, 13 Dec 2023 14:20:33 -0500 Subject: [PATCH 01/11] Allow multiple JWT audiences to be configured (closes #108) Signed-off-by: Keegan Witt --- pkg/sidecar/config.go | 32 +++++++++++++++++++++++------ pkg/sidecar/config_test.go | 33 ++++++++++++++++++++++-------- pkg/sidecar/sidecar.go | 41 ++++++++++++++++++++++++-------------- 3 files changed, 77 insertions(+), 29 deletions(-) diff --git a/pkg/sidecar/config.go b/pkg/sidecar/config.go index e83c30bd..08138ce1 100644 --- a/pkg/sidecar/config.go +++ b/pkg/sidecar/config.go @@ -32,9 +32,10 @@ type Config struct { RenewSignalDeprecated string `hcl:"renewSignal"` // JWT configuration - JWTAudience string `hcl:"jwt_audience"` - JWTSvidFilename string `hcl:"jwt_svid_file_name"` - JWTBundleFilename string `hcl:"jwt_bundle_file_name"` + JwtSvids []JwtConfig `hcl:"jwt_svids"` + JWTAudienceDeprecated string `hcl:"jwt_audience"` + JWTSvidFilenameDeprecated string `hcl:"jwt_svid_file_name"` + JWTBundleFilename string `hcl:"jwt_bundle_file_name"` // TODO: is there a reason for this to be exposed? and inside of config? ReloadExternalProcess func() error @@ -42,6 +43,11 @@ type Config struct { Log logrus.FieldLogger } +type JwtConfig struct { + JWTAudience string `hcl:"jwt_audience"` + JWTSvidFilename string `hcl:"jwt_svid_file_name"` +} + // ParseConfig parses the given HCL file into a SidecarConfig struct func ParseConfig(file string) (*Config, error) { sidecarConfig := new(Config) @@ -120,11 +126,17 @@ func ValidateConfig(c *Config) error { c.RenewSignal = c.RenewSignalDeprecated } + for _, jwtConfig := range c.JwtSvids { + if countEmpty(jwtConfig.JWTSvidFilename, jwtConfig.JWTAudience) > 0 { + return errors.New("both 'jwt_file_name' and 'jwt_audience' are required in 'jwt_svids'") + } + } + x509EmptyCount := countEmpty(c.SvidFileName, c.SvidBundleFileName, c.SvidKeyFileName) - jwtSVIDEmptyCount := countEmpty(c.JWTSvidFilename, c.JWTAudience) + jwtSVIDEmptyCount := countEmpty(c.JWTSvidFilenameDeprecated, c.JWTAudienceDeprecated) jwtBundleEmptyCount := countEmpty(c.SvidBundleFileName) - if x509EmptyCount == 3 && jwtSVIDEmptyCount == 2 && jwtBundleEmptyCount == 1 { - return errors.New("at least one of the sets ('svid_file_name', 'svid_key_file_name', 'svid_bundle_file_name'), ('jwt_file_name', 'jwt_audience'), or ('jwt_bundle_file_name') must be fully specified") + if x509EmptyCount == 3 && jwtSVIDEmptyCount == 2 && c.JwtSvids == nil && jwtBundleEmptyCount == 1 { + return errors.New("at least one of the sets ('svid_file_name', 'svid_key_file_name', 'svid_bundle_file_name'), ('jwt_file_name', 'jwt_audience'), 'jwt_svids', or ('jwt_bundle_file_name') must be fully specified") } if x509EmptyCount != 0 && x509EmptyCount != 3 { @@ -135,6 +147,14 @@ func ValidateConfig(c *Config) error { return errors.New("all or none of 'jwt_file_name', 'jwt_audience' must be specified") } + if jwtSVIDEmptyCount == 0 { + c.Log.Warn(getWarning("jwt_file_name and jwt_audience", "jwt_svids")) + } + + if jwtSVIDEmptyCount != 0 && c.JwtSvids == nil { + return errors.New("must not specify deprecated JWT configs ('jwt_file_name' and 'jwt_audience') and new JWT config ('jwt_svids')") + } + return nil } diff --git a/pkg/sidecar/config_test.go b/pkg/sidecar/config_test.go index 36fad5b4..400e4103 100644 --- a/pkg/sidecar/config_test.go +++ b/pkg/sidecar/config_test.go @@ -34,9 +34,9 @@ func TestParseConfig(t *testing.T) { assert.Equal(t, expectedSvidFileName, c.SvidFileName) assert.Equal(t, expectedKeyFileName, c.SvidKeyFileName) assert.Equal(t, expectedSvidBundleFileName, c.SvidBundleFileName) - assert.Equal(t, expectedJWTSVIDFileName, c.JWTSvidFilename) + assert.Equal(t, expectedJWTSVIDFileName, c.JWTSvidFilenameDeprecated) assert.Equal(t, expectedJWTBundleFileName, c.JWTBundleFilename) - assert.Equal(t, expectedJWTAudience, c.JWTAudience) + assert.Equal(t, expectedJWTAudience, c.JWTAudienceDeprecated) assert.True(t, c.AddIntermediatesToBundle) } @@ -56,12 +56,29 @@ func TestValidateConfig(t *testing.T) { SvidBundleFileName: "bundle.pem", }, }, + { + name: "warns on deprecated jwt configs", + config: &Config{ + AgentAddress: "path", + JWTAudienceDeprecated: "your-audience", + JWTSvidFilenameDeprecated: "jwt.token", + JWTBundleFilename: "bundle.json", + }, + expectLogs: []shortEntry{ + { + Level: logrus.WarnLevel, + Message: "jwt_file_name and jwt_audience will be deprecated, should be used as jwt_svids", + }, + }, + }, { name: "no error", config: &Config{ - AgentAddress: "path", - JWTAudience: "your-audience", - JWTSvidFilename: "jwt.token", + AgentAddress: "path", + JwtSvids: []JwtConfig{{ + JWTSvidFilename: "jwt.token", + JWTAudience: "your-audience", + }}, JWTBundleFilename: "bundle.json", }, }, @@ -70,7 +87,7 @@ func TestValidateConfig(t *testing.T) { config: &Config{ AgentAddress: "path", }, - expectError: "at least one of the sets ('svid_file_name', 'svid_key_file_name', 'svid_bundle_file_name'), ('jwt_file_name', 'jwt_audience'), or ('jwt_bundle_file_name') must be fully specified", + expectError: "at least one of the sets ('svid_file_name', 'svid_key_file_name', 'svid_bundle_file_name'), ('jwt_file_name', 'jwt_audience'), 'jwt_svids', or ('jwt_bundle_file_name') must be fully specified", }, { name: "missing svid config", @@ -83,8 +100,8 @@ func TestValidateConfig(t *testing.T) { { name: "missing jwt config", config: &Config{ - AgentAddress: "path", - JWTSvidFilename: "cert.pem", + AgentAddress: "path", + JWTSvidFilenameDeprecated: "cert.pem", }, expectError: "all or none of 'jwt_file_name', 'jwt_audience' must be specified", }, diff --git a/pkg/sidecar/sidecar.go b/pkg/sidecar/sidecar.go index a9a31a49..1370bddc 100644 --- a/pkg/sidecar/sidecar.go +++ b/pkg/sidecar/sidecar.go @@ -103,7 +103,7 @@ func (s *Sidecar) RunDaemon(ctx context.Context) error { }() } - if s.config.JWTSvidFilename != "" && s.config.JWTAudience != "" { + if s.config.JWTSvidFilenameDeprecated != "" && s.config.JWTAudienceDeprecated != "" { jwtSource, err := workloadapi.NewJWTSource(ctx, workloadapi.WithClientOptions(s.getWorkloadAPIAdress())) if err != nil { s.config.Log.Fatalf("Error watching JWT svid updates: %v", err) @@ -111,11 +111,22 @@ func (s *Sidecar) RunDaemon(ctx context.Context) error { s.jwtSource = jwtSource defer s.jwtSource.Close() - wg.Add(1) - go func() { - defer wg.Done() - s.updateJWTSVID(ctx) - }() + if s.config.JwtSvids != nil { + for _, jwtConfig := range s.config.JwtSvids { + jwtConfig := jwtConfig + wg.Add(1) + go func() { + defer wg.Done() + s.updateJWTSVID(ctx, jwtConfig.JWTAudience, jwtConfig.JWTSvidFilename) + }() + } + } else { + wg.Add(1) + go func() { + defer wg.Done() + s.updateJWTSVID(ctx, s.config.JWTAudienceDeprecated, s.config.JWTSvidFilenameDeprecated) + }() + } } wg.Wait() @@ -274,14 +285,14 @@ func (s *Sidecar) updateJWTBundle(jwkSet *jwtbundle.Set) { } } -func (s *Sidecar) fetchJWTSVID(ctx context.Context) (*jwtsvid.SVID, error) { - jwtSVID, err := s.jwtSource.FetchJWTSVID(ctx, jwtsvid.Params{Audience: s.config.JWTAudience}) +func (s *Sidecar) fetchJWTSVIDs(ctx context.Context, jwtAudience string) (*jwtsvid.SVID, error) { + jwtSVID, err := s.jwtSource.FetchJWTSVID(ctx, jwtsvid.Params{Audience: jwtAudience}) if err != nil { s.config.Log.Errorf("Unable to fetch JWT SVID: %v", err) return nil, err } - _, err = jwtsvid.ParseAndValidate(jwtSVID.Marshal(), s.jwtSource, []string{s.config.JWTAudience}) + _, err = jwtsvid.ParseAndValidate(jwtSVID.Marshal(), s.jwtSource, []string{jwtAudience}) if err != nil { s.config.Log.Errorf("Unable to parse or validate token: %v", err) return nil, err @@ -312,16 +323,16 @@ func getRefreshInterval(svid *jwtsvid.SVID) time.Duration { return time.Until(svid.Expiry)/2 + time.Second } -func (s *Sidecar) performJWTSVIDUpdate(ctx context.Context) (*jwtsvid.SVID, error) { +func (s *Sidecar) performJWTSVIDUpdate(ctx context.Context, jwtAudience string, jwtSvidFilename string) (*jwtsvid.SVID, error) { s.config.Log.Debug("Updating JWT SVID") - jwtSVID, err := s.fetchJWTSVID(ctx) + jwtSVID, err := s.fetchJWTSVIDs(ctx, jwtAudience) if err != nil { s.config.Log.Errorf("Unable to update JWT SVID: %v", err) return nil, err } - filePath := path.Join(s.config.CertDir, s.config.JWTSvidFilename) + filePath := path.Join(s.config.CertDir, jwtSvidFilename) if err = os.WriteFile(filePath, []byte(jwtSVID.Marshal()), os.ModePerm); err != nil { s.config.Log.Errorf("Unable to update JWT SVID: %v", err) return nil, err @@ -331,10 +342,10 @@ func (s *Sidecar) performJWTSVIDUpdate(ctx context.Context) (*jwtsvid.SVID, erro return jwtSVID, nil } -func (s *Sidecar) updateJWTSVID(ctx context.Context) { +func (s *Sidecar) updateJWTSVID(ctx context.Context, jwtAudience string, jwtSvidFilename string) { retryInterval := createRetryIntervalFunc() var initialInterval time.Duration - jwtSVID, err := s.performJWTSVIDUpdate(ctx) + jwtSVID, err := s.performJWTSVIDUpdate(ctx, jwtAudience, jwtSvidFilename) if err != nil { // If the first update fails, use the retry interval initialInterval = retryInterval() @@ -350,7 +361,7 @@ func (s *Sidecar) updateJWTSVID(ctx context.Context) { case <-ctx.Done(): return case <-ticker.C: - jwtSVID, err = s.performJWTSVIDUpdate(ctx) + jwtSVID, err = s.performJWTSVIDUpdate(ctx, jwtAudience, jwtSvidFilename) if err == nil { retryInterval = createRetryIntervalFunc() ticker.Reset(getRefreshInterval(jwtSVID)) From 4ca75de5d788a5395b03e6b92876c0bedd2621b2 Mon Sep 17 00:00:00 2001 From: Keegan Witt Date: Mon, 18 Dec 2023 21:36:52 -0500 Subject: [PATCH 02/11] Remove deprecated parameters Signed-off-by: Keegan Witt --- pkg/sidecar/config.go | 21 +++------------------ pkg/sidecar/config_test.go | 27 +++++++-------------------- pkg/sidecar/sidecar.go | 16 ++++------------ test/fixture/config/helper.conf | 8 ++++++-- 4 files changed, 20 insertions(+), 52 deletions(-) diff --git a/pkg/sidecar/config.go b/pkg/sidecar/config.go index 08138ce1..2f88bb8d 100644 --- a/pkg/sidecar/config.go +++ b/pkg/sidecar/config.go @@ -32,10 +32,8 @@ type Config struct { RenewSignalDeprecated string `hcl:"renewSignal"` // JWT configuration - JwtSvids []JwtConfig `hcl:"jwt_svids"` - JWTAudienceDeprecated string `hcl:"jwt_audience"` - JWTSvidFilenameDeprecated string `hcl:"jwt_svid_file_name"` - JWTBundleFilename string `hcl:"jwt_bundle_file_name"` + JwtSvids []JwtConfig `hcl:"jwt_svids"` + JWTBundleFilename string `hcl:"jwt_bundle_file_name"` // TODO: is there a reason for this to be exposed? and inside of config? ReloadExternalProcess func() error @@ -133,9 +131,8 @@ func ValidateConfig(c *Config) error { } x509EmptyCount := countEmpty(c.SvidFileName, c.SvidBundleFileName, c.SvidKeyFileName) - jwtSVIDEmptyCount := countEmpty(c.JWTSvidFilenameDeprecated, c.JWTAudienceDeprecated) jwtBundleEmptyCount := countEmpty(c.SvidBundleFileName) - if x509EmptyCount == 3 && jwtSVIDEmptyCount == 2 && c.JwtSvids == nil && jwtBundleEmptyCount == 1 { + if x509EmptyCount == 3 && c.JwtSvids == nil && jwtBundleEmptyCount == 1 { return errors.New("at least one of the sets ('svid_file_name', 'svid_key_file_name', 'svid_bundle_file_name'), ('jwt_file_name', 'jwt_audience'), 'jwt_svids', or ('jwt_bundle_file_name') must be fully specified") } @@ -143,18 +140,6 @@ func ValidateConfig(c *Config) error { return errors.New("all or none of 'svid_file_name', 'svid_key_file_name', 'svid_bundle_file_name' must be specified") } - if jwtSVIDEmptyCount != 0 && jwtSVIDEmptyCount != 2 { - return errors.New("all or none of 'jwt_file_name', 'jwt_audience' must be specified") - } - - if jwtSVIDEmptyCount == 0 { - c.Log.Warn(getWarning("jwt_file_name and jwt_audience", "jwt_svids")) - } - - if jwtSVIDEmptyCount != 0 && c.JwtSvids == nil { - return errors.New("must not specify deprecated JWT configs ('jwt_file_name' and 'jwt_audience') and new JWT config ('jwt_svids')") - } - return nil } diff --git a/pkg/sidecar/config_test.go b/pkg/sidecar/config_test.go index 400e4103..03f17431 100644 --- a/pkg/sidecar/config_test.go +++ b/pkg/sidecar/config_test.go @@ -34,9 +34,9 @@ func TestParseConfig(t *testing.T) { assert.Equal(t, expectedSvidFileName, c.SvidFileName) assert.Equal(t, expectedKeyFileName, c.SvidKeyFileName) assert.Equal(t, expectedSvidBundleFileName, c.SvidBundleFileName) - assert.Equal(t, expectedJWTSVIDFileName, c.JWTSvidFilenameDeprecated) + assert.Equal(t, expectedJWTSVIDFileName, c.JwtSvids[0].JWTSvidFilename) assert.Equal(t, expectedJWTBundleFileName, c.JWTBundleFilename) - assert.Equal(t, expectedJWTAudience, c.JWTAudienceDeprecated) + assert.Equal(t, expectedJWTAudience, c.JwtSvids[0].JWTAudience) assert.True(t, c.AddIntermediatesToBundle) } @@ -56,21 +56,6 @@ func TestValidateConfig(t *testing.T) { SvidBundleFileName: "bundle.pem", }, }, - { - name: "warns on deprecated jwt configs", - config: &Config{ - AgentAddress: "path", - JWTAudienceDeprecated: "your-audience", - JWTSvidFilenameDeprecated: "jwt.token", - JWTBundleFilename: "bundle.json", - }, - expectLogs: []shortEntry{ - { - Level: logrus.WarnLevel, - Message: "jwt_file_name and jwt_audience will be deprecated, should be used as jwt_svids", - }, - }, - }, { name: "no error", config: &Config{ @@ -100,10 +85,12 @@ func TestValidateConfig(t *testing.T) { { name: "missing jwt config", config: &Config{ - AgentAddress: "path", - JWTSvidFilenameDeprecated: "cert.pem", + AgentAddress: "path", + JwtSvids: []JwtConfig{{ + JWTSvidFilename: "jwt.token", + }}, }, - expectError: "all or none of 'jwt_file_name', 'jwt_audience' must be specified", + expectError: "both 'jwt_file_name' and 'jwt_audience' are required in 'jwt_svids'", }, // Duplicated field error: { diff --git a/pkg/sidecar/sidecar.go b/pkg/sidecar/sidecar.go index 1370bddc..a59f8f8d 100644 --- a/pkg/sidecar/sidecar.go +++ b/pkg/sidecar/sidecar.go @@ -103,7 +103,7 @@ func (s *Sidecar) RunDaemon(ctx context.Context) error { }() } - if s.config.JWTSvidFilenameDeprecated != "" && s.config.JWTAudienceDeprecated != "" { + if s.config.JwtSvids != nil { jwtSource, err := workloadapi.NewJWTSource(ctx, workloadapi.WithClientOptions(s.getWorkloadAPIAdress())) if err != nil { s.config.Log.Fatalf("Error watching JWT svid updates: %v", err) @@ -111,20 +111,12 @@ func (s *Sidecar) RunDaemon(ctx context.Context) error { s.jwtSource = jwtSource defer s.jwtSource.Close() - if s.config.JwtSvids != nil { - for _, jwtConfig := range s.config.JwtSvids { - jwtConfig := jwtConfig - wg.Add(1) - go func() { - defer wg.Done() - s.updateJWTSVID(ctx, jwtConfig.JWTAudience, jwtConfig.JWTSvidFilename) - }() - } - } else { + for _, jwtConfig := range s.config.JwtSvids { + jwtConfig := jwtConfig wg.Add(1) go func() { defer wg.Done() - s.updateJWTSVID(ctx, s.config.JWTAudienceDeprecated, s.config.JWTSvidFilenameDeprecated) + s.updateJWTSVID(ctx, jwtConfig.JWTAudience, jwtConfig.JWTSvidFilename) }() } } diff --git a/test/fixture/config/helper.conf b/test/fixture/config/helper.conf index 0ab57b78..7e637aec 100644 --- a/test/fixture/config/helper.conf +++ b/test/fixture/config/helper.conf @@ -6,8 +6,12 @@ renew_signal = "SIGHUP" svid_file_name = "svid.pem" svid_key_file_name = "svid_key.pem" svid_bundle_file_name = "svid_bundle.pem" -jwt_svid_file_name = "jwt_svid.token" jwt_bundle_file_name = "jwt_bundle.json" -jwt_audience = "your-audience" +jwt_svids = [ + { + jwt_svid_file_name = "jwt_svid.token" + jwt_audience = "your-audience" + } +] timeout = "10s" add_intermediates_to_bundle = true From a4a00957acff6d008a4f28f8f5e8013469be2bd3 Mon Sep 17 00:00:00 2001 From: Keegan Witt Date: Wed, 20 Dec 2023 09:08:24 -0500 Subject: [PATCH 03/11] Update readme Signed-off-by: Keegan Witt --- README.md | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 08f3dc07..f26ce3e2 100644 --- a/README.md +++ b/README.md @@ -16,20 +16,19 @@ If `-config` is not specified, the default value `helper.conf` is assumed. ## Configuration The configuration file is an [HCL](https://github.com/hashicorp/hcl) formatted file that defines the following configurations: - | Configuration | Description | Example Value | - |-----------------------------|----------------------------------------------------------------------------------------------------------------| -------------------- | - |`agent_address` | Socket address of SPIRE Agent. | `"/tmp/agent.sock"` | - |`cmd` | The path to the process to launch. | `"ghostunnel"` | - |`cmd_args` | The arguments of the process to launch. | `"server --listen localhost:8002 --target localhost:8001--keystore certs/svid_key.pem --cacert certs/svid_bundle.pem --allow-uri-san spiffe://example.org/Database"` | - |`cert_dir` | Directory name to store the fetched certificates. This directory must be created previously. | `"certs"` | - |`add_intermediates_to_bundle`| Add intermediate certificates into Bundle file instead of SVID file. | `true` | - |`renew_signal` | The signal that the process to be launched expects to reload the certificates. It is not supported on Windows. | `"SIGUSR1"` | - |`svid_file_name` | File name to be used to store the X.509 SVID public certificate in PEM format. | `"svid.pem"` | - |`svid_key_file_name` | File name to be used to store the X.509 SVID private key and public certificate in PEM format. | `"svid_key.pem"` | - |`svid_bundle_file_name` | File name to be used to store the X.509 SVID Bundle in PEM format. | `"svid_bundle.pem"` | - |`jwt_audience` | JWT SVID audience. | `"your-audience"` | - |`jwt_svid_file_name` | File name to be used to store JWT SVID in Base64-encoded string. | `"jwt_svid.token"` | - |`jwt_bundle_file_name` | File name to be used to store JWT Bundle in JSON format. | `"jwt_bundle.json"` | + | Configuration | Description | Example Value | + |-------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------| + | `agent_address` | Socket address of SPIRE Agent. | `"/tmp/agent.sock"` | + | `cmd` | The path to the process to launch. | `"ghostunnel"` | + | `cmd_args` | The arguments of the process to launch. | `"server --listen localhost:8002 --target localhost:8001--keystore certs/svid_key.pem --cacert certs/svid_bundle.pem --allow-uri-san spiffe://example.org/Database"` | + | `cert_dir` | Directory name to store the fetched certificates. This directory must be created previously. | `"certs"` | + | `add_intermediates_to_bundle` | Add intermediate certificates into Bundle file instead of SVID file. | `true` | + | `renew_signal` | The signal that the process to be launched expects to reload the certificates. It is not supported on Windows. | `"SIGUSR1"` | + | `svid_file_name` | File name to be used to store the X.509 SVID public certificate in PEM format. | `"svid.pem"` | + | `svid_key_file_name` | File name to be used to store the X.509 SVID private key and public certificate in PEM format. | `"svid_key.pem"` | + | `svid_bundle_file_name` | File name to be used to store the X.509 SVID Bundle in PEM format. | `"svid_bundle.pem"` | + | `jwt_svids` | An array of objects containing `jwt_audience` (which is the JWT SVID audience) and `jwt_svid_file_name` (which is the file name to be used to store JWT SVID in Base64-encoded string). | `[{jwt_audience="your-audience", jwt_svid_file_name="jwt_svid.token"}]` | + | `jwt_bundle_file_name` | File name to be used to store JWT Bundle in JSON format. | `"jwt_bundle.json"` | ### Configuration example ``` From ec9df942334109a7c16d01cc07ad4aa309ac4d74 Mon Sep 17 00:00:00 2001 From: Keegan Witt Date: Wed, 20 Dec 2023 10:36:24 -0500 Subject: [PATCH 04/11] Delete removed parameters from error message Signed-off-by: Keegan Witt --- pkg/sidecar/config.go | 2 +- pkg/sidecar/config_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/sidecar/config.go b/pkg/sidecar/config.go index 2f88bb8d..e2d8e5a3 100644 --- a/pkg/sidecar/config.go +++ b/pkg/sidecar/config.go @@ -133,7 +133,7 @@ func ValidateConfig(c *Config) error { x509EmptyCount := countEmpty(c.SvidFileName, c.SvidBundleFileName, c.SvidKeyFileName) jwtBundleEmptyCount := countEmpty(c.SvidBundleFileName) if x509EmptyCount == 3 && c.JwtSvids == nil && jwtBundleEmptyCount == 1 { - return errors.New("at least one of the sets ('svid_file_name', 'svid_key_file_name', 'svid_bundle_file_name'), ('jwt_file_name', 'jwt_audience'), 'jwt_svids', or ('jwt_bundle_file_name') must be fully specified") + return errors.New("at least one of the sets ('svid_file_name', 'svid_key_file_name', 'svid_bundle_file_name'), 'jwt_svids', or 'jwt_bundle_file_name' must be fully specified") } if x509EmptyCount != 0 && x509EmptyCount != 3 { diff --git a/pkg/sidecar/config_test.go b/pkg/sidecar/config_test.go index 03f17431..a242da1d 100644 --- a/pkg/sidecar/config_test.go +++ b/pkg/sidecar/config_test.go @@ -72,7 +72,7 @@ func TestValidateConfig(t *testing.T) { config: &Config{ AgentAddress: "path", }, - expectError: "at least one of the sets ('svid_file_name', 'svid_key_file_name', 'svid_bundle_file_name'), ('jwt_file_name', 'jwt_audience'), 'jwt_svids', or ('jwt_bundle_file_name') must be fully specified", + expectError: "at least one of the sets ('svid_file_name', 'svid_key_file_name', 'svid_bundle_file_name'), 'jwt_svids', or 'jwt_bundle_file_name' must be fully specified", }, { name: "missing svid config", From 988d9e674dceb407ad35ac609a0244eac1565760 Mon Sep 17 00:00:00 2001 From: Keegan Witt Date: Wed, 20 Dec 2023 10:37:10 -0500 Subject: [PATCH 05/11] Replace nil check with len > 0 Co-authored-by: Marcos Yacob Signed-off-by: Keegan Witt --- pkg/sidecar/sidecar.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sidecar/sidecar.go b/pkg/sidecar/sidecar.go index a59f8f8d..24585132 100644 --- a/pkg/sidecar/sidecar.go +++ b/pkg/sidecar/sidecar.go @@ -103,7 +103,7 @@ func (s *Sidecar) RunDaemon(ctx context.Context) error { }() } - if s.config.JwtSvids != nil { + if len(s.config.JwtSvids) > 0 { jwtSource, err := workloadapi.NewJWTSource(ctx, workloadapi.WithClientOptions(s.getWorkloadAPIAdress())) if err != nil { s.config.Log.Fatalf("Error watching JWT svid updates: %v", err) From 07f3bf017d359ce00c9a5725e06ac8883fcaaeca Mon Sep 17 00:00:00 2001 From: Keegan Witt Date: Wed, 20 Dec 2023 10:38:14 -0500 Subject: [PATCH 06/11] Replace nil check with len > 0 Signed-off-by: Keegan Witt --- pkg/sidecar/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sidecar/config.go b/pkg/sidecar/config.go index e2d8e5a3..23a4b1db 100644 --- a/pkg/sidecar/config.go +++ b/pkg/sidecar/config.go @@ -132,7 +132,7 @@ func ValidateConfig(c *Config) error { x509EmptyCount := countEmpty(c.SvidFileName, c.SvidBundleFileName, c.SvidKeyFileName) jwtBundleEmptyCount := countEmpty(c.SvidBundleFileName) - if x509EmptyCount == 3 && c.JwtSvids == nil && jwtBundleEmptyCount == 1 { + if x509EmptyCount == 3 && len(c.JwtSvids) == 0 && jwtBundleEmptyCount == 1 { return errors.New("at least one of the sets ('svid_file_name', 'svid_key_file_name', 'svid_bundle_file_name'), 'jwt_svids', or 'jwt_bundle_file_name' must be fully specified") } From c679edf86c128f1e2629cdc09bcf9a9f5278caf7 Mon Sep 17 00:00:00 2001 From: Keegan Witt Date: Wed, 20 Dec 2023 10:43:32 -0500 Subject: [PATCH 07/11] Improve jwt_svids missing parameters error messages Signed-off-by: Keegan Witt --- pkg/sidecar/config.go | 7 +++++-- pkg/sidecar/config_test.go | 14 ++++++++++++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/pkg/sidecar/config.go b/pkg/sidecar/config.go index 23a4b1db..7e660144 100644 --- a/pkg/sidecar/config.go +++ b/pkg/sidecar/config.go @@ -125,8 +125,11 @@ func ValidateConfig(c *Config) error { } for _, jwtConfig := range c.JwtSvids { - if countEmpty(jwtConfig.JWTSvidFilename, jwtConfig.JWTAudience) > 0 { - return errors.New("both 'jwt_file_name' and 'jwt_audience' are required in 'jwt_svids'") + if countEmpty(jwtConfig.JWTSvidFilename) > 0 { + return errors.New("'jwt_file_name' is required in 'jwt_svids'") + } + if countEmpty(jwtConfig.JWTAudience) > 0 { + return errors.New("'jwt_audience' is required in 'jwt_svids'") } } diff --git a/pkg/sidecar/config_test.go b/pkg/sidecar/config_test.go index a242da1d..9163c77e 100644 --- a/pkg/sidecar/config_test.go +++ b/pkg/sidecar/config_test.go @@ -83,14 +83,24 @@ func TestValidateConfig(t *testing.T) { expectError: "all or none of 'svid_file_name', 'svid_key_file_name', 'svid_bundle_file_name' must be specified", }, { - name: "missing jwt config", + name: "missing jwt audience", config: &Config{ AgentAddress: "path", JwtSvids: []JwtConfig{{ JWTSvidFilename: "jwt.token", }}, }, - expectError: "both 'jwt_file_name' and 'jwt_audience' are required in 'jwt_svids'", + expectError: "'jwt_audience' is required in 'jwt_svids'", + }, + { + name: "missing jwt path", + config: &Config{ + AgentAddress: "path", + JwtSvids: []JwtConfig{{ + JWTAudience: "my-audience", + }}, + }, + expectError: "'jwt_file_name' is required in 'jwt_svids'", }, // Duplicated field error: { From c34436faf49942e3f97c9a2eaaa438ef9d16b352 Mon Sep 17 00:00:00 2001 From: Keegan Witt Date: Wed, 20 Dec 2023 17:06:08 -0500 Subject: [PATCH 08/11] Update README.md Co-authored-by: Faisal Memon Signed-off-by: Keegan Witt --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f26ce3e2..4b2aac1d 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ The configuration file is an [HCL](https://github.com/hashicorp/hcl) formatted f | `svid_file_name` | File name to be used to store the X.509 SVID public certificate in PEM format. | `"svid.pem"` | | `svid_key_file_name` | File name to be used to store the X.509 SVID private key and public certificate in PEM format. | `"svid_key.pem"` | | `svid_bundle_file_name` | File name to be used to store the X.509 SVID Bundle in PEM format. | `"svid_bundle.pem"` | - | `jwt_svids` | An array of objects containing `jwt_audience` (which is the JWT SVID audience) and `jwt_svid_file_name` (which is the file name to be used to store JWT SVID in Base64-encoded string). | `[{jwt_audience="your-audience", jwt_svid_file_name="jwt_svid.token"}]` | + | `jwt_svids` | An array with the audience and file name to store the JWT SVIDs. File is Base64-encoded string). | `[{jwt_audience="your-audience", jwt_svid_file_name="jwt_svid.token"}]` | | `jwt_bundle_file_name` | File name to be used to store JWT Bundle in JSON format. | `"jwt_bundle.json"` | ### Configuration example From 6a5399cdad86b3b0f8c95c6e39edb87f2bd817a0 Mon Sep 17 00:00:00 2001 From: Keegan Witt Date: Wed, 20 Dec 2023 17:13:47 -0500 Subject: [PATCH 09/11] Update pkg/sidecar/config.go Co-authored-by: Faisal Memon Signed-off-by: Keegan Witt --- pkg/sidecar/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sidecar/config.go b/pkg/sidecar/config.go index 7e660144..1e40344b 100644 --- a/pkg/sidecar/config.go +++ b/pkg/sidecar/config.go @@ -125,7 +125,7 @@ func ValidateConfig(c *Config) error { } for _, jwtConfig := range c.JwtSvids { - if countEmpty(jwtConfig.JWTSvidFilename) > 0 { + if jwtConfig.JWTSvidFilename == "" { return errors.New("'jwt_file_name' is required in 'jwt_svids'") } if countEmpty(jwtConfig.JWTAudience) > 0 { From a0af7b3e684df2dfd3af88add0830e9193f65415 Mon Sep 17 00:00:00 2001 From: Keegan Witt Date: Wed, 20 Dec 2023 17:13:55 -0500 Subject: [PATCH 10/11] Update pkg/sidecar/config.go Co-authored-by: Faisal Memon Signed-off-by: Keegan Witt --- pkg/sidecar/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sidecar/config.go b/pkg/sidecar/config.go index 1e40344b..0a9fc839 100644 --- a/pkg/sidecar/config.go +++ b/pkg/sidecar/config.go @@ -128,7 +128,7 @@ func ValidateConfig(c *Config) error { if jwtConfig.JWTSvidFilename == "" { return errors.New("'jwt_file_name' is required in 'jwt_svids'") } - if countEmpty(jwtConfig.JWTAudience) > 0 { + if jwtConfig.JWTAudience == "" { return errors.New("'jwt_audience' is required in 'jwt_svids'") } } From 558ea303dfade166316298553b09548cf942f790 Mon Sep 17 00:00:00 2001 From: Keegan Witt Date: Wed, 20 Dec 2023 18:35:22 -0500 Subject: [PATCH 11/11] Fix table spacing Signed-off-by: Keegan Witt --- README.md | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index eaa6660c..a7d5a924 100644 --- a/README.md +++ b/README.md @@ -16,19 +16,19 @@ If `-config` is not specified, the default value `helper.conf` is assumed. ## Configuration The configuration file is an [HCL](https://github.com/hashicorp/hcl) formatted file that defines the following configurations: - | Configuration | Description | Example Value | - |-------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------| - | `agent_address` | Socket address of SPIRE Agent. | `"/tmp/agent.sock"` | - | `cmd` | The path to the process to launch. | `"ghostunnel"` | - | `cmd_args` | The arguments of the process to launch. | `"server --listen localhost:8002 --target localhost:8001--keystore certs/svid_key.pem --cacert certs/svid_bundle.pem --allow-uri-san spiffe://example.org/Database"` | - | `cert_dir` | Directory name to store the fetched certificates. This directory must be created previously. | `"certs"` | - | `add_intermediates_to_bundle` | Add intermediate certificates into Bundle file instead of SVID file. | `true` | - | `renew_signal` | The signal that the process to be launched expects to reload the certificates. It is not supported on Windows. | `"SIGUSR1"` | - | `svid_file_name` | File name to be used to store the X.509 SVID public certificate in PEM format. | `"svid.pem"` | - | `svid_key_file_name` | File name to be used to store the X.509 SVID private key and public certificate in PEM format. | `"svid_key.pem"` | - | `svid_bundle_file_name` | File name to be used to store the X.509 SVID Bundle in PEM format. | `"svid_bundle.pem"` | - | `jwt_svids` | An array with the audience and file name to store the JWT SVIDs. File is Base64-encoded string). | `[{jwt_audience="your-audience", jwt_svid_file_name="jwt_svid.token"}]` | - | `jwt_bundle_file_name` | File name to be used to store JWT Bundle in JSON format. | `"jwt_bundle.json"` | + | Configuration | Description | Example Value | + |-------------------------------|----------------------------------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------| + | `agent_address` | Socket address of SPIRE Agent. | `"/tmp/agent.sock"` | + | `cmd` | The path to the process to launch. | `"ghostunnel"` | + | `cmd_args` | The arguments of the process to launch. | `"server --listen localhost:8002 --target localhost:8001--keystore certs/svid_key.pem --cacert certs/svid_bundle.pem --allow-uri-san spiffe://example.org/Database"` | + | `cert_dir` | Directory name to store the fetched certificates. This directory must be created previously. | `"certs"` | + | `add_intermediates_to_bundle` | Add intermediate certificates into Bundle file instead of SVID file. | `true` | + | `renew_signal` | The signal that the process to be launched expects to reload the certificates. It is not supported on Windows. | `"SIGUSR1"` | + | `svid_file_name` | File name to be used to store the X.509 SVID public certificate in PEM format. | `"svid.pem"` | + | `svid_key_file_name` | File name to be used to store the X.509 SVID private key and public certificate in PEM format. | `"svid_key.pem"` | + | `svid_bundle_file_name` | File name to be used to store the X.509 SVID Bundle in PEM format. | `"svid_bundle.pem"` | + | `jwt_svids` | An array with the audience and file name to store the JWT SVIDs. File is Base64-encoded string). | `[{jwt_audience="your-audience", jwt_svid_file_name="jwt_svid.token"}]` | + | `jwt_bundle_file_name` | File name to be used to store JWT Bundle in JSON format. | `"jwt_bundle.json"` | ### Configuration example ```