Re: git: c210cac00f25 - main - dhclient: fix time parsing for leases expiring after 2038
Date: Mon, 10 Jul 2023 16:34:42 UTC
In message <202307101600.36AG0xNe015335@gitrepo.freebsd.org>, Eric van
Gyzen wr
ites:
> The branch main has been updated by vangyzen:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=c210cac00f2584f031b56b4cdd2e801d
> bb9e1348
>
> commit c210cac00f2584f031b56b4cdd2e801dbb9e1348
> Author: Alex Bahm <alexander.bahm@dell.com>
> AuthorDate: 2023-07-10 15:56:05 +0000
> Commit: Eric van Gyzen <vangyzen@FreeBSD.org>
> CommitDate: 2023-07-10 16:00:34 +0000
>
> dhclient: fix time parsing for leases expiring after 2038
>
> Convert lease parsing to timegm to calculate timestamp. For reference, wh
> en
> writing the lease, we use gmtime to convert the timestamp to struct tm.
>
> Reviewed By: markj, vangyzen
> MFC after: 2 weeks
> Sponsored by: Dell EMC Isilon
> Differential Revision: https://reviews.freebsd.org/D40760
> ---
> sbin/dhclient/parse.c | 28 ++-----------------
> sbin/dhclient/tests/Makefile | 3 +-
> sbin/dhclient/tests/fake.c | 15 ++++++++++
> sbin/dhclient/tests/option-domain-search.c | 45 ++++++++++++++++++++++++++++
> ++
> 4 files changed, 64 insertions(+), 27 deletions(-)
>
> diff --git a/sbin/dhclient/parse.c b/sbin/dhclient/parse.c
> index 4018f70f3a5d..70e2013d0813 100644
> --- a/sbin/dhclient/parse.c
> +++ b/sbin/dhclient/parse.c
> @@ -444,9 +444,7 @@ convert_num(unsigned char *buf, char *str, unsigned base,
> int size)
> time_t
> parse_date(FILE *cfile)
> {
> - static int months[11] = { 31, 59, 90, 120, 151, 181,
> - 212, 243, 273, 304, 334 };
> - int guess, token;
> + int token;
> struct tm tm;
> char *val;
>
> @@ -570,27 +568,5 @@ parse_date(FILE *cfile)
> return (0);
> }
>
> - /* Guess the time value... */
> - guess = ((((((365 * (tm.tm_year - 70) + /* Days in years since '70 */
> - (tm.tm_year - 69) / 4 + /* Leap days since '70 */
> - (tm.tm_mon /* Days in months this year */
> - ? months[tm.tm_mon - 1]
> - : 0) +
> - (tm.tm_mon > 1 && /* Leap day this year */
> - !((tm.tm_year - 72) & 3)) +
> - tm.tm_mday - 1) * 24) + /* Day of month */
> - tm.tm_hour) * 60) +
> - tm.tm_min) * 60) + tm.tm_sec;
> -
> - /*
> - * This guess could be wrong because of leap seconds or other
> - * weirdness we don't know about that the system does. For
> - * now, we're just going to accept the guess, but at some point
> - * it might be nice to do a successive approximation here to get
> - * an exact value. Even if the error is small, if the server
> - * is restarted frequently (and thus the lease database is
> - * reread), the error could accumulate into something
> - * significant.
> - */
> - return (guess);
> + return (timegm(&tm));
> }
> diff --git a/sbin/dhclient/tests/Makefile b/sbin/dhclient/tests/Makefile
> index ce4c7acb822e..790d3dbcccce 100644
> --- a/sbin/dhclient/tests/Makefile
> +++ b/sbin/dhclient/tests/Makefile
> @@ -6,7 +6,8 @@ ATF_TESTS_SH= pcp
>
> PLAIN_TESTS_C= option-domain-search_test
> SRCS.option-domain-search_test= alloc.c convert.c hash.c option
> s.c \
> - tables.c fake.c option-domain-search.c
> + tables.c parse.c conflex.c tree.c fake.
> c \
> + option-domain-search.c
> CFLAGS.option-domain-search_test+= -I${.CURDIR:H}
> LIBADD.option-domain-search_test= util
>
> diff --git a/sbin/dhclient/tests/fake.c b/sbin/dhclient/tests/fake.c
> index 6a170953beb0..17b721527f04 100644
> --- a/sbin/dhclient/tests/fake.c
> +++ b/sbin/dhclient/tests/fake.c
> @@ -7,6 +7,7 @@
> #include "dhcpd.h"
>
> extern jmp_buf env;
> +int warnings_occurred;
>
> void
> error(const char *fmt, ...)
> @@ -52,6 +53,20 @@ note(const char *fmt, ...)
> return ret;
> }
>
> +int
> +parse_warn(const char *fmt, ...)
> +{
> + int ret;
> + va_list ap;
> +
> + va_start(ap, fmt);
> + ret = vfprintf(stderr, fmt, ap);
> + va_end(ap);
> + fprintf(stderr, "\n");
> +
> + return ret;
> +}
> +
> void
> bootp(struct packet *packet)
> {
> diff --git a/sbin/dhclient/tests/option-domain-search.c b/sbin/dhclient/tests
> /option-domain-search.c
> index b79f9a560137..a3517c9c1dc1 100644
> --- a/sbin/dhclient/tests/option-domain-search.c
> +++ b/sbin/dhclient/tests/option-domain-search.c
> @@ -303,6 +303,49 @@ multiple_domains_valid()
> free(option->data);
> }
>
> +static
> +void
> +parse_date_helper(const char *string, time_t timestamp)
> +{
> + int ret = 0;
> + FILE *file = NULL;
> + time_t ts;
> +
> + file = fopen("/tmp/dhclient.test", "w");
> + if (!file)
> + abort();
> +
> + ret = fwrite(string, strlen(string), 1, file);
> + if (ret <= 0)
> + abort();
> +
> + fclose(file);
> +
> + file = fopen("/tmp/dhclient.test", "r");
> + if (!file)
> + abort();
> +
> + new_parse("test");
> + ts = parse_date(file);
> + if (ts != timestamp)
> + abort();
> +
> + fclose(file);
> +}
> +
> +void
> +parse_date_valid(void)
> +{
> + int ret;
> +
> + ret = setjmp(env);
> + if (ret != 0)
> + abort();
> +
> + parse_date_helper(" 2 2024/7/2 20:25:50;\n", 1719951950);
> + parse_date_helper(" 1 2091/7/2 20:25:50;\n", 3834246350);
> +}
> +
> int
> main(int argc, char *argv[])
> {
> @@ -324,5 +367,7 @@ main(int argc, char *argv[])
>
> multiple_domains_valid();
>
> + parse_date_valid();
> +
> return (0);
> }
>
This fails to build on i386.
===> libexec/bootpd/tools/bootptest (all)
--- all_subdir_sbin ---
/opt/src/git-src/sbin/dhclient/tests/option-domain-search.c:346:47: error:
implicit conversion from 'long long' to 'time_t' (aka 'int') changes value
from 3834246350 to -460720946 [-Werror,-Wconstant-conversion]
parse_date_helper(" 1 2091/7/2 20:25:50;\n", 3834246350);
~~~~~~~~~~~~~~~~~ ^~~~~~~~~~
1 error generated.
4.16 real 0.37 user 0.36 sys
make[1]: stopped in /opt/src/git-src
make: stopped in /opt/src/git-src
_buildsys 58228 - - i386 build failed, continuing
_buildsys 58233 - - i386 failed: build failure
_buildsys 58238 - - exiting, RC=0: /usr/local/bin/_buildsys -x -uz -Nb
-Ai386 -j8
Finished Mon Jul 10 09:31:24 PDT 2023, RC=0
--
Cheers,
Cy Schubert <Cy.Schubert@cschubert.com>
FreeBSD UNIX: <cy@FreeBSD.org> Web: https://FreeBSD.org
NTP: <cy@nwtime.org> Web: https://nwtime.org
e^(i*pi)+1=0