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

POSIX shell changes #12

Merged
merged 3 commits into from
Jun 18, 2024
Merged

POSIX shell changes #12

merged 3 commits into from
Jun 18, 2024

Conversation

bewinsnw
Copy link
Contributor

POSIX date does not support the %N format, also POSIX shell does not support the substring expansion format, or arrays.

Another problem with the original script is the use of dd, which may not return the number of bytes expected (see https://unix.stackexchange.com/questions/121865/create-random-data-with-dd-and-get-partial-read-warning-is-the-data-after-the#121888)

The original also did not format the resulting string like the other scripts in 8-4-4-4-6 format.

I think it's fine to change the original script to specify bash as the shell not sh, but just to point out the issues, here's a stab at a posixly correct version? (shellcheck at least does not complain)

POSIX date does not support the %N format, also POSIX shell
does not support the substring expansion format, or arrays.

Another problem with the original script is the use of dd, which
may not return the number of bytes expected (see https://unix.stackexchange.com/questions/121865/create-random-data-with-dd-and-get-partial-read-warning-is-the-data-after-the#121888)

The original also did not format the resulting string like the
other scripts in 8-4-4-4-6 format.

I think it's fine to change the original script to specify bash
as the shell not sh, but just to point out the issues, here's a stab at a posixly correct version? (shellcheck at least
does not complain)
@nalgeon
Copy link
Owner

nalgeon commented Jun 17, 2024

This implementation yields incorrect results:

0190273c-00a0-7000-8000-

script.sh: line 10: xxd: command not found
script.sh: line 15: xxd: command not found
script.sh: line 19: xxd: command not found

https://onecompiler.com/bash/42ghbcfd4

@bewinsnw
Copy link
Contributor Author

in that case the original implementation is also incorrect, because it also depends on xxd

This version will run on onecompiler and should still be posixly correct. (ie bourne shell not bash)

Instead of taking the first n bytes out of urandom and converting them to hex (using xxd) the trick here is to take bytes dropping any that are not in the range of allowed hex characters. This still ends up with the right number of hex digits, tho it's not particularly efficient.
@bewinsnw
Copy link
Contributor Author

Figured out how to drop xxd from the script if I can use tr, which is also defined by POSIX. https://pubs.opengroup.org/onlinepubs/9699919799/nframe.html

this works, but is a bit of a hack... (inevitable if you stick to bourne shell). I tried it in that onecompiler environment as well as locally and it works just fine.

timestamp_lo is 16 bits but the mask only took 8.
@nalgeon nalgeon merged commit 12d580a into nalgeon:main Jun 18, 2024
@nalgeon
Copy link
Owner

nalgeon commented Jun 18, 2024

Awesome! Thank you so much!

@Summertime
Copy link

POSIX date does not support the %N format

It also does not support %s https://pubs.opengroup.org/onlinepubs/9699919799/utilities/date.html


head does not support -c https://pubs.opengroup.org/onlinepubs/9699919799/utilities/head.html


dd should be safe due to

When calling read(2) for the device /dev/urandom, reads of up to 256 bytes will return as many bytes as are requested and will not be interrupted by a signal handler.

from urandom(4)

@bewinsnw
Copy link
Contributor Author

fair points all. Any suggestions for fixing it? We can use dd to replace head, but date's more of an issue.

One possibility is awk: awk 'BEGIN{print srand(srand())"000"}' (from https://unix.stackexchange.com/a/703145)

@Summertime
Copy link

I wrote up a version here: https://posts.summerti.me/posix-compliant/

A fully POSIX approach is fairly painful and slow, so I'd recommend avoiding such considerations and just label it as Bash on GNU/Linux


Additionally: /dev/urandom isn't part of POSIX, so for a truely POSIX implementation, one would need to consider making an RNG in POSIX shell and somehow have that be a CSPRNG and somehow seed that in a cryptographically sane way

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.

3 participants