controller: add official-dns-name and autocert-url attributes #6363

Merged
merged 1 commit into from Sep 30, 2016

Conversation

Projects
None yet
5 participants
Owner

rogpeppe commented Sep 30, 2016

This adds a controller configuration option to explicitly enable
autocert retrieval - a request at any other DNS name will use
the self-generated certificate. This means that we won't
be vulnerable to the DoS attack mentioned in https://godoc.org/golang.org/x/crypto/acme/autocert#Manager under the HostPolicy field.

As autocert generation doesn't work unless the controller is listening
on port 443, we make the bootstrap command warn if that's not the
case (but provide an override in case someone wants to do port
forwarding).

To QA, bootstrap a controller with:

juju bootstrap --debug --config 'autocert-dns-name=<your domain name>' --config 'api-port=443' ec2 aws

Then configure the DNS record for your domain name to point to the IP address of the newly
bootstrapped controller, and change the api-endpoints entries in controllers.yaml to refer to :443 and delete the ca-cert entry.

Then you should be able to connect OK to it.

👍

Looks good with some minor changes.

apiserver/apiserver.go
-func (srv *Server) newTLSConfig() *tls.Config {
+func (srv *Server) newTLSConfig(cfg ServerConfig) *tls.Config {
+ tlsConfig := utils.SecureTLSConfig()
+ logger.Infof("newTLSConfig, OfficialDNSName %q", cfg.OfficialDNSName)
apiserver/cert_test.go
@@ -13,6 +14,7 @@ import (
"github.com/juju/juju/apiserver/params"
"github.com/juju/juju/cert"
coretesting "github.com/juju/juju/testing"
+ "github.com/juju/loggo"
@frankban

frankban Sep 30, 2016

Member

This goes in the second import group.

@@ -70,3 +72,83 @@ func (s *certSuite) TestUpdateCert(c *gc.C) {
err = conn.Ping()
c.Assert(err, jc.ErrorIsNil)
}
+
+func (s *certSuite) TestAutocertFailure(c *gc.C) {
+ // We don't have a fake autocert server, but we can at lease
@frankban

frankban Sep 30, 2016

Member

s/at lease/at least/

+ _, err := tls.Dial("tcp", apiInfo.Addrs[0], &tls.Config{
+ ServerName: "somewhere.example",
+ })
+ // If we can't get an autocert certificate, so we'll fall back to the local certificate
+ _, err := tls.Dial("tcp", apiInfo.Addrs[0], &tls.Config{
+ ServerName: "somewhere.else",
+ })
+ // If we can't get an autocert certificate, so we'll fall back to the local certificate
+ _, err := tls.Dial("tcp", apiInfo.Addrs[0], &tls.Config{
+ ServerName: "somewhere.example",
+ })
+ // If we can't get an autocert certificate, so we'll fall back to the local certificate
+ })
+ // If we can't get an autocert certificate, so we'll fall back to the local certificate
+ // which isn't valid for connecting to somewhere.example.
+ c.Assert(err, gc.ErrorMatches, `x509: certificate is valid for \*, not somewhere.example`)
@frankban

frankban Sep 30, 2016

Member

Is this the correct error message in this case?

@rogpeppe

rogpeppe Sep 30, 2016

Owner

I believe so. Why don't you think it is?

cmd/juju/commands/bootstrap.go
@@ -556,6 +558,10 @@ func (c *bootstrapCommand) Run(ctx *cmd.Context) (resultErr error) {
if err != nil {
return errors.Annotate(err, "constructing controller config")
}
+ if controllerConfig.OfficialDNSName() != "" && controllerConfig.APIPort() != 443 && !c.ForceAPIPort {
+ return errors.Errorf("official-dns-name is set but it's not usually possible official certificates without api-port=443 config; use --force-api-port to override this if you plan on using a port forwarder")
@frankban

frankban Sep 30, 2016

Member

This sentence doesn't sound correct to me.

Owner

rogpeppe commented Sep 30, 2016

$$merge$$

Contributor

jujubot commented Sep 30, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented Sep 30, 2016

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/9391

Owner

rogpeppe commented Sep 30, 2016

$$merge$$

Contributor

jujubot commented Sep 30, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit efdcb16 into juju:master Sep 30, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment