You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by GitBox <gi...@apache.org> on 2020/04/23 11:48:35 UTC

[GitHub] [incubator-brpc] feifeiiong opened a new issue #1096: SSL 相关问题。

feifeiiong opened a new issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096


   **Describe the bug (描述bug)**
   
   io_buf中处理ssl相关的逻辑,未考虑SSL_write和SSL_read可能出现的所有情况,导致出现异常。
   **To Reproduce (复现方法)**
   建立ssl连接后,第三方ssl服务端因为某些原因关闭socket,客户端持续写入/读取,框架处理出现异常,出现异常的代码位置:
   socket.cpp 
   1855行,DoRead阶段对ssl的错误码处理。
   1699行,DoWrite阶段对ssl的错误码处理。
   iobuf.cpp
   1665行,append_from_SSL_channel 函数中的SSL_read:
   const int rc = SSL_read(ssl, _block->data + _block->size, read_len);
   *ssl_error = SSL_get_error(ssl, rc);
   rc =0 时,可能收到了close_notify,此时应判断ssl_error,以处理可能的EOF,代码中未见。
   此外,也可能出现SSL_ERROR_ZERO_RETURN,也未见处理。
   
   970行:write相关逻辑:
   cut_into_SSL_channel函数
   const int nw = SSL_write(ssl, r.block->data + r.offset, r.length);
       if (nw > 0) {
           pop_front(nw);
       }
       *ssl_error = SSL_get_error(ssl, nw);
   
   未见处理nw = 0,以及相应的ssl error,此处可能socket已关闭。
   
   **Expected behavior (期望行为)**
   正确处理SSL相关逻辑。
   此外,还有一些疑问:
   1. 查看代码发现SSL相关逻辑混用BIO和SSL_read/write 接口,但这种写法似乎不推荐,大佬这么写的原因是?
   2. 上述未正确处理的错误,都是通过查看 man的ssl相关api得出,并且实际也遇到了这些问题,当时未处理这些错误的原因是什么?
   3.socket.cpp 未调用SSL_shutdown相关逻辑,不调用该函数的原因是?
   4.对于一条已经过ssl认证的tcp,从池中再次拿出该socket,为什么需要再次发起握手流程?
   **Versions (各种版本)**
   OS:
   Compiler:
   brpc:
   protobuf:
   
   **Additional context/screenshots (更多上下文/截图)**
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] feifeiiong commented on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
feifeiiong commented on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-619385096


   感谢您的回复!第四点确实是如您所说。关于1,3点,有一些不同的看法:


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] feifeiiong edited a comment on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
feifeiiong edited a comment on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-619385096


   感谢您的回复!第四点确实是如您所说。关于1,3点,有一些不同的看法:
   DoRead 的设计确实是读到0即代表约定遇到EOF,但是SSL_read为0时的处理并不完备,以下内容摘自man:
   SSL_read    return values:
   
   '>' 0
   The read operation was successful; the return value is the number of bytes actually read from the TLS/SSL connection.
   
   0
   
   The read operation was not successful. The reason may either be a clean shutdown due to a "close notify" alert sent by the peer (in which case the SSL_RECEIVED_SHUTDOWN flag in the ssl shutdown state is set (see ssl_shutdown(3), ssl_set_shutdown(3)). It is also possible, that the peer simply shut down the underlying transport and the shutdown is incomplete. Call SSL_get_error() with the return value ret to find out, whether an error occurred or the connection was shut down cleanly ( SSL_ERROR_ZERO_RETURN 
   
   SSL_get_error Return Values:
   SSL_ERROR_ZERO_RETURN
   The TLS/SSL connection has been closed.
   SSL_ERROR_SYSCALL
   Some I/O error occurred. The OpenSSL error queue may contain more information on the error. If the error queue is empty (i.e. ERR_get_error() returns 0), ret can be used to find out more about the error: If ret == 0, an EOF was observed that violates the protocol. If ret == -1, the underlying BIO reported an I/O error (for socket I/O on Unix systems, consult errno for details).
   
   在iobuf中是这样处理的:
   ```
   
            const int rc = SSL_read(ssl, _block->data + _block->size, read_len);
           *ssl_error = SSL_get_error(ssl, rc);
           if (rc > 0) {
               const IOBuf::BlockRef r = { (uint32_t)_block->size, (uint32_t)rc, _block };
               _push_back_ref(r);
               _block->size += rc;
               if (_block->full()) {
                   Block* const saved_next = _block->portal_next;
                   _block->dec_ref();  // _block may be deleted
                   _block = saved_next;
               }
               nr += rc;
           } else {
               if (rc < 0) {
                   if (*ssl_error == SSL_ERROR_WANT_READ
                       || (*ssl_error == SSL_ERROR_SYSCALL
                           && BIO_fd_non_fatal_error(errno) == 1)) {
                       // Non fatal error, tell caller to read again
                       *ssl_error = SSL_ERROR_WANT_READ;
                   } else {
                       // Other errors are fatal
                       return rc;
                   }
               }
               return (nr > 0 ? nr : rc);
   ```
   在rc == 0时,返回,并置错误码,此处的错误码,有可能是SSL_ERROR_SYSCALL/SSL_ERROR_ZERO_RETURN, 在socket.cpp:
   
   int ssl_error = 0;
       ssize_t nr = _read_buf.append_from_SSL_channel(_ssl_session, &ssl_error, size_hint);
       switch (ssl_error) {
       case SSL_ERROR_NONE:  // `nr' > 0
           break;
               
       case SSL_ERROR_WANT_READ:
           // Regard this error as EAGAIN
           errno = EAGAIN;
           break;
               
       case SSL_ERROR_WANT_WRITE:
           // Disable renegotiation
           errno = EPROTO;
           return -1;
   
       default: {
           const unsigned long e = ERR_get_error();
           if (nr == 0) {
               // Socket EOF or SSL session EOF
           } else if (e != 0) {
               LOG(WARNING) << "Fail to read from ssl_fd=" << fd()
                            << ": " << SSLError(e);
               errno = ESSL;
           } else {
               // System error with corresponding errno set
               PLOG(WARNING) << "Fail to read from ssl_fd=" << fd();
           }
           break;
       }
   如上,以上错误码被default捕获,而此刻nr 有可能!=0 (iobuf.cpp : return (nr > 0 ? nr : rc);), 从而导致该EOF被忽略,导致socket无法关闭。在实际应用中,该问题已经频繁被触发,打印日志类似:
    Fail to read from ssl_fd=id:Success
   DoWrite有类似的问题,这里就不贴代码了,烦请您看看!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] old-bear edited a comment on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
old-bear edited a comment on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-619035162


   1. `DoRead`这个函数设计返回0即约定表示遇到EOF,这也和非SSL情况统一,同时外层`input_messenger.cpp`中也会相应处理,因此SSL_read return 0直接返回0就行。由于brpc中不允许半关闭读写的情况,对于对端EOF情况只需直接close本端就行。
   2. 另外brpc中一个连接要么一直是SSL,要么永远是非SSL,不会来回再SSL状态间切换,因此也不需要SSL_shutdown。
   3. `cut_into_SSL_channel`在nw=0时也会设置ssl_error,外层`DoWrite`也会捕获感知设置errno。
   4. 从池中(`SocketPool`)拿出的Socket,是不会再次ssl_handshake的。因为连接池模式保证一次请求结束前独占这个Socket,后续拿出时其ssl_state应该set过了。
   5. BIO的api调用,一部分是为了证书buffer,另一部分是为了能让用户设置SSL底层读写的buffer大小,具体读写没有用BIO_read/write


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] old-bear commented on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
old-bear commented on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-619035162


   `DoRead`这个函数设计返回0即约定表示遇到EOF,这也和非SSL情况统一,同时外层`input_messenger.cpp`中也会相应处理,因此SSL_read return 0直接返回0就行。由于brpc中不允许半关闭读写的情况,对于对端EOF情况只需直接close本端就行。
   另外brpc中一个连接要么一直是SSL,要么永远是非SSL,不会来回再SSL状态间切换,因此也不需要SSL_shutdown。
   `cut_into_SSL_channel`在nw=0时也会设置ssl_error,外层`DoWrite`也会捕获感知设置errno。
   从池中(`SocketPool`)拿出的Socket,是不会再次ssl_handshake的。因为连接池模式保证一次请求结束前独占这个Socket,后续拿出时其ssl_state应该set过了。
   BIO的api调用,一部分是为了证书buffer,另一部分是为了能让用户设置SSL底层读写的buffer大小,具体读写没有用BIO_read/write


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] feifeiiong edited a comment on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
feifeiiong edited a comment on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-619385096


   感谢您的回复!第四点确实是如您所说。关于1,3点,有一些不同的看法:
   DoRead 的设计确实是读到0即代表约定遇到EOF,但是SSL_read为0时的处理并不完备,以下内容摘自man:
   SSL_read    return values:
   
   '>' 0
   The read operation was successful; the return value is the number of bytes actually read from the TLS/SSL connection.
   
   0
   
   The read operation was not successful. The reason may either be a clean shutdown due to a "close notify" alert sent by the peer (in which case the SSL_RECEIVED_SHUTDOWN flag in the ssl shutdown state is set (see ssl_shutdown(3), ssl_set_shutdown(3)). It is also possible, that the peer simply shut down the underlying transport and the shutdown is incomplete. Call SSL_get_error() with the return value ret to find out, whether an error occurred or the connection was shut down cleanly ( SSL_ERROR_ZERO_RETURN 
   
   SSL_get_error Return Values:
   SSL_ERROR_ZERO_RETURN
   The TLS/SSL connection has been closed.
   SSL_ERROR_SYSCALL
   Some I/O error occurred. The OpenSSL error queue may contain more information on the error. If the error queue is empty (i.e. ERR_get_error() returns 0), ret can be used to find out more about the error: If ret == 0, an EOF was observed that violates the protocol. If ret == -1, the underlying BIO reported an I/O error (for socket I/O on Unix systems, consult errno for details).
   
   在iobuf中是这样处理的:
   ```
   const int rc = SSL_read(ssl, _block->data + _block->size, read_len);
           *ssl_error = SSL_get_error(ssl, rc);
           if (rc > 0) {
               const IOBuf::BlockRef r = { (uint32_t)_block->size, (uint32_t)rc, _block };
               _push_back_ref(r);
               _block->size += rc;
               if (_block->full()) {
                   Block* const saved_next = _block->portal_next;
                   _block->dec_ref();  // _block may be deleted
                   _block = saved_next;
               }
               nr += rc;
           } else {
               if (rc < 0) {
                   if (*ssl_error == SSL_ERROR_WANT_READ
                       || (*ssl_error == SSL_ERROR_SYSCALL
                           && BIO_fd_non_fatal_error(errno) == 1)) {
                       // Non fatal error, tell caller to read again
                       *ssl_error = SSL_ERROR_WANT_READ;
                   } else {
                       // Other errors are fatal
                       return rc;
                   }
               }
               return (nr > 0 ? nr : rc);
   ```
   在rc == 0时,返回,并置错误码,此处的错误码,有可能是SSL_ERROR_SYSCALL/SSL_ERROR_ZERO_RETURN, 在socket.cpp:
   
   int ssl_error = 0;
       ssize_t nr = _read_buf.append_from_SSL_channel(_ssl_session, &ssl_error, size_hint);
       switch (ssl_error) {
       case SSL_ERROR_NONE:  // `nr' > 0
           break;
               
       case SSL_ERROR_WANT_READ:
           // Regard this error as EAGAIN
           errno = EAGAIN;
           break;
               
       case SSL_ERROR_WANT_WRITE:
           // Disable renegotiation
           errno = EPROTO;
           return -1;
   
       default: {
           const unsigned long e = ERR_get_error();
           if (nr == 0) {
               // Socket EOF or SSL session EOF
           } else if (e != 0) {
               LOG(WARNING) << "Fail to read from ssl_fd=" << fd()
                            << ": " << SSLError(e);
               errno = ESSL;
           } else {
               // System error with corresponding errno set
               PLOG(WARNING) << "Fail to read from ssl_fd=" << fd();
           }
           break;
       }
   如上,以上错误码被default捕获,而此刻nr 有可能!=0 (iobuf.cpp : return (nr > 0 ? nr : rc);), 从而导致该EOF被忽略,导致socket无法关闭。在实际应用中,该问题已经频繁被触发,打印日志类似:
    Fail to read from ssl_fd=id:Success
   DoWrite有类似的问题,这里就不贴代码了,烦请您看看!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] feifeiiong edited a comment on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
feifeiiong edited a comment on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-619385096


   感谢您的回复!第四点确实是如您所说。关于1,3点,有一些不同的看法:
   DoRead 的设计确实是读到0即代表约定遇到EOF,但是SSL_read为0时的处理并不完备,以下内容摘自man:
   SSL_read    return values:
   
   '>' 0
   The read operation was successful; the return value is the number of bytes actually read from the TLS/SSL connection.
   
   0
   
   The read operation was not successful. The reason may either be a clean shutdown due to a "close notify" alert sent by the peer (in which case the SSL_RECEIVED_SHUTDOWN flag in the ssl shutdown state is set (see ssl_shutdown(3), ssl_set_shutdown(3)). It is also possible, that the peer simply shut down the underlying transport and the shutdown is incomplete. Call SSL_get_error() with the return value ret to find out, whether an error occurred or the connection was shut down cleanly ( SSL_ERROR_ZERO_RETURN 
   
   SSL_get_error Return Values:
   SSL_ERROR_ZERO_RETURN
   The TLS/SSL connection has been closed.
   SSL_ERROR_SYSCALL
   Some I/O error occurred. The OpenSSL error queue may contain more information on the error. If the error queue is empty (i.e. ERR_get_error() returns 0), ret can be used to find out more about the error: If ret == 0, an EOF was observed that violates the protocol. If ret == -1, the underlying BIO reported an I/O error (for socket I/O on Unix systems, consult errno for details).
   
   在iobuf中是这样处理的:
   ```
   
            const int rc = SSL_read(ssl, _block->data + _block->size, read_len);
           *ssl_error = SSL_get_error(ssl, rc);
           if (rc > 0) {
               const IOBuf::BlockRef r = { (uint32_t)_block->size, (uint32_t)rc, _block };
               _push_back_ref(r);
               _block->size += rc;
               if (_block->full()) {
                   Block* const saved_next = _block->portal_next;
                   _block->dec_ref();  // _block may be deleted
                   _block = saved_next;
               }
               nr += rc;
           } else {
               if (rc < 0) {
                   if (*ssl_error == SSL_ERROR_WANT_READ
                       || (*ssl_error == SSL_ERROR_SYSCALL
                           && BIO_fd_non_fatal_error(errno) == 1)) {
                       // Non fatal error, tell caller to read again
                       *ssl_error = SSL_ERROR_WANT_READ;
                   } else {
                       // Other errors are fatal
                       return rc;
                   }
               }
               return (nr > 0 ? nr : rc);
   ```
   在rc == 0时,返回,并置错误码,此处的错误码,有可能是SSL_ERROR_SYSCALL/SSL_ERROR_ZERO_RETURN, 在socket.cpp:
   ```
       int ssl_error = 0;
       ssize_t nr = _read_buf.append_from_SSL_channel(_ssl_session, &ssl_error, size_hint);
       switch (ssl_error) {
       case SSL_ERROR_NONE:  // `nr' > 0
           break;
               
       case SSL_ERROR_WANT_READ:
           // Regard this error as EAGAIN
           errno = EAGAIN;
           break;
               
       case SSL_ERROR_WANT_WRITE:
           // Disable renegotiation
           errno = EPROTO;
           return -1;
   
       default: {
           const unsigned long e = ERR_get_error();
           if (nr == 0) {
               // Socket EOF or SSL session EOF
           } else if (e != 0) {
               LOG(WARNING) << "Fail to read from ssl_fd=" << fd()
                            << ": " << SSLError(e);
               errno = ESSL;
           } else {
               // System error with corresponding errno set
               PLOG(WARNING) << "Fail to read from ssl_fd=" << fd();
           }
           break;
       }
   
   ```
   如上,以上错误码被default捕获,而此刻nr 有可能!=0 (iobuf.cpp : return (nr > 0 ? nr : rc);), 从而导致该EOF被忽略,导致socket无法关闭。在实际应用中,该问题已经频繁被触发,打印日志类似:
    Fail to read from ssl_fd=id:Success
   DoWrite有类似的问题,这里就不贴代码了,烦请您看看!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] feifeiiong edited a comment on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
feifeiiong edited a comment on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-619385096


   感谢您的回复!第四点确实是如您所说。关于1,3点,有一些不同的看法:
   1. DoRead 的设计确实是读到0即代表约定遇到EOF,但是SSL_read为0时的处理并不完备,以下内容摘自man:
   The following return values can occur:
   
   >0
   The read operation was successful; the return value is the number of bytes actually read from the TLS/SSL connection.
   
   0
   
   The read operation was not successful. The reason may either be a clean shutdown due to a "close notify" alert sent by the peer (in which case the SSL_RECEIVED_SHUTDOWN flag in the ssl shutdown state is set (see ssl_shutdown(3), ssl_set_shutdown(3)). It is also possible, that the peer simply shut down the underlying transport and the shutdown is incomplete. Call SSL_get_error() with the return value ret to find out, whether an error occurred or the connection was shut down cleanly ( SSL_ERROR_ZERO_RETURN 
   
   SSL_get_error:
   SSL_ERROR_ZERO_RETURN
   The TLS/SSL connection has been closed.
   SSL_ERROR_SYSCALL
   Some I/O error occurred. The OpenSSL error queue may contain more information on the error. If the error queue is empty (i.e. ERR_get_error() returns 0), ret can be used to find out more about the error: If ret == 0, an EOF was observed that violates the protocol. If ret == -1, the underlying BIO reported an I/O error (for socket I/O on Unix systems, consult errno for details).
   
   在iobuf中是这样处理的:
   ```
   const int rc = SSL_read(ssl, _block->data + _block->size, read_len);
           *ssl_error = SSL_get_error(ssl, rc);
           if (rc > 0) {
               const IOBuf::BlockRef r = { (uint32_t)_block->size, (uint32_t)rc, _block };
               _push_back_ref(r);
               _block->size += rc;
               if (_block->full()) {
                   Block* const saved_next = _block->portal_next;
                   _block->dec_ref();  // _block may be deleted
                   _block = saved_next;
               }
               nr += rc;
           } else {
               if (rc < 0) {
                   if (*ssl_error == SSL_ERROR_WANT_READ
                       || (*ssl_error == SSL_ERROR_SYSCALL
                           && BIO_fd_non_fatal_error(errno) == 1)) {
                       // Non fatal error, tell caller to read again
                       *ssl_error = SSL_ERROR_WANT_READ;
                   } else {
                       // Other errors are fatal
                       return rc;
                   }
               }
               return (nr > 0 ? nr : rc);
   ```
   在rc == 0时,返回,并置错误码,此处的错误码,有可能是SSL_ERROR_SYSCALL/SSL_ERROR_ZERO_RETURN, 在socket.cpp:
   
   int ssl_error = 0;
       ssize_t nr = _read_buf.append_from_SSL_channel(_ssl_session, &ssl_error, size_hint);
       switch (ssl_error) {
       case SSL_ERROR_NONE:  // `nr' > 0
           break;
               
       case SSL_ERROR_WANT_READ:
           // Regard this error as EAGAIN
           errno = EAGAIN;
           break;
               
       case SSL_ERROR_WANT_WRITE:
           // Disable renegotiation
           errno = EPROTO;
           return -1;
   
       default: {
           const unsigned long e = ERR_get_error();
           if (nr == 0) {
               // Socket EOF or SSL session EOF
           } else if (e != 0) {
               LOG(WARNING) << "Fail to read from ssl_fd=" << fd()
                            << ": " << SSLError(e);
               errno = ESSL;
           } else {
               // System error with corresponding errno set
               PLOG(WARNING) << "Fail to read from ssl_fd=" << fd();
           }
           break;
       }
   如上,以上错误码被default捕获,而此刻nr 有可能!=0 (iobuf.cpp : return (nr > 0 ? nr : rc);), 从而导致该EOF被忽略,导致socket无法关闭。在实际应用中,该问题已经频繁被触发,DoWrite有类似的问题,这里就不贴代码了,烦请您看看!相关联的问题是:


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] memory-overflow commented on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
memory-overflow commented on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-749389923


   我也遇到一样的问题。
   
   ![image](https://user-images.githubusercontent.com/15645203/102860542-0ae58180-4469-11eb-8fbd-29ddbc45e707.png)
   
   会一只打印类似的日志,然后服务器磁盘被这个日志写满了。也是因为这个bug引起的吗?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] old-bear commented on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
old-bear commented on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-620589519


   感谢指正,我看了下 DoRead中确实存在bug,但具体问题好像与你描述的不同:
   1. `append_from_SSL_channel`中不断调用`SSL_read`,已经读入了一些内容,`nr>0`,此时最后一次`SSL_read`返回`rc=0`,而`append_from_SSL_channel`返回`nr (>0)`
   2. `DoRead`中进入default分支,打印了`Fail to read from ssl_fd=id`,返回`nr (>0)`
   3. `InputMessenger`中调用`DoRead`得到`nr>0`,处理消息,然后循环`while (!read_eof)`,继续调用`DoRead -> SSL_read`,此时则rc=0,一路return出去,被当做eof
   
   因此,我这边看这个bug主要在于会错误打印一句warning日志,应该不影响fd关闭或者fd泄漏
   如果你这边有具体复现场景描述,或者相关日志,希望能提供一下,谢谢
   
   另外,SSL握手目前被视作一种**连通性**指标,如果server拒绝与client建立SSL(无论何种因素),那么后续任何请求肯定无法成功。


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] feifeiiong edited a comment on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
feifeiiong edited a comment on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-619385096


   感谢您的回复!第四点确实是如您所说。关于1,3点,有一些不同的看法:
   1. DoRead 的设计确实是读到0即代表约定遇到EOF,但是SSL可能存在读到非0数据但表示为tcp关闭的情况,以下摘自man:


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] feifeiiong commented on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
feifeiiong commented on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-619396997


   另外,关于main socket,健康检查时,如果设置了SSL options,会尝试进行ssl连接。想问下这个操作是否有必要?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] memory-overflow edited a comment on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
memory-overflow edited a comment on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-749389923


   我也遇到一样的问题。
   
   ![image](https://user-images.githubusercontent.com/15645203/102860542-0ae58180-4469-11eb-8fbd-29ddbc45e707.png)
   
   会一直打印类似的日志,然后服务器磁盘被这个日志写满了。也是因为这个bug引起的吗?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] feifeiiong edited a comment on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
feifeiiong edited a comment on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-619385096


   感谢您的回复!第四点确实是如您所说。关于1,3点,有一些不同的看法:
   DoRead 的设计确实是读到0即代表约定遇到EOF,但是SSL_read为0时的处理并不完备,以下内容摘自man:
   SSL_read    return values:
   
   '>' 0
   The read operation was successful; the return value is the number of bytes actually read from the TLS/SSL connection.
   
   0
   
   The read operation was not successful. The reason may either be a clean shutdown due to a "close notify" alert sent by the peer (in which case the SSL_RECEIVED_SHUTDOWN flag in the ssl shutdown state is set (see ssl_shutdown(3), ssl_set_shutdown(3)). It is also possible, that the peer simply shut down the underlying transport and the shutdown is incomplete. Call SSL_get_error() with the return value ret to find out, whether an error occurred or the connection was shut down cleanly ( SSL_ERROR_ZERO_RETURN 
   
   SSL_get_error Return Values:
   SSL_ERROR_ZERO_RETURN
   The TLS/SSL connection has been closed.
   SSL_ERROR_SYSCALL
   Some I/O error occurred. The OpenSSL error queue may contain more information on the error. If the error queue is empty (i.e. ERR_get_error() returns 0), ret can be used to find out more about the error: If ret == 0, an EOF was observed that violates the protocol. If ret == -1, the underlying BIO reported an I/O error (for socket I/O on Unix systems, consult errno for details).
   
   在iobuf中是这样处理的:
   ```
   
            const int rc = SSL_read(ssl, _block->data + _block->size, read_len);
           *ssl_error = SSL_get_error(ssl, rc);
           if (rc > 0) {
               const IOBuf::BlockRef r = { (uint32_t)_block->size, (uint32_t)rc, _block };
               _push_back_ref(r);
               _block->size += rc;
               if (_block->full()) {
                   Block* const saved_next = _block->portal_next;
                   _block->dec_ref();  // _block may be deleted
                   _block = saved_next;
               }
               nr += rc;
           } else {
               if (rc < 0) {
                   if (*ssl_error == SSL_ERROR_WANT_READ
                       || (*ssl_error == SSL_ERROR_SYSCALL
                           && BIO_fd_non_fatal_error(errno) == 1)) {
                       // Non fatal error, tell caller to read again
                       *ssl_error = SSL_ERROR_WANT_READ;
                   } else {
                       // Other errors are fatal
                       return rc;
                   }
               }
               return (nr > 0 ? nr : rc);
   ```
   在rc == 0时,返回,并置错误码,此处的错误码,有可能是SSL_ERROR_SYSCALL/SSL_ERROR_ZERO_RETURN, 在socket.cpp:
   ```
       int ssl_error = 0;
       ssize_t nr = _read_buf.append_from_SSL_channel(_ssl_session, &ssl_error, size_hint);
       switch (ssl_error) {
       case SSL_ERROR_NONE:  // `nr' > 0
           break;
               
       case SSL_ERROR_WANT_READ:
           // Regard this error as EAGAIN
           errno = EAGAIN;
           break;
               
       case SSL_ERROR_WANT_WRITE:
           // Disable renegotiation
           errno = EPROTO;
           return -1;
   
       default: {
           const unsigned long e = ERR_get_error();
           if (nr == 0) {
               // Socket EOF or SSL session EOF
           } else if (e != 0) {
               LOG(WARNING) << "Fail to read from ssl_fd=" << fd()
                            << ": " << SSLError(e);
               errno = ESSL;
           } else {
               // System error with corresponding errno set
               PLOG(WARNING) << "Fail to read from ssl_fd=" << fd();
           }
           break;
       }
   如上,以上错误码被default捕获,而此刻nr 有可能!=0 (iobuf.cpp : return (nr > 0 ? nr : rc);), 从而导致该EOF被忽略,导致socket无法关闭。在实际应用中,该问题已经频繁被触发,打印日志类似:
    Fail to read from ssl_fd=id:Success
   DoWrite有类似的问题,这里就不贴代码了,烦请您看看!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] feifeiiong commented on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
feifeiiong commented on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-618985736


   这个问题有什么进展吗,各位大佬?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] feifeiiong edited a comment on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
feifeiiong edited a comment on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-619385096


   感谢您的回复!第四点确实是如您所说。关于1,3点,有一些不同的看法:
   1. DoRead 的设计确实是读到0即代表约定遇到EOF,但是SSL_read为0时的处理并不完备,以下内容摘自man:
   SSL_read    return values:
   
   '>' 0
   The read operation was successful; the return value is the number of bytes actually read from the TLS/SSL connection.
   
   0
   
   The read operation was not successful. The reason may either be a clean shutdown due to a "close notify" alert sent by the peer (in which case the SSL_RECEIVED_SHUTDOWN flag in the ssl shutdown state is set (see ssl_shutdown(3), ssl_set_shutdown(3)). It is also possible, that the peer simply shut down the underlying transport and the shutdown is incomplete. Call SSL_get_error() with the return value ret to find out, whether an error occurred or the connection was shut down cleanly ( SSL_ERROR_ZERO_RETURN 
   
   SSL_get_error Return Values:
   SSL_ERROR_ZERO_RETURN
   The TLS/SSL connection has been closed.
   SSL_ERROR_SYSCALL
   Some I/O error occurred. The OpenSSL error queue may contain more information on the error. If the error queue is empty (i.e. ERR_get_error() returns 0), ret can be used to find out more about the error: If ret == 0, an EOF was observed that violates the protocol. If ret == -1, the underlying BIO reported an I/O error (for socket I/O on Unix systems, consult errno for details).
   
   在iobuf中是这样处理的:
   ```
   const int rc = SSL_read(ssl, _block->data + _block->size, read_len);
           *ssl_error = SSL_get_error(ssl, rc);
           if (rc > 0) {
               const IOBuf::BlockRef r = { (uint32_t)_block->size, (uint32_t)rc, _block };
               _push_back_ref(r);
               _block->size += rc;
               if (_block->full()) {
                   Block* const saved_next = _block->portal_next;
                   _block->dec_ref();  // _block may be deleted
                   _block = saved_next;
               }
               nr += rc;
           } else {
               if (rc < 0) {
                   if (*ssl_error == SSL_ERROR_WANT_READ
                       || (*ssl_error == SSL_ERROR_SYSCALL
                           && BIO_fd_non_fatal_error(errno) == 1)) {
                       // Non fatal error, tell caller to read again
                       *ssl_error = SSL_ERROR_WANT_READ;
                   } else {
                       // Other errors are fatal
                       return rc;
                   }
               }
               return (nr > 0 ? nr : rc);
   ```
   在rc == 0时,返回,并置错误码,此处的错误码,有可能是SSL_ERROR_SYSCALL/SSL_ERROR_ZERO_RETURN, 在socket.cpp:
   
   int ssl_error = 0;
       ssize_t nr = _read_buf.append_from_SSL_channel(_ssl_session, &ssl_error, size_hint);
       switch (ssl_error) {
       case SSL_ERROR_NONE:  // `nr' > 0
           break;
               
       case SSL_ERROR_WANT_READ:
           // Regard this error as EAGAIN
           errno = EAGAIN;
           break;
               
       case SSL_ERROR_WANT_WRITE:
           // Disable renegotiation
           errno = EPROTO;
           return -1;
   
       default: {
           const unsigned long e = ERR_get_error();
           if (nr == 0) {
               // Socket EOF or SSL session EOF
           } else if (e != 0) {
               LOG(WARNING) << "Fail to read from ssl_fd=" << fd()
                            << ": " << SSLError(e);
               errno = ESSL;
           } else {
               // System error with corresponding errno set
               PLOG(WARNING) << "Fail to read from ssl_fd=" << fd();
           }
           break;
       }
   如上,以上错误码被default捕获,而此刻nr 有可能!=0 (iobuf.cpp : return (nr > 0 ? nr : rc);), 从而导致该EOF被忽略,导致socket无法关闭。在实际应用中,该问题已经频繁被触发,打印日志类似:
    Fail to read from ssl_fd=id:Success
   DoWrite有类似的问题,这里就不贴代码了,烦请您看看!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] feifeiiong edited a comment on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
feifeiiong edited a comment on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-619385096


   感谢您的回复!第四点确实是如您所说。关于1,3点,有一些不同的看法:
   1. DoRead 的设计确实是读到0即代表约定遇到EOF,但是SSL_read为0时的处理并不完备,以下内容摘自man:
   The following return values can occur:
   
   >0
   The read operation was successful; the return value is the number of bytes actually read from the TLS/SSL connection.
   
   0
   
   The read operation was not successful. The reason may either be a clean shutdown due to a "close notify" alert sent by the peer (in which case the SSL_RECEIVED_SHUTDOWN flag in the ssl shutdown state is set (see ssl_shutdown(3), ssl_set_shutdown(3)). It is also possible, that the peer simply shut down the underlying transport and the shutdown is incomplete. Call SSL_get_error() with the return value ret to find out, whether an error occurred or the connection was shut down cleanly ( SSL_ERROR_ZERO_RETURN 
   
   SSL_get_error:
   SSL_ERROR_ZERO_RETURN
   The TLS/SSL connection has been closed.
   SSL_ERROR_SYSCALL
   Some I/O error occurred. The OpenSSL error queue may contain more information on the error. If the error queue is empty (i.e. ERR_get_error() returns 0), ret can be used to find out more about the error: If ret == 0, an EOF was observed that violates the protocol. If ret == -1, the underlying BIO reported an I/O error (for socket I/O on Unix systems, consult errno for details).
   
   在iobuf中是这样处理的:
   ```
   const int rc = SSL_read(ssl, _block->data + _block->size, read_len);
           *ssl_error = SSL_get_error(ssl, rc);
           if (rc > 0) {
               const IOBuf::BlockRef r = { (uint32_t)_block->size, (uint32_t)rc, _block };
               _push_back_ref(r);
               _block->size += rc;
               if (_block->full()) {
                   Block* const saved_next = _block->portal_next;
                   _block->dec_ref();  // _block may be deleted
                   _block = saved_next;
               }
               nr += rc;
           } else {
               if (rc < 0) {
                   if (*ssl_error == SSL_ERROR_WANT_READ
                       || (*ssl_error == SSL_ERROR_SYSCALL
                           && BIO_fd_non_fatal_error(errno) == 1)) {
                       // Non fatal error, tell caller to read again
                       *ssl_error = SSL_ERROR_WANT_READ;
                   } else {
                       // Other errors are fatal
                       return rc;
                   }
               }
               return (nr > 0 ? nr : rc);
   ```
   在rc == 0时,返回,并置错误码,此处的错误码,有可能是SSL_ERROR_SYSCALL/SSL_ERROR_ZERO_RETURN, 在socket.cpp:
   
   int ssl_error = 0;
       ssize_t nr = _read_buf.append_from_SSL_channel(_ssl_session, &ssl_error, size_hint);
       switch (ssl_error) {
       case SSL_ERROR_NONE:  // `nr' > 0
           break;
               
       case SSL_ERROR_WANT_READ:
           // Regard this error as EAGAIN
           errno = EAGAIN;
           break;
               
       case SSL_ERROR_WANT_WRITE:
           // Disable renegotiation
           errno = EPROTO;
           return -1;
   
       default: {
           const unsigned long e = ERR_get_error();
           if (nr == 0) {
               // Socket EOF or SSL session EOF
           } else if (e != 0) {
               LOG(WARNING) << "Fail to read from ssl_fd=" << fd()
                            << ": " << SSLError(e);
               errno = ESSL;
           } else {
               // System error with corresponding errno set
               PLOG(WARNING) << "Fail to read from ssl_fd=" << fd();
           }
           break;
       }
   如上,以上错误码被default捕获,而此刻nr 有可能!=0 (iobuf.cpp : return (nr > 0 ? nr : rc);), 从而导致该EOF被忽略,导致socket无法关闭。在实际应用中,该问题已经频繁被触发,打印日志类似:
    Fail to read from ssl_fd=id:Success
   DoWrite有类似的问题,这里就不贴代码了,烦请您看看!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] feifeiiong edited a comment on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
feifeiiong edited a comment on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-619385096


   感谢您的回复!第四点确实是如您所说。关于1,3点,有一些不同的看法:
   1. DoRead 的设计确实是读到0即代表约定遇到EOF,但是SSL_read为0时的处理并不完备,以下内容摘自man:
   SSL_read    return values:
   
   >0
   The read operation was successful; the return value is the number of bytes actually read from the TLS/SSL connection.
   
   0
   
   The read operation was not successful. The reason may either be a clean shutdown due to a "close notify" alert sent by the peer (in which case the SSL_RECEIVED_SHUTDOWN flag in the ssl shutdown state is set (see ssl_shutdown(3), ssl_set_shutdown(3)). It is also possible, that the peer simply shut down the underlying transport and the shutdown is incomplete. Call SSL_get_error() with the return value ret to find out, whether an error occurred or the connection was shut down cleanly ( SSL_ERROR_ZERO_RETURN 
   
   SSL_get_error Return Values:
   SSL_ERROR_ZERO_RETURN
   The TLS/SSL connection has been closed.
   SSL_ERROR_SYSCALL
   Some I/O error occurred. The OpenSSL error queue may contain more information on the error. If the error queue is empty (i.e. ERR_get_error() returns 0), ret can be used to find out more about the error: If ret == 0, an EOF was observed that violates the protocol. If ret == -1, the underlying BIO reported an I/O error (for socket I/O on Unix systems, consult errno for details).
   
   在iobuf中是这样处理的:
   ```
   const int rc = SSL_read(ssl, _block->data + _block->size, read_len);
           *ssl_error = SSL_get_error(ssl, rc);
           if (rc > 0) {
               const IOBuf::BlockRef r = { (uint32_t)_block->size, (uint32_t)rc, _block };
               _push_back_ref(r);
               _block->size += rc;
               if (_block->full()) {
                   Block* const saved_next = _block->portal_next;
                   _block->dec_ref();  // _block may be deleted
                   _block = saved_next;
               }
               nr += rc;
           } else {
               if (rc < 0) {
                   if (*ssl_error == SSL_ERROR_WANT_READ
                       || (*ssl_error == SSL_ERROR_SYSCALL
                           && BIO_fd_non_fatal_error(errno) == 1)) {
                       // Non fatal error, tell caller to read again
                       *ssl_error = SSL_ERROR_WANT_READ;
                   } else {
                       // Other errors are fatal
                       return rc;
                   }
               }
               return (nr > 0 ? nr : rc);
   ```
   在rc == 0时,返回,并置错误码,此处的错误码,有可能是SSL_ERROR_SYSCALL/SSL_ERROR_ZERO_RETURN, 在socket.cpp:
   
   int ssl_error = 0;
       ssize_t nr = _read_buf.append_from_SSL_channel(_ssl_session, &ssl_error, size_hint);
       switch (ssl_error) {
       case SSL_ERROR_NONE:  // `nr' > 0
           break;
               
       case SSL_ERROR_WANT_READ:
           // Regard this error as EAGAIN
           errno = EAGAIN;
           break;
               
       case SSL_ERROR_WANT_WRITE:
           // Disable renegotiation
           errno = EPROTO;
           return -1;
   
       default: {
           const unsigned long e = ERR_get_error();
           if (nr == 0) {
               // Socket EOF or SSL session EOF
           } else if (e != 0) {
               LOG(WARNING) << "Fail to read from ssl_fd=" << fd()
                            << ": " << SSLError(e);
               errno = ESSL;
           } else {
               // System error with corresponding errno set
               PLOG(WARNING) << "Fail to read from ssl_fd=" << fd();
           }
           break;
       }
   如上,以上错误码被default捕获,而此刻nr 有可能!=0 (iobuf.cpp : return (nr > 0 ? nr : rc);), 从而导致该EOF被忽略,导致socket无法关闭。在实际应用中,该问题已经频繁被触发,打印日志类似:
    Fail to read from ssl_fd=id:Success
   DoWrite有类似的问题,这里就不贴代码了,烦请您看看!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] feifeiiong edited a comment on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
feifeiiong edited a comment on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-619385096


   感谢您的回复!第四点确实是如您所说。关于1,3点,有一些不同的看法:
   1. DoRead 的设计确实是读到0即代表约定遇到EOF,但是SSL_read为0时的处理并不完备,以下内容摘自man:
   SSL_read    return values:
   The following return values can occur:
   
   >0
   The read operation was successful; the return value is the number of bytes actually read from the TLS/SSL connection.
   
   0
   
   The read operation was not successful. The reason may either be a clean shutdown due to a "close notify" alert sent by the peer (in which case the SSL_RECEIVED_SHUTDOWN flag in the ssl shutdown state is set (see ssl_shutdown(3), ssl_set_shutdown(3)). It is also possible, that the peer simply shut down the underlying transport and the shutdown is incomplete. Call SSL_get_error() with the return value ret to find out, whether an error occurred or the connection was shut down cleanly ( SSL_ERROR_ZERO_RETURN 
   
   SSL_get_error Return Values:
   SSL_ERROR_ZERO_RETURN
   The TLS/SSL connection has been closed.
   SSL_ERROR_SYSCALL
   Some I/O error occurred. The OpenSSL error queue may contain more information on the error. If the error queue is empty (i.e. ERR_get_error() returns 0), ret can be used to find out more about the error: If ret == 0, an EOF was observed that violates the protocol. If ret == -1, the underlying BIO reported an I/O error (for socket I/O on Unix systems, consult errno for details).
   
   在iobuf中是这样处理的:
   ```
   const int rc = SSL_read(ssl, _block->data + _block->size, read_len);
           *ssl_error = SSL_get_error(ssl, rc);
           if (rc > 0) {
               const IOBuf::BlockRef r = { (uint32_t)_block->size, (uint32_t)rc, _block };
               _push_back_ref(r);
               _block->size += rc;
               if (_block->full()) {
                   Block* const saved_next = _block->portal_next;
                   _block->dec_ref();  // _block may be deleted
                   _block = saved_next;
               }
               nr += rc;
           } else {
               if (rc < 0) {
                   if (*ssl_error == SSL_ERROR_WANT_READ
                       || (*ssl_error == SSL_ERROR_SYSCALL
                           && BIO_fd_non_fatal_error(errno) == 1)) {
                       // Non fatal error, tell caller to read again
                       *ssl_error = SSL_ERROR_WANT_READ;
                   } else {
                       // Other errors are fatal
                       return rc;
                   }
               }
               return (nr > 0 ? nr : rc);
   ```
   在rc == 0时,返回,并置错误码,此处的错误码,有可能是SSL_ERROR_SYSCALL/SSL_ERROR_ZERO_RETURN, 在socket.cpp:
   
   int ssl_error = 0;
       ssize_t nr = _read_buf.append_from_SSL_channel(_ssl_session, &ssl_error, size_hint);
       switch (ssl_error) {
       case SSL_ERROR_NONE:  // `nr' > 0
           break;
               
       case SSL_ERROR_WANT_READ:
           // Regard this error as EAGAIN
           errno = EAGAIN;
           break;
               
       case SSL_ERROR_WANT_WRITE:
           // Disable renegotiation
           errno = EPROTO;
           return -1;
   
       default: {
           const unsigned long e = ERR_get_error();
           if (nr == 0) {
               // Socket EOF or SSL session EOF
           } else if (e != 0) {
               LOG(WARNING) << "Fail to read from ssl_fd=" << fd()
                            << ": " << SSLError(e);
               errno = ESSL;
           } else {
               // System error with corresponding errno set
               PLOG(WARNING) << "Fail to read from ssl_fd=" << fd();
           }
           break;
       }
   如上,以上错误码被default捕获,而此刻nr 有可能!=0 (iobuf.cpp : return (nr > 0 ? nr : rc);), 从而导致该EOF被忽略,导致socket无法关闭。在实际应用中,该问题已经频繁被触发,打印日志类似:
    Fail to read from ssl_fd=id:Success
   DoWrite有类似的问题,这里就不贴代码了,烦请您看看!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


[GitHub] [incubator-brpc] feifeiiong edited a comment on issue #1096: SSL 相关问题。

Posted by GitBox <gi...@apache.org>.
feifeiiong edited a comment on issue #1096:
URL: https://github.com/apache/incubator-brpc/issues/1096#issuecomment-619385096


   感谢您的回复!第四点确实是如您所说。关于1,3点,有一些不同的看法:
   1. DoRead 的设计确实是读到0即代表约定遇到EOF,但是SSL_read为0时的处理并不完备,以下内容摘自man:
   SSL_read    return values:
   
   > 0
   The read operation was successful; the return value is the number of bytes actually read from the TLS/SSL connection.
   
   0
   
   The read operation was not successful. The reason may either be a clean shutdown due to a "close notify" alert sent by the peer (in which case the SSL_RECEIVED_SHUTDOWN flag in the ssl shutdown state is set (see ssl_shutdown(3), ssl_set_shutdown(3)). It is also possible, that the peer simply shut down the underlying transport and the shutdown is incomplete. Call SSL_get_error() with the return value ret to find out, whether an error occurred or the connection was shut down cleanly ( SSL_ERROR_ZERO_RETURN 
   
   SSL_get_error Return Values:
   SSL_ERROR_ZERO_RETURN
   The TLS/SSL connection has been closed.
   SSL_ERROR_SYSCALL
   Some I/O error occurred. The OpenSSL error queue may contain more information on the error. If the error queue is empty (i.e. ERR_get_error() returns 0), ret can be used to find out more about the error: If ret == 0, an EOF was observed that violates the protocol. If ret == -1, the underlying BIO reported an I/O error (for socket I/O on Unix systems, consult errno for details).
   
   在iobuf中是这样处理的:
   ```
   const int rc = SSL_read(ssl, _block->data + _block->size, read_len);
           *ssl_error = SSL_get_error(ssl, rc);
           if (rc > 0) {
               const IOBuf::BlockRef r = { (uint32_t)_block->size, (uint32_t)rc, _block };
               _push_back_ref(r);
               _block->size += rc;
               if (_block->full()) {
                   Block* const saved_next = _block->portal_next;
                   _block->dec_ref();  // _block may be deleted
                   _block = saved_next;
               }
               nr += rc;
           } else {
               if (rc < 0) {
                   if (*ssl_error == SSL_ERROR_WANT_READ
                       || (*ssl_error == SSL_ERROR_SYSCALL
                           && BIO_fd_non_fatal_error(errno) == 1)) {
                       // Non fatal error, tell caller to read again
                       *ssl_error = SSL_ERROR_WANT_READ;
                   } else {
                       // Other errors are fatal
                       return rc;
                   }
               }
               return (nr > 0 ? nr : rc);
   ```
   在rc == 0时,返回,并置错误码,此处的错误码,有可能是SSL_ERROR_SYSCALL/SSL_ERROR_ZERO_RETURN, 在socket.cpp:
   
   int ssl_error = 0;
       ssize_t nr = _read_buf.append_from_SSL_channel(_ssl_session, &ssl_error, size_hint);
       switch (ssl_error) {
       case SSL_ERROR_NONE:  // `nr' > 0
           break;
               
       case SSL_ERROR_WANT_READ:
           // Regard this error as EAGAIN
           errno = EAGAIN;
           break;
               
       case SSL_ERROR_WANT_WRITE:
           // Disable renegotiation
           errno = EPROTO;
           return -1;
   
       default: {
           const unsigned long e = ERR_get_error();
           if (nr == 0) {
               // Socket EOF or SSL session EOF
           } else if (e != 0) {
               LOG(WARNING) << "Fail to read from ssl_fd=" << fd()
                            << ": " << SSLError(e);
               errno = ESSL;
           } else {
               // System error with corresponding errno set
               PLOG(WARNING) << "Fail to read from ssl_fd=" << fd();
           }
           break;
       }
   如上,以上错误码被default捕获,而此刻nr 有可能!=0 (iobuf.cpp : return (nr > 0 ? nr : rc);), 从而导致该EOF被忽略,导致socket无法关闭。在实际应用中,该问题已经频繁被触发,打印日志类似:
    Fail to read from ssl_fd=id:Success
   DoWrite有类似的问题,这里就不贴代码了,烦请您看看!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org