Skip to content
This repository has been archived by the owner on Sep 5, 2019. It is now read-only.

TLS doesn't support connection TCPKeepAlive Option #85

Closed
yanfali opened this issue Aug 12, 2016 · 8 comments
Closed

TLS doesn't support connection TCPKeepAlive Option #85

yanfali opened this issue Aug 12, 2016 · 8 comments

Comments

@yanfali
Copy link

yanfali commented Aug 12, 2016

This issue on etcd explains the issue.

etcd-io/etcd#2185

@yanfali
Copy link
Author

yanfali commented Aug 12, 2016

To reproduce do something like this:

        srv := &graceful.Server{
            Timeout: 10 * time.Second,
            TCPKeepAlive: 3 * time.Minute,
            Server:  &http.Server{Addr: fmt.Sprintf(":%d", port), Handler: n},
            Logger:  graceful.DefaultLogger(),
        }

        err = srv.ListenAndServeTLSConfig(tlsConfig)

Then try and connect to port with a browser.

panic: interface conversion: *tls.Conn is not graceful.keepAliveConn: missing method SetKeepAlive

@yanfali
Copy link
Author

yanfali commented Aug 12, 2016

I can try and work up a PR if you're interested, but it seems relatively straightforward to fix.

@F21
Copy link
Collaborator

F21 commented Aug 13, 2016

@yanfali Thanks for reporting the issue! 😄 If you could open a PR, that would be really awesome too!

yanfali pushed a commit to awakesecurity/graceful that referenced this issue Aug 14, 2016
Order matters when enabling TCP Keep Alives. Keep alives need to
be configured before wrapping them with a TLS Listener. In the current
implementation the TCP Keep Alive listener wraps after TLS is enabled
and TLS Listener doesn't have methods to access the lower layer TCP
behavior.

1. remove the TCP Keep Alive Listner from Serve
2. re-add it after creating a tcp Listener and before wrapping in TLS
3. update TLS unit tests to always specify TCPKeepAlive values to catch
   regressions.
yanfali pushed a commit to awakesecurity/graceful that referenced this issue Aug 14, 2016
Order matters when enabling TCP Keep Alives. Keep alives need to
be configured before wrapping them with a TLS Listener. In the current
implementation the TCP Keep Alive listener wraps after TLS is enabled
and TLS Listener doesn't have methods to access the lower layer TCP
behavior.

1. remove the TCP Keep Alive Listener from Serve
2. re-add TCP Keep Alive after creating a tcp Listener but before wrapping in TLS
3. update TLS unit tests to always specify TCPKeepAlive values to catch
   regressions.
yanfali pushed a commit to awakesecurity/graceful that referenced this issue Aug 14, 2016
Order matters when enabling TCP Keep Alives. Keep alives need to
be configured before wrapping them with a TLS Listener. In the current
implementation the TCP Keep Alive listener wraps after TLS is enabled
and TLS Listener doesn't have methods to access the lower layer TCP
behavior.

1. remove the TCP Keep Alive Listener from server.Serve
2. create a newTCPListener to server which returns wrapped instanced
3. update TLS unit tests to always specify TCPKeepAlive values to catch
   regressions.
@yanfali
Copy link
Author

yanfali commented Aug 14, 2016

@F21 hope this works for the project.

yanfali pushed a commit to awakesecurity/graceful that referenced this issue Aug 14, 2016
Order matters when enabling TCP Keep Alives. Keep alives need to
be configured before wrapping them with a TLS Listener. In the current
implementation the TCP Keep Alive listener wraps after TLS is enabled
and TLS Listener doesn't have methods to access the lower layer TCP
behavior.

1. remove the TCP Keep Alive Listener from server.Serve
2. create a newTCPListener() on server that returns keep alive
   wrapped instances on demand
3. update TLS unit tests to always specify TCPKeepAlive values to catch
   regressions.
F21 added a commit that referenced this issue Aug 15, 2016
ISSUE #85: TCP Keep Alive cannot wrap TLS
@F21
Copy link
Collaborator

F21 commented Aug 15, 2016

Closed via #86

@F21 F21 closed this as completed Aug 15, 2016
@yanfali
Copy link
Author

yanfali commented Aug 15, 2016

@F21 Any timeline when this is going to be released to gopkg.in? I'm using vendoring so I can switch back github, but if you're going to push a release I can wait. Thanks

@F21
Copy link
Collaborator

F21 commented Aug 15, 2016

@yanfali Thanks for reminding me! I just pushed v1.2.12

@yanfali
Copy link
Author

yanfali commented Aug 15, 2016

Awesome! Thank you.

zjs added a commit to zjs/vic that referenced this issue Sep 26, 2017
This was a mechanical change using the following commands:
  gvt delete github.com/tylerb/graceful
  gvt fetch -revision 4654dfbb6ad53cb5e27f37d99b02e16c1872fbbb \
      https://github.com/tylerb/graceful

