[
https://issues.apache.org/jira/browse/THRIFT-5996?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18080981#comment-18080981
]
Marin Nozhchev commented on THRIFT-5996:
----------------------------------------
> I don't see a difference in risk in syscall.Read and syscall.Recvfrom .
[https://man7.org/linux/man-pages/man2/recvmsg.2.html] confirms that read and
recvfrom are equivalent in this context . Reading that documentation confirms
that the likelyhood to corrupt the communication going over the socket is none
- with either the code in mysql or thrift.
> 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)
