Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop LSOF_CCDATE in order to ensure reproducible builds #150

Merged
merged 1 commit into from
Feb 16, 2021

Conversation

dilinger
Copy link
Contributor

LSOF_CCDATE is used to embed the build time/date string into the lsof binary.
This string is displayed as part of 'lsof -v' output. However, having that
embedded string breaks reproducible builds - we want to ensure that subsequent
builds (using the same exact build environemnt) produce the same exact binary,
bit-for-bit.

Debian has a patch that allows overriding the variable, similar to what lsof allows
for LSOF_HOST (and others). Let me know if you'd prefer that patch. However,
I'm of the opinion that LSOF_CCDATE should just be dropped completely. We
can see the version to tell us what the corresponding source code is, and we have
file timestamps to tell us when a binary was installed. LSOF_CCDATE provides
no security, as an attacker could hardcode the older date/time string in their
malicious binary. So, this pull request just drops the variable completely.

@dilinger
Copy link
Contributor Author

By the way - there are additional variables that seem completely unnecessary that I could also remove if you'd like. On my system after I build, I see the following output from 'lsof -v':

lsof version information:
revision: 4.93.2
latest revision: https://github.com/lsof-org/lsof
latest FAQ: https://github.com/lsof-org/lsof/blob/master/00FAQ
latest (non-formatted) man page: https://github.com/lsof-org/lsof/blob/master/Lsof.8
constructed: Tue 26 Jan 2021 07:04:04 AM UTC
constructed by and on: root@e7470
compiler: cc
compiler version: 10.2.1 20210110 (Debian 10.2.1-6)
compiler flags: -g -O2 -ffile-prefix-map=/x/lsof-4.93.2+dfsg=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -DLINUXV=510009 -DGLIBCV=231 -DHASIPv6 -DNEEDS_NETINET_TCPH -DHASSELINUX -DHASUXSOCKEPT -DHASPTYEPT -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE -DHAS_STRFTIME -DLSOF_VSTR="5.10.9" -O
loader flags: -L./lib -llsof -Wl,-z,relro -Wl,-z,now -lselinux
system info: Linux e7470 5.10.0-1-amd64 #1 SMP Debian 5.10.4-1 (2020-12-31) x86_64 GNU/Linux
Anyone can list all files.
/dev warnings are disabled.
Kernel ID check is disabled.

Distributions often override those variables (Debian overrides LSOF_CCDATE, LSOF_HOST, LSOF_LOGNAME, LSOF_SYSINFO, and LSOF_USER).

I'm not sure who actually cares what compiler was used (LSOF_CC), or when it was built (LSOF_CCDATE) and by who (LSOF_USER/LSOF_LOGNAME) and where (LSOF_HOST). I'm happy to add additional commits to this branch to remove more of those variables if you agree with removing any of them. LSOF_SYSINFO in particular seems very unneccessary.

@masatake
Copy link
Contributor

We have already an applied patch for the same purpose.
https://github.com/lsof-org/lsof/pull/94/files
How do you think about this?

@masatake
Copy link
Contributor

Dropping the coverage is not important in this pull request, of course.

@dilinger
Copy link
Contributor Author

We have already an applied patch for the same purpose.
https://github.com/lsof-org/lsof/pull/94/files
How do you think about this?

That will also work. The Debian patch (which you can see here:
https://sources.debian.org/src/lsof/4.93.2+dfsg-1.1/debian/patches/preset-ccdate/ ) changes it for all dialects. For Debian, the only ones we actually care about are Linux and FreeBSD. I can submit a separate PR for freebsd if you'd like.

Personally I'd still rather see a bunch of the noise removed, but it's your choice.

@masatake masatake self-assigned this Jan 28, 2021
@masatake
Copy link
Contributor

masatake commented Feb 8, 2021

Could you consider writing an entry for your change at the end of 00DIST file?

@dilinger
Copy link
Contributor Author

Sure, but how should I be updating this branch? Removing CCDATE or adding the freebsd override using SOURCE_DATE_EPOCH?

@masatake
Copy link
Contributor

Let's remove CCDATE.

LSOF_CCDATE is used to embed the build time/date string into the lsof binary.
This string is displayed as part of 'lsof -v' output. However, having that
embedded string breaks reproducible builds - we want to ensure that subsequent
builds (using the same exact build environemnt) produce the same exact binary,
bit-for-bit.

The Linux dialect replaced the LSOF_CCDATE variable with the SOURCE_DATE_EPOCH
variable for this reason. However, it doesn't make sense to even include the
build date. We have 'ls -l' and version strings to give us information about
an installed binary. So let's just drop the build time/date string.

SOURCE_DATE_EPOCH was overriding the LSOF_HOST, LSOF_LOGNAME, LSOF_SYSINFO,
and LSOF_USER variables on Linux. These variables are already capable of being
overridden, and having "SOURCE_DATE_EPOCH" as a shortcut for overriding random
other variables (even if it is being standardized by the reproducible-builds.org
folks) doesn't make much sense. So this also gets rid of SOURCE_DATE_EPOCH.
@masatake
Copy link
Contributor

Thank you. I will review (and merge).

@masatake masatake merged commit a4a8155 into lsof-org:master Feb 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants