Merge lp:~dimitern/juju-core/147-apiprovisioner-blank-env-secrets into lp:~go-bot/juju-core/trunk
Status: | Merged |
---|---|
Approved by: | Dimiter Naydenov |
Approved revision: | no longer in the source branch. |
Merged at revision: | 1892 |
Proposed branch: | lp:~dimitern/juju-core/147-apiprovisioner-blank-env-secrets |
Merge into: | lp:~go-bot/juju-core/trunk |
Prerequisite: | lp:~dimitern/juju-core/146-apiprovisioner-addresses |
Diff against target: |
66 lines (+37/-1) 2 files modified
state/apiserver/provisioner/provisioner.go (+20/-1) state/apiserver/provisioner/provisioner_test.go (+17/-0) |
To merge this branch: | bzr merge lp:~dimitern/juju-core/147-apiprovisioner-blank-env-secrets |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Dimiter Naydenov (community) | Approve | ||
Review via email:
|
Commit message
apiserver/
This changes the provisioner API EnvironConfig()
call to mask out any secret attributes before
returning the config to non-manager agents.
The environment manager agents still get everything.
This closes off a security issue with LXC provisioner
having access to the environment configuration and
its secrets via the API.
Live tested on EC2 successfully.
https:/
R=dimitern, jameinel
Description of the change
apiserver/
This changes the provisioner API EnvironConfig()
call to mask out any secret attributes before
returning the config to non-manager agents.
The environment manager agents still get everything.
This closes off a security issue with LXC provisioner
having access to the environment configuration and
its secrets via the API.
Live tested on EC2 successfully.
Reviewers: mp+187738_ code.launchpad. net,
Message:
Please take a look.
Description: provisioner: Mask out secret env attrs
apiserver/
This changes the provisioner API EnvironConfig()
call to mask out any secret attributes before
returning the config to non-manager agents.
The environment manager agents still get everything.
This closes off a security issue with LXC provisioner
having access to the environment configuration and
its secrets via the API.
Live tested on EC2 successfully.
https:/ /code.launchpad .net/~dimitern/ juju-core/ 147-apiprovisio ner-blank- env-secrets/ +merge/ 187738
Requires: /code.launchpad .net/~dimitern/ juju-core/ 146-apiprovisio ner-addresses/ +merge/ 187719
https:/
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/13964043/
Affected files (+38, -1 lines): /provisioner/ provisioner. go /provisioner/ provisioner_ test.go
A [revision details]
M state/apiserver
M state/apiserver
Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:
<email address hidden>
+New revision:
<email address hidden>
Index: state/apiserver /provisioner/ provisioner. go apiserver/ provisioner/ provisioner. go' /provisioner/ provisioner. go 2013-09-26 10:38:59 +0000 /provisioner/ provisioner. go 2013-09-26 11:15:31 +0000 AuthEnvironMana ger() { New(config) ).SecretAttrs( config)
=== modified file 'state/
--- state/apiserver
+++ state/apiserver
@@ -157,7 +157,25 @@
if err != nil {
return result, err
}
- result.Config = config.AllAttrs()
+ allAttrs := config.AllAttrs()
+ if !p.authorizer.
+ // Mask out any secrets in the environment configuration
+ // with values of the same type, so it'll pass validation.
+ //
+ // TODO(dimitern) 201309-26 bug #1231384
+ // This needs to change so we won't return anything to
+ // entities other than the environment manager, but the
+ // provisioner code should be refactored first.
+ env, err := environs.
+ if err != nil {
+ return result, err
+ }
+ secretAttrs, err := env.Provider(
+ for k := range secretAttrs {
+ allAttrs[k] = "not available"
+ }
+ }
+ result.Config = allAttrs
return result, nil
}
Index: state/apiserver /provisioner/ provisioner_ test.go apiserver/ provisioner/ provisioner_ test.go' /provisioner/ provisioner_ test.go 2013-09-26 10:38:59 /provisioner/ provisioner_ test.go 2013-09-26 11:15:31 result. Error, gc.IsNil) result. Config, gc.DeepEquals, Config( envConfig. AllAttrs( ))) MachineAgent = true Manager = false NewProvisionerA PI(s.State, s.resources,
=== modified file 'state/
--- state/apiserver
+0000
+++ state/apiserver
+0000
@@ -422,6 +422,23 @@
c.Assert(err, gc.IsNil)
c.Assert(
c.Assert(
params.
+
+ // Now test it with a non-environment manager and make sure
+ // the secret attributes are masked.
+ anAuthorizer := s.authorizer
+ anAuthorizer.
+ anAuthorizer.
+ aProvisioner, err := provisioner.
+ anAuthorizer)
+ c.Assert(err, gc.IsNil)
+
+ // We need to see the secret attributes masked out, and for
+ // the dummy pro...