fix: SSLDataEvent's fd is 0 Error #642

yuweizzz · 2024-09-30T07:34:54Z

Fixes: #596

this error happend because application use BIO instead of set socket fd into the SSL layers.

the default fd value in SSLDataEvent struct is 0, When application use SSL_set_fd, the error will not happend, we can get corret fd value.

When application use SSL_set_bio, the fd value in SSLDataEvent struct will keep default value.

App Example:

# curl v7.74.0 lib/vtls/openssl.c
static CURLcode ossl_connect_step1(struct connectdata *conn, int sockindex) {
......
    if(!SSL_set_fd(backend->handle, (int)sockfd)) {
    /* pass the raw socket into the SSL layers */
    failf(data, "SSL: SSL_set_fd failed: %s",
          ossl_strerror(ERR_get_error(), error_buffer, sizeof(error_buffer)));
    return CURLE_SSL_CONNECT_ERROR;
  }

  connssl->connecting_state = ssl_connect_2;

  return CURLE_OK;
}

# curl v7.88.1 lib/vtls/openssl.c
static CURLcode ossl_connect_step1(struct Curl_cfilter *cf, struct Curl_easy *data) {
......
  backend->bio_method = bio_cf_method_create();
  if(!backend->bio_method)
    return CURLE_OUT_OF_MEMORY;
  bio = BIO_new(backend->bio_method);
  if(!bio)
    return CURLE_OUT_OF_MEMORY;

  BIO_set_data(bio, cf);
#ifdef HAVE_SSL_SET0_WBIO
  /* with OpenSSL v1.1.1 we get an alternative to SSL_set_bio() that works
   * without backward compat quirks. Every call takes one reference, so we
   * up it and pass. SSL* then owns it and will free.
   * We check on the function in configure, since libressl and friends
   * each have their own versions to add support for this. */
  BIO_up_ref(bio);
  SSL_set0_rbio(backend->handle, bio);
  SSL_set0_wbio(backend->handle, bio);
#else
  SSL_set_bio(backend->handle, bio, bio);
#endif
  connssl->connecting_state = ssl_connect_2;

  return CURLE_OK;
}

yuweizzz · 2024-09-30T13:15:07Z

if we get corret fd value, means we are not in BIO mode.

/* There are the classes of BIOs */
# define BIO_TYPE_DESCRIPTOR     0x0100 /* socket, fd, connect or accept */
# define BIO_TYPE_FILTER         0x0200
# define BIO_TYPE_SOURCE_SINK    0x0400

/* These are the 'types' of BIOs */
# define BIO_TYPE_NONE             0
# define BIO_TYPE_MEM            ( 1|BIO_TYPE_SOURCE_SINK)
# define BIO_TYPE_FILE           ( 2|BIO_TYPE_SOURCE_SINK)

# define BIO_TYPE_FD             ( 4|BIO_TYPE_SOURCE_SINK|BIO_TYPE_DESCRIPTOR)
# define BIO_TYPE_SOCKET         ( 5|BIO_TYPE_SOURCE_SINK|BIO_TYPE_DESCRIPTOR)
# define BIO_TYPE_NULL           ( 6|BIO_TYPE_SOURCE_SINK)
# define BIO_TYPE_SSL            ( 7|BIO_TYPE_FILTER)
# define BIO_TYPE_MD             ( 8|BIO_TYPE_FILTER)
# define BIO_TYPE_BUFFER         ( 9|BIO_TYPE_FILTER)
# define BIO_TYPE_CIPHER         (10|BIO_TYPE_FILTER)
# define BIO_TYPE_BASE64         (11|BIO_TYPE_FILTER)
# define BIO_TYPE_CONNECT        (12|BIO_TYPE_SOURCE_SINK|BIO_TYPE_DESCRIPTOR)
# define BIO_TYPE_ACCEPT         (13|BIO_TYPE_SOURCE_SINK|BIO_TYPE_DESCRIPTOR)

