Skip to content

Commit

Permalink
Make the sp_lstchg shadow field reproducible.
Browse files Browse the repository at this point in the history
The third field in the /etc/shadow file (sp_lstchg) contains the date of
the last password change expressed as the number of days since Jan 1, 1970.
As this is a relative time, creating a user today will result in:

   username:17238:0:99999:7:::

whilst creating the same user tomorrow will result in:

    username:17239:0:99999:7:::

This has an impact for the Reproducible Builds[0] project where we aim to
be independent of as many elements the build environment as possible,
including the current date.

This patch changes the behaviour to use the SOURCE_DATE_EPOCH[1]
environment variable (instead of Jan 1, 1970) if valid.

 [0] https://reproducible-builds.org/
 [1] https://reproducible-builds.org/specs/source-date-epoch/

Signed-off-by: Chris Lamb <lamby@debian.org>
  • Loading branch information
lamby committed Apr 10, 2017
1 parent 2f36da5 commit cb610d5
Show file tree
Hide file tree
Showing 8 changed files with 100 additions and 7 deletions.
3 changes: 3 additions & 0 deletions lib/prototypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ extern int getrange (char *range,
unsigned long *min, bool *has_min,
unsigned long *max, bool *has_max);

/* gettime.c */
extern time_t gettime ();

/* get_uid.c */
extern int get_uid (const char *uidstr, uid_t *uid);

Expand Down
1 change: 1 addition & 0 deletions libmisc/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ libmisc_a_SOURCES = \
getdate.y \
getgr_nam_gid.c \
getrange.c \
gettime.c \
hushed.c \
idmapping.h \
idmapping.c \
Expand Down
89 changes: 89 additions & 0 deletions libmisc/gettime.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*
* Copyright (c) 2017, Chris Lamb
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
* 3. The name of the copyright holders or contributors may not be used to
* endorse or promote products derived from this software without
* specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
* PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
* HOLDERS OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

#include <config.h>

#ident "$Id$"

#include <errno.h>
#include <limits.h>
#include <stdio.h>
#include "defines.h"
#include "prototypes.h"

