Skip to content

Allow to choose TLS ciphersuites #110

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.

Sign up for GitHub

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

Merged
summerwind merged 1 commit into from
Dec 27, 2019
+73 −1
Merged

Allow to choose TLS ciphersuites #110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions cmd/h2spec/h2spec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func main() {
flags.BoolP("strict", "S", false, "Run all test cases including strict test cases")
flags.Bool("dryrun", false, "Display only the title of test cases")
flags.BoolP("tls", "t", false, "Connect over TLS")
flags.StringP("ciphers", "c", "", "List of colon-separated TLS cipher names")
flags.BoolP("insecure", "k", false, "Don't verify server's certificate")
flags.BoolP("verbose", "v", false, "Output verbose log")
flags.Bool("version", false, "Display version information and exit")
Expand Down Expand Up @@ -106,6 +107,11 @@ func run(cmd *cobra.Command, args []string) error {
return err
}

ciphers, err := flags.GetString("ciphers")
if err != nil {
return err
}

insecure, err := flags.GetBool("insecure")
if err != nil {
return err
Expand Down Expand Up @@ -134,6 +140,7 @@ func run(cmd *cobra.Command, args []string) error {
Strict: strict,
DryRun: dryRun,
TLS: tls,
Ciphers: ciphers,
Insecure: insecure,
Verbose: verbose,
Sections: args,
Expand Down
67 changes: 66 additions & 1 deletion config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type Config struct {
Strict bool
DryRun bool
TLS bool
Ciphers string
Insecure bool
Verbose bool
Sections []string
Expand All @@ -34,7 +35,7 @@ type Config struct {
FromPort int
}

// Addr returns the string concatinated with hostname and port number.
// Addr returns the string concatenated with hostname and port number.
Copy link
Copy Markdown
Owner

@summerwind summerwind Dec 25, 2019

Choose a reason for hiding this comment

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

👍

func (c *Config) Addr() string {
return fmt.Sprintf("%s:%d", c.Host, c.Port)
}
Expand All @@ -47,6 +48,69 @@ func (c *Config) Scheme() string {
}
}

func CiphersuiteByName(name string) uint16 {
switch name {
case "TLS_RSA_WITH_RC4_128_SHA":
return tls.TLS_RSA_WITH_RC4_128_SHA
Copy link
Copy Markdown
Owner

@summerwind summerwind Dec 25, 2019

Choose a reason for hiding this comment

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

RC4 and 3DES are not recommended to use now. Do you have any good reason to support them?

Copy link
Copy Markdown
Contributor Author

@vankoven vankoven Dec 26, 2019

Choose a reason for hiding this comment

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

Thank you for review!

For some reason Golang didn't provide literal names for ciphersuites and it wasn't possible to get ciphersuite id by name. So I had to add a such convert function on my own. I've listed all TLS1.0-1.2 ciphersuites here without any security considerations just to keep the function more library-like. TLS 1.3 ciphersuites are not listed there since they are not configurable. That's why I've covered all ciphersuites listed in crtypto/tls module.

But I actually was surprised that master branch of Golang was updated after this PR was created. In upcoming 1.14 release they have added a CipherSuite() function, which provide the missing link between literal names and ids which can be used to get ciphersuite id by literal name (simillary to new cipherSuiteByID() function).

I can adapt PR to changes in upcoming 1.4 Golang release, but then I will need to bump go version in go.mod and the PR will have to be postponed until Golang 1.4 is released (it's in beta now).
Or I can leave the changes as is, then the code will work fine in both Go 1.2 and 1.4. Which variant do you prefer?

Copy link
Copy Markdown
Owner

@summerwind summerwind Dec 27, 2019

Choose a reason for hiding this comment

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

Thank you for describing!
It's OK to use your current code because Go 1.4 has not released yet.

case "TLS_RSA_WITH_3DES_EDE_CBC_SHA":
return tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA
case "TLS_RSA_WITH_AES_128_CBC_SHA":
return tls.TLS_RSA_WITH_AES_128_CBC_SHA
case "TLS_RSA_WITH_AES_128_CBC_SHA256":
return tls.TLS_RSA_WITH_AES_128_CBC_SHA256
case "TLS_RSA_WITH_AES_256_GCM_SHA384":
return tls.TLS_RSA_WITH_AES_256_GCM_SHA384
case "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA":
return tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA
case "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA":
return tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA
case "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA":
return tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA
case "TLS_ECDHE_RSA_WITH_RC4_128_SHA":
return tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA
case "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA":
return tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA
case "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA":
return tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA
case "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA":
return tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA
case "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256":
return tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256
case "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256":
return tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256
case "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256":
return tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
case "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256":
return tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
case "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384":
return tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
case "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384":
return tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
case "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305":
return tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305
case "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305":
return tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305
}
return 0
}

// Decode the user-defined list of allowed cipher suites from string
// representation.
// TODO: now Golang doesn't provide a way to convert ciphersuite name to ID,
// thus manual implementation is required.
func (c *Config) GetCiphersuites() []uint16 {
var ids []uint16

for _, name := range strings.Split(c.Ciphers, ":") {
id := CiphersuiteByName(name)
if id != 0 {
ids = append(ids, id)
}
}

return ids
}

// TLSConfig returns a tls.Config based on the configuration of h2spec.
func (c *Config) TLSConfig() (*tls.Config, error) {
if !c.TLS {
Expand All @@ -55,6 +119,7 @@ func (c *Config) TLSConfig() (*tls.Config, error) {

config := tls.Config{
InsecureSkipVerify: c.Insecure,
CipherSuites: c.GetCiphersuites(),
}

if config.NextProtos == nil {
Expand Down