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

Fix cacert-${d}.pem.sha256 files to actually refer to the correct file #267

Closed
wants to merge 2 commits into from

Conversation

gevaerts
Copy link
Contributor

I put a retroactive fix snippet to a separate commit. I don't have the necessary files locally, so not actually tested...

ca/update.sh Outdated
@@ -9,9 +9,16 @@ if test $? -gt "0"; then
d="$(date +%Y-%m-%d)"
cp cacert.pem "cacert-${d}.pem"
sha256sum cacert.pem > cacert.pem.sha256
cp cacert.pem.sha256 "cacert-${d}.pem.sha256"
sha256sum "cacert-${d}.pem" > "cacert-${d}.pem.sha256"
Copy link
Member

Choose a reason for hiding this comment

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

what is wrong with the old code that copies the hash file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The sha256 file contains the filename, and if that filename doesn't match you can't use sha256sum --check, so actually using the sha256 file is harder than necessary

Copy link
Member

Choose a reason for hiding this comment

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

ah right i forgot about that. the rebuild should be a one-time thing so it may be easier for @bagder to just run it once

Copy link
Member

@vszakats vszakats Jun 26, 2023

Choose a reason for hiding this comment

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

I'd be happier with a solution that doesn't require regenerating the hash. One option is to replace cp with sed.

diff --git a/ca/update.sh b/ca/update.sh
index 9d030b03..3497a745 100755
--- a/ca/update.sh
+++ b/ca/update.sh
@@ -1,5 +1,12 @@
 #!/bin/sh
 
+# fixup filenames in .sha256 files retroactively
+for f in cacert-*.pem.sha256; do
+  if grep -q -F 'cacert.pem' "${f}"; then
+    sed -i.bak "s/cacert.pem/${f}/" "${f}"
+  fi
+done
+
 # get the cert and create ca-bundle.crt
 perl ../cvssource/scripts/mk-ca-bundle.pl
 
@@ -9,7 +16,7 @@ if test $? -gt "0"; then
   d="$(date +%Y-%m-%d)"
   cp cacert.pem "cacert-${d}.pem"
   sha256sum cacert.pem > cacert.pem.sha256
-  cp cacert.pem.sha256 "cacert-${d}.pem.sha256"
+  sed "s/cacert.pem/cacert-${d}.pem/" < cacert.pem.sha256 > "cacert-${d}.pem.sha256"
   gzip -c cacert.pem > cacert.pem.gz
   xz -c cacert.pem > cacert.pem.xz
   perl ./listpem.pl > pemlist.gen

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point. I'll update my tree later today

Copy link
Member

@vszakats vszakats Jun 26, 2023

Choose a reason for hiding this comment

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

I've opened #269 with the above snippet earlier.

Frank Gevaerts added 2 commits June 26, 2023 20:28
Currently they refer to the plain cacert.pem files, which makes
"simple" checking fail.
(reported by ursa on irc, with suggestions from Viktor Szakats)
@gevaerts
Copy link
Contributor Author

I've updated this to use sed as suggested.

vszakats added a commit to vszakats/curl-www that referenced this pull request Jul 1, 2023
Co-authored-by: Frank Gevaerts
Closes curl#267
@vszakats vszakats closed this in #269 Jul 1, 2023
vszakats added a commit that referenced this pull request Jul 1, 2023
Co-authored-by: Frank Gevaerts
Closes #267
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