/*
* gettime() returns the time as the number of seconds since the Epoch
*
* Like time(), gettime() returns the time as the number of seconds since the
* Epoch, 1970-01-01 00:00:00 +0000 (UTC), except that if the SOURCE_DATE_EPOCH
* environment variable is exported it will use that instead.
*/
/*@observer@*/time_t gettime ()
{
char *endptr;
char *source_date_epoch;
time_t fallback;
unsigned long long epoch;

fallback = time (NULL);
source_date_epoch = getenv ("SOURCE_DATE_EPOCH");

if (!source_date_epoch)
return fallback;

errno = 0;
epoch = strtoull (source_date_epoch, &endptr, 10);
if ((errno == ERANGE && (epoch == ULLONG_MAX || epoch == 0))
|| (errno != 0 && epoch == 0)) {
fprintf (stderr,
_("Environment variable $SOURCE_DATE_EPOCH: strtoull: %s\n"),
strerror(errno));
} else if (endptr == source_date_epoch) {
fprintf (stderr,
_("Environment variable $SOURCE_DATE_EPOCH: No digits were found: %s\n"),
endptr);
} else if (*endptr != '\0') {
fprintf (stderr,
_("Environment variable $SOURCE_DATE_EPOCH: Trailing garbage: %s\n"),
endptr);
} else if (epoch > ULONG_MAX) {

This comment has been minimized.

Copy link
@alejandro-colomar

alejandro-colomar Jan 6, 2024

Collaborator

Was there any reason to reject >ULONG_MAX? I'm touching this code, and don't see a reason for it; it looks very arbitrary; especially since some systems can have 32-bit long, but 64-bit time_t. Should I just drop that check, or keep it? And why?

Cc: @lamby , @hallyn

This comment has been minimized.

Copy link
@lamby

lamby Jan 7, 2024

Author Contributor

Was there any reason to reject >ULONG_MAX

I'm a little confused; the code doesn't reject >ULONG_MAX - indeed, that is what line 75 does. Or are you asking why we check for this at all...?

This comment has been minimized.

Copy link
@alejandro-colomar

alejandro-colomar Jan 7, 2024

Collaborator

Yes, I'm asking why we check for epoch > ULONG_MAX in line 75. The code in lines 75-78 checks if >ULONG_MAX, and rejects those values; why are we rejecting them?

This comment has been minimized.

Copy link
@lamby

lamby Jan 8, 2024

Author Contributor

re. "some systems can have 32-bit long, but 64-bit time_t" - is this ... new? I'd have to ping whoever originally wrote this snippet back in ~2014. I do seem to recall that this part was queried before and there was a good reason...

This comment has been minimized.

Copy link
@alejandro-colomar

alejandro-colomar Jan 8, 2024

Collaborator

Hmmm, I don't know if back in 2014 there was 64-bit time_t in 32-bit systems. Maybe not. Maybe that's the reason, and ULONG_MAX was being used as a hypothetical TIME_MAX because time_t was known to be 32-bit too. That starts to make sense in my head.

If we all agree, let's replace that test by a test that it fits time_t.

This comment has been minimized.

Copy link
@lamby

lamby Jan 8, 2024

Author Contributor

Not doubting your logic, but do let me run this past the folks on the Reproducible Builds mailing list first? This area of C is not my forte and there are some very low-level folks on there that will be able to confirm and reassure beyond all doubt. (Also, this code snippet exists in a large number of codebases...)

This comment has been minimized.

Copy link
@alejandro-colomar

alejandro-colomar Jan 8, 2024

Collaborator

Sure, please!

Cc: Alejandro Colomar <alx@kernel.org>

This comment has been minimized.

Copy link
@alejandro-colomar

alejandro-colomar Jan 8, 2024

Collaborator

I have written a set of fixes for this function, in this PR: #893 (still a draft).

Here are the relevant patches:

8423d2e#diff-f90bbb842bf44f70863a9cdf0e21d0d7eb7c4c83aa79ba44cf1b0c9155babff4
cf39f81 (updated version: b097ddc)

(The commit messages need some updates, after the info learnt in this discussion.)

These may be interesting to those guys for fixing this same snippet in other code bases (I'd be willing to help in that).

This comment has been minimized.

Copy link
@lamby

lamby Jan 19, 2024

Author Contributor

This comment has been minimized.

Copy link
@pkitszel

pkitszel Mar 6, 2024

Hi, I see no replies on mailing list and the PR linked above is rather huge and still unmerged, so:

The above check resulting in rejection would just cause build to be not reproducible (for upstream builds fallback time will be used) or would cause given user trying to repro to have a failed result (with a clear warning).

My suggestion is to have better snippet since now onward, perhaps even consider just failing the build instead of fallbacking.
(In a way that not providing env variable at all will assing current time and print "use SOURCE_DATE_EPOCH=123456...123 to reproduce this build")

This comment has been minimized.

Copy link
@alejandro-colomar

alejandro-colomar Mar 6, 2024

Collaborator
  • Please continue the discussion in #961.
fprintf (stderr,
_("Environment variable $SOURCE_DATE_EPOCH: value must be smaller than or equal to %lu but was found to be: %llu\n"),
ULONG_MAX, epoch);
} else if (epoch > fallback) {
fprintf (stderr,
_("Environment variable $SOURCE_DATE_EPOCH: value must be smaller than or equal to the current time (%lu) but was found to be: %llu\n"),
fallback, epoch);
} else {
/* Valid */
return (time_t)epoch;
}

return fallback;
}
2 changes: 1 addition & 1 deletion src/chpasswd.c
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ int main (int argc, char **argv)
if (NULL != sp) {
newsp = *sp;
newsp.sp_pwdp = cp;
newsp.sp_lstchg = (long) time ((time_t *)NULL) / SCALE;
newsp.sp_lstchg = (long) gettime () / SCALE;
if (0 == newsp.sp_lstchg) {
/* Better disable aging than requiring a
* password change */
Expand Down
4 changes: 2 additions & 2 deletions src/newusers.c
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ static int add_passwd (struct passwd *pwd, const char *password)
}
spent.sp_pwdp = cp;
}
spent.sp_lstchg = (long) time ((time_t *) 0) / SCALE;
spent.sp_lstchg = (long) gettime () / SCALE;
if (0 == spent.sp_lstchg) {
/* Better disable aging than requiring a password
* change */
Expand Down Expand Up @@ -553,7 +553,7 @@ static int add_passwd (struct passwd *pwd, const char *password)
*/
spent.sp_pwdp = "!";
#endif
spent.sp_lstchg = (long) time ((time_t *) 0) / SCALE;
spent.sp_lstchg = (long) gettime () / SCALE;
if (0 == spent.sp_lstchg) {
/* Better disable aging than requiring a password change */
spent.sp_lstchg = -1;
Expand Down
2 changes: 1 addition & 1 deletion src/passwd.c
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,7 @@ static void update_shadow (void)
}
#ifndef USE_PAM
if (do_update_age) {
nsp->sp_lstchg = (long) time ((time_t *) 0) / SCALE;
nsp->sp_lstchg = (long) gettime () / SCALE;
if (0 == nsp->sp_lstchg) {
/* Better disable aging than requiring a password
* change */
Expand Down
2 changes: 1 addition & 1 deletion src/useradd.c
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,7 @@ static void new_spent (struct spwd *spent)
memzero (spent, sizeof *spent);
spent->sp_namp = (char *) user_name;
spent->sp_pwdp = (char *) user_pass;
spent->sp_lstchg = (long) time ((time_t *) 0) / SCALE;
spent->sp_lstchg = (long) gettime () / SCALE;
if (0 == spent->sp_lstchg) {
/* Better disable aging than requiring a password change */
spent->sp_lstchg = -1;
Expand Down
4 changes: 2 additions & 2 deletions src/usermod.c
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ static void new_spent (struct spwd *spent)
spent->sp_pwdp = new_pw_passwd (spent->sp_pwdp);

if (pflg) {
spent->sp_lstchg = (long) time ((time_t *) 0) / SCALE;
spent->sp_lstchg = (long) gettime () / SCALE;
if (0 == spent->sp_lstchg) {
/* Better disable aging than requiring a password
* change. */
Expand Down Expand Up @@ -1673,7 +1673,7 @@ static void usr_update (void)
spent.sp_pwdp = xstrdup (pwent.pw_passwd);
pwent.pw_passwd = xstrdup (SHADOW_PASSWD_STRING);

spent.sp_lstchg = (long) time ((time_t *) 0) / SCALE;
spent.sp_lstchg = (long) gettime () / SCALE;
if (0 == spent.sp_lstchg) {
/* Better disable aging than
* requiring a password change */
Expand Down

0 comments on commit cb610d5

Please sign in to comment.