Skip to content

Feature/zcs 11179#5

Merged
umagmrit merged 28 commits intozimbra/developfrom
feature/ZCS-11179
Feb 7, 2024
Merged

Feature/zcs 11179#5
umagmrit merged 28 commits intozimbra/developfrom
feature/ZCS-11179

Conversation

@SuryawanshiAmol
Copy link

  • The core, event, and http directory code has been merged from branches/stable 1.24 to the current zimbra/nginx.

…inx repository and merge them with the Zimbra files.

- The core, event, and http directory code has been merged from branches/stable 1.24 to the current zimbra/nginx.
Barry de Graaff and others added 20 commits August 22, 2023 08:20
…inx repository and merge them with the Zimbra files.

- The auto directory code has been merged from branches/stable 1.24 to the current zimbra/nginx.
…inx repository and merge them with the Zimbra files.

- The conf directory code has been merged from branches/stable 1.24 to the current zimbra/nginx.
… nginx repository and merge them with the Zimbra files.

- The contrib directory code has been merged from branches/stable 1.24 to the current zimbra/nginx.
…inx repository and merge them with the Zimbra files.

- The docs directory code has been merged from branches/stable 1.24 to the current zimbra/nginx.
…inx repository and merge them with the Zimbra files.

- The misc directory code has been merged from branches/stable 1.24 to the current zimbra/nginx.
…lic nginx repository and merge them with the Zimbra files.

- The src/stream directory code has been merged from branches/stable 1.24 to the current zimbra/nginx.
…nginx repository and merge them with the Zimbra files.

- The src/os directory code has been merged from branches/stable 1.24 to the current zimbra/nginx.
…inx repository and merge them with the Zimbra files.

- src/mail/ngx_mail_smtp_module.c has been merged from branches/stable 1.24 to the current zimbra/nginx.
…ginx repository and merge them with the Zimbra files.

- src/mail/ngx_mail_proxy_module.c has been merged from branches/stable 1.24 to the current zimbra/nginx.

The following commits from upstream have been skipped as our implementation is different at these points:
- nginx@d96d60d#diff-e9062f9616b15ece5fadfd80a39969f44b8db5a8712900d67392b51e66c7e2b1
- nginx@d96d60d#diff-e9062f9616b15ece5fadfd80a39969f44b8db5a8712900d67392b51e66c7e2b1
- nginx@2ca4355
…inx repository and merge them with the Zimbra files.

- src/mail/ngx_mail_pop3_module.c has been merged from branches/stable 1.24 to the current zimbra/nginx.
…ectory.

- Review comments are discussed with barry and applied them
…ginx repository.

- Merged the files with latest nginx version stable-1.24.0
- Remaining file
ZCOMT-2579: Discuss the review comment and apply it to the "core" dir…
- AMB : Code is commented in order to move compilation forward and detect any errors that arise.
See https://trac.nginx.org/nginx/ticket/2312
@ghen2
Copy link

ghen2 commented Sep 19, 2023

Btw minor build warning:
./auto/configure: warning: the "--with-ipv6" option is deprecated
(since nginx 1.11.5)

SuryawanshiAmol and others added 2 commits October 2, 2023 16:37
@ghost
Copy link

ghost commented Oct 2, 2023

When comparing our changes from the 1.20 version next to our changes in the 1.24 version, it seems like we omit some of our customizations in ngx_mail_parse.c beginning under /* IMAP tag */ we used to add case '\x0' but we do not anymore. Can you explain why we make a different approach now?

ngx_mail_parse c

@ghost ghost mentioned this pull request Oct 6, 2023
@SuryawanshiAmol
Copy link
Author

When comparing our changes from the 1.20 version next to our changes in the 1.24 version, it seems like we omit some of our customizations in ngx_mail_parse.c beginning under /* IMAP tag */ we used to add case '\x0' but we do not anymore. Can you explain why we make a different approach now?
Answer: I've left it out because it's now handled in the default case. See the code below the '\x0' will satisfy the if condition.
u_char ch('\x0');
if ((ch < 'A' || ch > 'Z') && (ch < 'a' || ch > 'z')
&& (ch < '0' || ch > '9') && ch != '-' && ch != '.'
&& ch != '_')
{
cout << "goto Invalid";
}

Or we can handle it as below:
case CR:
case LF:
case '\x0':
goto invalid;

@SuryawanshiAmol
Copy link
Author

The --with-ipv6 option no longer needed. But no harm keeping it. Its not added recently its their since beginning.
Please See:
https://nginx.org/en/CHANGES-1.20

Changes with nginx 1.11.5 11 Oct 2016

*) Change: the --with-ipv6 configure option was removed, **now IPv6
   support is configured automatically.**

@ghost
Copy link

ghost commented Oct 9, 2023

In ngx_mail_parse.c the \x0 is also missing swi_spaces_before_command method. If that is fixed, the rest of the customization in this file look the same as in the 1.20 version.

@ghost ghost requested a review from ashishkataria86 October 13, 2023 06:49
@ghen2
Copy link

ghen2 commented Oct 14, 2023

Hi

Could you consider including nginx@6ceef19 to mitigate CVE-2023-44487: HTTP/2 Rapid Reset Attack? This patch will be part of an upcoming nginx 1.24.x release.

It applies cleanly to nginx 1.20.0 as well.

@ghen2
Copy link

ghen2 commented Oct 23, 2023

I created PR #7 for Zimbra's nginx 1.20.0, we're already running with this patch in production.

@umagmrit umagmrit merged commit cf37e06 into zimbra/develop Feb 7, 2024
@umagmrit umagmrit deleted the feature/ZCS-11179 branch February 7, 2024 05:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants