Merge lp:~axwalk/juju-core/null-provider-storage-auth into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

Wire up authenticating httpstorage

There are two things happening here:
 - httpstorage authentication changes
 - wiring up to null provider/localstorage worker.

The httpstorage authentication mechanism has
changed to use an auth key (randomly generated
as part of the boilerplate), rather than client
certificates. This gives us more options for
using the storage, since now you don't need the
client to share the CA key.

Also, when serving HTTPS, HTTP is *also* provided,
but only for non-modifying operations. This allows
wget et al. to fetch files from the storage without
bypassing certificate validation. The HTTP interface
disallows modifying operations if there is an HTTPS
interface backing it. To get an HTTPS URL, a HEAD
request may be performed on the HTTP interface.

https://codereview.appspot.com/14011044/

Wire up authenticating httpstorage

There are two things happening here:
 - httpstorage authentication changes
 - wiring up to null provider/localstorage worker.

The httpstorage authentication mechanism has
changed to use an auth key (randomly generated
as part of the boilerplate), rather than client
certificates. This gives us more options for
using the storage, since now you don't need the
client to share the CA key.

Also, when serving HTTPS, HTTP is *also* provided,
but only for non-modifying operations. This allows
wget et al. to fetch files from the storage without
bypassing certificate validation. The HTTP interface
disallows modifying operations if there is an HTTPS
interface backing it. To get an HTTPS URL, a HEAD
request may be performed on the HTTP interface.

https://codereview.appspot.com/14011044/

[Andrew Wilkins (axwalk)]

Reviewers: mp+187964_code.launchpad.net,

Message:
Please take a look.

Description:
Wire up authenticating httpstorage

There are two things happening here:
  - httpstorage authentication changes
  - wiring up to null provider/localstorage worker.

The httpstorage authentication mechanism has
changed to use an auth key (randomly generated
as part of the boilerplate), rather than client
certificates. This gives us more options for
using the storage, since now you don't need the
client to share the CA key.

Also, when serving HTTPS, HTTP is *also* provided,
but only for non-modifying operations. This allows
wget et al. to fetch files from the storage without
bypassing certificate validation. The HTTP interface
disallows modifying operations if there is an HTTPS
interface backing it. To get an HTTPS URL, a HEAD
request may be performed on the HTTP interface.

https://code.launchpad.net/~axwalk/juju-core/null-provider-storage-auth/+merge/187964

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/14011044/

Affected files (+507, -108 lines):
   A [revision details]
   M environs/httpstorage/backend.go
   M environs/httpstorage/backend_test.go
   M environs/httpstorage/storage.go
   M environs/httpstorage/storage_test.go
   M environs/manual/bootstrap.go
   M provider/null/config.go
   M provider/null/config_test.go
   M provider/null/environ.go
   M provider/null/environ_test.go
   M provider/null/provider.go
   M worker/localstorage/config.go
   A worker/localstorage/config_test.go
   M worker/localstorage/worker.go

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/14011044/

[Tim Penhey (thumper)]

LGTM, few comments

https://codereview.appspot.com/14011044/diff/4001/environs/httpstorage/backend.go
File environs/httpstorage/backend.go (right):

https://codereview.appspot.com/14011044/diff/4001/environs/httpstorage/backend.go#newcode78
environs/httpstorage/backend.go:78: http.Error(w, "method HEAD is not
supported", http.StatusMethodNotAllowed)
shouldn't we return here? So as not to change the header to 200 OK?

https://codereview.appspot.com/14011044/diff/4001/provider/null/environ.go
File provider/null/environ.go (right):

https://codereview.appspot.com/14011044/diff/4001/provider/null/environ.go#newcode150
provider/null/environ.go:150: return
httpstorage.Client(e.envConfig().storageAddr())
If this is an error condition, shouldn't we log an error (which the
users now see) and return nil rather than http storage?

https://codereview.appspot.com/14011044/

[Andrew Wilkins (axwalk)]

https://codereview.appspot.com/14011044/diff/4001/environs/httpstorage/backend.go
File environs/httpstorage/backend.go (right):

https://codereview.appspot.com/14011044/diff/4001/environs/httpstorage/backend.go#newcode78
environs/httpstorage/backend.go:78: http.Error(w, "method HEAD is not
supported", http.StatusMethodNotAllowed)
On 2013/10/02 05:07:25, thumper wrote:
> shouldn't we return here? So as not to change the header to 200 OK?

It doesn't actually change in practice, as the first header wins. But
this was an oversight and I've fixed it. I added a test while I was at
it. Thanks.

https://codereview.appspot.com/14011044/diff/4001/provider/null/environ.go
File provider/null/environ.go (right):

https://codereview.appspot.com/14011044/diff/4001/provider/null/environ.go#newcode150
provider/null/environ.go:150: return
httpstorage.Client(e.envConfig().storageAddr())
On 2013/10/02 05:07:25, thumper wrote:
> If this is an error condition, shouldn't we log an error (which the
users now
> see) and return nil rather than http storage?

Done.

https://codereview.appspot.com/14011044/

=== modified file 'environs/httpstorage/backend.go'
--- environs/httpstorage/backend.go	2013-09-24 05:42:43 +0000
+++ environs/httpstorage/backend.go	2013-10-02 05:48:33 +0000
@@ -18,24 +18,39 @@
 	"launchpad.net/juju-core/environs/storage"
 )
 
