[exim] Re: Problem with prvs functions

Startseite
Nachricht löschen
Nachricht beantworten
Autor: Brian Candler
Datum:  
To: exim-users
Betreff: [exim] Re: Problem with prvs functions
On Wed, Jul 13, 2005 at 03:23:42PM +0100, Brian Candler wrote:
> OK so far - but if I break either the signature or the secret, I still get a
> validated answer:
>
> # exim -be '${prvscheck {prvs=brian/0123456789@???}{foobar}{$prvscheck_result}}'
> brian@???
>
> # exim -be '${prvscheck {prvs=brian/19844dfc9a@???}{wibble}{$prvscheck_result}}'
> brian@???
>
> Actually, according to doc/NewStuff, $prvscheck_result should either be
> empty string or "1", not an E-mail address.
>
> Am I doing something wrong, or is the doc broken, or is the code broken?


I think I can answer my own question. ${prvscheck} does not work in the same
way as other Exim expansions.

Firstly, it expands its parameter 3 *before* deciding whether the test
passed or not. Secondly, even though it doesn't allow parameter 3 to be
omitted, it treats empty-string as a special case; an empty string here
returns the contents of $prvscheck_address (the unencoded address), whether
or not the cryptographic validation passed or failed.

So:

# exim -d+expand -be '${prvscheck {prvs=brian/0123456789@???}{foobar}{ok}}'
returns 'ok', even though the crypto validation failed

# exim -d+expand -be '${prvscheck {prvs=brian/0123456789@???}{foobar}{$prvscheck_result}}'
returns the E-mail address, because $prvscheck_result is the empty string

# exim -d+expand -be '${prvscheck {prvs=brian/19844dfc9a@???}{foobar}{$prvscheck_result}}'
is the same, even though the signature is valid, because $prvscheck_result
was empty at the time the string was expanded.

So, the only way I can think of to get a sensible answer out of this is:

# exim -d+expand -be '${prvscheck {prvs=brian/19844dfc9a@???}{foobar}{0}}$prvscheck_result'

which returns "0" for failure and "01" for success (yuk).

I think it would be better if it worked like ${expand}: expand parameter 3
if success, expand parameter 4 if fail, and allow 'fail' for explicit
expansion failure in that case. I don't think there's any need to return the
decoded address in the event that cryptographic validation fails; there's
always $prvscheck_address if you need it.

Even better, make prvscheck a condition for use in ${if}, like crypteq or
match.

The patch below fixes some spelling errors in debug messages, but I don't
know enough about exim internals to make ${prvscheck} work in a more
exim-like way.

Also, I'm pretty sure the iexpire/inow logic is hosed. I think it should
check that inow lies between iexpire-7 and iexpire (modulo 1000). At the
moment, a case like
      iexpire = 900
      inow = 100
will be accepted, even though we are 200 days past the signature's expiry
date. To be fair, the BATV draft is not well-designed here.


Regards,

Brian.

--- exim-4.52/src/expand.c.orig    Fri Jul  1 12:09:15 2005
+++ exim-4.52/src/expand.c    Wed Jul 13 17:03:56 2005
@@ -3371,7 +3371,7 @@
       /* Ugliness: We want to expand parameter 1 first, then set
          up expansion variables that are used in the expansion of
          parameter 2. So we clone the string for the first
-         expansion, where we only expand paramter 1. */
+         expansion, where we only expand parameter 1. */
       uschar *s_backup = string_copy(s);


       /* Reset expansion variables */
@@ -3448,18 +3448,18 @@
           if (iexpire > inow)
             {
             prvscheck_result = US"1";
-            DEBUG(D_expand) debug_printf("prvscheck: success, $pvrs_result set to 1\n");
+            DEBUG(D_expand) debug_printf("prvscheck: success, $prvscheck_result set to 1\n");
             }
             else
             {
             prvscheck_result = NULL;
-            DEBUG(D_expand) debug_printf("prvscheck: signature expired, $pvrs_result unset\n");
+            DEBUG(D_expand) debug_printf("prvscheck: signature expired, $prvscheck_result unset\n");
             }
           }
         else
           {
           prvscheck_result = NULL;
-          DEBUG(D_expand) debug_printf("prvscheck: hash failure, $pvrs_result unset\n");
+          DEBUG(D_expand) debug_printf("prvscheck: hash failure, $prvscheck_result unset\n");
           }
       }
       else