[Date Prev][Date Next]
[Chronological]
[Thread]
[Top]
Re: (ITS#9055) contrib/slapd-modules/passwd/totp improvements
On Thu, Sep 05, 2019 at 10:59:46AM -0400, Greg Veldman wrote:
> On Thu, Sep 05, 2019 at 11:52:45AM +0200, Ond??ej Kuzn??k wrote:
>> - could you split it in two patches, one to check the previous time step
>> (+doc) and one to support the new schemes (+doc)?
>
> Working on it, will have updated patches up shortly...
Thanks for the updated patches.
>> - I don't think you need to allocate a copy of the passwd just come in,
>> you can just frame it into separate bervals reusing the provided
>> buffer so long as you keep in mind they are not NUL-terminated
>> properly.
>
> Are you referring to the chk_totp_and_pw() function? If so,
> since the expected format is <password><totp> with no seperator,
> if I terminated the password part that would overwrite the first
> char of totp, yes? That's the reason I make a copy and allocate
> an extra byte for the NUL.
I mean the ber_str2bv(,,1,) in both new functions. Not sure which code
you think would overwrite parts of the buffer? ber_str2bv(,,0,) never
touches it, manually initialising the berval certainly wouldn't either.
And then you have fewer memory regions to scrub.
Since you already know the length, you can also pass it in so ber_str2bv
can skip its strlen() check (and since anything can be in a {PLAINTEXT}
password, you're now embedded NUL safe).
>> Just a style note, if there's an else coming, could you make sure both
>> the if and the else blocks are in {}?
>
> Implemented, it will be included in the updated patches.
I think I mentioned this before as something worth changing: rather
than call time(0L), you can use op->o_time which is stable and the
closest you can get to the time the operation was received.
BTW, scinet.supercomputing.org's HTTPS cert is signed by Let's Encrypt
Authority X3 as an intermediate, but isn't sending it during the
handshake, so wget/curl aren't happy trusting it (I think browsers cache
it or already have a copy).
Thanks,
--
OndÅ?ej KuznÃk
Senior Software Engineer
Symas Corporation http://www.symas.com
Packaged, certified, and supported LDAP solutions powered by OpenLDAP