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

[cron] sleep random seconds (<59), if not interactive or forced #5215

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

oittaa
Copy link
Contributor

@oittaa oittaa commented Jul 20, 2024

#944 (comment)

A Let's Encrypt employee said in the comments "we do see peaks at the beginning of minutes and even seconds; the finer-grained time randomization, the better."

This adds a random amount of sleep seconds before beginning the cron job. I considered reading from /dev/urandom and so on, but we aren't doing anything security critical here so I thought that just using the process number modulo 59 (the largest prime <= 60) should give decent variability across the systems. The starting hour and minute are already randomized during the installation.

acmesh-official#944 (comment)

Let's Encrypt employee said in the comments "we do see peaks at the beginning of minutes and even seconds; the finer-grained time randomization, the better."

This adds a random amount of sleep second before beginning the cron job. I considered reading from `/dev/urandom` and so on, but we aren't doing anything security critical here so I thought that just using the process number modulo 59 (the largest prime <= 60) should give decent variability across the systems. The starting hour and minute are already randomized during the installation.
@user8446
Copy link

user8446 commented Aug 7, 2024

Unfortunately that code doesn't work on OpenWrt which is ash and not bash.

Also, a sleep of 1-5 or 55-59 would probably defeat the purpose due to being too close to the peaks at the beginning of each second.

So, playing around with it what about a random number between 6 and 54 seconds?

Working on both:

random_sec=$(( $RANDOM % 55 +6 ))
echo Sleeping for $random_sec seconds.
sleep $random_sec

@oittaa
Copy link
Contributor Author

oittaa commented Aug 9, 2024

I don't have an OpenWRT here, but $$ should be available in Almquist Shell. On a Debian:

$ /bin/dash
$ echo $SHELL
/bin/bash
$ dpkg -l | grep dash
ii  dash                                 0.5.12-2                         arm64        POSIX-compliant shell
$ echo $$
939274

@oittaa
Copy link
Contributor Author

oittaa commented Aug 16, 2024

Is that test broken or what happened there?

/home/runner/work/_temp/1c8ca25f-9a95-441c-90d5-270c7ee76975.sh: line 1: docker-compose: command not found
Error: Process completed with exit code 127.

https://github.com/acmesh-official/acme.sh/actions/runs/10019783975/job/28809783408?pr=5215#step:4:18

@oittaa
Copy link
Contributor Author

oittaa commented Aug 16, 2024

random_sec=$(( $RANDOM % 55 +6 ))
echo Sleeping for $random_sec seconds.
sleep $random_sec

And $RANDOM isn't POSIX standard. You can test that with shellcheck or something similar.

#!/bin/sh

echo $RANDOM
$ shellcheck myscript
 
[Line 3:](javascript:setPosition(3, 6))
echo $RANDOM
     ^-- [SC3028](https://www.shellcheck.net/wiki/SC3028) (warning): In POSIX sh, RANDOM is undefined.

Compared to

#!/bin/sh

echo $$
$ shellcheck myscript2
No issues detected!

@user8446
Copy link

user8446 commented Aug 16, 2024

Thank you for the info.

Since the PID won't change but on reboot, what would be the solution to make it random like $RANDOM?

root@OpenWrt:~# echo $$
17372
root@OpenWrt:~# echo $$
17372
root@OpenWrt:~# echo $RANDOM
23144
root@OpenWrt:~# echo $RANDOM
31538

@vmmello
Copy link

vmmello commented Aug 16, 2024

@oittaa
Copy link
Contributor Author

oittaa commented Aug 18, 2024

@user8446 Of course you will get the same process within the same shell session. The cron job gets a new process id every time. Google how POSIX process IDs work if you want to know more.

$ dash -c 'echo $$'
3317086
$ dash -c 'echo $$'
3317116

@vmmello I literally referenced that in the first comment. Imagine a hypothetical random_second=$(_math $_t % 60) and what would it be?

It would be zero always since the cron job starts when the minute changes.

@oittaa
Copy link
Contributor Author

oittaa commented Aug 18, 2024

Is that test broken or what happened there?

/home/runner/work/_temp/1c8ca25f-9a95-441c-90d5-270c7ee76975.sh: line 1: docker-compose: command not found
Error: Process completed with exit code 127.

https://github.com/acmesh-official/acme.sh/actions/runs/10019783975/job/28809783408?pr=5215#step:4:18

Seems like docker-compose was removed from the runners and you should use docker compose actions/runner-images#9692

@oittaa
Copy link
Contributor Author

oittaa commented Aug 18, 2024

The staging tests seem to be broken at the moment, but the all the other tests pass now.

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.

None yet

3 participants