TLS based etcd can't get certs info from request.

[ringtail]

Hi,I found a problem in release-2.2，I need to read cert info from request.here is the request handler.

func (h *keysHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) 

cert info will be parsed from r

r.TLS.PeerCertificates

but It doesn't work in release-2.2. so I found the facts.

        if fdLimit, err := runtimeutil.FDLimit(); err == nil {
                    if fdLimit <= reservedInternalFDNum {
                        plog.Fatalf("file descriptor limit[%d] of etcd process is too low, and should be set higher than %d to ensure internal usage", fdLimit, reservedInternalFDNum)
                    }
                    l = netutil.LimitListener(l, int(fdLimit-reservedInternalFDNum))
                }


//code in github.com/coreos/etcd/Godeps/_workspace/src/golang.org/x/net/netutil
func (l *limitListener) Accept() (net.Conn, error) {
    l.acquire()
    c, err := l.Listener.Accept()
    if err != nil {
        l.release()
        return nil, err
    }
    return &limitListenerConn{Conn: c, release: l.release}, nil
}

type limitListenerConn struct {
    net.Conn
    releaseOnce sync.Once
    release     func()
}

so when parse cert info

if tlsConn, ok := c.rwc.(*tls.Conn); ok {
        if d := c.server.ReadTimeout; d != 0 {
            c.rwc.SetReadDeadline(time.Now().Add(d))
        }
        if d := c.server.WriteTimeout; d != 0 {
            c.rwc.SetWriteDeadline(time.Now().Add(d))
        }
        if err := tlsConn.Handshake(); err != nil {
            c.server.logf("http: TLS handshake error from %s: %v", c.rwc.RemoteAddr(), err)
            return
        }
        c.tlsState = new(tls.ConnectionState)
        *c.tlsState = tlsConn.ConnectionState()
        if proto := c.tlsState.NegotiatedProtocol; validNPN(proto) {
            if fn := c.server.TLSNextProto[proto]; fn != nil {
                h := initNPNRequest{tlsConn, serverHandler{c.server}}
                fn(c.server, tlsConn, h)
            }
            return
        }
    }

It can't convert to *tls.con. the simplest way is to add config filed in limitListenerConn.

[philips]

Would you be able to write a unit test to cover this?

[ringtail]

I found the problem is that &limitListenerConn{Conn: c, release: l.release} can't not convert to a *tls.con

[ringtail]

I simply comment the code in etcd/etcdmain/etcd.go below,every things works fine.

        if fdLimit, err := runtimeutil.FDLimit(); err == nil {
                    if fdLimit <= reservedInternalFDNum {
                        plog.Fatalf("file descriptor limit[%d] of etcd process is too low, and should be set higher than %d to ensure internal usage", fdLimit, reservedInternalFDNum)
                    }
                    l = netutil.LimitListener(l, int(fdLimit-reservedInternalFDNum))
                }

[ringtail]

TLS connection is like this

&Conn{conn: conn, config: config}

so we can simple add limitListener a field called config,and it will be fine.

[xiang90]

@ringtail Are you trying to embed etcd into your project? We are not going to maintain a stable API or implementation guarantees for etcd itself. We do not have the bandwidth right now and we plan to change the implementation of etcd internally quite a bit. However, if you meet any issue we are happy to discuss and review pull requests.

For your issue, do you want to get the TLSConn or just the static configuration? The static configuration is passed by the user, you can simply cache it when you pass it in. So you do not need to add a field.

[ringtail]

@xiang90 Hi,Maybe I don't describe the problem clearly. The scenario is that I enhance etcd by certs with specific role in it. you can sign a client cert with some specific role. And I need to get cert info from
keysHandler in etcdserver/etcdhttp/client.go

func (h *keysHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
      // you can get certChains from r.TLS  
     certChains:= r.TLS.PeerCertificates
}

but in relase-2.2, we can't get r.TLS.PeerCertificates because of the code below

                if fdLimit, err := runtimeutil.FDLimit(); err == nil {
                    if fdLimit <= reservedInternalFDNum {
                        plog.Fatalf("file descriptor limit[%d] of etcd process is too low, and should be set higher than %d to ensure internal usage", fdLimit, reservedInternalFDNum)
                    }
                    l = netutil.LimitListener(l, int(fdLimit-reservedInternalFDNum))
                }

because of the tls listener is wrapped by netutil.LimitListener, but netutil.LimitListener can't not create a *tls.con， so the handshake will not be successful and we can't get client cert from request in keysHandler

[xiang90]

@ringtail If you want to make sure etcd upgrades do not break you, you'd better try to contribute your patch to upstream. If you want to fix this issue, you can try to submit a patch. We do not plan to provide any guarantees about any sub pkgs structs or API inside etcd.

[xiang90]

but netutil.LimitListener can't not create a *tls.con， so the handshake will not be successful and we can't get client cert from request in keysHandler

I do not quite understand this. LimitListener is just a wrapper around the original Listener. I do not think there is any issue with handshake. Can you reproduce the handshake failure with LimitListener?

[xiang90]

@ringtail

OK. After checking the go std lib, I found

if tlsConn, ok := c.rwc.(*tls.Conn); ok {
        if d := c.server.ReadTimeout; d != 0 {
            c.rwc.SetReadDeadline(time.Now().Add(d))
        }
        if d := c.server.WriteTimeout; d != 0 {
            c.rwc.SetWriteDeadline(time.Now().Add(d))
        }
        if err := tlsConn.Handshake(); err != nil {
            c.server.logf("http: TLS handshake error from %s: %v", c.rwc.RemoteAddr(), err)
            return
        }
        c.tlsState = new(tls.ConnectionState)
        *c.tlsState = tlsConn.ConnectionState()
        if proto := c.tlsState.NegotiatedProtocol; validNPN(proto) {
            if fn := c.server.TLSNextProto[proto]; fn != nil {
                h := initNPNRequest{tlsConn, serverHandler{c.server}}
                fn(c.server, tlsConn, h)
            }
            return
        }
    }

is in std library, not in your code. I will try to reproduce the issue myself then.

[xiang90]

@ringtail

Also netutil is an external dependency at https://github.com/golang/net/tree/master/netutil.

Can you try to file an issue there?

[ringtail]

@xiang90 yes! That's exactly the problem. I'll open a issue in https://github.com/golang/net/tree/master/netutil.
Thanks a lot!

[xiang90]

@ringtail

I got a fix at etcd side at #4153.

Can you try to pull down this branch and give it a try. At meantime, I would suggest you not keep a private fork of etcd.

[ringtail]

yes, I keep a private fork in my gitlab and the problem is raised when I try to upgrade my etcd version from 2.1.3 to 2.2.2☺️

[xiang90]

@ringtail Anyway, please give #4153 a try. Thanks!

[ringtail]

OK，I'll try it. thank you!

[xiang90]

@ringtail Why this is closed?

[ringtail]

It works☺️

[xiang90]

@ringtail Thanks!