Re: git master branch going unstable
On Wed, Jan 12, 2011 at 01:12:30PM +1100, Greg Banks wrote: > On 12/01/11 09:47, Bron Gondwana wrote: > > > >Basically it (Greg, correct me if I've messed anything up here...): > > > >[...] > >There's a bit more magic to do with remembering which folders > >are "touched" by each conversation to make the implementation more > >efficient. And of course making reconstruct work, and able to > >repair a damaged conversations database. > > And replication using sync_client/sync_server, and expiring entries > in the conversations database. Minor implementation details ;) Bron.
Re: Crash in timsieved's cmd_authenticate() on 2.4.6
G'day Florian,
On 12/01/11 02:21, Florian Pflug wrote:
Hi
I've just found a bug in timsieved's cmd_authenticate on cyrus 2.4.6.
If the authenticating user is an admin, we proceed even if the mailbox
lookup fails. In this case, the mboxlist_lookup seems to leave the
mboxlist_entry uninitialized, making the code believe that the mailbox
is remote if the bit MBTYPE_REMOTE happens to be set in mbentry.mvtype.
The crash then happens when xstrdup tried to copy mbentry.partition.
Initializing mbentry to zero in cmd_authenticate() fixes the bug and
allows admins without mailboxes (like root) to authenticate again
on my system.
Thanks, your analysis is correct, but I think a better fix might be the
attached (untested) patch.
Bron changed the mboxlist_lookup() API very recently (not yet in a
release), which means that patch won't apply to ToT, but the bug is
still there. As coincidentally I touched that code yesterday, I'll fix it.
--
Greg.
commit a19110e73a9df20969d9763e0ceff2e1fb992c32
Author: Greg Banks
Date: Wed Jan 12 13:29:47 2011 +1100
timsieved: fix crash in cmd_authenticate
Found and analysed by Florian Pflug. If the mboxlist_lookup() fails, the
uninitialised mbentry is examined and decisions made based on random
stack garbage.
diff --git a/timsieved/parser.c b/timsieved/parser.c
index dd8aaa6..1e16e80 100644
--- a/timsieved/parser.c
+++ b/timsieved/parser.c
@@ -714,7 +714,7 @@ static int cmd_authenticate(struct protstream *sieved_out,
goto cleanup;
}
- if(mbentry.mbtype & MBTYPE_REMOTE) {
+ if (!r && mbentry.mbtype & MBTYPE_REMOTE) {
/* It's a remote mailbox */
if (config_getswitch(IMAPOPT_SIEVE_ALLOWREFERRALS)) {
/* We want to set up a referral */
Re: git master branch going unstable
On 12/01/11 09:47, Bron Gondwana wrote: Basically it (Greg, correct me if I've messed anything up here...): [...] There's a bit more magic to do with remembering which folders are "touched" by each conversation to make the implementation more efficient. And of course making reconstruct work, and able to repair a damaged conversations database. And replication using sync_client/sync_server, and expiring entries in the conversations database. -- Greg.
Re: git master branch going unstable
(I hope you don't mind me CC'ing this back to the list so everyone can read it!) On Tue, Jan 11, 2011 at 04:58:06PM +, Alexey Melnikov wrote: > Hi Bron, > > Bron Gondwana wrote: > > >Actually - it's been unstable for a few days now since I added the > >"specialuse" patches > > > I meant to ask earlier: is there a public list of things that you > are planning in 2.5? In particular I am interested in knowing which > IMAP extensions you and your co-workers are planning to add. We should really discuss them on cyrus-devel and the IRC channel. We've been quite lax about that :( Do you have any particular extentions you're interested in? > >* conversations (cross folder thread) initial support from Greg > > Do you have a pointer to how this works/what does this do? No - we'll have to write something up on this. It's going to be a non-standard extention to start off with. Hopefully once we've ironed out the details we can write an RFC. Basically it (Greg, correct me if I've messed anything up here...): *) adds a 64 bit "cid" (conversation id) to each index record. *) adds a separate database (per user) which contains a map of message-id header to cid. *) for each appended message, gets every "message-id" from the message header (Message-Id, In-Reply-To, References) and looks them up in the conversations database. *) if a related message is found, it keeps the same cid. If no related message is found, it allocates a "random" cid (first 64 bits of the sha1 of the current message in fact) *) SPECIAL CASE: if TWO SEPARATE cids are found then we have a message which links two previously separate conversations. In this case, bias upwards - choose the higher cid and re-number all the messages with the lower cid. (the same renumbering logic will be used by replication to merge different cids from different ends) There's a bit more magic to do with remembering which folders are "touched" by each conversation to make the implementation more efficient. And of course making reconstruct work, and able to repair a damaged conversations database. . We're about here now. The next step is defining a good interface to access this data :) In particular, there's a few things we need: >From Folder/UidValidity/Uid triplet - get a conversation ID. >From a triplet or a conversation ID - get a list of all "related" messages (those with the same conversation ID) across ALL folders belonging to the same user. SEARCH - within a conversation SORT - within a conversation . Kind of somewhat separately, we want some form of sorted search windowing... where you can say "give me the first 10 messages which match this search within this sort - and it will sort the messages (efficient) first and then apply the search (usually more expensive) to each message until it's found enough matches - then return! At the moment it always searches first, then sorts the search results, which can be very expensive for a body search across all folders - which is what customers often want, funnily enough. Bron.
Re: Universal tool - /usr/bin/cyrus
Ondřej Surý wrote: > Ok, > > can I get the responses on the mailing list as "yes, we are interested > in such universal tool in case it keeps backward compatibility and > switches to cyrus user?". > Yes. ;-) > What about the name? Is /usr/bin/cyrus good? > /usr/bin/cyrus, /usr/bin/cyrus-tool (as suggested later in this thread) both seem fine to me. > In case the answer to both question is "yes" then I'll improve the > tool to fix those issues mentioned in the mailing list (switching to > cyrus user, compile time path) and then rewrite the tool into the C > (so it can call mailbox_reconstruct() and others directly) with > keeping backwards compatibility (or the usual plan - keep both in one > major release (2.5.x), print deprecation warnings in next major > release (2.6.x) and remove them in next+1 major release (2.7.x)). > > How does that sound? > I would not bother as much with backwards compatibility unless you want to (for the mentally challenging aspect of it, for example). To say the least, I would not consider it a requirement for the tool to be accepted. While you work on this, may I just bluntly invite you to do so in the upstream GIT directly? If interested, please send me your SSH public key, and I'll return with more precise instructions. Kind regards, Jeroen van Meeuwen -- Senior Engineer, Kolab Systems AG e: vanmeeu...@kolabsys.com t: +316 42 801 403 w: http://www.kolabsys.com pgp: 9342 BF08
Re: git master branch going unstable
Bron Gondwana wrote: > Anyway: > > ** > ** > * !! CONSIDER THIS A WARNING !!! * > ** > ** > > Use git master at your own risk, punk. > I suppose this also means we'll be churning out regular, unstable snapshot releases gearing towards 2.5. If you feel like contributing, testing those for regressions might be just up your sleeve! Kind regards, Jeroen van Meeuwen -- Senior Engineer, Kolab Systems AG e: vanmeeu...@kolabsys.com t: +316 42 801 403 w: http://www.kolabsys.com pgp: 9342 BF08
Crash in timsieved's cmd_authenticate() on 2.4.6
Hi I've just found a bug in timsieved's cmd_authenticate on cyrus 2.4.6. If the authenticating user is an admin, we proceed even if the mailbox lookup fails. In this case, the mboxlist_lookup seems to leave the mboxlist_entry uninitialized, making the code believe that the mailbox is remote if the bit MBTYPE_REMOTE happens to be set in mbentry.mvtype. The crash then happens when xstrdup tried to copy mbentry.partition. Initializing mbentry to zero in cmd_authenticate() fixes the bug and allows admins without mailboxes (like root) to authenticate again on my system. best regards, Florian Pflug
Re: Universal tool - /usr/bin/cyrus
cyrus-tool sounds good to me. O. On Tue, Jan 11, 2011 at 15:14, Adam Tauno Williams wrote: > On Tue, 2011-01-11 at 13:45 +0100, Ondřej Surý wrote: >> Ok, >> can I get the responses on the mailing list as "yes, we are interested >> in such universal tool in case it keeps backward compatibility and >> switches to cyrus user?". > >> What about the name? Is /usr/bin/cyrus good? > > I'd go with cyrus-tool, like Samba has switched to a single tool named > samba-tool > > backward-compat doesn't matter much to me, but I'm just one random guy. > I've always thought the tools, while pretty good, where/are themselves a > bit random/arbitrary in name and syntax. [But then I'm a big fan of > Powershell's verb-noun consistency] > >> In case the answer to both question is "yes" then I'll improve the >> tool to fix those issues mentioned in the mailing list (switching to >> cyrus user, compile time path) and then rewrite the tool into the C >> (so it can call mailbox_reconstruct() and others directly) with >> keeping backwards compatibility (or the usual plan - keep both in one >> major release (2.5.x), print deprecation warnings in next major >> release (2.6.x) and remove them in next+1 major release (2.7.x)). >> How does that sound? > > > > -- Ondřej Surý
Re: Universal tool - /usr/bin/cyrus
On Tue, 2011-01-11 at 13:45 +0100, Ondřej Surý wrote: > Ok, > can I get the responses on the mailing list as "yes, we are interested > in such universal tool in case it keeps backward compatibility and > switches to cyrus user?". > What about the name? Is /usr/bin/cyrus good? I'd go with cyrus-tool, like Samba has switched to a single tool named samba-tool backward-compat doesn't matter much to me, but I'm just one random guy. I've always thought the tools, while pretty good, where/are themselves a bit random/arbitrary in name and syntax. [But then I'm a big fan of Powershell's verb-noun consistency] > In case the answer to both question is "yes" then I'll improve the > tool to fix those issues mentioned in the mailing list (switching to > cyrus user, compile time path) and then rewrite the tool into the C > (so it can call mailbox_reconstruct() and others directly) with > keeping backwards compatibility (or the usual plan - keep both in one > major release (2.5.x), print deprecation warnings in next major > release (2.6.x) and remove them in next+1 major release (2.7.x)). > How does that sound?
Re: Universal tool - /usr/bin/cyrus
Ok, can I get the responses on the mailing list as "yes, we are interested in such universal tool in case it keeps backward compatibility and switches to cyrus user?". What about the name? Is /usr/bin/cyrus good? In case the answer to both question is "yes" then I'll improve the tool to fix those issues mentioned in the mailing list (switching to cyrus user, compile time path) and then rewrite the tool into the C (so it can call mailbox_reconstruct() and others directly) with keeping backwards compatibility (or the usual plan - keep both in one major release (2.5.x), print deprecation warnings in next major release (2.6.x) and remove them in next+1 major release (2.7.x)). How does that sound? Regards, Ondrej On Thu, Jan 6, 2011 at 19:02, Jeroen van Meeuwen (Kolab Systems) wrote: > Ondřej Surý wrote: > >> Hi, > >> > > Hi Ondrej, > > First of all, apologies for my belated response. > >> as a part of packaging cyrus-imapd for debian we have talked (and I > >> have coded it) about universal tool (like f.e. git has) which will > >> support all those commands in $libdir/bin/ directory. > >> > >> I have called it /usr/bin/cyrus, but it's not published yet, so if we > >> can agree on a need to have such command (and since there is several > >> name clash if the cyrus binaries are installed into /usr/bin I think > >> it's good idea to have such command). > >> > >> I would be happy to recode this into something more universal than > >> just a wrapper script to call /usr/lib/cyrus/bin/ so new > >> commands (like make_sha1) could be implemented directly as a > >> subcommand of this universal tool - with fallback to calling external > >> commands. (Look at git source it has a nice example how to implement > >> that.) > >> > > I suppose I have two remarks: > > - switching to the cyrus user (with complementary group mail) or any such > configurable would prove usefull in terms of adoption into different > distributions, and switching to the cyrus user would perhaps make the > wrapper a /usr/sbin/ utility (/usr/bin/cyrus of course could be linked to > consolehelper), > > - As the /usr/lib/cyrus/bin/ path is actually a compile time configurable, I > suppose this wrapper can make use of that fact, and be included into > upstream in full. > > The former notwithstanding, I like what I'm seeing. > > I suppose for the switching to the cyrus user, what I always do is, for > example: > > # su - -s /bin/bash cyrus -c '/usr/lib/cyrus-imapd/ctl_mboxlist -d' > > Because the cyrus user does not have a valid shell in RPM based > distributions. The '-' (for login) is there to make sure we not only *gain* > cyrus user privileges but also drop the privileges we used to switch to the > cyrus user (not sure this is damn accurate BTW). > > Thank you, > > Kind regards, > > Jeroen van Meeuwen > > -- > > Senior Engineer, Kolab Systems AG > > e: vanmeeu...@kolabsys.com > > t: +316 42 801 403 > > w: http://www.kolabsys.com > > pgp: 9342 BF08 -- Ondřej Surý
git master branch going unstable
Actually - it's been unstable for a few days now since I added the "specialuse" patches - but it's about to get more unstable pretty fast. Greg (gnb) has about 100 minor fixups to be applied, and then we're going to start pushing on to big ticket items: * conversations (cross folder thread) initial support from Greg * and work on merging replication into imapd from me * autosieve and autocreate patches * charset handling RFCs That's the user visible stuff on my internal timeline for 2.5. There's also a pile of work to be done in simplifying and completing the 'buf', 'prot' and 'dlist' APIs and using them in considerably more places rather than hand-spun data structures everywhere. This should make memory management simpler and more robust, and memory lifetimes a lot easier to track (at the expense of a few more mallocs - but honestly, they're cheap compared to complex tracking of who gets to free what) Anyway: ** ** * !! CONSIDER THIS A WARNING !!! * ** ** Use git master at your own risk, punk. Thanks for your attention, Bron ( if you're not using a fixed-width font, enjoy the space damage above )
