On Mon, Aug 05, 2024 at 10:13:16AM -0700, Russ Allbery wrote: > Niko Tyni <nt...@debian.org> writes:
> > If it's really the right thing to do, I suppose casting to an IV would > > be more correct than either int or long int. But I suspect that the > > intention might be to dereference the pointer instead. FWIW I tried that > > at home and the test suite still passed, so clearly it's not covering > > these parts. > > I agree. I think the upstream code is buggy here and was incorrectly > returning the value of the pointer rather than the underlying property > value. Thanks for confirming! I propose the attached patch on top of Étienne's. My understanding of for example https://www.sendmail.org/~ca/email/cyrus2/mechanisms.html is that the security strength factor (SSF) is always zero for PLAIN connections, so I'm testing for that value. I've checked that the new test fails as expected (yielding a pointer value as a large integer) without the code change. > > I don't see any reverse dependencies, so removal is also an option. > > Particularly as this seems security sensitive and abandoned upstream... > > The lack of dependencies is somewhat deceptive because this module is > transparently used by Authen::SASL (which is, somewhat surprisingly, > missing any relevant dependency, even at the Suggests level; that's > probably a different bug). I believe it prefers Authen::SASL::XS if it is > installed. > > The Perl implementation for Authen::SASL works fine for clients, but if > you want to write a server, you need Authen::SASL::XS if you're using any > mechanism other than the simple password ones. See Authen::SASL::Perl: > > As for server support, only *PLAIN*, *LOGIN* and *DIGEST-MD5* are > supported at the time of this writing. Okay, let's keep it then :) Thanks again. -- Niko
From: Niko Tyni <nt...@debian.org> Date: Mon, 5 Aug 2024 16:50:56 +0100 X-Dgit-Generated: 1.00-2 fe76997d50267530dd5a5f73995d11987547ac4d Subject: Fix SASL_SSF and SASL_MAXOUTBUF property handling sasl_getprop() returns a pointer which needs to be dereferenced to get the actual value. Bug-Debian: https://bugs.debian.org/1075146 --- diff --git a/XS.xs b/XS.xs index eaeef7e..d6d6f4b 100755 --- a/XS.xs +++ b/XS.xs @@ -1883,7 +1883,7 @@ PPCODE: break; case SASL_SSF: case SASL_MAXOUTBUF: - XPUSHi((long int)value); + XPUSHi(*((IV *)value)); break; #ifdef SASL2 case SASL_IPLOCALPORT: diff --git a/t/plain.t b/t/plain.t index 6776cf6..4730905 100755 --- a/t/plain.t +++ b/t/plain.t @@ -1,7 +1,7 @@ use strict; use Authen::SASL qw(XS); # Only use XS plugin -use Test::Simple tests => 5; +use Test::Simple tests => 6; our $me; require "t/common.pl"; @@ -38,6 +38,9 @@ sleep(1); print $conn->listmech("","|",""),"\n"; + my $ssf = $conn->property('ssf'); + ok($ssf == 0, "security strength factor $ssf is zero as expected"); + sendreply( $conn->server_start( getreply(\*FROM_CLIENT) ),\*TO_CLIENT ); ok(1);