CandyShop / gerrit

Automatically exported from code.google.com/p/gerrit
Apache License 2.0
1 stars 0 forks source link

Gmail TLS auth incorrect #1213

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Affected Version:

What steps will reproduce the problem?
1.
Mail confuig like this
[sendemail]
    enable = true
    smtpServer = smtp.gmail.com
    smtpServerPort = 587
    smtpEncryption = TLS
    smtpUser = no-one@domain.co.uk   <-- this is the same as server.email btw
    smtpPass = ******
    sslVerify = false
    from = server

now get gerrit to send an email e.g. register new email address

What is the expected output?
  Send an email

 What do you see instead?
from error_log
[2011-12-16 14:43:23,997] ERROR 
com.google.gerrit.httpd.rpc.account.AccountSecurityImpl : Cannot send email 
verification message to simon@surf.org.uk
com.google.gerrit.server.mail.EmailException: Connection closed without 
indication.
    at

Please provide any additional information below.

manual testing shows this
simon@madoka:~$ gnutls-cli --crlf --starttls --port 25 smtp.gmail.com
Resolving 'smtp.gmail.com'...
Connecting to '209.85.229.109:25'...

- Simple Client Mode:

220 mx.google.com ESMTP eo4sm13318686wib.19
ehlo sjjss
250-mx.google.com at your service, [79.173.186.214]
250-SIZE 35882577
250-8BITMIME
250-XXXXXXXA
250 ENHANCEDSTATUSCODES
STARTTLS
502 5.5.1 Unrecognized command. eo4sm13318686wib.19
quit
221 2.0.0 closing connection eo4sm13318686wib.19
- Peer has closed the GnuTLS connection
simon@ubuntu:~$ gnutls-cli --crlf --starttls --port 587 smtp.gmail.com
Resolving 'smtp.gmail.com'...
Connecting to '209.85.229.108:587'...

- Simple Client Mode:

220 mx.google.com ESMTP u5sm14828533wbm.2
ehlo sjsjs
250-mx.google.com at your service, [79.173.186.214]
250-SIZE 35882577
250-8BITMIME
250-STARTTLS
250 ENHANCEDSTATUSCODES
starttls 
220 2.0.0 Ready to start TLS
ehlo sjsjs
- Peer has closed the GnuTLS connection

So I would suggest the 2nd client.login after starttls in SmtpEmailSender.java 
is incorrect and should be removed. 

Simon Loader simon@plumbee.co.uk

Original issue reported on code.google.com by simon.lo...@plumbee.co.uk on 16 Dec 2011 at 2:47

GoogleCodeExporter commented 9 years ago
I think this is the same issue as 1032 but with more detail.

Original comment by simon.lo...@plumbee.co.uk on 16 Dec 2011 at 2:48

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
And to prove the point switching to SSL fixes the problem.

Original comment by simon.lo...@plumbee.co.uk on 16 Dec 2011 at 2:56

GoogleCodeExporter commented 9 years ago

Suggested patch

git diff
diff --git 
a/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java

b/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java
index f681710..6a7fdbb 100644
--- 
a/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java
+++ 
b/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java
@@ -244,10 +244,6 @@ public class SmtpEmailSender implements EmailSender {
         if (!client.startTLS(smtpHost, smtpPort, sslVerify)) {
           throw new EmailException("SMTP server does not support TLS");
         }
-        if (!client.login()) {
-          String e = client.getReplyString();
-          throw new EmailException("SMTP server rejected login: " + e);
-        }
       }

       if (smtpUser != null && !client.auth(smtpUser, smtpPass)) {

Original comment by simon.lo...@plumbee.co.uk on 16 Dec 2011 at 3:01