This is needed to pick up the fix for tylerstillwater/graceful#85
zjs added a commit to zjs/vic that referenced this issue Sep 28, 2017
This was a mechanical change using the following commands:
  gvt delete github.com/tylerb/graceful
  gvt fetch -revision 4654dfbb6ad53cb5e27f37d99b02e16c1872fbbb \
      https://github.com/tylerb/graceful

This is needed to pick up the fix for tylerstillwater/graceful#85
zjs added a commit to vmware/vic that referenced this issue Sep 30, 2017
This was a mechanical change using the following commands:
  gvt delete github.com/tylerb/graceful
  gvt fetch -revision 4654dfbb6ad53cb5e27f37d99b02e16c1872fbbb \
      https://github.com/tylerb/graceful

This is needed to pick up the fix for tylerstillwater/graceful#85
zjs added a commit to zjs/vic that referenced this issue Oct 9, 2017
This was a mechanical change using the following commands:
  gvt delete github.com/tylerb/graceful
  gvt fetch -revision 4654dfbb6ad53cb5e27f37d99b02e16c1872fbbb \
      https://github.com/tylerb/graceful

This is needed to pick up the fix for tylerstillwater/graceful#85
zjs added a commit to vmware/vic that referenced this issue Oct 25, 2017
This was a mechanical change using the following commands:
  gvt delete github.com/tylerb/graceful
  gvt fetch -revision 4654dfbb6ad53cb5e27f37d99b02e16c1872fbbb \
      https://github.com/tylerb/graceful

This is needed to pick up the fix for tylerstillwater/graceful#85
zjs added a commit to vmware/vic that referenced this issue Oct 31, 2017
This was a mechanical change using the following commands:
  gvt delete github.com/tylerb/graceful
  gvt fetch -revision 4654dfbb6ad53cb5e27f37d99b02e16c1872fbbb \
      https://github.com/tylerb/graceful

This is needed to pick up the fix for tylerstillwater/graceful#85
zjs added a commit to vmware/vic that referenced this issue Nov 6, 2017
This was a mechanical change using the following commands:
  gvt delete github.com/tylerb/graceful
  gvt fetch -revision 4654dfbb6ad53cb5e27f37d99b02e16c1872fbbb \
      https://github.com/tylerb/graceful

This is needed to pick up the fix for tylerstillwater/graceful#85
zjs added a commit to vmware/vic that referenced this issue Nov 6, 2017
This was a mechanical change using the following commands:
  gvt delete github.com/tylerb/graceful
  gvt fetch -revision 4654dfbb6ad53cb5e27f37d99b02e16c1872fbbb \
      https://github.com/tylerb/graceful

This is needed to pick up the fix for tylerstillwater/graceful#85
zjs added a commit to zjs/vic that referenced this issue Nov 7, 2017
This was a mechanical change using the following commands:
  gvt delete github.com/tylerb/graceful
  gvt fetch -revision 4654dfbb6ad53cb5e27f37d99b02e16c1872fbbb \
      https://github.com/tylerb/graceful

This is needed to pick up the fix for tylerstillwater/graceful#85
zjs added a commit to zjs/vic that referenced this issue Nov 7, 2017
This was a mechanical change using the following commands:
  gvt delete github.com/tylerb/graceful
  gvt fetch -revision 4654dfbb6ad53cb5e27f37d99b02e16c1872fbbb \
      https://github.com/tylerb/graceful

This is needed to pick up the fix for tylerstillwater/graceful#85
zjs added a commit to vmware/vic that referenced this issue Nov 15, 2017
This was a mechanical change using the following commands:
  gvt delete github.com/tylerb/graceful
  gvt fetch -revision 4654dfbb6ad53cb5e27f37d99b02e16c1872fbbb \
      https://github.com/tylerb/graceful

This is needed to pick up the fix for tylerstillwater/graceful#85
zjs added a commit to vmware/vic that referenced this issue Nov 16, 2017
This was a mechanical change using the following commands:
  gvt delete github.com/tylerb/graceful
  gvt fetch -revision 4654dfbb6ad53cb5e27f37d99b02e16c1872fbbb \
      https://github.com/tylerb/graceful

This is needed to pick up the fix for tylerstillwater/graceful#85
zjs added a commit to vmware/vic that referenced this issue Nov 20, 2017
This was a mechanical change using the following commands:
  gvt delete github.com/tylerb/graceful
  gvt fetch -revision 4654dfbb6ad53cb5e27f37d99b02e16c1872fbbb \
      https://github.com/tylerb/graceful

This is needed to pick up the fix for tylerstillwater/graceful#85
AngieCris pushed a commit to AngieCris/vic that referenced this issue Nov 20, 2017
This was a mechanical change using the following commands:
  gvt delete github.com/tylerb/graceful
  gvt fetch -revision 4654dfbb6ad53cb5e27f37d99b02e16c1872fbbb \
      https://github.com/tylerb/graceful

This is needed to pick up the fix for tylerstillwater/graceful#85
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants