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.

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

Jump to bottom

Generating TLS rules with "go generate" fails #190

Closed
saintaardvark opened this issue Mar 9, 2018 · 5 comments
Closed

Generating TLS rules with "go generate" fails #190

saintaardvark opened this issue Mar 9, 2018 · 5 comments

Comments

@saintaardvark
Copy link

Summary

Following the directions in the README for generating up-to-date TLS rules do not seem to work for me:

  • If I follow the instructions, I see warnings during the generate step, and the build itself fails
  • If I remove rules/tls.go, then do go generate, then re-add rules/tls.go, I can build gas with up-to-date TLS rules

Steps to reproduce the behavior

I've confirmed this in a docker container (version 1.10 of go), which has a clean environment:

$ docker run -it golang:1 /bin/sh
# go version
go version go1.10 linux/amd64
# go get github.com/GoASTScanner/gas/cmd/tlsconfig/...
# cd /go/src/github.com/GoASTScanner/gas
# go generate ./...
2018/03/09 22:23:46 Warning: cannot map cipher 'ECDHE-ECDSA-CHACHA20-POLY1305'
2018/03/09 22:23:46 Warning: cannot map cipher 'ECDHE-RSA-CHACHA20-POLY1305'
2018/03/09 22:23:46 Warning: cannot map cipher 'ECDHE-ECDSA-CHACHA20-POLY1305'
2018/03/09 22:23:46 Warning: cannot map cipher 'ECDHE-RSA-CHACHA20-POLY1305'
2018/03/09 22:23:46 Warning: cannot map cipher 'ECDHE-ECDSA-CHACHA20-POLY1305'
2018/03/09 22:23:46 Warning: cannot map cipher 'ECDHE-RSA-CHACHA20-POLY1305'
2018/03/09 22:23:46 Warning: cannot map cipher 'DHE-RSA-CHACHA20-POLY1305'

If I try to force a build by removing the gas binary and re-running "go get", I get a slightly different error:

# rm /go/bin/gas
# go get github.com/GoASTScanner/gas/...
# github.com/GoASTScanner/gas/rules
rules/rulelist.go:85:52: cannot use NewIntermediateTLSCheck (type func(gas.Config) (gas.Rule, []ast.Node)) as type gas.RuleBuilder in field value
# /go/bin/gas
/bin/sh: 13: /go/bin/gas: not found

In order to successfully build gas with the generated rules, here's the procedure I have to follow:

go get -d github.com/GoASTScanner/gas/...
go get github.com/GoASTScanner/gas/cmd/tlsconfig/...
cd /go/src/github.com/GoASTScanner/gas
rm rules/tls.go
go generate ./...
git checkout rules/tls.go
go get github.com/GoASTScanner/gas/...

(Probably not the most efficient way...I'm a bit of a go newbie.)

Gas version

Current HEAD (7116c4d)

Go version (output of 'go version')

go version go1.10 linux/amd64

Operating system / Environment

  • Docker container (linux, amd64)

Expected behavior

Successful build of gas, with up-to-date TLS rules, following the instructions in the README

Actual behavior

Have to remove rules/tls.go, then do go generate, then re-add rules/tls.go in order to build gas with up-to-date TLS rules
@gcmurphy
Copy link
Member

The file the is generated is rules/tls_config.go, it contains the preferred ciphersuites. rules/tls.go contains the rule logic.

@cosmincojocar Are the warnings anything to be concerned about?

@ccojocar ccojocar mentioned this issue Mar 11, 2018
Merged
@ccojocar
Copy link
Member

@saintaardvark @gcmurphy There was a recent refactoring and it seems that the generated code was edited manually. I update the template to reflect the changes.

The warnings are ciphers published by Mozilla which are not supported in Go. I'll update the log message to be more specific.

@cji
Copy link

cji commented Mar 27, 2018

Some of the ciphers that tlsconfig are saying it cannot map do appear to be in crypto/tls. Is this an issue with the tls observatory code tlsconfig uses?

https://golang.org/src/crypto/tls/cipher_suites.go

Both ECDHE-ECDSA-CHACHA20-POLY1305 and ECDHE-RSA-CHACHA20-POLY1305 are included.

@gcmurphy gcmurphy reopened this Mar 27, 2018
@gcmurphy
Copy link
Member

@cosmincojocar can you comment on this?

@ccojocar ccojocar mentioned this issue Mar 28, 2018
Merged
@ccojocar
Copy link
Member

@cji It seems that the name mapping is missing from the tls observatory. Thanks for pointing this out. I created this PR mozilla/tls-observatory#319.

@ccojocar ccojocar mentioned this issue Apr 10, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@gcmurphy @cji @ccojocar @saintaardvark