sieve addflag/setflag for seen flag doesn't work with keep/implicit keep

The whole mailbox/append code leaves me a bit lost, but I've
tracked down 
the general area of the problem, Ken maybe you can work out
what the right 
fix is, I don't think it's hard.

sieve_fileinto() does the following:

1. cast void * sc -> script_data_t *sd
2. pass "sd->username" as the
"authuser" param to deliver_mailbox()
3. deliver_mailbox() calls append_setup() with
"authuser" as the "userid" 
param
4. append_setup() copys the "userid" parameter
into the "appendstate.userid" 
struct area
5. during append_commit() "appendstate.userid" is
used as the username to 
add the seen flag to

That all works fine.

However, sieve_keep() does the following:

1. cast void *mc -> deliver_data_t *mydata
2. pass "mydata" as the "mydata" param
to deliver_local()
3. deliver_local() calls deliver_mailbox(), with
"mydata->authuser" as the 
"userid" param

The value in mydata->authuser is not the username, in
fact they're empty:

(gdb) p *mydata
$29 = { ..., authuser = 0x0, authstate = 0x0}

This means the seen flag never gets stored correctly if
you're using keep or 
the implicit keep, you have to use fileinto.

I'm not sure of the best way of fixing this. I can see the
obvious solution 
(in sieve_keep, cast void * sc -> script_data_t *sd, and
copy authuser and 
authstate sd to mydata), but that doesn't feel right to me.

Ken, do you know what the right solution is here?

Rob

I plan on looking into this on Monday.  I need to look at
this code 
anyways to make fileinto work across backends.


Rob Mueller wrote:
> The whole mailbox/append code leaves me a bit lost, but
I've tracked 
> down the general area of the problem, Ken maybe you can
work out what 
> the right fix is, I don't think it's hard.
> 
> sieve_fileinto() does the following:
> 
> 1. cast void * sc -> script_data_t *sd
> 2. pass "sd->username" as the
"authuser" param to deliver_mailbox()
> 3. deliver_mailbox() calls append_setup() with
"authuser" as the 
> "userid" param
> 4. append_setup() copys the "userid"
parameter into the 
> "appendstate.userid" struct area
> 5. during append_commit()
"appendstate.userid" is used as the username 
> to add the seen flag to
> 
> That all works fine.
> 
> However, sieve_keep() does the following:
> 
> 1. cast void *mc -> deliver_data_t *mydata
> 2. pass "mydata" as the "mydata"
param to deliver_local()
> 3. deliver_local() calls deliver_mailbox(), with
"mydata->authuser" as 
> the "userid" param
> 
> The value in mydata->authuser is not the username,
in fact they're empty:
> 
> (gdb) p *mydata
> $29 = { ..., authuser = 0x0, authstate = 0x0}
> 
> This means the seen flag never gets stored correctly if
you're using 
> keep or the implicit keep, you have to use fileinto.
> 
> I'm not sure of the best way of fixing this. I can see
the obvious 
> solution (in sieve_keep, cast void * sc ->
script_data_t *sd, and copy 
> authuser and authstate sd to mydata), but that doesn't
feel right to me.
> 
> Ken, do you know what the right solution is here?
> 
> Rob
> 
> 


-- 
Kenneth Murchison
Systems Programmer
Project Cyrus Developer/Maintainer
Carnegie Mellon University

Sorry for the delay in looking into this.  I looked at the
code, and 
finally realized why it was written in the way that it was:

- For fileinto, we use the credentials of the script owner
when 
delivering to the mailbox and optionally setting IMAP flags.
 We know 
that the script owner has explicitly told us what mailbox to
deliver to 
and with which flags.  Of course, the script owner must
still have the 
appropriate ACL.

- For keep, we use the credentials of the LMTP
authenticated/authorized 
user (if any).  We treat keep no different than a regular
LMTP delivery 
where no sieve script is executed.  Yes, an explicit keep is
a result of 
a sieve script, but since the destination mailbox was not
explicitly 
provided by the script owner, how is the script engine
supposed to know 
if the recipient wants some anonymous user to be able to
deliver to the 
mailbox and optionally set IMAP flags on messages?  If the
ACL is set 
accordingly for a particular auth'd sender (or anonymous),
then the 
message can be delivered and possibly have flags set.

If anyone has any thoughts on how to improve on this without
creating a 
security hole, I'm all ears.


