> On Jun 29, 2016, at 9:30 AM, Johan Hattne <jo...@hattne.se> wrote:
>
>
>>> On Jun 29, 2016, at 09:11, Ken Murchison <mu...@andrew.cmu.edu> wrote:
>>>
>>> On Jun 29, 2016, at 8:55 AM, Johan Hattne <jo...@hattne.se> wrote:
>>>
>>> Hi Ellie & Ken;
>>>
>>>> On Jun 28, 2016, at 20:47, ellie timoney <el...@fastmail.com> wrote:
>>>>
>>>> Hi Johan,
>>>>
>>>>> In the unpatched code, a sasl_http_request_t structure is created on the
>>>>> stack and a pointer to it is copied to httpd_saslconn using
>>>>> sasl_setprop(). When the structure goes out of scope, there is no
>>>>> guarantee that its members will be preserved. A diff against the code I
>>>>> just cloned (cyrus-imapd-3.0.0-beta2-294-ga42b500) is attached.
>>>>
>>>> I think you're onto something here (though the attachment is missing):
>>>>
>>>> Looking only at the cyrus-imapd code, I would have supposed that
>>>> sasl_setprop() promised to take a copy of the provided value, not just
>>>> the pointer, and that it was therefore fine for the original
>>>> sasl_http_request_t structure to go out of scope.
>>>>
>>>> But looking at cyrus-sasl code, it looks like sasl_setprop() *generally*
>>>> takes a copy of the provided value, but for a few cases -- one of which
>>>> is SASL_HTTP_REQUEST -- it just copies the pointer.
>>>>
>>>> I'm not sure if this is a bug in cyrus-sasl, or in cyrus-imapd, but one
>>>> of them is clearly wrong. The man page for sasl_setprop() makes no
>>>> promises in either direction, but also doesn't document
>>>> SASL_HTTP_REQUEST at all, so the intent remains unclear.
>>>>
>>>> Ken, what do you think? IMO, sasl_setprop() should strictly and
>>>> consistently make its own copy of the provided value, and promise thus,
>>>> in which case the cyrus-imapd code is good. But there might be API
>>>> change ramifications here to be wary of?
>>>>
>>>> I'm curious, how did you come across this?
>>>
>>> I’m not sure how to best deal with it; hence a patch (regenerated and
>>> attached this time) instead of a pull request.
>>>
>>> I ran into this while playing around with digest authentication in carddav.
>>> Authentication relies on the method, which is passed to libsasl in the
>>> sasl_http_request_t structure.
>>
>>
>> Which CardDAV client uses Digest? Just curious. I've found out after
>> implementing it that HTTP Digest is a mess and doesn't interoperate very
>> well. Last I checked Apples calendar app wasn't bumping the nonce count
>> correctly.
>
> I’m toying around with Apple Contacts (which appears count the total number
> of times the nonce goes through the wire instead of the number of times the
> it responds with it). So I completely agree with Ken's assessment, but I
> also think that the problem in httpd is real.
Agreed.
>
>
>>>>>> On Tue, Jun 28, 2016, at 05:31 PM, Johan Hattne wrote:
>>>>>>
>>>>>> On Jun 26, 2016, at 20:02, ellie timoney <el...@fastmail.com> wrote:
>>>>>>
>>>>>> Hi Johan,
>>>>>>
>>>>>> I don't know how your git repository got into that state...
>>>>>>
>>>>>>> $ git describe
>>>>>>> cyrus-imapd-2.5-snapshot-autoconf-and-automake-3857-g4049dd2
>>>>>>
>>>>>> This says that the nearest tag your git can find is
>>>>>> "cyrus-imapd-2.5-snapshot-autoconf-and-automake"(which is over four
>>>>>> years old), and that your branch is 3857 commits ahead of that tag (but
>>>>>> the "cyrus-imapd-2.5.0" release tag is only 1796 commits ahead, so why
>>>>>> didn't describe find that, instead?), and that the commit id of your
>>>>>> branch tip is "4049dd2" (which I don't have, presumably because it's
>>>>>> your local commit containing your changes).
>>>>>>
>>>>>> You probably want to refetch entirely I suspect [but keep reading first]
>>>>>>
>>>>>>> * remote origin
>>>>>>> Fetch URL: git://git.cyrusimap.org/cyrus-imapd
>>>>>>> Push URL: git://git.cyrusimap.org/cyrus-imapd
>>>>>>> HEAD branch: master
>>>>>>
>>>>>> I think these URLs are wrong/old. If you can even fetch from them
>>>>>> anymore, do they update? Look at the dates on the recent commits -- are
>>>>>> they ancient? ('git log --format=fuller' to see commit dates).
>>>>>>
>>>>>> But anyway, we just moved our repositories to github last week, so
>>>>>> anything you get from your old origin is going to be stale now
>>>>>> regardless. https://github.com/cyrusimap/cyrus-imapd is the github page
>>>>>> with the clone/fork/etc links.
>>>>>
>>>>> Thanks a lot, Ellie;
>>>>>
>>>>> I’m confused about the repositories; I didn’t commit anything and the
>>>>> last log entries are from August 2015. Nevertheless, I cloned from
>>>>> GitHub as per above and imap-makefile.patch is indeed obsolete. However,
>>>>> I find that the httpd patch, or something addressing the same symptom, is
>>>>> still necessary.
>>>>>
>>>>> In the unpatched code, a sasl_http_request_t structure is created on the
>>>>> stack and a pointer to it is copied to httpd_saslconn using
>>>>> sasl_setprop(). When the structure goes out of scope, there is no
>>>>> guarantee that its members will be preserved. A diff against the code I
>>>>> just cloned (cyrus-imapd-3.0.0-beta2-294-ga42b500) is attached.
>>>>>
>>>>>>> I got this all from
>>>>>>> https://cyrusimap.org/mediawiki/index.php/Contribute#Anonymous_GIT_Access;
>>>>>>> the text indicates that development is actually happening elsewhere, but
>>>>>>> it has never been clear to me what the relationship between the
>>>>>>> repositories is.
>>>>>>
>>>>>> Okay, that mediawiki is ancient -- I actually thought it was gone
>>>>>> already -- so that explains the old info. I guess you got there from
>>>>>> Google, because it hasn't been linked from cyrusimap.org for ages.
>>>>>>
>>>>>> Anyway, I suspect once you set up the correct git remote, and fetch from
>>>>>> that, your build troubles will go away.
>>>>>
>>>>> Yes, I think I got to the mediawiki via Google.
>>>>>
>>>>> // Best wishes; Johan
>>>>>
>>>>>
>>>>>>> On Fri, Jun 24, 2016, at 11:33 PM, Johan Hattne wrote:
>>>>>>> Hi Ellie;
>>>>>>>
>>>>>>> $ git remote show origin | head -n 4
>>>>>>> * remote origin
>>>>>>> Fetch URL: git://git.cyrusimap.org/cyrus-imapd
>>>>>>> Push URL: git://git.cyrusimap.org/cyrus-imapd
>>>>>>> HEAD branch: master
>>>>>>> $ git describe
>>>>>>> cyrus-imapd-2.5-snapshot-autoconf-and-automake-3857-g4049dd2
>>>>>>>
>>>>>>> I got this all from
>>>>>>> https://cyrusimap.org/mediawiki/index.php/Contribute#Anonymous_GIT_Access;
>>>>>>> the text indicates that development is actually happening elsewhere, but
>>>>>>> it has never been clear to me what the relationship between the
>>>>>>> repositories is.
>>>>>>>
>>>>>>> // Best wishes; Johan
>>>>>>>
>>>>>>>> On Jun 23, 2016, at 19:08, ellie timoney via Info-cyrus
>>>>>>>> <info-cyrus@lists.andrew.cmu.edu> wrote:
>>>>>>>>
>>>>>>>> Hi Johan,
>>>>>>>>
>>>>>>>> What revision are these patches against?
>>>>>>>>
>>>>>>>> The Makefile.am patch is unnecessary, and doesn't apply cleanly anyway,
>>>>>>>> so I suspect you're looking at an old revision.
>>>>>>>>
>>>>>>>> I haven't studied the httpd patch in depth yet.
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>>
>>>>>>>> ellie
>>>>>>>>
>>>>>>>>> On Thu, Jun 23, 2016, at 12:59 AM, Johan Hattne via Info-cyrus wrote:
>>>>>>>>> Dear all;
>>>>>>>>>
>>>>>>>>> To make carddav run from git sources
>>>>>>>>> (git://git.cyrusimap.org/cyrus-imapd) I had to apply attached tiny
>>>>>>>>> patches; the Makefile.am patch addresses a use-before-definition issue
>>>>>>>>> which causes automake-1.14.1 to fall over, the httpd patch ensures
>>>>>>>>> that
>>>>>>>>> the sasl_http_request_t structure which is set as a property of
>>>>>>>>> httpd_saslconn does not go out of scope before it is used (I suppose
>>>>>>>>> this
>>>>>>>>> behavior is a tad system-dependent). Apologies if both these issues
>>>>>>>>> have
>>>>>>>>> already been addressed elsewhere.
>>>>>>>>>
>>>>>>>>> // Best wishes; Johan
>>>>>>>>>
>>>>>>>>> ----
>>>>>>>>> Cyrus Home Page: http://www.cyrusimap.org/
>>>>>>>>> List Archives/Info: http://lists.andrew.cmu.edu/pipermail/info-cyrus/
>>>>>>>>> To Unsubscribe:
>>>>>>>>> https://lists.andrew.cmu.edu/mailman/listinfo/info-cyrus
>>>>>>>>> Email had 2 attachments:
>>>>>>>>> + imap-httpd.patch
>>>>>>>>> 1k (text/plain)
>>>>>>>>> + imap-makefile.patch
>>>>>>>>> 1k (text/plain)
>>>>>>>> ----
>>>>>>>> Cyrus Home Page: http://www.cyrusimap.org/
>>>>>>>> List Archives/Info: http://lists.andrew.cmu.edu/pipermail/info-cyrus/
>>>>>>>> To Unsubscribe:
>>>>>>>> https://lists.andrew.cmu.edu/mailman/listinfo/info-cyrus
>>> <imap-httpd.patch>
>>
>> --
>> Kenneth Murchison
>> Principal Systems Software Engineer
>> Carnegie Mellon University
>
----
Cyrus Home Page: http://www.cyrusimap.org/
List Archives/Info: http://lists.andrew.cmu.edu/pipermail/info-cyrus/
To Unsubscribe:
https://lists.andrew.cmu.edu/mailman/listinfo/info-cyrus
