conn: do not prompt for username with client certificate

[balejk]

• What does this PR do?

Currently it is not possible to authenticate to a SMTP server using a
client certificate only as neomutt prompts for an username if one is not
provided in the configuration file. This will fail if the server doesn't
support username authentication as seems to be common with servers
expecting a client certificate.

This patch changes the behaviour so that username is only used if it's
explicitly set before the operation is started (e. g. via the
configuration file).

I haven't been able to verify whether it is possible for a server to require
both an username and a client certificate - please let me know if you are - but
this (i. e. not being prompted for the username and having to set it
beforehand) seems to be at least better than not being able to use the client
certificate functionality at all.

Possibly a better solution would be to make cancelling the prompt an option
without interrupting the sending of the message.

• Does this PR meet the acceptance criteria? (This is just a reminder for you,
  this section can be removed if you fulfill it.)

  • Documentation created/updated (you have to edit
    docs/manual.xml.head
    for that)

No.

• All builds and tests are passing

mutt_path_to_absolute is failing but that doesn't seem to be related.

• Added doxygen code documentation
  syntax

Updated.

• Code follows the style guide

I believe I have not introduced any violation.

• What are the relevant issue numbers?

Should close #3681.

Cc: @luchr

[balejk]

Relevant mutt issue. I shoud have looked at it sooner as it seems their solution is a bit more complicated than what I did and they claim it does what seems as the most reasonable solution to me. I am converting this into a draft until I look into it properly.

[flatcap]

Thanks for pushing ahead with this, @balejk.

Sorry for avoiding the issue.
This is one of those complicated bits of old code.
Nobody really understands them and nobody's brave enough to alter them.

I'll have a read of Mutt's patch and try to understand what they've done and why.

[balejk]

No problem, @flatcap.

If I thought to check how mutt handles this before, I wouldn't have opened this until I understood what they did and why.

I intend to see this through, it looks to be a nice excercise, but it might be rather a while until I have time for it, so if you have already started looking into it, feel free to fix it yourself :-)

[flatcap]

I haven't started yet -- I'm deep in quite a few other things,

it might be rather a while until I have time for it

There's no rush.

The code's a lot to take in, so ask lots of questions.
Either here on in IRC: #neomutt on irc.libera.chat (web client)

Thanks!

[balejk]

Will do, thanks for now!

[vuori]

I can test this if needed since I have an SMTP server that accepts client certs. I haven't used (Neo)mutt's client cert support or looked at the code, so just a few general comments on the matter:

The question of user names/other auth with SMTP (and IMAP) is… complicated. Probably the most common scenario for SMTP is that just having the client cert in the TLS is nego is enough ("implicit client certificate authentication").

Use of the SASL EXTERNAL mechanism is common with IMAP, but rare with SMTP. IIRC neither Thunderbird nor K-9 supports it.

You can have a belt&suspenders configuration where a server requires both client cert and password auth. I would expect this to be quite a bit more uncommon than implicit client cert, but both Thunderbird and K-9 support it. (edit: people who terminate TLS at a proxy and validate the client cert there, but still want user info on the SMTP server are the most likely users for this kind of setup.)

What I think should happen is that if username+password are configured AND the server advertises password mechs in EHLO response, then do the AUTH as usual. If either one is missing, don't prompt and just try to send the mail.

[balejk]

@vuori Thank you for the remarks. Sadly, I still haven't found the time to get into this (and I'm afraid it will still be like that for quiet some time), but I will be sure to ping you when there is something to test. Do I understand correctly that you have such a SMTP server to which you have admin access? That could be really useful for testing, although I could probably also try to set it up locally. Even if that is not the case then it will still be good if several people test it - I have user access to such a server as well, although it may no longer be the case, once I find the time for this. For (my) future reference: I have looked very briefly at the original mutt code some time ago and I noticed that the commit 91474fdf is likely also relevant to the problem.

[vuori]

@vuori Thank you for the remarks. Sadly, I still haven't found the time to get into this (and I'm afraid it will still be like that for quiet some time), but I will be sure to ping you when there is something to test. Do I understand correctly that you have such a SMTP server to which you have admin access? That could be really useful for testing, although I could probably also try to set it up locally.

Yes, I can issue you a cert and a username/password if/when you need it. Send me a mail when the time is right.

[balejk]

Awesome! It will definitely be a huge help that I won't have to set up a server myself, thanks :-)

[balejk]

So I have finally started looking into this again. I have gone through the mutt
commits and added a second commit to change the conditions for invoking
smtp_authenticate. The behaviour should now be basically as @vuori suggested,
with the addition of handling of client certificate SASL EXTERNAL.

I have tested this with two of my SMTP providers, one requiring client
certificate and no password AUTH, the other one using just AUTH. With the
former, sending works regardless of whether the username is set (with just
ssl_client_cert, smtp_url, from and optionally smtp_user in neomuttrc).
With the latter (smtp_url, smtp_user, smtp_pass and from in neomuttrc),
sending works too, except for when smtp_user is not set in which case neomutt
doesn't prompt. This is a shame, since smtp_auth_plain does have this
capability but never gets invoked as smtp_authenticate doesn't either because
of the missing username. However it is not a regression -- the current code
also does not call smtp_authenticate with missing username either. In ideal
case we should be able to call smtp_authenticate only when the server
advertises AUTH, however the 91474fdff648 mutt commit (or rather the therein
referenced Debian bug report) claims that sometimes the server can advertise it
even though it is not required.

It remains to test this with a server which requires both the certificate and
AUTH -- @vuori, would you please be able to test this? Or, if you prefer, you
can email me the authentication information for your server to the commit
author email address.

[vuori]

My usual SMTP server uses implicit client cert auth, but I'll see about setting up a test server that wants both client cert and plain and try it out.

[vuori]

I didn't have a chance to test the PR itself yet, but I mailed you the keys for a test server that expects client cert + password auth.

[balejk]

vuori, 2024-05-21T12:33:33-07:00:

I didn't have a chance to test the PR itself yet, but I mailed you the keys for a test server that expects client cert + password auth.

Thank you truly very much, this is a huge help! I have tried what I wrote before with your server and all works as expected, so I'm marking this as ready for @flatcap's review. I have also thought a little more about the second commit and I'm wondering whether it should be dropped after all. The effect this commit has is that if the username is missing but cert is set, neomutt will prompt for the username if the server advertises AUTH. Which is great, but makes the behaviour somewhat inconsistent for when there is no certificate authentication involved, in which case the credentials are not prompted for and the sending fails. Also, the second commit should make SASL EXTERNAL certificate authentication possible, but this we did not test plus you said that it is much less common than the implicit certificate authentication which I tested with my server. Hence leaving the authentication conditions as they were (i. e. dropping the second commit) seems to make some sense to me: setting the username explicitly will serve for forcing the authentication which will not be performed otherwise. The username will never be prompted for (as is the case now) and we won't be able to do client certificate-based SASL EXTERNAL. As I mentioned before, it would seem optimal if we were able to perform the authentication if and only if the server advertises it, which is what mutt tried to do with their first relevant commit, because then we could take advantage of the username prompt. This would however require to have a way to force skipping the authentication even in the case the server advertises it, not to cause regression for people with the setup mentioned in the Debian ticket, which doesn't seem to be possible now (such as by making `smtp_authenticators` empty). Possibly adding something like a `smtp_force_skip_authentication` configuration option could make sense? But this is really just a random idea somewhat beyond the scope of what I originally wanted to achieve here. Also, to be fair, I suppose that usually one knows the authentication requirements of their server beforehand making the prompt somewhat obsolete, except maybe for situations where one does not want to store the credentials in neomuttrc, which can however still be worked around by running `set` manually from within neomutt, which is why I'm more inclined toward leaving the authentication condition as is, depending only on the username being set. I will be happy to hear any more feedback of any kind!

[balejk]

I have rebased and updated commit messages. If we decide to keep the second
commit, I'd probably want to split out the certificate SASL EXTERNAL enablement
into a separate one for clarity before it's merged.

[vuori]

Great that you could get your changes tested.

1. Username + cert auth: I think it's better to not prompt if client cert is set and AUTH is advertised. It's probably a common configuration for servers that support both client cert and password auth to always have AUTH available in EHLO with the cert present, even if it's not required. Exim can be configured to not advertise AUTH if the client cert was accepted, but it's extra work and I don't know whether other SMTP servers support such configuration. Also client certs are a fairly exotic configuration so people who have set one up probably know what they want.

2. SASL EXTERNAL: IMO spending time on this is probably not super useful, but it could be nice to have. I can make the test server support it if needed. Let me know if you need it and whether you want it separately (= keep the current setup intact on the current port and have only SASL EXTERNAL available when connecting through another port) or just make it available in addition to PLAIN.

3. Forced auth prompt if server advertises AUTH: I think at minimum implementing one of the bypass mechanisms is a must. Probably nowadays a majority of servers require some kind of password/cert auth to send, but if a server supports both password-authenticated and traditional IP-authenticated sending, it will most likely always be advertising AUTH in EHLO. Requiring credentials with no opt-out will probably annoy some group of users. However since that group is likely not very large these days, having the prompting happen as default could be ok; I'll defer to flatcap et al. on this.

[flatcap]

Thanks for your work on this!
The code changes look fine to me.

As for the second commit, I don't know.
You have a much better understanding of whether it's necessary or not.

Upstream do have the commit, so I'd be inclined to merge it,
but I don't understand the problems it might cause / solve.

[balejk]

Thank you both for the remarks. Let's keep this minimal then, only addressing the issue that I have encountered myself, that is the impossibility to have implicit client certificate authentication when no username is expected. This is addressed by the first commit, which I'm now force pushing to be the only one here. To sum up the implications: the authentication will only be attempted if the username is set, irrespective of the certificate or what the SMTP advertises (the authentication will however exit with error if the server does not advertise it). This makes it possible to use both servers that only require the certificate implicitly and those that require normal auth in addition. I'm not entirely sure what the implications for SASL EXTERNAL are -- having only the cert set will not trigger authentication and hence SASL EXTERNAL will not happen, it should however be possible to force the authentication by setting the username, which is probably not entirely straightforward as I believe the username is not always needed for this kind of authentication (if ever) potentially forcing users to have to set dummy usernames just to trigger the auth (and I'm not sure if having the username set could then cause some problems during the authentication if the server would only want the certificate). If it was worth the trouble for you to set up the server certificate SASL EXTERNAL support, @vuori, I would be interested in trying to answer this concern out of curiosity (in this case I would probably prefer to have it separately on a different port), however I do not intend to look into it any deeper. Please correct me if I'm making wrong assumptions somewhere. If it turns out that this logic still causes problems for some setups, it can always be modified then with a clearer idea of what is desired. Hopefully the conversation that happened here will be of some help in that case. Also just to make it clear for @flatcap, as the above SASL EXTERNAL-related paragraph may raise the concern: there should be no regression for potential certificate-based SASL EXTERNAL users (except maybe for the removed username prompt). All that I write above should hold now already and shouldn't be affected by my changes. I have reiterated it only for convenience since we have discussed SASL EXTERNAL before.

[vuori]

IIRC the parameter to SASL EXTERNAL is mostly useless with client certs. In the rare case that EXTERNAL is used with SMTP, I expect doing anything with the parameter on the server would be even rarer because it's just an untrusted string from the client; everything you want to know should be in the certificate anyway (maybe it's useful for proxy auth scenarios for specifying the user you want to proxy to? this would be more of an IMAP thing though). In other words, sending an empty string as the parameter to EXTERNAL should be ok most of the time. Sending the username is probably fine too.

I'll see if setting up EXTERNAL is simple and get back to you in mail about that. I don't think we should hold up this PR with related concerns though.

[balejk]

Thank you for the information. I will be looking forward to your mail.

I don't think we should hold up this PR with related concerns though.

Agreed. @flatcap, I believe this is ready to go.

[flatcap]

Thanks for the recap.
Great work!

[balejk]

Richard Russon, 2024-05-25T03:56:56-07:00:

Thanks for the recap. Great work!

Thank you and thanks for your patience -- it is slightly amusing that patch-wise, the final form is exactly the same as what we started with almost a year ago. Not to mention that the changes themselves are trivial, but not so the consideration and understanding of their implications of course. And again, big thanks to @vuori without whose experienced insights I would struggle with the latter much more, not to mention the kind provision of a testing facility.

[balejk]

So I have had the opportunity to test the behaviour with a server which supports certitificate-based SASL EXTERNAL (again graciously set up by @vuori) and it seems the situation is as follows.

With the current main (so the the merge of this PR), this type of authentication works with no problems if the server wants a username in addition. If the username is not set in neomutt, smtp_authenticate is never invoked despite the certificate being available and sending fails because the sender is not authenticated (so this only concerns the case when the authentication is required by the server).

The second commit which I had here helps with that, triggering the authentication in case the username is missing but cert is set (at the same time, it does not try to authenticate if the server does not advertise it as otherwise it would break implicit client cert auth again). Here is the patch for convenience:

diff --git a/send/smtp.c b/send/smtp.c
index b09fad17c493..145ca2c0ef08 100644
--- a/send/smtp.c
+++ b/send/smtp.c
@@ -1071,16 +1071,14 @@ static int smtp_open(struct SmtpAccountData *adata, bool esmtp)
   }
 #endif
 
-  if (force_auth || adata->conn->account.flags & MUTT_ACCT_USER)
-  {
-    if (!(adata->capabilities & SMTP_CAP_AUTH))
-    {
-      mutt_error(_("SMTP server does not support authentication"));
-      return -1;
-    }
-
+  if (adata->capabilities & SMTP_CAP_AUTH &&
+      (adata->conn->account.flags & MUTT_ACCT_USER ||
+       force_auth
+#ifdef USE_SSL
+       || cs_subset_path(NeoMutt->sub, "ssl_client_cert")
+#endif
+      ))
     return smtp_authenticate(adata);
-  }
 
   return 0;
 }

However, in this case neomutt still prompts for the username and if the prompt is left empty it sends the empty string. This should be no problem with Exim which apparently either ignores the username altogether or can expect a specific one(s) (so smtp_user will have a valid value and it will work even without the above patch -- note however that setting smtp_user to an empty string seems to be the same as if it was not set at all, so in such case the patch is still needed). However it's not clear whether other server implementations behave the same and thus whether it could be an issue that it doesn't seem possible to prevent neomutt from sending any username at all (while also having smtp_authenticate run). So it might turn out to be most robust to just remove all the prompts and rely only on what the user has put in neomuttrc (or :set at runtime) as we probably usually expect the user to know the necessary configuration, especially with such an exotic setup.

I hope this information may prove useful to someone in the future but I leave further investigation and possible neomutt patches to them :-)