Hello!

On Fri, Aug 13, 2021 at 06:26:52PM +1000, Robert Mueller wrote:

> # HG changeset patch
> # User Rob Mueller <robm@fastmail.fm>
> # Date 1628841467 14400
> # Fri Aug 13 03:57:47 2021 -0400
> # Node ID 6ea8e179293dbd5d09218658220a64a9ce20cb8a
> # Parent dda421871bc213dd2eb3da0015d6228839323583
> Mail: Add Auth-SSL-Cipher header to each imap/pop/smtp auth request

Something like:

Mail: added Auth-SSL-Cipher header.

would be enough.

>
> This adds a new Auth-SSL-Cipher header to the mail proxy auth
> protocol when SSL is enabled the reports the SSL cipher that
> was negotiated.

It look like there are grammar issues here, "the reports ..." part
is not related to the text. Probably should be omitted as it is
obvious anyway.

>
> This can be useful for detecting users using older clients that
> negotiate old ciphers when you want to upgrade to newer
> TLS versions of remove suppport for old and insecure ciphers.

.... oR remove?
.... suPPort?

Shouldn't we also add Auth-SSL-Protocol if one of the declared use
cases is to upgrade to newer TLS versions?

In general this looks close to ticket #2134
(https://trac.nginx.org/nginx/ticket/2134), so it is good idea to
mention it in the commit log.

> You can use your auth backend to notify these users before the
> upgrade that they either need to upgrade their client software
> or contact your support team to work out an upgrade path.
>
> diff -r dda421871bc2 -r 6ea8e179293d src/mail/ngx_mail_auth_http_module.c
> --- a/src/mail/ngx_mail_auth_http_module.c Tue Aug 10 23:43:17 2021 +0300
> +++ b/src/mail/ngx_mail_auth_http_module.c Fri Aug 13 03:57:47 2021 -0400
> @@ -1138,7 +1138,7 @@
> ngx_connection_t *c;
> #if (NGX_MAIL_SSL)
> ngx_str_t verify, subject, issuer, serial, fingerprint,
> - raw_cert, cert;
> + raw_cert, cert, cipher;
> ngx_mail_ssl_conf_t *sslcf;
> #endif
> ngx_mail_core_srv_conf_t *cscf;
> @@ -1157,6 +1157,15 @@
>
> sslcf = ngx_mail_get_module_srv_conf(s, ngx_mail_ssl_module);
>
> + if (c->ssl) {
> + if (ngx_ssl_get_cipher_name(c, pool, &cipher) != NGX_OK) {
> + return NULL;
> + }
> + cipher.len = ngx_strlen(cipher.data);
> + } else {
> + ngx_str_null(&cipher);
> + }
> +

This does not use sslcf, so probably should be before it is
obtained. Also, it lacks empty lines expected per style.

> if (c->ssl && sslcf->verify) {
>
> /* certificate details */
> @@ -1252,6 +1261,8 @@
>
> if (c->ssl) {
> len += sizeof("Auth-SSL: on" CRLF) - 1
> + + sizeof("Auth-SSL-Cipher: ") - 1 + cipher.len
> + + sizeof(CRLF) - 1
> + sizeof("Auth-SSL-Verify: ") - 1 + verify.len
> + sizeof(CRLF) - 1
> + sizeof("Auth-SSL-Subject: ") - 1 + subject.len
> @@ -1373,6 +1384,13 @@
> b->last = ngx_cpymem(b->last, "Auth-SSL: on" CRLF,
> sizeof("Auth-SSL: on" CRLF) - 1);
>
> + if (cipher.len) {
> + b->last = ngx_cpymem(b->last, "Auth-SSL-Cipher: ",
> + sizeof("Auth-SSL-Cipher: ") - 1);
> + b->last = ngx_copy(b->last, cipher.data, cipher.len);
> + *b->last++ = CR; *b->last++ = LF;
> + }
> +
> if (verify.len) {
> b->last = ngx_cpymem(b->last, "Auth-SSL-Verify: ",
> sizeof("Auth-SSL-Verify: ") - 1);

The patch which addresses above comments, please take a look if it
works for you:

# HG changeset patch
# User Rob Mueller <robm@fastmail.fm>
# Date 1628841467 14400
# Fri Aug 13 03:57:47 2021 -0400
# Node ID b5d159018a6ad7ecaf52dedd5ff19798cf360d45
# Parent dda421871bc213dd2eb3da0015d6228839323583
Mail: Auth-SSL-Protocol and Auth-SSL-Cipher headers (ticket #2134).

This adds new Auth-SSL-Protocol and Auth-SSL-Cipher headers to
the mail proxy auth protocol when SSL is enabled.

This can be useful for detecting users using older clients that
negotiate old ciphers when you want to upgrade to newer
TLS versions of remove suppport for old and insecure ciphers.
You can use your auth backend to notify these users before the
upgrade that they either need to upgrade their client software
or contact your support team to work out an upgrade path.

diff --git a/src/mail/ngx_mail_auth_http_module.c b/src/mail/ngx_mail_auth_http_module.c
--- a/src/mail/ngx_mail_auth_http_module.c
+++ b/src/mail/ngx_mail_auth_http_module.c
@@ -1137,8 +1137,8 @@ ngx_mail_auth_http_create_request(ngx_ma
ngx_str_t login, passwd;
ngx_connection_t *c;
#if (NGX_MAIL_SSL)
- ngx_str_t verify, subject, issuer, serial, fingerprint,
- raw_cert, cert;
+ ngx_str_t protocol, cipher, verify, subject, issuer,
+ serial, fingerprint, raw_cert, cert;
ngx_mail_ssl_conf_t *sslcf;
#endif
ngx_mail_core_srv_conf_t *cscf;
@@ -1155,6 +1155,25 @@ ngx_mail_auth_http_create_request(ngx_ma

#if (NGX_MAIL_SSL)

+ if (c->ssl) {
+
+ if (ngx_ssl_get_protocol(c, pool, &protocol) != NGX_OK) {
+ return NULL;
+ }
+
+ protocol.len = ngx_strlen(protocol.data);
+
+ if (ngx_ssl_get_cipher_name(c, pool, &cipher) != NGX_OK) {
+ return NULL;
+ }
+
+ cipher.len = ngx_strlen(cipher.data);
+
+ } else {
+ ngx_str_null(&protocol);
+ ngx_str_null(&cipher);
+ }
+
sslcf = ngx_mail_get_module_srv_conf(s, ngx_mail_ssl_module);

if (c->ssl && sslcf->verify) {
@@ -1252,6 +1271,10 @@ ngx_mail_auth_http_create_request(ngx_ma

if (c->ssl) {
len += sizeof("Auth-SSL: on" CRLF) - 1
+ + sizeof("Auth-SSL-Protocol: ") - 1 + protocol.len
+ + sizeof(CRLF) - 1
+ + sizeof("Auth-SSL-Cipher: ") - 1 + cipher.len
+ + sizeof(CRLF) - 1
+ sizeof("Auth-SSL-Verify: ") - 1 + verify.len
+ sizeof(CRLF) - 1
+ sizeof("Auth-SSL-Subject: ") - 1 + subject.len
@@ -1373,6 +1396,20 @@ ngx_mail_auth_http_create_request(ngx_ma
b->last = ngx_cpymem(b->last, "Auth-SSL: on" CRLF,
sizeof("Auth-SSL: on" CRLF) - 1);

+ if (protocol.len) {
+ b->last = ngx_cpymem(b->last, "Auth-SSL-Protocol: ",
+ sizeof("Auth-SSL-Protocol: ") - 1);
+ b->last = ngx_copy(b->last, protocol.data, protocol.len);
+ *b->last++ = CR; *b->last++ = LF;
+ }
+
+ if (cipher.len) {
+ b->last = ngx_cpymem(b->last, "Auth-SSL-Cipher: ",
+ sizeof("Auth-SSL-Cipher: ") - 1);
+ b->last = ngx_copy(b->last, cipher.data, cipher.len);
+ *b->last++ = CR; *b->last++ = LF;
+ }
+
if (verify.len) {
b->last = ngx_cpymem(b->last, "Auth-SSL-Verify: ",
sizeof("Auth-SSL-Verify: ") - 1);

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel