engine: update chmod command for gpg keys on debian

[dvdksn]

Proposed changes

Related issues (optional)

Follow-up: #16674

[netlify]

✅ Deploy Preview for docsdocker ready!

Name	Link
🔨 Latest commit	d421693
🔍 Latest deploy log	https://app.netlify.com/sites/docsdocker/deploys/643197d5b8a44300087bd046
😎 Deploy Preview	https://deploy-preview-17070--docsdocker.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[thaJeztah]

Left a comment (we may have to check what we do in other examples)

[thaJeztah]

I think this may be a bit too eager;

mkdir keyrings
touch keyrings/one.sh
touch keyrings/docker.gpg
ls -al keyrings/
total 8
drwxr-xr-x 2 root root 4096 Apr 8 11:41 .
drwxr-xr-x 3 root root 4096 Apr 8 11:41 ..
-rw-r--r-- 1 root root 0 Apr 8 11:41 docker.gpg
-rw-r--r-- 1 root root 0 Apr 8 11:41 one.sh

Notice that all files in the directory now have their executable bit set (including files we don't own);

chmod -R a+rx keyrings
ls -l keyrings/
total 8
drwxr-xr-x 2 root root 4096 Apr 8 11:41 .
drwxr-xr-x 3 root root 4096 Apr 8 11:41 ..
-rwxr-xr-x 1 root root 0 Apr 8 11:41 docker.gpg
-rwxr-xr-x 1 root root 0 Apr 8 11:41 one.sh

I think we need two steps;

• one to set the executable bit on the directory to make it browsable (using capitalX to only touch the directory itself)
• one to set the permissinos on the docker.gpg (we should not touch other files that we don't own)

mkdir keyrings2
touch keyrings2/one.sh
touch keyrings2/docker.gpg
chmod -R a+rX keyrings2
chmod -R a+r keyrings2/docker.gpg
ls -al keyrings2/
total 8
drwxr-xr-x 2 root root 4096 Apr 8 11:41 .
drwxr-xr-x 4 root root 4096 Apr 8 11:41 ..
-rw-r--r-- 1 root root 0 Apr 8 11:41 docker.gpg
-rw-r--r-- 1 root root 0 Apr 8 11:41 one.sh

[thaJeztah]

I'm actually wondering, perhaps we could update the install steps to use install instead of mkdir; #16654 updated the instructions for creating the keyrings to set the right permissions, but if the directory already existed, it won't update those permissions (which is probably the issue we're running into here).

Here, I mimic a situation where the directory already exists with 0700 perms, and use mkdir -p (current installation steps); in that case, the -p just makes the mkdir stop early (directory exists, nothing to do), and it won't update any existing permissions;

mkdir -m 0700 keyrings
ls -l
total 4
drwx------ 2 root root 4096 Apr 8 11:54 keyrings
mkdir -m 0755 -p keyrings
ls -l
total 4
drwx------ 2 root root 4096 Apr 8 11:54 keyrings

install, on the other hand, allows you to do the same (create the directory if missing), but (unlike mkdir) also sets the permissions if they don't match;

ls -l
total 8
drwx------ 2 root root 4096 Apr 8 11:54 keyrings
install -m 0755 -d keyrings
ls -l
total 8
drwxr-xr-x 2 root root 4096 Apr 8 11:54 keyrings
install -m 0755 -d keyrings2
ls -l
total 8
drwxr-xr-x 2 root root 4096 Apr 8 11:54 keyrings
drwxr-xr-x 2 root root 4096 Apr 8 11:55 keyrings2

[thaJeztah]

I think with the install approach, we could even drop the "note" altogether 🤔

[dvdksn]

Nice, I like it. I had a go at updating to use install. (It's the first time I hear about this command 🙈 you live and you learn!)

Please take a look!

[thaJeztah]

(It's the first time I hear about this command 🙈 you live and you learn!)

I didn't know about it until "not too long ago", when I stumbled upon it when working on some of our packaging scripts. It's definitely not a command I think of "all the time", but it's pretty useful actually!

[thaJeztah]

🎉

before we fully celebrate, let's also get some more eyes on this in case we're overlooking scenarios where someone could still run into this.

Overall I think we should be fine (assuming users just ran the steps above), and I guess if someone ran those steps with the old variant, they would already have run into the issue.

[thaJeztah]

Oh! 🤦 I think there is still a situation; we now fixed the directory permissions, but we don't explicitly set read permissions on the docker.gpg itself.

I guess we could either keep a note (but I hate "too many notes" as they distract from the overall flow), or we could add a line to always set the permissions;

$ curl -fsSL {{ download-url-base }}/gpg | sudo gpg --dearmor -o /etc/apt/keyrings/docker.gpg
$ sudo chmod a+r /etc/apt/keyrings/docker.gpg

I double-checked and yes, looks like we still may have to (here's with a 0066 umask);

umask 0066
curl -fsSL https://download.docker.com/linux/ubuntu/gpg | gpg --dearmor -o /etc/apt/keyrings/docker2.gpg
ls -l /etc/apt/keyrings/
total 8
-rw-r--r-- 1 root root 2760 Jun 7 2022 docker.gpg
-rw------- 1 root root 2760 Apr 8 16:20 docker2.gpg
umask 0022

[dvdksn]

added chmod to the previous step - I agree that too many notes creates clutter and makes it hard to read

[thaJeztah]

LGTM

@tianon@neersighted perhaps you want to give this a sanity check?

[tianon]

Seems reasonable to me -- I worried for a moment about users who might have had different permissions on /etc/apt/keyrings on purpose, but those users aren't likely to be blindly copy/pasting that install line from these docs (just recording the thought process 👍).

[neersighted]

Lost this in my notifications; seems reasonable to me as well/matches what I do on my systems.