# define BIO_TYPE_NBIO_TEST      (16|BIO_TYPE_FILTER)/* server proxy BIO */
# define BIO_TYPE_NULL_FILTER    (17|BIO_TYPE_FILTER)
# define BIO_TYPE_BIO            (19|BIO_TYPE_SOURCE_SINK)/* half a BIO pair */
# define BIO_TYPE_LINEBUFFER     (20|BIO_TYPE_FILTER)
# define BIO_TYPE_DGRAM          (21|BIO_TYPE_SOURCE_SINK|BIO_TYPE_DESCRIPTOR)
# define BIO_TYPE_ASN1           (22|BIO_TYPE_FILTER)
# define BIO_TYPE_COMP           (23|BIO_TYPE_FILTER)
# ifndef OPENSSL_NO_SCTP
#  define BIO_TYPE_DGRAM_SCTP    (24|BIO_TYPE_SOURCE_SINK|BIO_TYPE_DESCRIPTOR)
# endif
# define BIO_TYPE_CORE_TO_PROV   (25|BIO_TYPE_SOURCE_SINK)
# define BIO_TYPE_DGRAM_PAIR     (26|BIO_TYPE_SOURCE_SINK)
# define BIO_TYPE_DGRAM_MEM      (27|BIO_TYPE_SOURCE_SINK)

cfc4n · 2024-09-30T14:11:29Z

if we get corret fd value, means we are not in BIO mode.

If the value of fd is 0, can it be determined that it definitely isn't BIO mode?

Additionally, from the solution you've fixed, if fd is zero, then this result should be disregarded. By doing so, would it miss any messages? Does this imply that all BIO mode information with an fd of 0 can be ignored and will always be overwritten by new event that is not zero?

cfc4n · 2024-10-01T17:47:49Z

kern/openssl.h

        s32 version = active_ssl_buf_t->version;
        bpf_probe_read(&buf, sizeof(const char*), &active_ssl_buf_t->buf);
-        process_SSL_data(ctx, current_pid_tgid, kSSLRead, buf, fd, version);
+        process_SSL_data(ctx, current_pid_tgid, kSSLRead, buf, fd, version, is_set_fd);


When is_set_fd is 0, can the event be discarded within the kernel?

yuweizzz · 2024-10-03T08:44:35Z

Hi, @cfc4n, I found a new way to determine BIO type, but this is a huge pull request. Now my dev environment is poor; I will finish this after vacation.

mooncityorg

# define BIO_TYPE_SOCKET         ( 5|BIO_TYPE_SOURCE_SINK|BIO_TYPE_DESCRIPTOR)
# define BIO_TYPE_NULL           ( 6|BIO_TYPE_SOURCE_SINK)
# define BIO_TYPE_SSL            ( 7|BIO_TYPE_FILTER)
# define BIO_TYPE_MD             ( 8|BIO_TYPE_FILTER)
# define BIO_TYPE_BUFFER         ( 9|BIO_TYPE_FILTER)
# define BIO_TYPE_CIPHER         (10|BIO_TYPE_FILTER)

cfc4n · 2024-10-11T15:06:13Z

user/module/probe_openssl.go

