> On Jun 29, 2016, at 8:55 AM, Johan Hattne <johan@xxxxxxxxx> wrote:
>
> Hi Ellie & Ken;
>
>> On Jun 28, 2016, at 20:47, ellie timoney <ellie@xxxxxxxxxxxx> 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.
>
>>> On Tue, Jun 28, 2016, at 05:31 PM, Johan Hattne wrote:
>>>
>>>> On Jun 26, 2016, at 20:02, ellie timoney <ellie@xxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx> 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
