SVN r851 is broken!

[rowen]

The basic flow of code in managesieve.lib.php at version 1.9.9 is as follows.

(1) connect and parse server banner storing available auth methods as given by server
(1b) parse legacy banner (I'm afraid I have not studied this legacy code)
(2) start TLS if needed
(3a) if legacy server ask for new banner
(3) parse new post-STARTTS server Banner and store auth methods in lower case without clearing storage first
(4) compare user provided (probably uppercase) auth method list with stored versions and store fist that matches into auth_in_use
(5) Case statement compares auth_in_use to ONLY UPPERCASE options

SVN revision 851 adds 2 strtolower calls that result in the lower casing mentioned in (3) above.
Also you can see that only uppercase auth methods can ever result in a match in (5)
Also at (3) the auth method storage ($this->capabilities) should really be cleaned up as the new post-TLS banner really should overwrite the pre-TLS banner.

I would argue that IF case insensitivity is desirable (which I doubt) then one should upper-case everything as (5) expects upper case.

As you can see (1) and (1b) store auth methods as given by the server at present... a fixed r851 should uppercase here.
(3a) and (3) should clear atleast the previous auth methods from $this->capabilities and possibly clear all of capabilities before parsing the new banner, and when parsing the new banner uppercase the auth methods as storing them.

At (4) the user provided config of wanted auth methods should also be converted to uppercase before being compared to those stored at (1) or (3).

Then at (5) we can be assured that the case statement will be comparing uppercase with uppercase which is at the nub of this issue.

I'm afraid my php is not great else I would write a patch myself, though I seem to be able to read php... so half way there!

This issue is being tracked as debian bug 618569: ​http://bugs.debian.org/618569

My personal view is that r851 should be reverted as I see little benefit in making this part of avelsieve case insensitive.

Regards
Alex Owen

[rowen]

Hello,
I see you have gone for the all strlower approach. I have patched my production server with changeset 1144 and initial tests show that it works.

Many thanks!

I would still note that perhaps the auth capabilities should be reset to be an empty list on STARTTLS but that is not an active problem in my production setup.

I will try and get debain to include this patch in the next debian release.

Thanks again.
Alex Owen