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

regression: zfs load-key on tty broken for keyformat=passphrase #10264

Closed
adamdmoss opened this issue Apr 29, 2020 · 2 comments · Fixed by #10265
Closed

regression: zfs load-key on tty broken for keyformat=passphrase #10264

adamdmoss opened this issue Apr 29, 2020 · 2 comments · Fixed by #10265
Labels
Type: Defect Incorrect behavior (e.g. crash, hang) Type: Regression Indicates a functional regression

Comments

@adamdmoss
Copy link
Contributor

adamdmoss commented Apr 29, 2020

System information

Type Version/Name
Distribution Name ubuntu
Distribution Version 18.04.x
Linux Kernel 5.3.0-46-generic
Architecture x64
ZFS Version c14ca14
SPL Version c14ca14

Describe the problem you're observing

At current git master HEAD - c14ca14 -

Attempts to enter keyphrase with
zfs load-key -a -r
are failing:

Enter passphrase for 'backups-on-6tbusb':
Key load error: Passphrase too short (min 8).

n.b. my passphrase isn't actually too short and this was working at 47c9299

At a guess, the regression is due to c14ca14

@adamdmoss
Copy link
Contributor Author

Confirmed that 89a6610 is fine so I guess it probably is c14ca14 that's troublesome.

@adamdmoss
Copy link
Contributor Author

adamdmoss commented Apr 29, 2020

Think I see the problem in c14ca14 .


	case ZFS_KEYLOCATION_PROMPT:
		if (isatty(fileno(stdin))) {
			can_retry = B_TRUE;
			ret = get_key_interactive(hdl, fsname, keyformat,
			    do_verify, newkey, km_out, kmlen_out);

...

	if ((ret = validate_key(hdl, keyformat, (const char *)km, kmlen)) != 0)
		goto error;

get_key_material() validates km/kmlen, but for the ZFS_KEYLOCATION_PROMPT isatty() case the output is put into km_out/kmlen_out instead.

adamdmoss added a commit to adamdmoss/zfs that referenced this issue Apr 29, 2020
- use correct output vars when stdin is an interactive terminal.
adamdmoss added a commit to adamdmoss/zfs that referenced this issue Apr 29, 2020
'zfs load-key broken for keyformat=passphrase'
- use correct output vars when stdin is an interactive terminal.

Signed-off-by: adam moss <c@yotes.com>
@adamdmoss adamdmoss changed the title zfs load-key broken for keyformat=passphrase regression: zfs load-key broken for keyformat=passphrase Apr 29, 2020
@adamdmoss adamdmoss changed the title regression: zfs load-key broken for keyformat=passphrase regression: zfs load-key on tty broken for keyformat=passphrase Apr 29, 2020
@behlendorf behlendorf added Type: Defect Incorrect behavior (e.g. crash, hang) Type: Regression Indicates a functional regression labels Apr 29, 2020
behlendorf pushed a commit that referenced this issue Apr 30, 2020
The 'zfs load-key' command was broken for 'keyformat=passphrase'.
Use the correct output vars when stdin is an interactive terminal.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: adam moss <c@yotes.com>
Closes #10264 
Closes #10265
adamdmoss added a commit to adamdmoss/zfs that referenced this issue Oct 4, 2020
Passphrase from tty and passphrase from pipe are separate
codepaths with - historically - different bugs, so test
the tty case.

Follow-up from PR openzfs#10265 and issue openzfs#10264
Closes PR openzfs#11010

Signed-off-by: Adam Moss <c@yotes.com>
jsai20 pushed a commit to jsai20/zfs that referenced this issue Mar 30, 2021
The 'zfs load-key' command was broken for 'keyformat=passphrase'.
Use the correct output vars when stdin is an interactive terminal.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: adam moss <c@yotes.com>
Closes openzfs#10264 
Closes openzfs#10265
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Defect Incorrect behavior (e.g. crash, hang) Type: Regression Indicates a functional regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants