Re: [exim] Re: Problem with prvs functions

Página superior
Eliminar este mensaje
Responder a este mensaje
Autor: Brian Candler
Fecha:  
A: Tom Kistner
Cc: exim-users
Asunto: Re: [exim] Re: Problem with prvs functions
On Wed, Jul 13, 2005 at 09:06:30PM +0200, Tom Kistner wrote:
> >which returns "0" for failure and "01" for success (yuk).
>
> I overlooked this. I presume you want to check for validity in the
> router section? I don't do that. My use case is this:
>
> a) Block stuff in ACLs
>
> # Bounces: drop unsigned addresses for BATV senders
>       deny message = This address does not send an unsigned reverse path.
>            senders = :
>            recipients = +batv_recipients

>
>       # Bounces: In case of prvs-signed address, check signature.
>       deny message = Invalid reverse path signature.
>            senders = :
>            condition = ${prvscheck {$local_part@$domain}{PRVSCHECK_SQL}{1}}
>            !condition = $prvscheck_result


That second example is my expected use case. Your particular workaround is
to apply two conditions, where I think only one would be expected - i.e. I'd
expect {1} to be returned in the first condition only if prvscheck passes.
In fact you return {1} always, as long as the address is in a prvs
syntactically-valid form.

I can see there could be cases where you want to check whether an address
has a prvs structure, but not to validate it (e.g. to prevent double-signing
of a message). But I'm not sure that having it "pass" the prvscheck but not
set $prvscheck_result is the best way to indicate that.

Incidentally: I haven't checked whether ${prvs} refuses to sign an address
which is already in prvs-format, but I think it probably should; I don't
think the prvs syntax lends itself to nesting.

    fred@???
->  prvs=fred/1234567890@???
->  prvs=prvs=fred/1234567890/abcdef1234@???


The BNF in the draft doesn't allow this to be unambiguously parsed:

local-part       = tag-type "=" loc-core 
                         *( "/" [tag-val-tag "="] tag-val )


although I suppose it could be parsed in the specific case of prvs, if you
work from right to left and left to right simultaneously.

[The simple rule may be to refuse to prvs-sign any address where the
local-part contains a slash]

> b) If any prvs encoded address has made it past the ACLs, it deserves to
> be redirected to its original address:
>
> batv_redirect:
>         driver = redirect
>         data = ${prvscheck {$local_part@$domain}{PRVSCHECK_SQL}{}}


But that last line highlights the problem; even though you've gone to the
trouble of extracting the secret key again (using the PRVSCHECK_SQL macro),
you're not actually using it! i.e. the expansion returns the *same* value
here whether or not the re-verification actually succeeds, as long as the
address is of a prvs form; so passing PRVSCHECK_SQL as a parameter is at
best confusing. This is very non-obvious behaviour.

It seems to me the actual operations we might want are:

1a. check whether an address is in prvs format
1b. check whether an address is in prvs format, and if so, extract the
    unencoded address
2a. check whether an address is in prvs format and is cryptographically valid
2b. ...and also extract the unencoded address


Your examples don't need (1a) or (2b), but I'll assume they are required for
completeness.

[I guess there's the question of what you want to do with users who aren't
using BATV signing, but the incoming message is in BATV format anyway]

Anyway, the approach you've taken is always to validate the signature, even
if it's not necessary. Given that it's a fairly cheap operation that's
probably OK. But in that case I'd expect prvscheck to say 'valid' only if
the address is completely valid. e.g.

1a/b = ${prvscheck {$local_part@$domain} {$prvs_address} {$prvs_address}}
2a = ${prvscheck {$local_part@$domain} {1} {0}}
2b = ${prvscheck {$local_part@$domain} {$prvs_address} fail}

I think it's still worth considering the 'if' form though: it's a familiar
Exim construct and is hardly any more verbose.

       deny message = Invalid reverse path signature.
            senders = :
            condition = ${if prvscheck {$local_part@$domain}{PRVSCHECK_SQL}{0}{1}}


# I would put a forced 'fail' in the PRVSCHECK_SQL macro, so that users
# without a secret key have that condition skipped. That means that a
# previous test on +batv_recipients isn't required.

batv_redirect:
        driver = redirect
        data = ${if prvscheck {$local_part@$domain}{PRVSCHECK_SQL}{$prvscheck_address}fail}


It might also be worth considering a separate condition just to extract from
prvs format, without crypto validation, for batv_redirect. I suppose this
could be done if there's a null secret, although I don't especially like
that (because a badly written condition could end up with this after a
failed lookup, and then *any* prvs-signed address would validate)

Hmm. I suppose it would be reasonable if:

   ${prvs {addr} {secret}}  acts as a passthrough if {secret} is empty
                            or the address is not signable


   ${if prvscheck {addr} {} ... }    checks for prvs syntax only, sets
                                     prvscheck_address/keynum


   ${if prvscheck {addr} {secret} ... }   also does full crypto & timestamp
                                          validation


Oh well, just thinking aloud...

Regards,

Brian.