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

Improve the Debian/Ubuntu instructions for adding the OBS repository #1189

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

stdedos
Copy link

@stdedos stdedos commented Apr 16, 2022

I has come to my attention that the apt-key practice
(as well as the trusted.gpg.d) are not recommended by the Debian team
(https://wiki.debian.org/DebianRepository/UseThirdParty#OpenPGP_Key_distribution),
and instead deb [ signed-by=... ] is the recommended practice.

I cannot find an official source on why is that in "nice terms", but this article
https://michael-prokop.at/blog/2021/02/16/how-to-properly-use-3rd-party-debian-repository-signing-keys-with-apt/
gives more insight into the issue.

Additionally, for Ubuntu wget > curl, since curl needs to be installed separately.

The issue described in the h6 above is still "relatively true", but it's less severe.

Tested in a home-brewed test, based on the
https://software.opensuse.org/download.html?project=home:vtrefny&package=blivet-gui
instructions, inside an Ubuntu docker container (with the necessary setup).

Signed-off-by: Stavros Ntentos [email protected]


Fixes #981 (partially?)

  • I've included before / after screenshots or did not change the UI

@hellcp
Copy link
Member

hellcp commented Apr 16, 2022

Running wget with sudo sounds a little ill-advised.

@stdedos
Copy link
Author

stdedos commented Apr 17, 2022

Oops 😅

I was using -O <filepath> directly, because gpg --dearmor is a lil bit unnecessary on nowadays SecureApt (hence the sudo).

However, I've decided against it in the end.

Is there anything else that's troubling the definitive reviewer of this that I should fix? Or is this okay code-wise?

@hellcp
Copy link
Member

hellcp commented Apr 17, 2022

I'm not an expert on dpkg/apt, so this looks fine to me otherwise

@stdedos
Copy link
Author

stdedos commented Jun 26, 2022

Uumm ... how can I move this PR forward? 😕

I has come to my attention that the `apt-key` practice
(as well as the `trusted.gpg.d`) are not recommended by the Debian team
(https://wiki.debian.org/DebianRepository/UseThirdParty#OpenPGP_Key_distribution),
and instead `deb [ signed-by=... ]` is the recommended practice.

I cannot find an official source on why is that in "nice terms", but this article
https://michael-prokop.at/blog/2021/02/16/how-to-properly-use-3rd-party-debian-repository-signing-keys-with-apt/
gives more insight into the issue.

Additionally, for Ubuntu `wget` > `curl`, since `curl` needs to be installed separately.

The issue described in the `h6` above is still "relatively true", but it's less severe.

Tested in a home-brewed test, based on the
https://software.opensuse.org/download.html?project=home:vtrefny&package=blivet-gui
instructions, inside an Ubuntu docker container (with the necessary setup).

Signed-off-by: Stavros Ntentos <[email protected]>
Oops 😅

`gpg --dearmor` is a little bit unnecessary on nowadays `SecureApt`.
However, let's not change this to preserve backwards-compatibility instead.

Signed-off-by: Stavros Ntentos <[email protected]>
@graue70
Copy link

graue70 commented Dec 8, 2023

Is there a reason why you chose a different location ('/usr/share/keychain') for the key than in the article?

It seems to me like '/usr/share/keyrings' has become the de-facto standard.

@stdedos
Copy link
Author

stdedos commented Dec 8, 2023

Is there a reason why you chose a different location ('/usr/share/keychain') for the key than in the article?

It seems to me like '/usr/share/keyrings' has become the de-facto standard.

Oops, typo. Good catch though.

... when Maintainers decide to notice me, I'll do the update

@@ -61,7 +61,7 @@ pacman -Sy <%= repo_name %>/<%= @package %></pre>
<pre><%=
# don't use apt-add-repository wrapper for Ubuntu for now, because it adds source repo which we don't provide
# "apt-add-repository deb #{v[:repo]} /\napt-get update\napt-get install #{@package}"
"echo 'deb #{v[:repo].gsub(/(\w):(\w)/, '\1:/\2').gsub(/^https/, 'http')} /' | sudo tee /etc/apt/sources.list.d/#{@project}.list\ncurl -fsSL #{v[:repo]}Release.key | gpg --dearmor | sudo tee /etc/apt/trusted.gpg.d/#{@project.gsub(':', '_')}.gpg > /dev/null\nsudo apt update\nsudo apt install #{@package}"
"echo 'deb [ signed-by=/usr/share/keychain/#{@project.gsub(':', '_')}.gpg ] #{v[:repo].gsub(/(\w):(\w)/, '\1:/\2').gsub(/^https/, 'http')} /' | sudo tee /etc/apt/sources.list.d/#{@project}.list\nwget #{v[:repo]}Release.key -O - | gpg --dearmor | sudo tee /usr/share/keychain/#{@project.gsub(':', '_')}.gpg > /dev/null\nsudo apt update\nsudo apt install #{@package}"
Copy link
Author

Choose a reason for hiding this comment

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

Suggested change
"echo 'deb [ signed-by=/usr/share/keychain/#{@project.gsub(':', '_')}.gpg ] #{v[:repo].gsub(/(\w):(\w)/, '\1:/\2').gsub(/^https/, 'http')} /' | sudo tee /etc/apt/sources.list.d/#{@project}.list\nwget #{v[:repo]}Release.key -O - | gpg --dearmor | sudo tee /usr/share/keychain/#{@project.gsub(':', '_')}.gpg > /dev/null\nsudo apt update\nsudo apt install #{@package}"
"echo 'deb [ signed-by=/usr/share/keychain/#{@project.gsub(':', '_')}.gpg ] #{v[:repo].gsub(/(\w):(\w)/, '\1:/\2').gsub(/^https/, 'http')} /' | sudo tee /etc/apt/sources.list.d/#{@project}.list\nwget #{v[:repo]}Release.key -O - | gpg --dearmor | sudo tee /usr/share/keyrings/#{@project.gsub(':', '_')}.gpg > /dev/null\nsudo apt update\nsudo apt install #{@package}"

If the MR moves forward

Copy link

Choose a reason for hiding this comment

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

The path appears twice and you only changed one. 😉

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.

security improvements for debian repo instructions
3 participants