Obey ssl-hostname-verification for provider-state

Obey ssl-hostname-verification for provider-state

When reading the provider-state file from cloud storage on
bootstrap, skip validating the https cert if the config
ssl-hostname-verification is set to false.

Change simplified to not test the valid cert case, avoiding
the need for a usable testing https server.

https://code.launchpad.net/~gz/juju-core/1.16_ssl_verification_bootstrap_state_1268913/+merge/203037

(do not edit description out of merge proposal)

[gz, 2014-01-27 18:11:55]

Please take a look.

[axw, 2014-01-29 07:53:19]

On 2014/01/27 18:11:55, gz wrote:
> Please take a look.

"On trunk, I don't see why ClientTLS shouldn't be switched
to just use https all the time."

I suppose we could just use HTTPS, but disable host key verification. The
certificate isn't known to clients other than management nodes/CLI.

[axw, 2014-01-29 08:12:59]

https://codereview.appspot.com/56560043/diff/20001/environs/httpstorage/stora...
File environs/httpstorage/storage.go (right):

https://codereview.appspot.com/56560043/diff/20001/environs/httpstorage/stora...
environs/httpstorage/storage.go:26: type PromotableStorage interface {
I'm not keen on this. I think a better way to test, rather than further exposing
the guts of this package, would be to use a net/http/httputil.ReverseProxy.

https://codereview.appspot.com/56560043/diff/20001/provider/common/state.go
File provider/common/state.go (right):

https://codereview.appspot.com/56560043/diff/20001/provider/common/state.go#n...
provider/common/state.go:71: func LoadStateFromURL(url string,
disableSSLHostnameVerification bool) (*BootstrapState, error) {
This is fine for now, but I wonder if we shouldn't have a method of obtaining an
http.Client for an environment. We now have HTTP proxy settings and SSL hostname
verification as options. We could centralise all that and pass an http.Client in
here.

[axw, 2014-01-29 09:13:38]

On 2014/01/29 07:53:19, axw wrote:
> On 2014/01/27 18:11:55, gz wrote:
> > Please take a look.
> 
> "On trunk, I don't see why ClientTLS shouldn't be switched
> to just use https all the time."
> 
> I suppose we could just use HTTPS, but disable host key verification. The
> certificate isn't known to clients other than management nodes/CLI.

William pointed out to me that actually, yes, we do have the (public) CA cert.
So we could indeed use HTTPS all the time. It would require us to somehow get
the cert into cloud-init so wget can use it when fetching tools, and to use it
when fetching from storage generally.

[gz, 2014-01-30 13:34:26]

Please take a look.

https://codereview.appspot.com/56560043/diff/20001/environs/httpstorage/stora...
File environs/httpstorage/storage.go (right):

https://codereview.appspot.com/56560043/diff/20001/environs/httpstorage/stora...
environs/httpstorage/storage.go:26: type PromotableStorage interface {
On 2014/01/29 08:12:59, axw wrote:
> I'm not keen on this. I think a better way to test, rather than further
exposing
> the guts of this package, would be to use a net/http/httputil.ReverseProxy.

Okay, I'll revert most of this and write something independant.

https://codereview.appspot.com/56560043/diff/20001/provider/common/state.go
File provider/common/state.go (right):

https://codereview.appspot.com/56560043/diff/20001/provider/common/state.go#n...
provider/common/state.go:71: func LoadStateFromURL(url string,
disableSSLHostnameVerification bool) (*BootstrapState, error) {
On 2014/01/29 08:12:59, axw wrote:
> This is fine for now, but I wonder if we shouldn't have a method of obtaining
an
> http.Client for an environment. We now have HTTP proxy settings and SSL
hostname
> verification as options. We could centralise all that and pass an http.Client
in
> here.

Yes, on trunk there should be some refactoring like that. There are a couple of
places that want a http.Client obeying thig setting and we may want more.

[gz, 2014-01-30 13:57:41]

Please take a look.

[axw, 2014-01-31 01:57:04]

On 2014/01/30 13:57:41, gz wrote:
> Please take a look.

Thanks, much nicer I think. LGTM.

[axw, 2014-01-31 01:57:27]

Oops, forgot to publish this.

https://codereview.appspot.com/56560043/diff/60001/provider/common/state_test.go
File provider/common/state_test.go (right):

https://codereview.appspot.com/56560043/diff/60001/provider/common/state_test...
provider/common/state_test.go:46: // testingHTTPServer creates a tempdir backed
https server with invalid certs
s/invalid/self-signed/?

[gz, 2014-01-31 12:33:34]

Please take a look.

https://codereview.appspot.com/56560043/diff/60001/provider/common/state_test.go
File provider/common/state_test.go (right):

https://codereview.appspot.com/56560043/diff/60001/provider/common/state_test...
provider/common/state_test.go:46: // testingHTTPServer creates a tempdir backed
https server with invalid certs
On 2014/01/31 01:57:27, axw wrote:
> s/invalid/self-signed/?

Good call. And really, it's just without certs right?

[axw, 2014-01-31 12:52:59]

On 2014/01/31 12:33:34, gz wrote:
> Please take a look.
> 
>
https://codereview.appspot.com/56560043/diff/60001/provider/common/state_test.go
> File provider/common/state_test.go (right):
> 
>
https://codereview.appspot.com/56560043/diff/60001/provider/common/state_test...
> provider/common/state_test.go:46: // testingHTTPServer creates a tempdir
backed
> https server with invalid certs
> On 2014/01/31 01:57:27, axw wrote:
> > s/invalid/self-signed/?
> 
> Good call. And really, it's just without certs right?

No, there's a certificate (there has to be), it's just self-signed. The SSL
certificate verification fails because the CA is unknown.

[gz, 2014-02-01 14:54:54]

On 2014/01/31 12:52:59, axw wrote:
> 
> No, there's a certificate (there has to be), it's just self-signed. The SSL
> certificate verification fails because the CA is unknown.

Hm, but we're not supplying it, so presumably the httptest server stuff has its
own one?

[axw, 2014-02-02 00:29:56]

On 2014/02/01 14:54:54, gz wrote:
> On 2014/01/31 12:52:59, axw wrote:
> > 
> > No, there's a certificate (there has to be), it's just self-signed. The SSL
> > certificate verification fails because the CA is unknown.
> 
> Hm, but we're not supplying it, so presumably the httptest server stuff has
its
> own one?

Yes: http://golang.org/src/pkg/net/http/httptest/server.go?s=3153:3180#L107