[Date Prev][Date Next]
[Chronological]
[Thread]
[Top]
Re: (ITS#8230) [PATCH] totp: bug fixes and improvements
- To: openldap-its@OpenLDAP.org
- Subject: Re: (ITS#8230) [PATCH] totp: bug fixes and improvements
- From: hyc@symas.com
- Date: Sat, 29 Aug 2015 15:44:09 +0000
- Auto-submitted: auto-generated (OpenLDAP-ITS)
peter@adpm.de wrote:
> Full_Name: Peter Marschall
> Version: 2.4.42
> OS: Linux
> URL: https://github.com/marschap/openldap/tree/contrib-totp
> Submission from: (NULL) (92.211.7.6)
>
>
> Hi,
>
> I have written some bugfixes & flexibilizations for the TOTP contrib module.
>
> You can find them in the github branch:
> https://github.com/marschap/openldap/tree/contrib-totp
>
> It differs from today's master by these commits:
> * https://github.com/marschap/openldap/commit/d67bffc4a361cecfce69fb4d14edb334d4e02c6a
> contrib/passwd/totp: flexibilize decoding key
>
> In function totp_b32_pton()
> - allow lowercase characters in encoded string too
This patch is incorrect. You must guard toupper(), it is not guaranteed not to
distort non-lowercase characters.
if (islower(x)) x = toupper(x)
> - allow padding to be omitted (totally, not only parts)
Why?
> In function chk_totp() determine the space required to hold the decoded
> key by calling totp_b32_pton() with a NULL argument for the target.
>
> * https://github.com/marschap/openldap/commit/435976d4f2468946bd0c5081ce7e2ae9fc0659fb
> contrib/passwd/totp: fix the big-endian case
>
> For the big-endian case, 'msg' wasn't set from 'tval' in generate().
This patch is wrong, as I commented on github.
> * https://github.com/marschap/openldap/commit/04c15b7b1e44d4d3167577702a8b77c5ee8bb7b6
> contrib/passwd/totp: fix decoding when padding is used
>
> In totp_b32_pton(), correctly count the number of '=' padding chars
> at the end of the base-32 encoded string.
>
> Note: '*str++' evaluates *str first and increases str later!
The current code correctly decodes known test data. What is your test case?
> * https://github.com/marschap/openldap/commit/04c15b7b1e44d4d3167577702a8b77c5ee8bb7b6
> contrib/passwd/totp: support compiling using nettle
The proper way to add this support is to add macros to hide all of the
differences in the initial ifdef block and not to add any additional ifdefs
anywhere in the main body of the code.
>
> that change the file
> contrib/slapd-modules/passwd/totp/slapd-totp.c | 67
> ++++++++++++++++++B%B++++++++++++++++++++++++++++++++++++------------
>
> I'd appreciate if you include them into OpenLDAP.
>
> The referenced patch files are derived from OpenLDAP Software.
> All of the modifications to OpenLDAP Software represented in the following
> patch(es) were developedy y Peter Marschall <peter@adpm.de>.
> I have not assigned rights and/or interest in this work to any party.
>
> The referenced modifications to OpenLDAP Software are subject to the following
> notice:
> Copyright 2015 Peter Marschall
> Redistribution and use in source and binary forms, with or without
> modification,
> are permitted only as authorized by the OpenLDAP Public License.
>
> Thanks in advance
> Peter
>
>
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/