Rob Mueller wrote:
> The whole mailbox/append code leaves me a bit lost, but
I've tracked 
> down the general area of the problem, Ken maybe you can
work out what 
> the right fix is, I don't think it's hard.
> 
> sieve_fileinto() does the following:
> 
> 1. cast void * sc -> script_data_t *sd
> 2. pass "sd->username" as the
"authuser" param to deliver_mailbox()
> 3. deliver_mailbox() calls append_setup() with
"authuser" as the 
> "userid" param
> 4. append_setup() copys the "userid"
parameter into the 
> "appendstate.userid" struct area
> 5. during append_commit()
"appendstate.userid" is used as the username 
> to add the seen flag to
> 
> That all works fine.
> 
> However, sieve_keep() does the following:
> 
> 1. cast void *mc -> deliver_data_t *mydata
> 2. pass "mydata" as the "mydata"
param to deliver_local()
> 3. deliver_local() calls deliver_mailbox(), with
"mydata->authuser" as 
> the "userid" param
> 
> The value in mydata->authuser is not the username,
in fact they're empty:
> 
> (gdb) p *mydata
> $29 = { ..., authuser = 0x0, authstate = 0x0}
> 
> This means the seen flag never gets stored correctly if
you're using 
> keep or the implicit keep, you have to use fileinto.
> 
> I'm not sure of the best way of fixing this. I can see
the obvious 
> solution (in sieve_keep, cast void * sc ->
script_data_t *sd, and copy 
> authuser and authstate sd to mydata), but that doesn't
feel right to me.
> 
> Ken, do you know what the right solution is here?
> 
> Rob
> 
> 


-- 
Kenneth Murchison
Systems Programmer
Project Cyrus Developer/Maintainer
Carnegie Mellon University

> - For keep, we use the credentials of the LMTP
authenticated/authorized
> user (if any).  We treat keep no different than a
regular LMTP delivery 
> where no sieve script is executed.  Yes, an explicit
keep is a result of a 
> sieve script, but since the destination mailbox was not
explicitly 
> provided by the script owner, how is the script engine
supposed to know if 
> the recipient wants some anonymous user to be able to
deliver to the 
> mailbox and optionally set IMAP flags on messages?  If
the ACL is set 
> accordingly for a particular auth'd sender (or
anonymous), then the 
> message can be delivered and possibly have flags set.

I don't have a problem with that logic for delivery.

However, the script did explicitly set the seen flag,
regardless of if it 
ends with a keep/fileinto, we should be setting the flag
correctly because 
the user explicitly said to.

In fact this does work for every other flag, except the seen
flag.

It seems to me the problem is that you're overloading the
use of "authuser" 
to mean the "seenuser" as well. Maybe you need an
explicit "seenuser" 
parameter that can be passed to deliver_mailbox()? Or if not
that, maybe we 
need to get the seenuser based on the actual mailbox name
itself that it's 
being delivered to? That seems reasonable, because only a
sieve script can 
set the seen flag, and at the moment there's no way to set
the seen flag 
for any user other than the mailbox it's being delivered
to?

Rob

>> The whole mailbox/append code leaves me a bit lost,
but I've tracked down 
>> the general area of the problem, Ken maybe you can
work out what the 
>> right fix is, I don't think it's hard.
>>
>> sieve_fileinto() does the following:
>>
>> 1. cast void * sc -> script_data_t *sd
>> 2. pass "sd->username" as the
"authuser" param to deliver_mailbox()
>> 3. deliver_mailbox() calls append_setup() with
"authuser" as the "userid" 
>> param
>> 4. append_setup() copys the "userid"
parameter into the 
>> "appendstate.userid" struct area
>> 5. during append_commit()
"appendstate.userid" is used as the username to 
>> add the seen flag to
>>
>> That all works fine.
>>
>> However, sieve_keep() does the following:
>>
>> 1. cast void *mc -> deliver_data_t *mydata
>> 2. pass "mydata" as the
"mydata" param to deliver_local()
>> 3. deliver_local() calls deliver_mailbox(), with
"mydata->authuser" as 
>> the "userid" param
>>
>> The value in mydata->authuser is not the
username, in fact they're empty:
>>
>> (gdb) p *mydata
>> $29 = { ..., authuser = 0x0, authstate = 0x0}
>>
>> This means the seen flag never gets stored
correctly if you're using keep 
>> or the implicit keep, you have to use fileinto.
>>
>> I'm not sure of the best way of fixing this. I can
see the obvious 
>> solution (in sieve_keep, cast void * sc ->
script_data_t *sd, and copy 
>> authuser and authstate sd to mydata), but that
doesn't feel right to me.
>>
>> Ken, do you know what the right solution is here?
>>
>> Rob
>>
>>
>
>
> -- 
> Kenneth Murchison
> Systems Programmer
> Project Cyrus Developer/Maintainer
> Carnegie Mellon University
>