[
https://issues.apache.org/jira/browse/THRIFT-5996?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Marin Nozhchev updated THRIFT-5996:
-----------------------------------
Description:
The connection check, implemented for THRIFT-5214 , works only for plain
sockets, not TLS. This is a significant deficiency.
Fortunately, the fix is simple and I happy to create a PR.
github.com/go-sql-driver/mysql that initially inspired THRIFT-5214 had this
working from the start - [https://github.com/go-sql-driver/mysql/pull/934] .
Essentially, in case if TLS, the conn check must access the underlying TCP
connection. In
[https://github.com/apache/thrift/blob/master/lib/go/thrift/socket_unix_conn.go#L40]
, checkConn() instead of
{code:java}
syscallConn, ok := sc.Conn.(syscall.Conn)
if !ok {
// No way to check, return nil
return nil
}{code}
we should have
{code:java}
conn := sc.Conn
if tlsConn, ok := conn.(*tls.Conn); ok {
conn = tlsConn.NetConn()
}
syscallConn, ok := conn.(syscall.Conn)
if !ok {
...{code}
*tls.Conn itself doesn't implement syscall.Conn .
The original implementation of THRIFT-5214 could not do this because NetConn()
wasn't available at the time - it was added in Go 1.18. The mysql
implementation managed without it because uses lower level API to open a TLS
connection.
was:
The connection check, implemented for THRIFT-5214 , works only for plain
sockets, not TLS. This is a significant deficiency.
Fortunately, the fix is simple and I happy to create a PR.
github.com/go-sql-driver/mysql that initially inspired THRIFT-5214 had this
working from the start - [https://github.com/go-sql-driver/mysql/pull/934] .
Essentially, in case if TLS, the conn check must access the underlying TCP
connection. In
[https://github.com/apache/thrift/blob/master/lib/go/thrift/socket_unix_conn.go#L40]
, checkConn() instead of
{code:java}
syscallConn, ok := sc.Conn.(syscall.Conn)
if !ok {
// No way to check, return nil
return nil
}{code}
we should have
{code:java}
conn := sc.Conn
if tlsConn, ok := conn.(*tls.Conn); ok {
conn = tlsConn.NetConn()
}
syscallConn, ok := conn.(syscall.Conn)
if !ok {
...{code}
*tls.Conn itself doesn't implement syscall.Conn .
The original implementation of THRIFT-5214 could not do this because NetConn()
wasn't available at the time - it was added in Go 1.18. The mysql
implementation managed without it because uses lower level API to open a TLS
connection.
> go: connection check should work for TLS sockets
> ------------------------------------------------
>
> Key: THRIFT-5996
> URL: https://issues.apache.org/jira/browse/THRIFT-5996
> Project: Thrift
> Issue Type: Improvement
> Components: Go - Library
> Affects Versions: 0.23.0
> Reporter: Marin Nozhchev
> Priority: Major
>
> The connection check, implemented for THRIFT-5214 , works only for plain
> sockets, not TLS. This is a significant deficiency.
> Fortunately, the fix is simple and I happy to create a PR.
> github.com/go-sql-driver/mysql that initially inspired THRIFT-5214 had this
> working from the start - [https://github.com/go-sql-driver/mysql/pull/934] .
> Essentially, in case if TLS, the conn check must access the underlying TCP
> connection. In
> [https://github.com/apache/thrift/blob/master/lib/go/thrift/socket_unix_conn.go#L40]
> , checkConn() instead of
>
> {code:java}
> syscallConn, ok := sc.Conn.(syscall.Conn)
> if !ok {
> // No way to check, return nil
> return nil
> }{code}
> we should have
> {code:java}
> conn := sc.Conn
> if tlsConn, ok := conn.(*tls.Conn); ok {
> conn = tlsConn.NetConn()
> }
> syscallConn, ok := conn.(syscall.Conn)
> if !ok {
> ...{code}
> *tls.Conn itself doesn't implement syscall.Conn .
> The original implementation of THRIFT-5214 could not do this because
> NetConn() wasn't available at the time - it was added in Go 1.18. The mysql
> implementation managed without it because uses lower level API to open a TLS
> connection.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
