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.

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

Secret Management #27794

Merged
merged 37 commits into from
Nov 9, 2016
Merged

Secret Management #27794

merged 37 commits into from
Nov 9, 2016

Conversation

ehazlett
Copy link
Contributor

@ehazlett ehazlett commented Oct 26, 2016

This adds support for the new Secret API in Swarmkit (moby/swarmkit#1567). This adds commands (create, inspect, ls, rm) for managing secrets in a Swarm cluster. This is currently for Swarm mode only as the backing store is Swarm and as such is only for Linux. This is the foundation for future secret support in Docker with potential improvements such as Windows support, different backing stores, etc.

This takes the ideas and collective recommendations from #13490 to set the groundwork. It exposes a file system based interface for secret access. The engine creates a tmpfs and mounts it in container (/run/secrets). A file is created for each secret that is assigned to the service. For example, if a secret named "ssh-dev" with a target of "id_rsa" is assigned to the service, a file with the secret contents will be created at /run/secrets/id_rsa.

TODO:

Examples (these will be turned into user docs):

Create a Secret:

$> cat test_key | docker secret create ssh-dev
qluh2o4xbvtu8rhvz0hueyeou

Create a Service with a Secret

$> docker service create --name nginx --secret ssh-dev nginx
t9kwy2v04l7zl3hl9dlvt5vq0

We can then check the created container for the secret:

$> docker exec -ti 06 mount | grep tmpfs
...
tmpfs on /run/secrets type tmpfs (ro,relatime)

$> docker exec -ti 06 cat /run/secrets/id_rsa
-----BEGIN RSA PRIVATE KEY-----
MIIEpAIBAAKCAQEA2Xz+yHs5pDgeCyv7ul10MkSYcAu79sRBh/VNGXBPMW1QbBcJ
...

Create with full options

$> docker service create --name nginx --secret source=ssh-dev,target=id_rsa,uid=1000,gid=1000,mode=0400 nginx
t9kwy2v04l7zl3hl9dlvt5vq0

refs := []*swarmapi.SecretReference{}
for _, s := range sr {
var mode swarmapi.SecretReference_Mode
switch s.Mode {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant switch. IMO should be

mode := swarmapi.SecretReference_FILE
if s.Mode == types.SecretReferenceSystem {
    mode = swarmapi.SecretReference_SYSTEM
}

refs := []*types.SecretReference{}
for _, s := range sr {
var mode types.SecretReferenceMode
switch s.Mode {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe make convertSecretReferenceMode function

}

// Meta
secret.Version.Index = s.Meta.Version.Index
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can embed Version and Spec in structure initialization


// SecretFromGRPC converts a grpc Secret to a Secret.
func SecretFromGRPC(s *swarmapi.Secret) swarmtypes.Secret {
logrus.Debugf("%+v", s)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add some words here.


// SecretSpecToGRPC converts Secret to a grpc Secret.
func SecretSpecToGRPC(s swarmtypes.SecretSpec) (swarmapi.SecretSpec, error) {
spec := swarmapi.SecretSpec{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just return this here

@@ -4,6 +4,7 @@ import (
executorpkg "github.com/docker/docker/daemon/cluster/executor"
"github.com/docker/swarmkit/api"
"golang.org/x/net/context"
"src/github.com/docker/swarmkit/agent/exec"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong import

return fmt.Errorf("error creating secret mount path: %s", err)
}

logrus.Debugf("injecting secret: name=%s path=%s", s.Name, fPath)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use WithFields for creating such structured messages :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ya thx :)


// create tmpfs
if err := os.MkdirAll(localMountPath, 0700); err != nil {
return fmt.Errorf("error creating secret local mount path: %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have pkg/errors vendored now. Would you mind to use errors.Wrap{f} in such cases?

@@ -175,6 +176,41 @@ func (daemon *Daemon) setupIpcDirs(c *container.Container) error {
return nil
}

func (daemon *Daemon) setupSecretDir(c *container.Container) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this func cleanup on error? I mean like unmount and remove secrets dir.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok i added a defer func that should cleanup. ptal thx!


func (daemon *Daemon) SetContainerSecrets(name string, secrets []*containertypes.ContainerSecret) error {
if !secretsSupported() {
logrus.Warn("secrets are not supported on this platform")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like this warning will be logged on each container run. Is that right?

Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having issues making this work:

$ echo foo=bar | docker secret create test
$ docker service create --name test --secret test:/foo busybox top
$ docker exec <cid> ls /foo

/foo does not exist.

I have concerns on supporting the format <name>:<target> which puts us in the same sticky situation we are in with -v on run.

rawVersion := r.URL.Query().Get("version")
version, err := strconv.ParseUint(rawVersion, 10, 64)
if err != nil {
return fmt.Errorf("Invalid secret version '%s': %s", rawVersion, err.Error())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use api/errors with something like a BadRequest?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also lower-case errors

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx -- i was following the convention. i'll update


id := vars["id"]
if err := sr.backend.UpdateSecret(id, version, secret); err != nil {
return fmt.Errorf("Error updating secret: %s", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this only ever be an internal error? If not I'd not wrap the error here and make sure the backend provides a sufficient message.

)

// parseSecretString parses the requested secret and returns the secret name
// and target. Expects format SECRET_NAME:TARGET
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👎
This format is nice in the happy case, but in the long run is a nightmare.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. do you have an alternative?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @stevvooe once suggested the unicode arrow ;)

But seriously, I think the code here has the right approach. The secret name and target need to be specified adjacent to each other instead of using separate options. We could use key-value pairs, but I think that would be really ugly and unintuitive.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpuguy83 @aaronlehmann what about something like

--secret ssh-dev,target=id_rsa,uid=1000,gid=1000,mode=0600

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any thoughts on the above @cpuguy83 @aaronlehmann ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say in general it looks good.
I think ssh-dev should be source=ssh-dev to match --mount (big discussion on whether to allow the positional syntax from that PR).

Only question I'd ask is, how does this work with if/when an alternative secrets backend is supported (e.g. Vault). Would the current options work on all backends/platforms? Can Windows support this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big fan of source=. It complicates the normal case where all you want to specify is a source, which now involves key/value pairs. --secret-add source=secretname doesn't look like a particularly friendly CLI to me.

What about keeping the key/value pairs as the general case, but allowing a shorthand of --secret-add secretname (i.e. if no = is found, take the argument as the source)? I'm not completely sure I like this, but I think I like it more than forcing key/value syntax for this argument.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Find by me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But mixing positional and k/v in the same spec (e.g. --secret-add secretname,target=mysecret) not so much.

Copy link
Contributor

@diogomonica diogomonica Nov 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @aaronlehmann's suggestion. We should have a shorthand of --secret-add secretname

}

// remount secrets ro
if err := mount.Mount("tmpfs", localMountPath, "tmpfs", "remount,ro"); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not ramfs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we decided to not use ramfs since the swarmkit storage swaps to disk so we don't gain anything by using ramfs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ehazlett does this mean secrets are effectively stored plain-text on disk?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the container they are stored in tmpfs (in memory). They are encrypted at rest in the Swarm storage.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok awesome, thanks @ehazlett 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yw :)

@@ -259,6 +266,26 @@ func (container *Container) IpcMounts() []Mount {
return mounts
}

// SecretMounts returns the list of Secret mounts
func (container *Container) SecretMounts() []Mount {
var mounts []Mount
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have a slice but only ever append one thing to it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was to add more in the future but good point. i'll remove the slice.

const DefaultSHMSize int64 = 67108864
const (
DefaultSHMSize int64 = 67108864
containerSecretMountPath = "/run/secrets"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't match the doc string

ExecCommands *exec.Store `json:"-"`
HostConfig *containertypes.HostConfig `json:"-"` // do not serialize the host config in the json, otherwise we'll make the container unportable
ExecCommands *exec.Store `json:"-"`
Secrets []*containertypes.ContainerSecret `json:"-"` // do not serialize
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't like that we're shoving secrets in here purely for indexing, but I guess we are doing it for other things as well right now.

ctx, cancel := c.getRequestContext()
defer cancel()

r, err := c.node.client.GetSecret(ctx, &swarmapi.GetSecretRequest{SecretID: id})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't work when a name is specified.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nor prefixes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct. swarmkit only supports getting by id atm

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

swarmkit can filter by name or name prefix using the ListSecrets RPC. This is the same as other object types like service and node - Get is always by ID.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we could implement a GetSecretByName that effectively uses ListSecrets with a name filter under it.

@cpuguy83
Copy link
Member

cpuguy83 commented Oct 26, 2016

Also sorry, I added some code review in there though it's still in design review.
I think overall the design is mostly good except for the format.

The other item would be only supporting stdin... but that's probably not a big deal... who knows for Windows though.

@cpuguy83
Copy link
Member

Tests are also failing because of the linter.

@ehazlett
Copy link
Contributor Author

lint issues should be fixed.

@cpuguy83 the design was to have the SECRET_NAME:TARGET_NAME but I'm not tied to it. The intended use (sorry for the lack of docs atm) was to not have a path for the target so we can have a simple directory instead of nesting. The latest update checks and returns an error if a path is specified.

@cpuguy83
Copy link
Member

Aha! It goes in /run/secrets of course... This makes sense. I think I'm ok with the format in that case. It could still end up tricky though.

@ehazlett
Copy link
Contributor Author

@cpuguy83 I was possibly thinking something like the more granular mount syntax in the future but it might be too much for now. idk

@LK4D4
Copy link
Contributor

LK4D4 commented Oct 26, 2016

@ehazlett looks like vendor needs to be updated.

type Secret struct {
ID string
Meta
Spec *SecretSpec `json:",omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spec should not be a pointer.

Meta
Spec *SecretSpec `json:",omitempty"`
Digest string `json:",omitempty"`
SecretSize int64 `json:",omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably don't want omitempty for SecretSize. Probably not for any of these fields, actually.


type SecretSpec struct {
Annotations
Data []byte `json:",omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, probably don't want to omit the Data field if the secret is zero-length.

sec = append(sec, &swarm.SecretReference{
SecretID: s,
Mode: swarm.SecretReferenceFile,
Target: "",
Copy link
Contributor

@aaronlehmann aaronlehmann Oct 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we still need to add the ability for the user to specify a custom target?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be an inconsistency between service create and service update. Shouldn't they both take secrets in the format that specifies a name and target (i.e. use parseSecrets)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ya we actually don't even need this because i set it outside in order to do the lookup. good catch thx!

tokens := strings.Split(secretString, ":")

secretName := strings.TrimSpace(tokens[0])
targetName := ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

targetName = secretName to avoid the else case below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1


foundSecrets := make(map[string]*swarmtypes.Secret)
for _, secret := range secrets {
foundSecrets[secret.Spec.Annotations.Name] = &secret
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong. It will set every map value to point to the same iterator. Just make the map values secret IDs instead of pointers to secrets. That will make things simpler.


// Error returns a string representation of a secretNotFoundError
func (e secretNotFoundError) Error() string {
return fmt.Sprintf("Error: No such secret: %s", e.name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no such secret: %s

}
}

return mount
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it correct to return an empty Mount structure in this case? I see that it's unconditionally appended to a slice in createSpec.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had this as a nil check before; i'll switch back

@vdemeester vdemeester added this to the 1.13.0 milestone Oct 26, 2016
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few questions :

What should happen if I remove a secret that is mounted ? Right now it's still available in the container, that might be by design, just asking 👼.

On service create, there is a problem if I specify multiple secrets (using --secret) ; I get the same content for all the secrets.

$ docker service create sec1 <<EOF
A
EOF
$ docker secret create sec2 <<EOF
B
EOF
$ docker service create --name test --secret sec1 --secret sec2 busybox top
# Task container is 2b66…
$ docker exec -ti 2b66 sh
$ ls /run/secrets
sec1 sec2
$ cat /run/secrets/sec1
B
# should have be A
$ cat /run/secrets/sec2
B

On service update, it doesn't seems it updates the secret or re-create the task. Not sure what is the preferred way to go (I would think re-create the container), but as of now, it doesn't have any effect. — just saw the checkbox, nevermind 👼

$ docker service ps test 
NAME                              IMAGE    NODE          DESIRED STATE  CURRENT STATE           ERROR
test.1.imutqdjh4k2cm24e90mpkt9vh  busybox  4c780a8f3912  Running        Running 37 seconds ago
$ docker secret create sec3 <<EOF
C
EOF
$ docker service update --secret sec3 test
test
$ docker service ps test
NAME                              IMAGE    NODE          DESIRED STATE  CURRENT STATE          ERROR
test.1.imutqdjh4k2cm24e90mpkt9vh  busybox  4c780a8f3912  Running        Running 7 minutes ago
$ docker exec -ti 2b6 sh
$ ls /run/secrets
sec1 sec2

For docker secret subcommands (like rm or inspect), should we support prefixes (like on all container subcommands) and names ? right now it just supports the full ID.

Other than that, it looks good 👼 🐸

@@ -40,5 +40,10 @@ func (sr *swarmRouter) initRoutes() {
router.NewPostRoute("/nodes/{id:.*}/update", sr.updateNode),
router.NewGetRoute("/tasks", sr.getTasks),
router.NewGetRoute("/tasks/{id:.*}", sr.getTask),
router.NewGetRoute("/secrets", sr.getSecrets),
router.NewPostRoute("/secrets/create", sr.createSecret),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be router.NewPostRoute("/secrets", sr.createSecret) 👼

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya I was trying to follow the convention but I like /secrets better. thx!

@aaronlehmann
Copy link
Contributor

On service create, there is a problem if I specify multiple secrets (using --secret) ; I get the same content for all the secrets.

That's probably because of this: https://github.com/docker/docker/pull/27794/files#r85236096

@@ -18,6 +18,10 @@ type executor struct {
backend executorpkg.Backend
}

type secretProvider interface {
Get(secretID string) *api.Secret
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it's unused.

log.Errorf("error cleaning up secret mount: %s", err)
}
}
}(setupErr)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Args to a deferred function are evaluated at the time of the defer statement, so this won't work as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ok thx

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm using a named return now so i think this should work. PTAL

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still looks wrong. You shouldn't pass setupErr as an argument to the deferred function. You should just let it capture the variable from the outer scope.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are correct. thx


// create tmpfs
if err := os.MkdirAll(localMountPath, 0700); err != nil {
setupErr = errors.Wrap(err, "error creating secret local mount path")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it correct to continue if the directory can't be created?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope -- i need to rework the cleanup

setupErr = errors.Wrap(err, "error creating secret local mount path")
}
if err := mount.Mount("tmpfs", localMountPath, "tmpfs", "nodev"); err != nil {
setupErr = errors.Wrap(err, "unable to setup secret mount")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it correct to continue if the mount fails?

@ehazlett
Copy link
Contributor Author

We need a way to define the secret on the container. Since we can't call the secret store direct from the engine side I chose to implement it here and have the cluster define it as discussed during design.

ContainerSecret does repeat most of the SecretReference however it also needs data. I chose to keep them separate instead of mix the types between swarm and the container types. I could embed if that's better but I felt it made a clear separation. This is the same for SecretRequestOption. This is similar to things like ContainerAttachConfig and ContainerAttachOptions where there are two fields different.

@stevvooe
Copy link
Contributor

@ehazlett ContainerSecret mixes the concepts of reference and secret.

In general, these types should be isomorphic to swarm types. We can share types with swarm types, just fine. We do this for mounts.

@xeor
Copy link

xeor commented Nov 16, 2016

Is the design of the secret management allowing for future features like one-time-read options? Or is the design not allowing that kind of extensions of dynamically adding/removing secrets in a secure way?

@stevvooe
Copy link
Contributor

Is the design of the secret management allowing for future features like one-time-read options? Or is the design not allowing that kind of extensions of dynamically adding/removing secrets in a secure way?

The design of the secrets functionality in this PR does not preclude such future features. Many of the existing features were designed with these use cases in mind for the future.

@thaJeztah
Copy link
Member

opened #28516 for tracking the remaining review comments

tianon added a commit to infosiftr/stackbrew that referenced this pull request Nov 17, 2016
- `cassandra`: 3.0.10
- `docker`: 1.13.0-rc1 (currently failing the `override-cmd` test; see moby/moby#28329), `docker daemon` -> `dockerd` (docker-library/docker#31)
- `drupal`: 8.2.3
- `java`: alpine 7.121.2.6.8-r0, 8.111.14-r0
- `openjdk`: alpine 7.121.2.6.8-r0, 8.111.14-r0
- `python`: 3.6.0b3 (docker-library/python#157), pip 9.0.1 (docker-library/python#154)
- `ruby`: 2.3.2, 2.2.6
- `tomcat`: 6.0.48, 7.0.73, 8.0.39, 8.5.8, 9.0.0.M13
- `wordpress`: add `WORDPRESS_*_FILE` support for passing parameters via files especially for [Docker secrets](moby/moby#27794) (docker-library/wordpress#186)
@RRAlex
Copy link

RRAlex commented Nov 18, 2016

@jefflill You might also want to make sure your second tmpfs is mounted securely, with rw,noexec,nosuid,nodev,size=1m
Hopefully all of those can also be passed to a service (I didn't verify).

Short term, I also wish the /run/secrets/ could have an option to simply o-rwx the directory (since it's read-only and we can't chmod it).
That could be simpler & easier to implement, until we get secrets that are read once, etc.
A root user could set things up before dropping its privileges (to a user), for example...

That said, read once sounds a lot better... :)

@jefflill
Copy link

Actually, I've verified that opts/mount: add tmpfs-specific options
#28150 works in recent 1.13 builds.

This is super clean now.

--Jeff

On Fri, Nov 18, 2016 at 7:30 AM, Alexandre Guédon [email protected]
wrote:

@jefflill https://github.com/jefflill You might also want to make sure
your second tmpfs is mounted securely, with rw,noexec,nosuid,nodev,size=1m
Hopefully all of those can also be passed to a service (I didn't verify).

Short term, I also wish the /run/secrets/ could have an option to simply
o-rwx the directory (since it's read-only and we can't chmod it).
That could be simpler & easier to implement, until we get secrets that are
read once, etc.
A root user could set things up before dropping its privileges (to a
user), for example...

That said, read once sounds a lot better... :)


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#27794 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AErlj8NdrY9xZ3ZpKoOPpVk3chpi6wtmks5q_cSXgaJpZM4Khe0a
.

@shouze
Copy link
Contributor

shouze commented Dec 14, 2016

@ehazlett wow it's pretty cool ! Was working on a vault integration of our own when I've seen that.

As the feature comes from swarmkit, could we imagine to have it in non swarm docker use (docker for mac, docker engine)?

And what about docker-compose support? There's already a discussion opened about secrets docker/compose#1534

To wum up, I would be very pleased to be able to use secrets on my local machine/ci/... the same way than on the big cluster ;)

@thaJeztah
Copy link
Member

@shouze this all works on docker for mac, but currently only for "services" (so if you have swarm mode enabled (docker swarm init). Future enhancements to allow it to be used for docker run, and docker build may be coming in future releases.

@shouze
Copy link
Contributor

shouze commented Dec 14, 2016

@thaJeztah ho nice! I've just tested it, very very nice ;) I guess that when enhancements in docker run/build will come it will also be integrated with docker compose?

@thaJeztah
Copy link
Member

@shouze docker-compose is a separate project, but I think that would be a reasonable assumption 😄 FWIW; @ehazlett started on a PR for build-time secrets; #28079, but it needed more time so couldn't make it into 1.13; for 1.14 it will probably need a rewrite, so it was closed in the meantime.

@ghost
Copy link

ghost commented Dec 21, 2016

Hi,

Sorry for the question but does the secrets are stored in an encrypted way ?

@thaJeztah
Copy link
Member

@nasskach yes; see the (work in progress) documentation; https://github.com/docker/docker.github.io/pull/568/files

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

Successfully merging this pull request may close these issues.