Bug 1543196

Created attachment 1392920 [details]
Patch which fixes the bug

The crash happens when using the following (non-default) configuration.
Note: sasl_pwcheck_method: auxprop configures cyrus to use /etc/sasldb2
as password-db directly (without any saslauthd necessary). Passwords are maintained with saslpasswd2

/etc/imapd.conf:
serverinfo: on                                                                                                                                                                                                
configdirectory: /var/lib/imap                                                                                                                                                                                
partition-default: /var/spool/imap                                                                                                                                                                            
admins: cyrus                                                                                                                                                                                                 
sievedir: /var/lib/imap/sieve                                                                                                                                                                                 
sendmail: /usr/sbin/sendmail                                                                                                                                                                                  
hashimapspool: true                                                                                                                                                                                           
sasl_pwcheck_method: auxprop                                                                                                                                                                                  
autocreate_quota: -1
unixhierarchysep: yes
autocreate_inbox_folders: Sent|Drafts|Trash|Templates|spam
sievenotifier: mailto
newsprefix: news
sieve_maxscriptsize: 64
duplicatesuppression: 0
altnamespace: 0
sasl_minimum_layer: 128
duplicate_db_path: /var/run/cyrus/deliver.db
ptscache_db_path:  /var/run/cyrus/ptscache.db
statuscache_db_path: /var/run/cyrus/statuscache.db
tls_sessions_db_path: /var/run/cyrus/tls_sessions.db
httpd_sasl_mech_list: PLAIN
httpmodules: caldav carddav
caldav_allowscheduling: on
caldav_create_default: 1
caldav_create_attach: 1
caldav_create_sched: 1
tls_server_cert: /etc/pki/cyrus-imapd/cyrus-imapd.crt
tls_server_key: /etc/pki/cyrus-imapd/cyrus-imapd.key
tls_ciphers: ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-DSS-AES128-GCM-SHA256:kEDH+AESGCM:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA:ECDHE-ECDSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-DSS-AES128-SHA256:DHE-RSA-AES256-SHA256:DHE-DSS-AES256-SHA:DHE-RSA-AES256-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:AES:CAMELLIA:DES-CBC3-SHA:!aNULL:!eNULL:!EXPORT:!DES:!RC4:!MD5:!PSK:!aECDH:!EDH-DSS-DES-CBC3-SHA:!EDH-RSA-DES-CBC3-SHA:!KRB5-DES-CBC3-SHA

/etc/cyrus.conf:
DAEMON {
  # this is only necessary if using idled for IMAP IDLE
  idled         cmd="idled"
}

# UNIX sockets start with a slash and are put into /var/lib/imap/sockets
SERVICES {
  # add or remove based on preferences
  imap          cmd="imapd" listen="192.168.2.17:imap" prefork=5
  imaps         cmd="imapd -s" listen="imaps" prefork=1
  pop3          cmd="pop3d" listen="192.168.2.17:pop3" prefork=3
  pop3s         cmd="pop3d -s" listen="pop3s" prefork=1
  sieve         cmd="timsieved" listen="192.168.2.17:sieve-filter" prefork=0

  # these are only necessary if receiving/exporting usenet via NNTP
  nntp          cmd="nntpd" listen="192.168.2.17:nntp" prefork=3
  nntps         cmd="nntpd -s" listen="192.168.2.17:nntps" prefork=1

  # these are only necessary if using HTTP for CalDAV, CardDAV, or RSS
  http          cmd="httpd -p 128" listen="127.0.0.1:50080" prefork=1
  #http          cmd="httpd -p 128" listen="50080" prefork=1
  https         cmd="httpd -s" listen="50443" prefork=1

  # at least one LMTP is required for delivery
#  lmtp         cmd="lmtpd" listen="lmtp" prefork=0
  lmtpunix      cmd="lmtpd" listen="/var/lib/imap/socket/lmtp" prefork=1

  # this is only necessary if using notifications
#  notify       cmd="notifyd" listen="/var/lib/imap/socket/notify" proto="udp" prefork=1
}

EVENTS {
  # this is required
  checkpoint    cmd="ctl_cyrusdb -c" period=30

  # this is only necessary if using duplicate delivery suppression,
  # Sieve or NNTP
  delprune      cmd="cyr_expire -E 3" at=0400

  # this is only necessary if caching TLS sessions
  tlsprune      cmd="tls_prune" at=0400

  # fetch news
  ### groups redacted ###
}

This is my original pull request at github that was rejected:

https://github.com/cyrusimap/cyrus-imapd/pull/1963

