httpd/carddav from git sources

Ken Murchison murch at andrew.cmu.edu
Wed Jun 29 09:11:07 EDT 2016 


> On Jun 29, 2016, at 8:55 AM, Johan Hattne <johan at hattne.se> wrote:
> 
> Hi Ellie & Ken;
> 
>> On Jun 28, 2016, at 20:47, ellie timoney <ellie at 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. 



> 
>>> On Tue, Jun 28, 2016, at 05:31 PM, Johan Hattne wrote:
>>> 
>>>> On Jun 26, 2016, at 20:02, ellie timoney <ellie at 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 at 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

More information about the Info-cyrus mailing list