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

OF-2877 Reproducible builds #2539

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

stokito
Copy link
Contributor

@stokito stokito commented Sep 22, 2024

Pin the build timestamp and disable Built-By, Created-By and Build-Jdk-Spec fields from generated manifest. The Maven JAR plugin is updated to latest version that has the new option addDefaultEntries

https://igniterealtime.atlassian.net/browse/OF-2877

@stokito
Copy link
Contributor Author

stokito commented Sep 22, 2024

Note: I also changed the plugins parent pom so plugins jars also should have the manifest without generated fields.

Copy link
Member

@guusdk guusdk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can/should we modify the static dates that are introduced in this PR with one that is more tied to the state of the code, like the date of the last commit, or something?

Some of these changes are likely going to confuse future developers ("This date shouldn't be static! Lets make it a variable!"). Can you please add a reference to OF-2877 in the comments that you've already added. That will help people find more of the background of this change.

@stokito
Copy link
Contributor Author

stokito commented Sep 23, 2024

yes, we can use the git commit date https://maven.apache.org/guides/mini/guide-reproducible-builds.html#faq
It looks like we already have the git-commit-id-plugin but with a different group id pl.project13.maven.

The timestamp is used inside of the JAR as mtime for entries. It looks like it doesn't really used elsewhere and not shown to a user. We may change to 1/1/1970 to make it for anyone to not worried to update it. The comment should be enough.

Also later I noticed that the xmppserver.jar is actually not stable. It contains the Admin Console with JSP files that may (almost) randomly compiled differently. Disassembly show that it may use or not use the early return in generated methods.
Still, now it's much easier to detect such a change. As a partial solution we may extract the admin console to a separate jar (war?). This may be good idea for someone who actually aren't interested in the Admin Console.
I'll back to this on the weekend.

Pin the build timestamp to 1980-02-01 as Gradle uses when preserveFileTimestamps = false.

Disable Built-By, Created-By and Build-Jdk-Spec fields from generated manifest.
The Maven JAR plugin is updated to latest version that has the new option addDefaultEntries
@stokito stokito force-pushed the OF-2877_reprodicible_builds branch from 87ca30a to e1c073f Compare October 5, 2024 22:51
@stokito
Copy link
Contributor Author

stokito commented Oct 5, 2024

I don't think it's a good idea to use a git tag as a build timestamp. It's not only over complication but also may make comparing artifacts of different versions harder.
The build timestamp itself may be confusing but the comment and the git blame should be enough.
But generally speaking this can be solved by introducing a special constant and I proposed this to Maven https://issues.apache.org/jira/browse/MJAR-314
Meanwhile I changed the timestamp to 1 Feb 1980 because this will be the default value in future.

Here remains one problem with CSS and images assets of the Web Admin Console. The browser uses If-Modified-Since header for caching and remembers the Last-Updated header which is taken from the mtime of a JAR. So users have to re-upload all assets.

It's not a big deal because the Admin Console is not a highly loaded server but only one-two users occasionally will open its web page.
But this may be a problem with some other plugins like Converse or maybe something else.

@stokito
Copy link
Contributor Author

stokito commented Oct 15, 2024

Once you'll have a minute please review the PR. I commented in the Jira
https://igniterealtime.atlassian.net/browse/OF-2877?focusedCommentId=23127

Copy link
Member

@Fishbowler Fishbowler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whilst I don't quite understand the end goal beyond what's stated on the Reproducible Builds website, I similarly have no objections to moving in that direction, assuming we're not making unreasonable compromises for other users' needs along the way.

@stokito
Copy link
Contributor Author

stokito commented Nov 22, 2024

The change will make it easier to find exploits in the distribution files. Nice thing to have but not critical.

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.

3 participants