Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixbug: fd may be 0 when BIO_get_fd return 0 #82

Open
wants to merge 1 commit into
base: master-babassl
Choose a base branch
from
Open

Fixbug: fd may be 0 when BIO_get_fd return 0 #82

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ssl/statem_ntls/statem_ntls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,14 +264,14 @@ int SSL_connection_is_ntls(SSL *s, int is_server)
* have to get the server version from clientHello
*/
if (SSL_IS_FIRST_HANDSHAKE(s) && SSL_in_before(s)) {
int ret, fd;
int ret, fd = -1;
PACKET pkt;
unsigned int version, type;
unsigned char buf[PEEK_HEADER_LENGTH];

ret = BIO_get_fd(s->rbio, &fd);
(void) BIO_get_fd(s->rbio, &fd);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

当fd为0的时候，ret不会返回-1么？

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

或者说fd没取到合法的值的时候，ret应该返回-1才对。还是说有可能ret和fd都是0？

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

或者说fd没取到合法的值的时候，ret应该返回-1才对。还是说有可能ret和fd都是0？

同意，如果有问题，也应该修改BIO_get_fd内部的逻辑

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我看了下BIO_get_fd的实现，如果该BIO没有初始化，应该返回-1，其余情况都是返回正常的fd值（即fd和ret值相等）。如果不是这种情况，那可能BIO_get_fd的实现就有问题了

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BIO_get_fd内部实现没有问题。内部调用的sock_ctrl 返回值成功的话，ret = bio->num，bio->num 可能为0。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果要是这样，那么逻辑应该是：以BIO_get_fd()的返回值作为判断是否调用成功的依据，如果是-1则直接失败，大于等于0的都认为是成功：

ret = BIO_get_fd(s->rbio, &fd);
if (ret < 0) {
     SSLfatal_ntls(...);
}

改一行即可

Copy link
Author

@easycoderwei easycoderwei Sep 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果要是这样，那么逻辑应该是：以BIO_get_fd()的返回值作为判断是否调用成功的依据，如果是-1则直接失败，大于等于0的都认为是成功：

ret = BIO_get_fd(s->rbio, &fd);
if (ret < 0) {
     SSLfatal_ntls(...);
}

改一行即可

改一行是不行的。ret=0有两种场景：
1.成功：调用的sock_ctrl（b->method->ctrl）返回值成功的话，ret = bio->num，bio->num 可能为0。
2.失败：BIO_get_fd失败也有可能返回0

验证BIO_get_fd宏得到fd=0的场景，可以通过静态编译链接到nginx，然后nginx全局配置daemon off; 单次请求必现。

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

正常启动进程fd=0会被占用，但是不能排除fd=0的场景。像nginx以非守护进程的方式启动就是例子。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ret=0有两种场景：
1.成功：调用的sock_ctrl（b->method->ctrl）返回值成功的话，ret = bio->num，bio->num 可能为0。
2.失败：BIO_get_fd失败也有可能返回0

失败不会反回0，失败只会返回-1。所以返回0的时候，fd必为0。在成功的情况下，ret的值和fd的值是相同的。失败的时候ret为-1

Copy link
Author

@easycoderwei easycoderwei Sep 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ret=0有两种场景：
1.成功：调用的sock_ctrl（b->method->ctrl）返回值成功的话，ret = bio->num，bio->num 可能为0。
2.失败：BIO_get_fd失败也有可能返回0

失败不会反回0，失败只会返回-1。所以返回0的时候，fd必为0。在成功的情况下，ret的值和fd的值是相同的。失败的时候ret为-1

失败返回0的情况只有在bio==NULL的场景，在SSL_connection_is_ntls函数调用时确实是不会出现的。
但是从openssl中使用 BIO_get_fd 这个宏的地方，一般都不会依赖于返回值，而是依赖于输出参数fd的值，fd>=0就认为是合法的。


if (ret <= 0) {
if (fd < 0) {
/* NTLS only support socket communication */
SSLfatal_ntls(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL_CONNECTION_IS_NTLS,
ERR_R_INTERNAL_ERROR);
Expand Down