If upstream has rejected the PR, I don't think it's particularly wise to carry it locally without understanding why it is required with just Fedora's cyrus-sasl version.  If this is really a problem with our cyrus-sasl package then I'd rather this bug be reported to the maintainers of that package instead.  I think there's a good chance that your patch is just a workaround and the real problem is not with cyrus-imapd at all.  Looking at the mailing list post, it does show the actual segfault to be in the cyrus-sasl code, not in cyrus-imapd.

Also, adding functionality by improving error messages is something which should go upstream separately and should definitely not be in a patch which aims to work around a segfault.  Remember that if this patch does go into this package, we (the maintainers of the package) will have to carry it and rebase it potentially forever.  So having extra little bits of functionality in with a bugfix is not a good thing.  It seems like the real patch you wanted to submit is just the commenting of a single line.

I probably expressed myself poorly (I'm not a native english speaker)

To clarify:

I don't think, that the patch is required with just Fedora's cyrus-sasl version.
This is only what I deducted from the claim "cyrus-imap master (3.0.x) works fine with cyrus-sasl master (soon-to-be 2.1.27)" made by the guy closing that PR at the time.
I did not try to prove it wrong, because cyrus-sasl's master branch is a moving target. The relevant code however in in cyrus-sasl/lib/common.c is still the same in upstreams master branch like in Fedora's cyrus-sasl. This makes me quite confident that using "their" pristine cyrus-sasl would just result in exactly the same failure *if* used with my specific configuration. They just didn't test that specific code flow.

Looking into cyrus-sasl's source now, also makes me realize that my statement in the initial bug message above "This in turn triggers a buffer overflow when used with Fedora's cyrus-sasl lib." was wrong. sasl_setprop() just returns an error because the minimum security-level in that specific case does not allow setting maxbuffersize to 0. (The buffer overflow was the second - more severe - error which they *did* fix - accepting a PR - at the time)

About the changes of the two error messages:
These changes enabled me to find the bug in the first place. therefore, I consider them part of the solution. Feel free to remove them, but please be aware that then all those error messages are completely useless because they all spell exactly the same and cannot be distinguished.

Your English is fine; no need to worry about that.

I've already communicated with Ken Murchison (who is the primary maintainer of cyrus-sasl and who is the one who rejected the pull request in the first place).  He is going to look at the issue again and I will pass your additional comments on to him.

This message is a reminder that Fedora 26 is nearing its end of life.
Approximately 4 (four) weeks from now Fedora will stop maintaining
and issuing updates for Fedora 26. It is Fedora's policy to close all
bug reports from releases that are no longer maintained. At that time
this bug will be closed as EOL if it remains open with a Fedora  'version'
of '26'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version'
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not
able to fix it before Fedora 26 is end of life. If you would still like
to see this bug fixed and are able to reproduce it against a later version
of Fedora, you are encouraged  change the 'version' to a later Fedora
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's
lifetime, sometimes those efforts are overtaken by events. Often a
more recent Fedora release includes newer upstream software that fixes
bugs or makes them obsolete.

I updated the Fedora version as this is still relevant and a PITA.
Since we did not hear anything from Ken Murchinson in the meantime:
Can you PLEASE just apply my patch already.

Thanks
 -Fritz

I'm sorry, but the patch isn't something I'm willing to apply at this time.  The bug is not in cyrus-imapd as far as I can tell, and I have no idea what side effects it might carry.  The problem needs to be fixed in the right place: cyrus-sasl.  This ticket doesn't carry any backtrace or other information which would suggest otherwise.

I guess the only possibly useful thing I can do is move this ticket over to cyrus-sasl.

Recently I rebased to latest pre-release cyrus-sasl version and got rid of most of the downstream patches, so the difference from master should be minimal. Can you verify that you can reproduce it with Fedora 28?

From the last comments, the issue was that Ken did not reproduce the issue with his setup. Do you have a clue how this can be fixed in cyrus-sasl?

This message is a reminder that Fedora 27 is nearing its end of life.
On 2018-Nov-30  Fedora will stop maintaining and issuing updates for
Fedora 27. It is Fedora's policy to close all bug reports from releases
that are no longer maintained. At that time this bug will be closed as
EOL if it remains open with a Fedora  'version' of '27'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora 27 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora, you are encouraged  change the 'version' to a later Fedora 
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

Fedora 27 changed to end-of-life (EOL) status on 2018-11-30. Fedora 27 is
no longer maintained, which means that it will not receive any further
security or bug fix updates. As a result we are closing this bug.

If you can reproduce this bug against a currently maintained version of
Fedora please feel free to reopen this bug against that version. If you
are unable to reopen this bug, please file a new report against the
current release. If you experience problems, please add a comment to this
bug.

Thank you for reporting this bug and we are sorry it could not be fixed.