-// TODO(axw) 2013-09-16 bug #1225916
-// Implement authentication for modifying storage.
-
 // storageBackend provides HTTP access to a storage object.
 type storageBackend struct {
 	backend storage.Storage
-	tls     bool
+
+	// httpsBaseURL is the base URL to send to clients
+	// if they perform a HEAD request.
+	httpsBaseURL string
+
+	// authkey is non-empty if modifying requests
+	// require an auth key.
+	authkey string
 }
 
 // ServeHTTP handles the HTTP requests to the container.
 func (s *storageBackend) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	switch req.Method {
+	case "PUT", "DELETE":
+		// Don't allow modifying operations if there's an HTTPS backend
+		// to handle that, and ensure the user is authorised/authenticated.
+		if s.httpsBaseURL != "" || !s.authorised(req) {
+			http.Error(w, "unauthorised access", http.StatusUnauthorized)
+			return
+		}
+	}
+	switch req.Method {
 	case "GET":
 		if strings.HasSuffix(req.URL.Path, "*") {
 			s.handleList(w, req)
 		} else {
 			s.handleGet(w, req)
 		}
+	case "HEAD":
+		s.handleHead(w, req)
 	case "PUT":
 		s.handlePut(w, req)
 	case "DELETE":
@@ -45,11 +60,25 @@
 	}
 }
 
-// authorised checks that either the storage does not require authorisation,
-// or the user has negotiated TLS with a valid certificate. Authentication
-// implies authorisation.
+// authorised checks that either the storage does not require
+// authorisation, or the user has specified the correct auth key.
 func (s *storageBackend) authorised(req *http.Request) bool {
-	return !s.tls || len(req.TLS.PeerCertificates) > 0
+	if s.authkey == "" {
+		return true
+	}
+	return req.URL.Query().Get("authkey") == s.authkey
+}
+
+// handleHead returns the HTTPS URL for the specified
+// path in the Location header.
+func (s *storageBackend) handleHead(w http.ResponseWriter, req *http.Request) {
+	if s.httpsBaseURL != "" {
+		w.Header().Set("Location", s.httpsBaseURL+req.URL.Path)
+	} else {
+		http.Error(w, "method HEAD is not supported", http.StatusMethodNotAllowed)
+		return
+	}
+	w.WriteHeader(http.StatusOK)
 }
 
 // handleGet returns a storage file to the client.
