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

docs: store ddev.gpg in keyrings instead of trusted.gpg.d #5172

Conversation

ted933
Copy link
Contributor

@ted933 ted933 commented Jul 20, 2023

The Issue

On Ubuntu systems keys stored in /etc/apt/trusted.gpg.d validate packages for the whole linux installation. Custom sources should not use this directory for storing their keys as this creates a vulnerability.

See https://www.digitalocean.com/community/tutorials/how-to-handle-apt-key-and-add-apt-repository-deprecation-using-gpg-to-add-external-repositories-on-ubuntu-22-04 for detailed explanation.

How This PR Solves The Issue

Storing the ddev.gpg key in /etc/apt/keyrings/ mitigates this vulnerability.

Manual Testing Instructions

  • Test docs on Ubuntu
  • Test the docker-ce WSL2 installation script
  • Test the docker desktop WSL2 installation script

Release/Deployment Notes

  • New deployment of gitpod image
  • New deployment of Codespaces package

@ted933 ted933 requested a review from a team as a code owner July 20, 2023 06:23
@rfay
Copy link
Member

rfay commented Jul 20, 2023

Please take a look at

and

There's more to be done than just this little bit. But those will give you more context. Definitely want to do it, would like to do more than you have here.

@rfay rfay changed the title Store ddev.gpg in keyrings instead of trusted.gpg.d docs: store ddev.gpg in keyrings instead of trusted.gpg.d Jul 20, 2023
@mattstein mattstein removed the request for review from a team July 20, 2023 22:29
@ted933
Copy link
Contributor Author

ted933 commented Jul 26, 2023

Setting access rights corresponding to https://docs.docker.com/engine/install/ubuntu/

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

I think we'll want to make the changes more than just in the docs. If you want to take that on it's welcome. See the previous PR, for example, and the discussion in the previous issue.

@ted933
Copy link
Contributor Author

ted933 commented Aug 5, 2023

I think we'll want to make the changes more than just in the docs.

Please be more precise. What else do you expect?

@rfay
Copy link
Member

rfay commented Aug 5, 2023

The things that have to be done are listed in the initial comment, #5172 (comment) - Just search for the many places that have to be changed as was done in #4961 and do them. That PR had a number of problems, but the idea was right.

On Ubuntu systems keys stored in /etc/apt/trusted.gpg.d validate packages for the whole linux installation. Custom sources should not use this directory for storing their keys as this creates a vulnerability.

See https://www.digitalocean.com/community/tutorials/how-to-handle-apt-key-and-add-apt-repository-deprecation-using-gpg-to-add-external-repositories-on-ubuntu-22-04 for detailed explanation.
Set propper access rights when creating /etc/apt/keyrings.
@rfay rfay force-pushed the 20230720_ted933_doc_installation_ubuntu_apt_vulnerability branch from 9a8214e to e8c21f1 Compare August 10, 2023 20:45
@rfay
Copy link
Member

rfay commented Aug 10, 2023

I'm working on adding a commit to do the things.

Co-authored-by: hakre <hanskrentel@yahoo.de>
@rfay rfay requested a review from a team as a code owner August 10, 2023 23:04
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Aug 10, 2023
@rfay
Copy link
Member

rfay commented Aug 10, 2023

@ted933 @hakre please review and see what you think, thanks!

@rfay rfay requested a review from gilbertsoft August 14, 2023 14:50
Copy link
Member

@gilbertsoft gilbertsoft left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@gilbertsoft gilbertsoft left a comment

Choose a reason for hiding this comment

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

This PR introduces two different locations for the keys, one in /usr/share/keyrings and a second in /etc/apt/keyrings, I think we should use one only!

@rfay
Copy link
Member

rfay commented Aug 14, 2023

@gilbertsoft The PR uses /usr/share/keyrings only for deb.sury.org packages, and it's because that's what they currently recommend for their case. https://packages.sury.org/php/README.txt

Copy link
Member

@gilbertsoft gilbertsoft left a comment

Choose a reason for hiding this comment

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

Thanks for the additional information. I had a look at the various links and it looks fine to me.

@rfay rfay merged commit 8c9b3c3 into ddev:master Aug 14, 2023
32 checks passed
@rfay
Copy link
Member

rfay commented Aug 14, 2023

Thanks @hakre and @ted933 for all your work on this. @hakre you also got commit credit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants