I have tried it for both the new code and both still don't output the
set_id. I am freebsd 9 if that helps at all.
----------------------------8< cut here >8------------------------------
/* success. we have a winner. */
{
int rc = auth_check_serv_cond(ablock);
return rc;
}
----------------------------8< cut here >8------------------------------
----------------------------8< cut here >8------------------------------
{
int rc = auth_check_serv_cond(ablock);
if (rc == OK)
{
auth_vars[0] = expand_nstring[1] = string_copy(msgbuf);
return OK;
}
CLEANUP_RETURN(rc);
}
----------------------------8< cut here >8------------------------------
exim -d -bV
Exim version 4.80.1 #0 (FreeBSD 9.0) built 06-Nov-2012 10:36:00
Copyright (c) University of Cambridge, 1995 - 2012
(c) The Exim Maintainers and contributors in ACKNOWLEDGMENTS file, 2007 -
2012
Probably Berkeley DB version 1.8x (native mode)
Support for: crypteq iconv() IPv6 use_setclassresources PAM Perl
Expand_dlfunc OpenSSL Content_Scanning DKIM Old_Demime Experimental_SPF
Lookups (built-in): lsearch wildlsearch nwildlsearch iplsearch cdb dbm
dbmjz dbmnz dnsdb dsearch mysql passwd
Authenticators: cram_md5 dovecot plaintext spa
Routers: accept dnslookup ipliteral manualroute queryprogram redirect
Transports: appendfile/maildir/mailstore/mbx autoreply lmtp pipe smtp
Fixed never_users: 0
Size of off_t: 8
Compiler: GCC [4.2.1 20070831 patched [FreeBSD]]
Library version: OpenSSL: Compile: OpenSSL 0.9.8q 2 Dec 2010
Runtime: OpenSSL 0.9.8q 2 Dec 2010
Library version: PCRE: Compile: 8.31
Runtime: 8.31 2012-07-06
Library version: MySQL: Compile: 5.5.27 [Source distribution]
Runtime: 5.5.27
WHITELIST_D_MACROS unset
TRUSTED_CONFIG_LIST unset
Exim version 4.80.1 (FreeBSD 9.0) uid=0 gid=0 pid=43640 D=fbb95cfd
changed uid/gid: forcing real = effective
uid=0 gid=0 pid=43640
auxiliary group list: 0
seeking password data for user "mailnull": using cached result
getpwnam() succeeded uid=26 gid=26
seeking password data for user "root": cache not available
getpwnam() succeeded uid=0 gid=0
changed uid/gid: calling tls_validate_require_cipher
uid=26 gid=6 pid=43641
auxiliary group list: 6
tls_validate_require_cipher child 43641 ended: status=0x0
configuration file is /usr/local/etc/exim/configure
log selectors = 00008ffe 002b380d
cwd=/usr/ports/mail/exim 3 args: exim -d -bV
trusted user
admin user
changed uid/gid: privilege not needed
uid=26 gid=6 pid=43640
auxiliary group list: 6
seeking password data for user "mailnull": cache not available
getpwnam() succeeded uid=26 gid=26
originator: uid=0 gid=0 login=root name=Charlie Root
sender address = root@<hostname>
Configuration file is /usr/local/etc/exim/configure
On Tue, Nov 6, 2012 at 9:43 AM, Phil Pennock <pdp@???> wrote:
> On 2012-11-05 at 18:29 +0200, James Isolder wrote:
> > The below auth when it fails
> >
> > spa_auth:
> > driver = spa
> > public_name = NTLM
> > hide server_password = ${lookup
> > mysql{NTLM_AUTH}{$value}fail}
> > server_set_id = $1
> >
> > used to give me
> >
> > 2012-11-05 17:20:18 spa_auth authenticator failed for <hostname> [IP
> > number]: 535 Incorrect authentication
> > data (set_id=<authid>)
> >
> > After the upgrade to 4.80.1 I only get
> >
> > 2012-11-05 18:10:59 spa_authenticator authenticator failed for <hostname>
> > [IP number]: 535 Incorrect authentication
> > data
>
> This is my fault, and only affects NTLM authentication. The bad commit
> is 08488c86; I was fixing Clang compiler warnings. In spa.c:
>
> ----------------------------8< cut here >8------------------------------
> +/* clean up globals which aren't referenced, but still shouldn't be left
> +pointing to stack memory */
> +#define CLEANUP_RETURN(Code) do { auth_vars[0] = expand_nstring[1] =
> NULL; \
> + expand_nlength[1] = expand_nmax = 0; return (Code); } while (0);
> ----------------------------8< cut here >8------------------------------
>
> This arose from:
> http://mejor.pl/exim-4.77/report-IORFNM.html#EndPath
>
> Basically, the address of stack memory was assigned to a global
> variable, which is then referenced after return, kind-of-before another
> function call. This is "problematic", so changing it was right. The
> change applied was to wipe the data as not being needed after return,
> which is false.
>
> To get it working quickly, in this section of code in src/auths/spa.c:
> ----------------------------8< cut here >8------------------------------
> 241 /* success. we have a winner. */
> 242 {
> 243 int rc = auth_check_serv_cond(ablock);
> 244 CLEANUP_RETURN(rc);
> 245 }
> ----------------------------8< cut here >8------------------------------
>
> you can change "CLEANUP_RETURN(rc);" to "return rc;".
>
> That will still leave stack memory in a global, it's still not great,
> but what the code in src/smtp_in.c does is this:
> ----------------------------8< cut here >8------------------------------
> c = (au->info->servercode)(au, smtp_cmd_data);
> if (au->set_id != NULL) set_id = expand_string(au->set_id);
> ----------------------------8< cut here >8------------------------------
>
> So immediately after the authenticator returns, set_id will be expanded,
> which uses the auth values. After _that_, nothing uses them. As long
> as expand_string didn't extend its stack memory usage to more than that
> of the SPA authenticator, we got away with it.
>
> For bonus points, please change the bad code block in spa.c starting at
> line 242 to this untested code: someone needs to test it, and I don't
> have a SPA setup in which to test (thus this
>
> ----------------------------8< cut here >8------------------------------
> {
> int rc = auth_check_serv_cond(ablock);
> if (rc == OK)
> {
> auth_vars[0] = expand_nstring[1] = string_copy(msgbuf);
> return OK;
> }
> CLEANUP_RETURN(rc);
> }
> ----------------------------8< cut here >8------------------------------
>
> Please do try that and let us know if it works; if it does, we'll add
> that to Exim. And fix the test suite, entry 3600, to test a little more
> for SPA.
>
> Thanks,
> -Phil
>