@@ -85,10 +114,6 @@
 
 // handlePut stores data from the client in the storage.
 func (s *storageBackend) handlePut(w http.ResponseWriter, req *http.Request) {
-	if !s.authorised(req) {
-		http.Error(w, "unauthorised access", http.StatusUnauthorized)
-		return
-	}
 	if req.ContentLength < 0 {
 		http.Error(w, "missing or invalid Content-Length header", http.StatusInternalServerError)
 		return
@@ -119,7 +144,7 @@
 // requests to the given storage implementation. It returns the network
 // listener. This can then be attached to with Client.
 func Serve(addr string, stor storage.Storage) (net.Listener, error) {
-	return serve(addr, stor, nil)
+	return serve(addr, stor, nil, "")
 }
 
 // ServeTLS runs a storage server on the given network address, relaying
@@ -130,7 +155,7 @@
 //
 // This method returns the network listener, which can then be attached
 // to with ClientTLS.
-func ServeTLS(addr string, stor storage.Storage, caCertPEM, caKeyPEM []byte, hostnames []string) (net.Listener, error) {
+func ServeTLS(addr string, stor storage.Storage, caCertPEM, caKeyPEM []byte, hostnames []string, authkey string) (net.Listener, error) {
 	expiry := time.Now().UTC().AddDate(10, 0, 0)
 	certPEM, keyPEM, err := cert.NewServer(caCertPEM, caKeyPEM, expiry, hostnames)
 	if err != nil {
@@ -150,21 +175,33 @@
 		ClientAuth:   tls.VerifyClientCertIfGiven,
 		ClientCAs:    caCerts,
 	}
-	return serve(addr, stor, config)
+	return serve(addr, stor, config, authkey)
 }
 
-func serve(addr string, stor storage.Storage, tlsConfig *tls.Config) (net.Listener, error) {
+func serve(addr string, stor storage.Storage, tlsConfig *tls.Config, authkey string) (net.Listener, error) {
 	listener, err := net.Listen("tcp", addr)
 	if err != nil {
 		return nil, fmt.Errorf("cannot start listener: %v", err)
 	}
 	backend := &storageBackend{backend: stor}
 	if tlsConfig != nil {
-		listener = tls.NewListener(listener, tlsConfig)
-		backend.tls = true
+		tlsBackend := &storageBackend{backend: stor, authkey: authkey}
+		tcpAddr := listener.Addr().(*net.TCPAddr)
+		tlsListener, err := tls.Listen("tcp", fmt.Sprintf("[%s]:0", tcpAddr.IP), tlsConfig)
+		if err != nil {
+			listener.Close()
+			return nil, fmt.Errorf("cannot start TLS listener: %v", err)
+		}
+		backend.httpsBaseURL = fmt.Sprintf("https://%s", tlsListener.Addr())
+		goServe(tlsListener, tlsBackend)
 	}
+	goServe(listener, backend)
+	return listener, nil
+}
+
+func goServe(listener net.Listener, backend *storageBackend) {
+	// Construct a NewServeMux to sanitise request paths.
 	mux := http.NewServeMux()
 	mux.Handle("/", backend)
 	go http.Serve(listener, mux)
-	return listener, nil
 }

=== modified file 'environs/httpstorage/backend_test.go'
--- environs/httpstorage/backend_test.go	2013-09-24 05:42:43 +0000
+++ environs/httpstorage/backend_test.go	2013-10-02 05:48:33 +0000
@@ -25,6 +25,8 @@
 	"launchpad.net/juju-core/testing/testbase"
 )
 
+const testAuthkey = "jabberwocky"
+
 func TestLocal(t *stdtesting.T) {
 	gc.TestingT(t)
 }
@@ -50,14 +52,16 @@
 // startServerTLS starts a new TLS-based local storage server
 // using a temporary directory and returns the listener,
 // a base URL for the server and the directory path.
-func startServerTLS(c *gc.C, caCertPEM, caKeyPEM []byte) (listener net.Listener, url, dataDir string) {
+func startServerTLS(c *gc.C) (listener net.Listener, url, dataDir string) {
 	dataDir = c.MkDir()
 	embedded, err := filestorage.NewFileStorageWriter(dataDir, filestorage.UseDefaultTmpDir)
 	c.Assert(err, gc.IsNil)
 	hostnames := []string{"127.0.0.1"}
-	listener, err = httpstorage.ServeTLS("127.0.0.1:0", embedded, caCertPEM, caKeyPEM, hostnames)
+	caCertPEM := []byte(coretesting.CACert)
+	caKeyPEM := []byte(coretesting.CAKey)
+	listener, err = httpstorage.ServeTLS("127.0.0.1:0", embedded, caCertPEM, caKeyPEM, hostnames, testAuthkey)
 	c.Assert(err, gc.IsNil)
-	return listener, fmt.Sprintf("https://%s/", listener.Addr()), dataDir
+	return listener, fmt.Sprintf("http://%s/", listener.Addr()), dataDir
 }
 
 type testCase struct {
@@ -130,6 +134,24 @@
 	},
 }
 
+func (s *backendSuite) TestHead(c *gc.C) {
+	// HEAD is unsupported for non-authenticating servers.
+	listener, url, _ := startServer(c)
+	defer listener.Close()
+	resp, err := http.Head(url)
+	c.Assert(err, gc.IsNil)
+	c.Assert(resp.StatusCode, gc.Equals, http.StatusMethodNotAllowed)
+
+	// HEAD on an authenticating server will return the HTTPS counterpart URL.
+	client, url, _ := s.tlsServerAndClient(c)
+	resp, err = client.Head(url + "arbitrary")
+	c.Assert(err, gc.IsNil)
+	c.Assert(resp.StatusCode, gc.Equals, http.StatusOK)
+	location, err := resp.Location()
+	c.Assert(err, gc.IsNil)
+	c.Assert(location.String(), gc.Matches, "https://127.0.0.1:[0-9]{5}/arbitrary")
+}
+
 func (s *backendSuite) TestGet(c *gc.C) {
 	// Test retrieving a file from a storage.
 	listener, url, dataDir := startServer(c)
@@ -373,12 +395,10 @@
 }
 
 func (b *backendSuite) tlsServerAndClient(c *gc.C) (client *http.Client, url, dataDir string) {
-	caCertPEM := []byte(coretesting.CACert)
-	caKeyPEM := []byte(coretesting.CAKey)
-	listener, url, dataDir := startServerTLS(c, caCertPEM, caKeyPEM)
+	listener, url, dataDir := startServerTLS(c)
 	b.AddCleanup(func(*gc.C) { listener.Close() })
 	caCerts := x509.NewCertPool()
-	c.Assert(caCerts.AppendCertsFromPEM(caCertPEM), jc.IsTrue)
+	c.Assert(caCerts.AppendCertsFromPEM([]byte(coretesting.CACert)), jc.IsTrue)
 	client = &http.Client{
 		Transport: &http.Transport{
 			TLSClientConfig: &tls.Config{RootCAs: caCerts},

=== modified file 'environs/httpstorage/storage.go'
--- environs/httpstorage/storage.go	2013-09-27 05:19:30 +0000
+++ environs/httpstorage/storage.go	2013-10-02 05:48:33 +0000
@@ -11,11 +11,11 @@
 	"io"
 	"io/ioutil"
 	"net/http"
+	"net/url"
 	"sort"
 	"strings"
-	"time"
+	"sync"
 
-	"launchpad.net/juju-core/cert"
 	"launchpad.net/juju-core/environs/storage"
 	coreerrors "launchpad.net/juju-core/errors"
 	"launchpad.net/juju-core/utils"
@@ -23,47 +23,60 @@
 
 // storage implements the storage.Storage interface.
 type localStorage struct {
-	baseURL string
-	client  *http.Client
+	addr   string
+	client *http.Client
+
+	authkey           string
+	httpsBaseURL      string
+	httpsBaseURLError error
+	httpsBaseURLOnce  sync.Once
 }
 
 // Client returns a storage object that will talk to the
 // storage server at the given network address (see Serve)
 func Client(addr string) storage.Storage {
 	return &localStorage{
-		baseURL: fmt.Sprintf("http://%s/", addr),
-		client:  http.DefaultClient,
+		addr:   addr,
+		client: http.DefaultClient,
 	}
 }
 
 // ClientTLS returns a storage object that will talk to the
 // storage server at the given network address (see Serve),
-// using TLS. The client will generate a client certificate,
-// signed with the given CA certificate/key, to authenticate.
-func ClientTLS(addr string, caCertPEM, caKeyPEM []byte) (storage.Storage, error) {
+// using TLS. The client is given an authentication key,
+// which the server will verify for Put and Remove* operations.
+func ClientTLS(addr string, caCertPEM []byte, authkey string) (storage.Storage, error) {
 	caCerts := x509.NewCertPool()
-	if !caCerts.AppendCertsFromPEM(caCertPEM) {
-		return nil, errors.New("error adding CA certificate to pool")
-	}
-	expiry := time.Now().UTC().AddDate(10, 0, 0)
-	clientCertPEM, clientKeyPEM, err := cert.NewClient(caCertPEM, caKeyPEM, expiry)
-	clientCert, err := tls.X509KeyPair(clientCertPEM, clientKeyPEM)
-	if err != nil {
-		return nil, err
+	if caCertPEM != nil {
+		if !caCerts.AppendCertsFromPEM(caCertPEM) {
+			return nil, errors.New("error adding CA certificate to pool")
+		}
 	}
 	return &localStorage{
-		baseURL: fmt.Sprintf("https://%s/", addr),
+		addr:    addr,
+		authkey: authkey,
 		client: &http.Client{
 			Transport: &http.Transport{
-				TLSClientConfig: &tls.Config{
-					Certificates: []tls.Certificate{clientCert},
-					RootCAs:      caCerts,
-				},
+				TLSClientConfig: &tls.Config{RootCAs: caCerts},
 			},
 		},
 	}, nil
 }
 
+func (s *localStorage) getHTTPSBaseURL() (string, error) {
+	url, _ := s.URL("/") // never fails
+	resp, err := s.client.Head(url)
+	if err != nil {
+		return "", err
+	}
+	resp.Body.Close()
+	httpsURL, err := resp.Location()
+	if err != nil {
+		return "", err
+	}
+	return httpsURL.String(), nil
+}
+
 // Get opens the given storage file and returns a ReadCloser
 // that can be used to read its contents. It is the caller's
 // responsibility to close it after use. If the name does not
@@ -119,9 +132,25 @@
 	return names, nil
 }
 
-// URL returns an URL that can be used to access the given storage file.
+// URL returns a URL that can be used to access the given storage file.
 func (s *localStorage) URL(name string) (string, error) {
-	return s.baseURL + name, nil
+	return fmt.Sprintf("http://%s/%s", s.addr, name), nil
+}
+
+// modURL returns a URL that can be used to modify the given storage file.
+func (s *localStorage) modURL(name string) (string, error) {
+	if s.client == http.DefaultClient {
+		return s.URL(name)
+	}
+	s.httpsBaseURLOnce.Do(func() {
+		s.httpsBaseURL, s.httpsBaseURLError = s.getHTTPSBaseURL()
+	})
+	if s.httpsBaseURLError != nil {
+		return "", s.httpsBaseURLError
+	}
+	v := url.Values{}
+	v.Set("authkey", s.authkey)
+	return fmt.Sprintf("%s%s?%s", s.httpsBaseURL, name, v.Encode()), nil
 }
 
 // ConsistencyStrategy is specified in the StorageReader interface.
@@ -137,7 +166,7 @@
 // Put reads from r and writes to the given storage file.
 // The length must be set to the total length of the file.
 func (s *localStorage) Put(name string, r io.Reader, length int64) error {
-	url, err := s.URL(name)
+	url, err := s.modURL(name)
 	if err != nil {
 		return err
 	}
@@ -168,7 +197,7 @@
 // storage. It should not return an error if the file does
 // not exist.
 func (s *localStorage) Remove(name string) error {
-	url, err := s.URL(name)
+	url, err := s.modURL(name)
 	if err != nil {
 		return err
 	}

=== modified file 'environs/httpstorage/storage_test.go'
--- environs/httpstorage/storage_test.go	2013-09-23 10:05:53 +0000
+++ environs/httpstorage/storage_test.go	2013-10-02 05:48:33 +0000
@@ -25,12 +25,9 @@
 var _ = gc.Suite(&storageSuite{})
 
 func (s *storageSuite) TestClientTLS(c *gc.C) {
-	caCertPEM := []byte(coretesting.CACert)
-	caKeyPEM := []byte(coretesting.CAKey)
-
-	listener, _, storageDir := startServerTLS(c, caCertPEM, caKeyPEM)
+	listener, _, storageDir := startServerTLS(c)
 	defer listener.Close()
-	stor, err := httpstorage.ClientTLS(listener.Addr().String(), caCertPEM, caKeyPEM)
+	stor, err := httpstorage.ClientTLS(listener.Addr().String(), []byte(coretesting.CACert), testAuthkey)
 	c.Assert(err, gc.IsNil)
 
 	data := []byte("hello")
@@ -41,13 +38,37 @@
 	c.Assert(names, gc.DeepEquals, []string{"filename"})
 	checkFileHasContents(c, stor, "filename", data)
 
-	// Now try Put, Remove and RemoveAll.
+	// Put, Remove and RemoveAll should all succeed.
 	checkPutFile(c, stor, "filenamethesecond", data)
 	checkFileHasContents(c, stor, "filenamethesecond", data)
 	c.Assert(stor.Remove("filenamethesecond"), gc.IsNil)
 	c.Assert(stor.RemoveAll(), gc.IsNil)
 }
 
+func (s *storageSuite) TestClientTLSInvalidAuth(c *gc.C) {
+	listener, _, storageDir := startServerTLS(c)
+	defer listener.Close()
+	const invalidAuthkey = testAuthkey + "!"
+	stor, err := httpstorage.ClientTLS(listener.Addr().String(), []byte(coretesting.CACert), invalidAuthkey)
+	c.Assert(err, gc.IsNil)
+
+	// Get and List should succeed.
+	data := []byte("hello")
+	err = ioutil.WriteFile(filepath.Join(storageDir, "filename"), data, 0644)
+	c.Assert(err, gc.IsNil)
+	names, err := storage.List(stor, "filename")
+	c.Assert(err, gc.IsNil)
+	c.Assert(names, gc.DeepEquals, []string{"filename"})
+	checkFileHasContents(c, stor, "filename", data)
+
+	// Put, Remove and RemoveAll should all fail.
+	const authErrorPattern = ".*401 Unauthorized"
+	err = putFile(c, stor, "filenamethesecond", data)
+	c.Assert(err, gc.ErrorMatches, authErrorPattern)
+	c.Assert(stor.Remove("filenamethesecond"), gc.ErrorMatches, authErrorPattern)
+	c.Assert(stor.RemoveAll(), gc.ErrorMatches, authErrorPattern)
+}
+
 func (s *storageSuite) TestList(c *gc.C) {
 	listener, _, _ := startServer(c)
 	defer listener.Close()
@@ -80,7 +101,6 @@
 	storage2 := httpstorage.Client(listener.Addr().String())
 	for _, name := range names {
 		checkFileHasContents(c, storage2, name, []byte(name))
-		checkFileURLHasContents(c, storage2, name, []byte(name))
 	}
 
 	// remove the first file and check that the others remain.
@@ -128,12 +148,17 @@
 	return nil
 }
 
-func checkPutFile(c *gc.C, stor storage.StorageWriter, name string, contents []byte) {
+func putFile(c *gc.C, stor storage.StorageWriter, name string, contents []byte) error {
 	c.Logf("check putting file %s ...", name)
 	reader := &readerWithClose{bytes.NewBuffer(contents), false}
 	err := stor.Put(name, reader, int64(len(contents)))
-	c.Assert(err, gc.IsNil)
 	c.Assert(reader.closeCalled, jc.IsFalse)
+	return err
+}
+
+func checkPutFile(c *gc.C, stor storage.StorageWriter, name string, contents []byte) {
+	err := putFile(c, stor, name, contents)
+	c.Assert(err, gc.IsNil)
 }
 
 func checkFileDoesNotExist(c *gc.C, stor storage.StorageReader, name string) {
@@ -147,19 +172,15 @@
 	c.Assert(err, gc.IsNil)
 	c.Check(r, gc.NotNil)
 	defer r.Close()
-
 	data, err := ioutil.ReadAll(r)
 	c.Check(err, gc.IsNil)
 	c.Check(data, gc.DeepEquals, contents)
-}
 
-func checkFileURLHasContents(c *gc.C, stor storage.StorageReader, name string, contents []byte) {
 	url, err := stor.URL(name)
 	c.Assert(err, gc.IsNil)
-
 	resp, err := http.Get(url)
 	c.Assert(err, gc.IsNil)
-	data, err := ioutil.ReadAll(resp.Body)
+	data, err = ioutil.ReadAll(resp.Body)
 	c.Assert(err, gc.IsNil)
 	defer resp.Body.Close()
 	c.Assert(resp.StatusCode, gc.Equals, http.StatusOK, gc.Commentf("error response: %s", data))

=== modified file 'environs/manual/bootstrap.go'
--- environs/manual/bootstrap.go	2013-10-02 05:18:02 +0000
+++ environs/manual/bootstrap.go	2013-10-02 05:48:33 +0000
@@ -7,7 +7,6 @@
 	"errors"
 	"fmt"
 
-	"launchpad.net/juju-core/agent"
 	"launchpad.net/juju-core/environs"
 	envtools "launchpad.net/juju-core/environs/tools"
 	"launchpad.net/juju-core/instance"
@@ -107,11 +106,9 @@
 	tools.URL = fmt.Sprintf("file://%s/%s", storageDir, toolsStorageName)
 
 	// Add the local storage configuration.
-	agentEnv := map[string]string{
-		agent.StorageAddr:       args.Environ.StorageAddr(),
-		agent.StorageDir:        storageDir,
-		agent.SharedStorageAddr: args.Environ.SharedStorageAddr(),
-		agent.SharedStorageDir:  args.Environ.SharedStorageDir(),
+	agentEnv, err := localstorage.StoreConfig(args.Environ)
+	if err != nil {
+		return err
 	}
 
 	// Finally, provision the machine agent.

=== modified file 'provider/null/config.go'
--- provider/null/config.go	2013-09-24 02:27:12 +0000
+++ provider/null/config.go	2013-10-02 05:48:33 +0000
@@ -16,11 +16,13 @@
 		"bootstrap-user":    schema.String(),
 		"storage-listen-ip": schema.String(),
 		"storage-port":      schema.Int(),
+		"storage-auth-key":  schema.String(),
 	}
 	configDefaults = schema.Defaults{
 		"bootstrap-user":    "",
 		"storage-listen-ip": "",
 		"storage-port":      8040,
+		"storage-auth-key":  schema.Omit,
 	}
 )
 
@@ -57,6 +59,10 @@
 	return int(c.attrs["storage-port"].(int64))
 }
 
+func (c *environConfig) storageAuthKey() string {
+	return c.attrs["storage-auth-key"].(string)
+}
+
 // storageAddr returns an address for connecting to the
 // bootstrap machine's localstorage.
 func (c *environConfig) storageAddr() string {

=== modified file 'provider/null/config_test.go'
--- provider/null/config_test.go	2013-09-24 03:02:54 +0000
+++ provider/null/config_test.go	2013-10-02 05:48:33 +0000
@@ -28,9 +28,10 @@
 
 func minimalConfigValues() map[string]interface{} {
 	return map[string]interface{}{
-		"name":           "test",
-		"type":           provider.Null,
-		"bootstrap-host": "hostname",
+		"name":             "test",
+		"type":             provider.Null,
+		"bootstrap-host":   "hostname",
+		"storage-auth-key": "whatever",
 		// While the ca-cert bits aren't entirely minimal, they avoid the need
 		// to set up a fake home.
 		"ca-cert":        coretesting.CACert,

=== modified file 'provider/null/environ.go'
--- provider/null/environ.go	2013-10-02 05:18:02 +0000
+++ provider/null/environ.go	2013-10-02 05:48:33 +0000
@@ -5,9 +5,12 @@
 
 import (
 	"errors"
+	"net"
 	"path"
 	"sync"
 
+	"launchpad.net/loggo"
+
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/cloudinit"
@@ -23,6 +26,7 @@
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
 	"launchpad.net/juju-core/tools"
+	"launchpad.net/juju-core/worker/localstorage"
 )
 
 const (
@@ -39,6 +43,8 @@
 	storageTmpSubdir = "storage-tmp"
 )
 
+var logger = loggo.GetLogger("juju.provider.null")
+
 type nullEnviron struct {
 	cfg                   *environConfig
 	cfgmutex              sync.Mutex
@@ -160,7 +166,20 @@
 	if e.bootstrapStorage != nil {
 		return e.bootstrapStorage
 	}
-	return httpstorage.Client(e.envConfig().storageAddr())
+	caCertPEM, caKeyPEM := e.StorageCACert(), e.StorageCAKey()
+	if caCertPEM != nil && caKeyPEM != nil {
+		authkey := e.StorageAuthKey()
+		storage, err := httpstorage.ClientTLS(e.envConfig().storageAddr(), caCertPEM, authkey)
+		if err != nil {
+			// Should be impossible, since ca-cert will always be validated.
+			logger.Errorf("initialising HTTPS storage failed: %v", err)
+		} else {
+			return storage
+		}
+	} else {
+		logger.Errorf("missing CA cert or private key")
+	}
+	return nil
 }
 
 func (e *nullEnviron) PublicStorage() storage.StorageReader {
@@ -202,3 +221,34 @@
 func (e *nullEnviron) SharedStorageDir() string {
 	return ""
 }
+
+func (e *nullEnviron) StorageCACert() []byte {
+	if bytes, ok := e.envConfig().CACert(); ok {
+		return bytes
+	}
+	return nil
+}
+
+func (e *nullEnviron) StorageCAKey() []byte {
+	if bytes, ok := e.envConfig().CAPrivateKey(); ok {
+		return bytes
+	}
+	return nil
+}
+
+func (e *nullEnviron) StorageHostnames() []string {
+	cfg := e.envConfig()
+	hostnames := []string{cfg.bootstrapHost()}
+	if ip := net.ParseIP(cfg.storageListenIPAddress()); ip != nil {
+		if !ip.IsUnspecified() {
+			hostnames = append(hostnames, ip.String())
+		}
+	}
+	return hostnames
+}
+
+func (e *nullEnviron) StorageAuthKey() string {
+	return e.envConfig().storageAuthKey()
+}
+
+var _ localstorage.LocalTLSStorageConfig = (*nullEnviron)(nil)

=== modified file 'provider/null/provider.go'
--- provider/null/provider.go	2013-09-26 11:21:52 +0000
+++ provider/null/provider.go	2013-10-02 05:48:33 +0000
@@ -99,12 +99,19 @@
         ## network interfaces.
         # storage-listen-ip:
         # storage-port: 8040
+        storage-auth-key: {{rand}}
 
 `
 }
 
-func (_ nullProvider) SecretAttrs(cfg *config.Config) (map[string]string, error) {
-	return make(map[string]string), nil
+func (p nullProvider) SecretAttrs(cfg *config.Config) (map[string]string, error) {
+	envConfig, err := p.validate(cfg, nil)
+	if err != nil {
+		return nil, err
+	}
+	attrs := make(map[string]string)
+	attrs["storage-auth-key"] = envConfig.storageAuthKey()
+	return attrs, nil
 }
 
 func (_ nullProvider) PublicAddress() (string, error) {

=== modified file 'worker/localstorage/config.go'
--- worker/localstorage/config.go	2013-09-16 02:03:12 +0000
+++ worker/localstorage/config.go	2013-10-02 05:48:33 +0000
@@ -3,6 +3,27 @@
 
 package localstorage
 
+import (
+	"launchpad.net/goyaml"
+
+	"launchpad.net/juju-core/agent"
+)
+
+const (
+	// TODO(axw) 2013-09-25 bug #1230131
+	// Move these variables out of agent when we can do upgrades in
+	// the right place. In this case, the local provider should do
+	// the envvar-to-agent.conf migration.
+	StorageDir        = agent.StorageDir
+	StorageAddr       = agent.StorageAddr
+	SharedStorageDir  = agent.SharedStorageDir
+	SharedStorageAddr = agent.SharedStorageAddr
+	StorageCACert     = "StorageCACert"
+	StorageCAKey      = "StorageCAKey"
+	StorageHostnames  = "StorageHostnames"
+	StorageAuthKey    = "StorageAuthKey"
+)
+
 // LocalStorageConfig is an interface that, if implemented, may be used
 // to configure a machine agent for use with the localstorage worker in
 // this package.
@@ -12,3 +33,94 @@
 	SharedStorageDir() string
 	SharedStorageAddr() string
 }
+
+// LocalTLSStorageConfig is an interface that extends LocalStorageConfig
+// to support serving storage over TLS.
+type LocalTLSStorageConfig interface {
+	LocalStorageConfig
+
+	// StorageCACert is the CA certificate in PEM format.
+	StorageCACert() []byte
+
+	// StorageCAKey is the CA private key in PEM format.
+	StorageCAKey() []byte
+
+	// StorageHostnames is the set of hostnames that will
+	// be assigned to the storage server's certificate.
+	StorageHostnames() []string
+
+	// StorageAuthKey is the key that clients must present
+	// to perform modifying operations.
+	StorageAuthKey() string
+}
+
+type config struct {
+	storageDir        string
+	storageAddr       string
+	sharedStorageDir  string
+	sharedStorageAddr string
+	caCertPEM         []byte
+	caKeyPEM          []byte
+	hostnames         []string
+	authkey           string
+}
+
+// StoreConfig takes a LocalStorageConfig (or derivative interface),
+// and stores it in a map[string]string suitable for updating an
+// agent.Config's key/value map.
+func StoreConfig(storageConfig LocalStorageConfig) (map[string]string, error) {
+	kv := make(map[string]string)
+	kv[StorageDir] = storageConfig.StorageDir()
+	kv[StorageAddr] = storageConfig.StorageAddr()
+	kv[SharedStorageDir] = storageConfig.SharedStorageDir()
+	kv[SharedStorageAddr] = storageConfig.SharedStorageAddr()
+	if tlsConfig, ok := storageConfig.(LocalTLSStorageConfig); ok {
+		if authkey := tlsConfig.StorageAuthKey(); authkey != "" {
+			kv[StorageAuthKey] = authkey
+		}
+		if cert := tlsConfig.StorageCACert(); cert != nil {
+			kv[StorageCACert] = string(cert)
+		}
+		if key := tlsConfig.StorageCAKey(); key != nil {
+			kv[StorageCAKey] = string(key)
+		}
+		if hostnames := tlsConfig.StorageHostnames(); len(hostnames) > 0 {
+			data, err := goyaml.Marshal(hostnames)
+			if err != nil {
+				return nil, err
+			}
+			kv[StorageHostnames] = string(data)
+		}
+	}
+	return kv, nil
+}
+
+func loadConfig(agentConfig agent.Config) (*config, error) {
+	config := &config{
+		storageDir:        agentConfig.Value(StorageDir),
+		storageAddr:       agentConfig.Value(StorageAddr),
+		sharedStorageDir:  agentConfig.Value(SharedStorageDir),
+		sharedStorageAddr: agentConfig.Value(SharedStorageAddr),
+		authkey:           agentConfig.Value(StorageAuthKey),
+	}
+
+	caCertPEM := agentConfig.Value(StorageCACert)
+	if len(caCertPEM) > 0 {
+		config.caCertPEM = []byte(caCertPEM)
+	}
+
+	caKeyPEM := agentConfig.Value(StorageCAKey)
+	if len(caKeyPEM) > 0 {
+		config.caKeyPEM = []byte(caKeyPEM)
+	}
+
+	hostnames := agentConfig.Value(StorageHostnames)
+	if len(hostnames) > 0 {
+		err := goyaml.Unmarshal([]byte(hostnames), &config.hostnames)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	return config, nil
+}

=== added file 'worker/localstorage/config_test.go'
--- worker/localstorage/config_test.go	1970-01-01 00:00:00 +0000
+++ worker/localstorage/config_test.go	2013-10-02 05:48:33 +0000
@@ -0,0 +1,132 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package localstorage_test
+
+import (
+	stdtesting "testing"
+
+	gc "launchpad.net/gocheck"
+	"launchpad.net/goyaml"
+
+	"launchpad.net/juju-core/worker/localstorage"
+)
+
+type configSuite struct{}
+
+var _ = gc.Suite(&configSuite{})
+
+func TestPackage(t *stdtesting.T) {
+	gc.TestingT(t)
+}
+
+type localStorageConfig struct {
+	storageDir        string
+	storageAddr       string
+	sharedStorageDir  string
+	sharedStorageAddr string
+}
+
+func (c *localStorageConfig) StorageDir() string {
+	return c.storageDir
+}
+
+func (c *localStorageConfig) StorageAddr() string {
+	return c.storageAddr
+}
+
+func (c *localStorageConfig) SharedStorageDir() string {
+	return c.sharedStorageDir
+}
+
+func (c *localStorageConfig) SharedStorageAddr() string {
+	return c.sharedStorageAddr
+}
+
+type localTLSStorageConfig struct {
+	localStorageConfig
+	caCertPEM []byte
+	caKeyPEM  []byte
+	hostnames []string
+	authkey   string
+}
+
+func (c *localTLSStorageConfig) StorageCACert() []byte {
+	return c.caCertPEM
+}
+
+func (c *localTLSStorageConfig) StorageCAKey() []byte {
+	return c.caKeyPEM
+}
+
+func (c *localTLSStorageConfig) StorageHostnames() []string {
+	return c.hostnames
+}
+
+func (c *localTLSStorageConfig) StorageAuthKey() string {
+	return c.authkey
+}
+
+func (*configSuite) TestStoreConfig(c *gc.C) {
+	var config localStorageConfig
+	m, err := localstorage.StoreConfig(&config)
+	c.Assert(err, gc.IsNil)
+	c.Assert(m, gc.DeepEquals, map[string]string{
+		localstorage.StorageDir:        "",
+		localstorage.StorageAddr:       "",
+		localstorage.SharedStorageDir:  "",
+		localstorage.SharedStorageAddr: "",
+	})
+
+	config.storageDir = "a"
+	config.storageAddr = "b"
+	config.sharedStorageDir = "c"
+	config.sharedStorageAddr = "d"
+	m, err = localstorage.StoreConfig(&config)
+	c.Assert(err, gc.IsNil)
+	c.Assert(m, gc.DeepEquals, map[string]string{
+		localstorage.StorageDir:        config.storageDir,
+		localstorage.StorageAddr:       config.storageAddr,
+		localstorage.SharedStorageDir:  config.sharedStorageDir,
+		localstorage.SharedStorageAddr: config.sharedStorageAddr,
+	})
+}
+
+func (*configSuite) TestStoreConfigTLS(c *gc.C) {
+	var config localTLSStorageConfig
+	m, err := localstorage.StoreConfig(&config)
+	c.Assert(err, gc.IsNil)
+	c.Assert(m, gc.DeepEquals, map[string]string{
+		localstorage.StorageDir:        "",
+		localstorage.StorageAddr:       "",
+		localstorage.SharedStorageDir:  "",
+		localstorage.SharedStorageAddr: "",
+	})
+
+	config.storageDir = "a"
+	config.storageAddr = "b"
+	config.sharedStorageDir = "c"
+	config.sharedStorageAddr = "d"
+	config.caCertPEM = []byte("heyhey")
+	config.caKeyPEM = []byte("hoho")
+	config.hostnames = []string{"easy", "as", "1.2.3"}
+	config.authkey = "password"
+	m, err = localstorage.StoreConfig(&config)
+	c.Assert(err, gc.IsNil)
+	c.Assert(m, gc.DeepEquals, map[string]string{
+		localstorage.StorageDir:        config.storageDir,
+		localstorage.StorageAddr:       config.storageAddr,
+		localstorage.SharedStorageDir:  config.sharedStorageDir,
+		localstorage.SharedStorageAddr: config.sharedStorageAddr,
+		localstorage.StorageCACert:     string(config.caCertPEM),
+		localstorage.StorageCAKey:      string(config.caKeyPEM),
+		localstorage.StorageHostnames:  mustMarshalYAML(c, config.hostnames),
+		localstorage.StorageAuthKey:    config.authkey,
+	})
+}
+
+func mustMarshalYAML(c *gc.C, v interface{}) string {
+	data, err := goyaml.Marshal(v)
+	c.Assert(err, gc.IsNil)
+	return string(data)
+}

=== modified file 'worker/localstorage/worker.go'
--- worker/localstorage/worker.go	2013-09-20 04:41:12 +0000
+++ worker/localstorage/worker.go	2013-10-02 05:48:33 +0000
@@ -4,6 +4,8 @@
 package localstorage
 
 import (
+	"net"
+
 	"launchpad.net/loggo"
 	"launchpad.net/tomb"
 
@@ -39,40 +41,53 @@
 	return s.tomb.Wait()
 }
 
+func (s *storageWorker) serveStorage(storageAddr, storageDir string, config *config) (net.Listener, error) {
+	authenticated := len(config.caCertPEM) > 0 && len(config.caKeyPEM) > 0
+	scheme := "http://"
+	if authenticated {
+		scheme = "https://"
+	}
+	logger.Infof("serving storage from %s to %s%s", storageDir, scheme, storageAddr)
+	storage, err := filestorage.NewFileStorageWriter(storageDir, filestorage.UseDefaultTmpDir)
+	if err != nil {
+		return nil, err
+	}
+	if authenticated {
+		return httpstorage.ServeTLS(
+			storageAddr,
+			storage,
+			config.caCertPEM,
+			config.caKeyPEM,
+			config.hostnames,
+			config.authkey,
+		)
+	}
+	return httpstorage.Serve(storageAddr, storage)
+}
+
 func (s *storageWorker) waitForDeath() error {
-	storageDir := s.config.Value(agent.StorageDir)
-	storageAddr := s.config.Value(agent.StorageAddr)
-	logger.Infof("serving %s on %s", storageDir, storageAddr)
+	config, err := loadConfig(s.config)
+	if err != nil {
+		logger.Errorf("error loading config: %v", err)
+		return err
+	}
 
-	storage, err := filestorage.NewFileStorageWriter(storageDir, filestorage.UseDefaultTmpDir)
-	if err != nil {
-		logger.Errorf("error with local storage: %v", err)
-		return err
-	}
-	storageListener, err := httpstorage.Serve(storageAddr, storage)
+	storageListener, err := s.serveStorage(config.storageAddr, config.storageDir, config)
 	if err != nil {
 		logger.Errorf("error with local storage: %v", err)
 		return err
 	}
 	defer storageListener.Close()
 
-	sharedStorageDir := s.config.Value(agent.SharedStorageDir)
-	sharedStorageAddr := s.config.Value(agent.SharedStorageAddr)
-	if sharedStorageAddr != "" && sharedStorageDir != "" {
-		logger.Infof("serving %s on %s", sharedStorageDir, sharedStorageAddr)
-		sharedStorage, err := filestorage.NewFileStorageWriter(sharedStorageDir, filestorage.UseDefaultTmpDir)
-		if err != nil {
-			logger.Errorf("error with local storage: %v", err)
-			return err
-		}
-		sharedStorageListener, err := httpstorage.Serve(sharedStorageAddr, sharedStorage)
+	if config.sharedStorageAddr != "" && config.sharedStorageDir != "" {
+		sharedStorageListener, err := s.serveStorage(config.sharedStorageAddr, config.sharedStorageDir, config)
 		if err != nil {
 			logger.Errorf("error with local storage: %v", err)
 			return err
 		}
 		defer sharedStorageListener.Close()
 	} else {
-		logger.Infof("no shared storage: dir=%q addr=%q", sharedStorageDir, sharedStorageAddr)
+		logger.Infof("no shared storage: dir=%q addr=%q", config.sharedStorageDir, config.sharedStorageAddr)
 	}
 
 	logger.Infof("storage routines started, awaiting death")