etcd-io / etcd

Distributed reliable key-value store for the most critical data of a distributed system
https://etcd.io
Apache License 2.0
47.79k stars 9.77k forks source link

Lock module does not work with https #636

Closed sigmunau closed 10 years ago

sigmunau commented 10 years ago

In my setup the lock module does not work at all. Any operation I try gives this error: 501: All the given peers are not reachable (Tried to connect to each peer twice and failed) [0] Normal operations on keys works as expected

We are running etcd commit ac872b385581ccd68895551deaf068ced72202e0 We are using TLS for both raft and client communications. The cluster has three nodes and each list the two others in it's config file after peers =

sigmunau commented 10 years ago

It seems the lock module unconditionally creates a go-etcd.Client and never creates a TLSClient, and thus will not work if etcd is configured to use https

benbjohnson commented 10 years ago

@sigmunau Thanks for reporting the bug. We'll get it fixed up.

Soulou commented 10 years ago

I'm encountering the same issue, is there something new?

Ulexus commented 10 years ago

There is a vagueness in the usability, here. The issue is that the go-etcd client, when used in TLS mode, essentially requires a client keypair.

For this to work, you would need to be able to supply a keypair for the client connection of etcd server, and there is not yet (to my knowledge) a facility or standard for declaring such. It seems bad practice to me to supply a keypair as part of a POST, even if that connection is, itself, encrypted. That leaves the keypair residing in the configuration of etcd itself.

In theory, we could use the TLSInfo from the etcd server, were it made available to the module handler (/mod/mod.go). My question would be whether that is actually a good idea: subrogating the TLS keypair from the server to the client.

Incidentally, this is all true for the leader module, as well.

Soulou commented 10 years ago

Excuse @Ulexus, I don't understand the link between what you said and the problem here

The sum up, the issue is the following:

curl --cacert ./cacert.pem --key ./file.key --cert ./file.crt 'https://localhost:4001/mod/v2/lock/test'
501: All the given peers are not reachable (Tried to connect to each peer twice and failed) [0]
curl http://localhost:4001/mod/v2/lock/test
100: Key not found (/_etcd/mod) [99318]

My cluster is composed of 3 nodes. HTTPS authentication is working perfectly in general. Why isn't it working for modules? I'm assuming that the TLS handshake and authentication are happening before any routing event. So my question is, why does the behaviour change between both situations?

Thank you!

Ulexus commented 10 years ago

The way the (leader and lock) modules work, is that they subtend a separate client connection to etcd on their own, internally from the module. This subtended connection has no facility inside the module code (though the client code itself does) to use a TLS connection. If your etcd servers are TLS-only, the lock module is not able to connect to etcd, so it fails to operate.

Soulou commented 10 years ago

Thank you for the explanation, it's a lot clearer now. The mods are essentially etcd client applications. For security reasons, the whole cluster is using certificate authentication, and I can't change it.

In a short-term, is the best turnaround would be implement myself a lock system inside my application? Is there another approach?

Ulexus commented 10 years ago

The module code would not be difficult to modify to use a TLS connection. The difficulty is in determining the keys for that connection. If you don't mind hard-coding them for your purposes, the patch to the module code would be fairly simple.

Ulexus commented 10 years ago

Essentially, that would be:

Change line 23 of /mod/lock/v2/handler.go from: client: etcd.NewClient([]string{addr}), to: client: etcd.NewTLSClient([]string{addr}, []string{cert}, []string{key }, []string{caCert}),

Change cert, key, and caCert to their appropriate (hard-coded) values for your system.

Soulou commented 10 years ago

That's true, but it would require to maintain a fork of etcd.

Adding the configuration flags --mods-tls-key and --mods-tls-crt would be a more usefull solution using a custom etcd with hard-coded data. Is it something which could be merged in the project?

Soulou commented 10 years ago

I'm hacking into th code right now, the main issue is, what is the cleanest way to pass the etcd.Config to the mods

Etcd creates a Server which calls Server.HTTPHandler, then mod.HTTPHandler is called and finally we are in the code of the module which creates the go-etcd Client. So basically sending the config object through all this functions to use it in the module code is a bit heavy.

So far the Config object stays in the Etcd object, I don't like using global variables / package variables, so I try to find the cleanest way to pass values from config file/env/flag to the module code.

I'd be happy to write a PR ;)

Ulexus commented 10 years ago

That is, likewise, where I got stuck, and why I posed the question. Any etcd maintainers have an opinion?

bmizerany commented 10 years ago

We are depreciating mods until after 0.4 to refocus on a new API. I am removing this from the 0.4 milestone.

Ulexus commented 10 years ago

Good idea, and thanks

Soulou commented 10 years ago

@Ulexus I've just developed this small go library to create locks https://github.com/Appsdeck/go-etcd-lock. I've tried to use the mod/lock logics, but in a simpler way.

Ulexus commented 10 years ago

Nice!

xiang90 commented 10 years ago

lock module is deprecated.