@@ -648,12 +648,13 @@ func (m *MOpenSSLProbe) Dispatcher(eventStruct event.IEventStruct) {
 }

 func (m *MOpenSSLProbe) dumpSslData(eventStruct *event.SSLDataEvent) {
-	if eventStruct.Fd <= 0 {
+	// BIO_TYPE_SOURCE_SINK|BIO_TYPE_DESCRIPTOR = 0x0400|0x0100 = 1280
+	if eventStruct.Fd <= 0 && eventStruct.BioType > 1280 {


use constant please.

cfc4n · 2024-10-11T15:11:49Z

kern/openssl_3_3_0_kern.c

@@ -68,6 +74,6 @@
 #define SSL_ST_RBIO SSL_CONNECTION_ST_RBIO

 #include "openssl.h"
-#include "openssl_masterkey_3.2.h"
+#include "openssl_masterkey_3.3.h"


Why did it change from 3.2 to 3.3?
As mentioned in

ecapture/utils/openssl_offset_3.3.sh

Line 24 in 7fc3da9

# openssl 3.3.* 跟 3.2.* 的offset一致。

, both 3.3 and 3.2 use the 3.2 header file.

I will check it again, maybe it was changed by utils srcipt.

cfc4n · 2024-10-11T15:13:43Z

kern/openssl.h

+        debug_bpf_printk(
+            "(OPENSSL) bpf_probe_read ssl_rbio_method_ptr failed, ret: %d\n",
+            ret);
+        return 0;


If the method and type of the BIO are not required, then I think it is possible to continue with the following process instead of returning 0.

those value didn't take effect in SSL_read/SSL_write, we can just make return 0 to // return 0.

user/module/probe_openssl.go

cfc4n · 2024-10-13T01:51:37Z

kern/openssl.h

+        debug_bpf_printk(
+            "(OPENSSL) bpf_probe_read ssl_wbio_method_ptr failed, ret: %d\n",
+            ret);
+        // return 0;


如果ssl_wbio_method_addr 获取失败，应该不能是简单的注释return 0，下面有使用ssl_wbio_method_addr，逻辑也会影响到，需要把下面的逻辑放到获取成功的逻辑分支里。针对BIO相关获取，单独剥离一个函数吧，逻辑清晰，也可以更早return。

yuweizzz · 2024-10-13T05:13:32Z

@cfc4n , please check again.

cfc4n

LGTM, Thanks.

fix

331341a

yuweizzz marked this pull request as draft September 30, 2024 12:06

yuweizzz marked this pull request as ready for review September 30, 2024 13:15

cfc4n added enhancement New feature or request fix bug fix PR labels Sep 30, 2024

cfc4n reviewed Oct 1, 2024

View reviewed changes

yuweizzz marked this pull request as draft October 3, 2024 08:44

mooncityorg reviewed Oct 7, 2024

View reviewed changes

yuweizzz added 5 commits October 8, 2024 10:25

update offset

eed7042

Merge branch 'master' into fix

6e27831

[skip ci] update openssl.h

c4126ae

[skip ci] update user layer

8c36f00

[skip ci] boringssl

4cb554f

yuweizzz marked this pull request as ready for review October 9, 2024 03:50

yuweizzz marked this pull request as draft October 9, 2024 03:50

yuweizzz force-pushed the fix branch 2 times, most recently from 716c2b5 to 9a98e1f Compare October 9, 2024 04:53

revert debug

a138de9

yuweizzz force-pushed the fix branch from 9a98e1f to a138de9 Compare October 9, 2024 06:19

yuweizzz marked this pull request as ready for review October 9, 2024 06:48

yuweizzz requested a review from cfc4n October 9, 2024 16:59

cfc4n requested changes Oct 11, 2024

View reviewed changes

update review

fed09c1

cfc4n reviewed Oct 13, 2024

View reviewed changes

user/module/probe_openssl.go Show resolved Hide resolved

cfc4n requested changes Oct 13, 2024

View reviewed changes

update review

01c106d

yuweizzz force-pushed the fix branch from e2059e1 to 01c106d Compare October 13, 2024 05:15

cfc4n approved these changes Oct 13, 2024

View reviewed changes

cfc4n merged commit 93cfff4 into gojue:master Oct 13, 2024
6 checks passed

yuweizzz mentioned this pull request Oct 26, 2024

Unable to access network,ERR SSLDataEvent's fd is 0 address= fd=0 pid=9059 #653

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: SSLDataEvent's fd is 0 Error #642

fix: SSLDataEvent's fd is 0 Error #642

yuweizzz commented Sep 30, 2024

yuweizzz commented Sep 30, 2024 •

edited

Loading

cfc4n commented Sep 30, 2024 •

edited

Loading

cfc4n Oct 1, 2024

yuweizzz commented Oct 3, 2024

mooncityorg left a comment •

edited by cfc4n

Loading

cfc4n Oct 11, 2024

cfc4n Oct 11, 2024

yuweizzz Oct 11, 2024

cfc4n Oct 11, 2024

yuweizzz Oct 11, 2024

cfc4n Oct 13, 2024

yuweizzz commented Oct 13, 2024

cfc4n left a comment

fix: SSLDataEvent's fd is 0 Error #642

fix: SSLDataEvent's fd is 0 Error #642

Conversation

yuweizzz commented Sep 30, 2024

yuweizzz commented Sep 30, 2024 • edited Loading

cfc4n commented Sep 30, 2024 • edited Loading

cfc4n Oct 1, 2024

Choose a reason for hiding this comment

yuweizzz commented Oct 3, 2024

mooncityorg left a comment • edited by cfc4n Loading

Choose a reason for hiding this comment

cfc4n Oct 11, 2024

Choose a reason for hiding this comment

cfc4n Oct 11, 2024

Choose a reason for hiding this comment

yuweizzz Oct 11, 2024

Choose a reason for hiding this comment

cfc4n Oct 11, 2024

Choose a reason for hiding this comment

yuweizzz Oct 11, 2024

Choose a reason for hiding this comment

cfc4n Oct 13, 2024

Choose a reason for hiding this comment

yuweizzz commented Oct 13, 2024

cfc4n left a comment

Choose a reason for hiding this comment

yuweizzz commented Sep 30, 2024 •

edited

Loading

cfc4n commented Sep 30, 2024 •

edited

Loading

mooncityorg left a comment •

edited by cfc4n

Loading