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

check md5 and length of downloads #6

Merged
merged 2 commits into from
Sep 8, 2023

Conversation

bo-tato
Copy link
Collaborator

@bo-tato bo-tato commented Sep 7, 2023

Since the quicklisp release contains the md5 and length of files, I figured we might as well check it when downloading. In the same spirit of ql-https just shelling out to curl, I just shell out to md5sum which is present on 99.9% of unix boxes.

If md5 was secure it would add significant security. Rather than getting a certificate authority to issue a false certificate or temporarily compromising the quicklisp server and backdooring some package, an attacker would have to also edit the dist to match their new hash and maintain the compromise unnoticed for months until you download the new dist and update some package. Unfortunately md5 is not secure and it's possible to make collisions with the same length, but it at least makes it a little harder for an attacker, and if quicklisp ever changes to use sha256sum or something then it will be trivial to change here and actually provide significant extra security. The quicklisp dist also contains a sha1 which would be better than md5, but that is generated by git which uses the time and author of the commit among other information we don't have in the simple release .tgz, so I don't think we can check it.

Copy link
Owner

@rudolfochrist rudolfochrist left a comment

Choose a reason for hiding this comment

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

Looks nice. Thank you.

@rudolfochrist rudolfochrist merged commit 8a2f999 into rudolfochrist:master Sep 8, 2023
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.

2 participants