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

Verbose output is suppressed by warning #316

Closed
msimonsson opened this issue Aug 28, 2021 · 5 comments
Closed

Verbose output is suppressed by warning #316

msimonsson opened this issue Aug 28, 2021 · 5 comments

Comments

@msimonsson
Copy link

scrypt enc --logN 20 -p 1 -r 8 -v -- test test.scrypt

Please enter passphrase:
Please confirm passphrase:
Parameters used: N = 1048576; r = 8; p = 1;
Decrypting this file requires at least 1.0 GB bytes of memory (1.1 GB available),
and will take approximately 0.3 seconds (limit: 5.0 seconds).

scrypt enc --logN 21 -p 1 -r 8 -v -- test test.scrypt

Please enter passphrase:
Please confirm passphrase:
scrypt: Warning: Explicit parameters might exceed memory limit

P.S. There is a typo in the man page for the verbose option --v (double-dash) not -v.

@msimonsson
Copy link
Author

Some additional thoughts.

I changed the following before testing:

% diff scrypt-master/lib/scryptenc/scryptenc.c scrypt/lib/scryptenc/scryptenc.c
203a204,206
>         if (verbose)
>             display_params(logN, r, p, memlimit, opps, maxtime);
>
214d216
<       }
216,217c218,220
<       if (verbose)
<               display_params(logN, r, p, memlimit, opps, maxtime);
---
>         if (verbose)
>             display_params(logN, r, p, memlimit, opps, maxtime);
>       }
% diff scrypt-master/lib-platform/util/memlimit.c scrypt/lib-platform/util/memlimit.c
29a30,31
> #define DEBUG
>

OS: FreeBSD 13

  1. Only one of the warnings will be shown when both limits are exceeded. SCRYPT_ETOOBIG is checked before SCRYPT_ETOOSLOW.

  2. The memory limit is checked against how much memory scrypt allowed, not available memory.

echo "foobar" | ./scrypt-test enc --logN 21 -p 1 -r 8 -P -v -- test /dev/null

Memory limits are:
usermem: 8980013056
memsize: 18446744073709551615
sysinfo: 18446744073709551615
rlimit: 18446744073709551615
sysconf: 17098350592
Allowing up to 1122501632 memory to be used
Parameters used: N = 2097152; r = 8; p = 1;
Decrypting this file requires at least 2.1 GB bytes of memory (1.1 GB available),
and will take approximately 0.6 seconds (limit: 5.0 seconds).
scrypt-head: Warning: Explicit parameters might exceed memory limit

  1. The time limit warning is shown even when the approximate time (1.2 sec) is less than the limit (5.0 sec).

echo "foobar" | ./scrypt-test enc --logN 20 -p 4 -r 8 -P -v -- test /dev/null

Memory limits are:
usermem: 8980000768
memsize: 18446744073709551615
sysinfo: 18446744073709551615
rlimit: 18446744073709551615
sysconf: 17098350592
Allowing up to 1122500096 memory to be used
Parameters used: N = 1048576; r = 8; p = 4;
Decrypting this file requires at least 1.0 GB bytes of memory (1.1 GB available),
and will take approximately 1.2 seconds (limit: 5.0 seconds).
scrypt-head: Warning: Explicit parameters might exceed time limit

@gperciva
Copy link
Member

Thank you, @msimonsson! I made some embarrassing mistakes here. Fixes in #317.

Two small notes:

  1. Fixing that we only display one warning message will be a bit more involved. I'm still working on that one.
  2. I'm not certain I understand this problem. Yes, scrypt only checks against the allowed memory. This can be increased with the -m 1.0 option (or explicitly specified with -M). Or are you suggesting that the message should be changed, i.e. instead of
    Decrypting this file requires at least 2.1 GB bytes of memory (1.1 GB available),
    it should say
    Decrypting this file requires at least 2.1 GB bytes of memory (1.1 GB allowed),
    ?

@msimonsson
Copy link
Author

That was fast, thank you!

2. I'm not certain I understand this problem.  Yes, scrypt only checks against the allowed memory.  This can be increased with the `-m 1.0` option (or explicitly specified with `-M`).  Or are you suggesting that the message should be changed, i.e. instead of
   `Decrypting this file requires at least 2.1 GB bytes of memory (1.1 GB available),`
   it should say
   `Decrypting this file requires at least 2.1 GB bytes of memory (1.1 GB allowed),`
   ?

The way I see it scrypt enc operates in one of two modes, (a) pick the encryption parameters or (b) explicitly provide the encryption parameters.

In mode (a) you can use -M maxmem, -m maxmemfrac and -t maxtime to help scrypt pick the encryption parameters. The warnings are never considered.

In mode (b) you must use --logN value, -p value and -r value. Here the man page says that "the resource limits set by -M, -m, and -t are not enforced". Here is my thought, why is there even a concept of "allowed" memory in this mode? If I tell scrypt to use 2 GB of memory and there is 2 GB available, there should be no problem. The more I think about it, I think the warnings are superfluous. In this mode scrypt should simply use the parameters given or fail.

P.S. I just noticed the redundant "bytes" in the display_params function "2.1 GB bytes of memory".

@gperciva
Copy link
Member

Aha, I see what you mean now! (As you may have noticed, specifying explicit --logN, p, -r values is a relatively recent addition to scrypt.)

The latest decision (on 2020-07-19) about warnings is "if you specify explicit parameters, you get what you asked for but you might also get a warning".
#263 (comment)

If you add -f (force) to the command-line, the warning is suppressed:

$ echo "foobar" | ./scrypt enc --logN 21 -p 1 -r 8 -P -- config.h /dev/null
scrypt: Warning: Explicit parameters might exceed memory limit
$ echo "foobar" | ./scrypt enc --logN 21 -p 1 -r 8 -P -f -- config.h /dev/null
$

That seems like a decent compromise to me, but I don't have strong feelings about this topic.

I like the idea of explicit parameters implying -m 1.0 (for -v and deciding whether to warn if running without -f); I'll submit a PR for that.

Thanks for these bug reports and checking, @msimonsson!

@msimonsson
Copy link
Author

If you add -f (force) to the command-line, the warning is suppressed:
...
That seems like a decent compromise to me, but I don't have strong feelings about this topic.

I like the idea of explicit parameters implying -m 1.0 (for -v and deciding whether to warn if running without -f); I'll submit a PR for that.

Sounds good to me! (Maybe this -f behavior with scrypt enc can be documented in the man page in the future.)

Great reply as always @gperciva!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants