Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add both uint64 and int64 to tlscommon types unpack methods #198

Merged
merged 4 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions transport/tlscommon/server_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,3 +173,80 @@ func Test_ServerConfig_Repack(t *testing.T) {
})
}
}

func Test_ServerConfig_RepackJSON(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we encounter json serialization in the error we observed but i've added tests in case that is the issue.
I've also added Unpack unit tests to all the types

tests := []struct {
name string
json string
auth *TLSClientAuth
}{{
name: "with client auth",
json: `{
"enabled": true,
"verification_mode": "certificate",
"supported_protocols": ["TLSv1.1", "TLSv1.2"],
"cipher_suites": ["RSA-AES-256-CBC-SHA"],
"certificate_authorities": ["/path/to/ca.crt"],
"certificate": "/path/to/cert.crt",
"key": "/path/to/key.crt",
"curve_types": "P-521",
"renegotiation": "freely",
"ca_sha256": ["example"],
"ca_trusted_fingerprint": "fingerprint",
"client_authentication": "optional"
}`,
auth: &optional,
}, {
name: "nil client auth",
json: `{
"enabled": true,
"verification_mode": "certificate",
"supported_protocols": ["TLSv1.1", "TLSv1.2"],
"cipher_suites": ["RSA-AES-256-CBC-SHA"],
"certificate_authorities": ["/path/to/ca.crt"],
"certificate": "/path/to/cert.crt",
"key": "/path/to/key.crt",
"curve_types": "P-521",
"renegotiation": "freely",
"ca_sha256": ["example"],
"ca_trusted_fingerprint": "fingerprint"
}`,
auth: &required,
}, {
name: "nil client auth, no cas",
json: `{
"enabled": true,
"verification_mode": "certificate",
"supported_protocols": ["TLSv1.1", "TLSv1.2"],
"cipher_suites": ["RSA-AES-256-CBC-SHA"],
"certificate": "/path/to/cert.crt",
"key": "/path/to/key.crt",
"curve_types": "P-521",
"renegotiation": "freely",
"ca_sha256": ["example"]
}`,
auth: nil,
}}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
cfg := mustLoadServerConfigJSON(t, tc.json)
if tc.auth != nil {
require.Equal(t, *tc.auth, *cfg.ClientAuth)
} else {
require.Nil(t, cfg.ClientAuth)
}

tmp, err := ucfg.NewFrom(cfg)
require.NoError(t, err)

err = tmp.Unpack(&cfg)
require.NoError(t, err)
if tc.auth != nil {
require.Equal(t, *tc.auth, *cfg.ClientAuth)
} else {
require.Nil(t, cfg.ClientAuth)
}
})
}
}
48 changes: 48 additions & 0 deletions transport/tlscommon/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ import (
"github.com/stretchr/testify/require"

"github.com/elastic/elastic-agent-libs/config"

ucfg "github.com/elastic/go-ucfg"
"github.com/elastic/go-ucfg/json"
)

const (
Expand Down Expand Up @@ -76,6 +79,50 @@ func mustLoad(t *testing.T, yamlStr string) *Config {
return cfg
}

// copied from config.fromConfig
func cfgConvert(in *ucfg.Config) *config.C {
return (*config.C)(in)
}

func loadJSON(jsonStr string) (*Config, error) {
var cfg Config
uc, err := json.NewConfig([]byte(jsonStr), ucfg.PathSep("."), ucfg.VarExp)
if err != nil {
return nil, err
}

c := cfgConvert(uc)

if err = c.Unpack(&cfg); err != nil {
return nil, err
}
return &cfg, nil
}

func loadServerConfigJSON(jsonStr string) (*ServerConfig, error) {
var cfg ServerConfig
uc, err := json.NewConfig([]byte(jsonStr), ucfg.PathSep("."), ucfg.VarExp)
if err != nil {
return nil, err
}

c := cfgConvert(uc)

if err = c.Unpack(&cfg); err != nil {
return nil, err
}
return &cfg, nil
}

func mustLoadServerConfigJSON(t *testing.T, jsonStr string) *ServerConfig {
t.Helper()
cfg, err := loadServerConfigJSON(jsonStr)
if err != nil {
t.Fatal(err)
}
return cfg
}

func writeTestFile(t *testing.T, content string) string {
t.Helper()
f, err := os.CreateTemp(t.TempDir(), "")
Expand Down Expand Up @@ -647,6 +694,7 @@ mrPVWmOCMtwHJrO7kF1ENDgHPkhoZFcpFhu3lzOY7mhpW5mPZPVs87ZmI75G7zMV
AcV8KJqa/7XTTpvIzXePw9FtSSux5SkU6iKAKqwUt82D1E73bbppSg==
-----END CERTIFICATE-----
`
//nolint:gosec // testing key
key := `
-----BEGIN RSA PRIVATE KEY-----
Proc-Type: 4,ENCRYPTED
Expand Down
8 changes: 8 additions & 0 deletions transport/tlscommon/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,8 @@ func (m *TLSVerificationMode) Unpack(in interface{}) error {
return fmt.Errorf("unknown verification mode '%v'", o)
}
*m = mode
case int64:
*m = TLSVerificationMode(o)
case uint64:
*m = TLSVerificationMode(o)
default:
Expand Down Expand Up @@ -244,6 +246,8 @@ func (cs *CipherSuite) Unpack(i interface{}) error {
}

*cs = suite
case int64:
*cs = CipherSuite(o)
case uint64:
*cs = CipherSuite(o)
default:
Expand All @@ -270,6 +274,8 @@ func (ct *tlsCurveType) Unpack(i interface{}) error {
}

*ct = t
case int64:
*ct = tlsCurveType(o)
case uint64:
*ct = tlsCurveType(o)
default:
Expand All @@ -296,6 +302,8 @@ func (r *TLSRenegotiationSupport) Unpack(i interface{}) error {
}

*r = t
case int64:
*r = TLSRenegotiationSupport(o)
case uint64:
*r = TLSRenegotiationSupport(o)
default:
Expand Down
Loading
Loading