Package: mutt
Version: 2.0.5-4
Severity: minor
Tags: patch
Hi,
Current implementation of mutt always seems to ask for SMTP username
even if one is not set in the config (smtp_url) and AUTH is not
an allowed option of submit server. This then results in an email
send failure
SMTP server does not support authentication
The patch that fixes this issue is at and also attached.
https://gitlab.com/muttmua/mutt/-/commit/191b0513b43d5e603f99292faa5f8ebcc1be3823.patch
I've tested this patch in the tagged version and the problem is solved.
Please consider adding it to mutt for next upload.
Thanks,
- Adam
>From 191b0513b43d5e603f99292faa5f8ebcc1be3823 Mon Sep 17 00:00:00 2001
From: Kevin McCarthy <ke...@8t8.us>
Date: Fri, 5 Mar 2021 18:52:48 -0800
Subject: [PATCH] Fix $ssl_client_cert usage with SMTP.
The ssl and gnutls client-cert setup code was calling
mutt_account_getuser(). This caused two problems. First, it's not
necessarily the case that there will be a username. Second,
populating the user would cause smtp_open() to check for AUTH
capabilities and call smtp_auth - even if the user is already
authenticated by the cert. The server won't advertize AUTH if they
already authenticated, causing a connection abort.
Remove prompt for mutt_account_getuser() in the ssl and gnutls client
certificate connection code. The SASL code has callbacks, so I don't
understand why it would need this. Let's take it out and see if
anyone screams 8-P. If necessary, we can add a mutt_account_getuser()
call to the very beginning of imap_auth_sasl().
Revamp the openssl ssl_passwd_cb() prompt. From the man pages, it
appears to be used for the cert decryption. There's no need to call
mutt_account_getuser() and use the generic mutt_account_getpass() just
to read a password in. Instead create a callback function version to
customize the prompt for a client cert with just the host.
Change the SMTP authentication test to check if the AUTH capabilities
are set, instead of if the user field is set before calling
smtp_auth().
---
account.c | 25 ++++++++++++++++++++-----
account.h | 2 ++
mutt_ssl.c | 32 +++++++++++++++++++++++++++-----
mutt_ssl_gnutls.c | 20 ++++++++++++++++++--
smtp.c | 11 +----------
5 files changed, 68 insertions(+), 22 deletions(-)
diff --git a/account.c b/account.c
index 28c0749b..09bf24d3 100644
--- a/account.c
+++ b/account.c
@@ -198,8 +198,19 @@ int mutt_account_getlogin (ACCOUNT* account)
return 0;
}
-/* mutt_account_getpass: fetch password into ACCOUNT, if necessary */
-int mutt_account_getpass (ACCOUNT* account)
+static void getpass_prompt (char *prompt, size_t prompt_size, ACCOUNT *account)
+{
+ /* L10N:
+ Prompt for an account password when connecting.
+ %s@%s is user@host
+ */
+ snprintf (prompt, prompt_size, _("Password for %s@%s: "),
+ account->flags & MUTT_ACCT_LOGIN ? account->login : account->user,
+ account->host);
+}
+
+int _mutt_account_getpass (ACCOUNT* account,
+ void (*prompt_func) (char *, size_t, ACCOUNT *))
{
char prompt[SHORT_STRING];
@@ -221,9 +232,7 @@ int mutt_account_getpass (ACCOUNT* account)
return -1;
else
{
- snprintf (prompt, sizeof (prompt), _("Password for %s@%s: "),
- account->flags & MUTT_ACCT_LOGIN ? account->login : account->user,
- account->host);
+ prompt_func (prompt, sizeof(prompt), account);
account->pass[0] = '\0';
if (mutt_get_password (prompt, account->pass, sizeof (account->pass)))
return -1;
@@ -234,6 +243,12 @@ int mutt_account_getpass (ACCOUNT* account)
return 0;
}
+/* mutt_account_getpass: fetch password into ACCOUNT, if necessary */
+int mutt_account_getpass (ACCOUNT *account)
+{
+ return _mutt_account_getpass (account, getpass_prompt);
+}
+
void mutt_account_unsetpass (ACCOUNT* account)
{
account->flags &= ~MUTT_ACCT_PASS;
diff --git a/account.h b/account.h
index 2eccb7f4..9f485cfd 100644
--- a/account.h
+++ b/account.h
@@ -55,6 +55,8 @@ int mutt_account_fromurl (ACCOUNT* account, ciss_url_t* url);
void mutt_account_tourl (ACCOUNT* account, ciss_url_t* url);
int mutt_account_getuser (ACCOUNT* account);
int mutt_account_getlogin (ACCOUNT* account);
+int _mutt_account_getpass (ACCOUNT* account,
+ void (*prompt_func) (char *, size_t, ACCOUNT *));
int mutt_account_getpass (ACCOUNT* account);
void mutt_account_unsetpass (ACCOUNT* account);
int mutt_account_getoauthbearer (ACCOUNT* account, BUFFER *authbearer, int xoauth2);
diff --git a/mutt_ssl.c b/mutt_ssl.c
index dd76cded..91507948 100644
--- a/mutt_ssl.c
+++ b/mutt_ssl.c
@@ -1399,22 +1399,44 @@ static void ssl_get_client_cert(sslsockdata *ssldata, CONNECTION *conn)
SSL_CTX_use_certificate_file(ssldata->ctx, SslClientCert, SSL_FILETYPE_PEM);
SSL_CTX_use_PrivateKey_file(ssldata->ctx, SslClientCert, SSL_FILETYPE_PEM);
+#if 0
+ /* This interferes with SMTP client-cert authentication that doesn't
+ * use AUTH EXTERNAL. (see gitlab #336)
+ *
+ * The mutt_sasl.c code sets up callbacks to get the login or
+ * user, and it looks like the Cyrus SASL external code calls
+ * those.
+ *
+ * Brendan doesn't recall if this really was necessary at one time, so
+ * I'm disabling it.
+ */
+
/* if we are using a client cert, SASL may expect an external auth name */
mutt_account_getuser (&conn->account);
+#endif
}
}
+static void client_cert_prompt (char *prompt, size_t prompt_size, ACCOUNT *account)
+{
+ /* L10N:
+ When using a $ssl_client_cert, OpenSSL may prompt for the password
+ to decrypt the cert. %s is the hostname.
+ */
+ snprintf (prompt, prompt_size, _("Password for %s client cert: "),
+ account->host);
+}
+
static int ssl_passwd_cb(char *buf, int size, int rwflag, void *userdata)
{
- ACCOUNT *account = (ACCOUNT*)userdata;
+ ACCOUNT *account;
- if (mutt_account_getuser (account))
+ if (!buf || size <= 0 || !userdata)
return 0;
- dprint (2, (debugfile, "ssl_passwd_cb: getting password for %s@%s:%u\n",
- account->user, account->host, account->port));
+ account = (ACCOUNT *) userdata;
- if (mutt_account_getpass (account))
+ if (_mutt_account_getpass (account, client_cert_prompt))
return 0;
return snprintf(buf, size, "%s", account->pass);
diff --git a/mutt_ssl_gnutls.c b/mutt_ssl_gnutls.c
index e9acc114..a576ab15 100644
--- a/mutt_ssl_gnutls.c
+++ b/mutt_ssl_gnutls.c
@@ -245,9 +245,19 @@ int mutt_ssl_starttls (CONNECTION* conn)
}
/* Note: this function grabs the CN out of the client
- * cert but appears to do nothing with it. It does contain a call
- * to mutt_account_getuser().
+ * cert but appears to do nothing with it.
+ *
+ * It does contain a call to mutt_account_getuser(), but this
+ * interferes with SMTP client-cert authentication that doesn't use
+ * AUTH EXTERNAL. (see gitlab #336)
+ *
+ * The mutt_sasl.c code sets up callbacks to get the login or user,
+ * and it looks like the Cyrus SASL external code calls those.
+ *
+ * Brendan doesn't recall if this really was necessary at one time, so
+ * I'm disabling it.
*/
+#if 0
static void tls_get_client_cert (CONNECTION* conn)
{
tlssockdata *data = conn->sockdata;
@@ -292,6 +302,7 @@ err:
FREE (&cn);
gnutls_x509_crt_deinit (clientcrt);
}
+#endif
#if HAVE_GNUTLS_PRIORITY_SET_DIRECT
static int tls_set_priority (tlssockdata *data)
@@ -496,7 +507,12 @@ static int tls_negotiate (CONNECTION * conn)
/* NB: gnutls_cipher_get_key_size() returns key length in bytes */
conn->ssf = gnutls_cipher_get_key_size (gnutls_cipher_get (data->state)) * 8;
+#if 0
+ /* See comment above the tls_get_client_cert() function for why this
+ * is ifdef'ed out. Also note the SslClientCert is already set up
+ * above. */
tls_get_client_cert (conn);
+#endif
if (!option (OPTNOCURSES))
{
diff --git a/smtp.c b/smtp.c
index 2ab45ec1..e34bdbfb 100644
--- a/smtp.c
+++ b/smtp.c
@@ -489,17 +489,8 @@ static int smtp_open (CONNECTION* conn)
}
#endif
- if (conn->account.flags & MUTT_ACCT_USER)
- {
- if (!mutt_bit_isset (Capabilities, AUTH))
- {
- mutt_error (_("SMTP server does not support authentication"));
- mutt_sleep (1);
- return -1;
- }
-
+ if (mutt_bit_isset (Capabilities, AUTH))
return smtp_auth (conn);
- }
return 0;
}
--
GitLab
