ipmitool: Fix ask_password getpass usage.

[mbap]

This is a quick fix for password matching. getpass was not being used
correctly because it uses a static buffer. Pointers to the getpass
return value will all point to that same static buffer. Comparing them
after multiple calls in incorrect.

This fix takes the return value to getpass and stores it into a local
buffer for comparison. There are additional improvements that could be
made such as wiping the pw buffers asap to reduce duration in memory.

[mbap]

$ipmitool user set password 9              
Password for user 9: 
Password for user 9: 
Set User Password command successful (user 9)

$ipmitool user set password 9 
Password for user 9: 
Password for user 9: 
Set User Password command successful (user 9)

$ipmitool user set password 9 
Password for user 9: 
Password for user 9: 
Passwords do not match or are longer than 20

$ipmitool user set password 9 
Password for user 9: 
Password for user 9: 
Set User Password command successful (user 9)

$ipmitool user set password 9 
Password for user 9: 
ipmitool: password input exceeded buffer

$ipmitool user set password 9 
Password for user 9: 
Password for user 9: 
Set User Password command successful (user 9)

$ipmitool user test 9 20 
Password for user 9: 
Failure: wrong password size

$ipmitool user set password 9 
Password for user 9: 
Password for user 9: 
Set User Password command successful (user 9)

$ipmitool user test 9 20      
Password for user 9: 
Success

[mbap]

Closes #229

[mbap]

Awaiting response on getpass/getpassphrase/readpassphrase discussion. Otherwise initial comments should be resolved.

[mbap]

Added a few more updates. I'll replace getpass with termios as well. Thanks for the review! Sorry its taking me a bit to get it all done. Will retest on Linux and FreeBSD when termios support is implemented.

[pstrinkle]

nit: you can reduce the indentation with some early bails - I don't know what the policy is on these, but if sbuf == NULL) { log; return ret; } etc etc

[mbap]

My apologies, I haven't had cycles to get the termios work started yet. Feel free to accept/deny this PR. If accepted, please feel free to file an issue and assign me to take to completion. I'm hoping to get some time this week to work this.

[mbap]

@pstrinkle Browser cache got me..

Will take a look at closer look at your review asap.

[mbap]

Ok, going to test these changes this week. Took at stab at it. It is likely not perfect, but hoping it is sufficient. Careful review would be awesome specifically looking for buffer overflow or careless errors in that regard.

[mbap]

Not totally sure whats going on with CI, can't seem to make catalina happy, however I'm building locally just fine on 10.15.7.
Still need to test on a machine that has IPMI support, but fixed linux compiler errors.

[mbap]

IPMITool user password testing.

Configure test user.

$./ipmitool  user set name 9 ""
$./ipmitool  user disable 9
$./ipmitool  user set name 9 "usertest"
$./ipmitool  user enable 9
$./ipmitool  user list
ID  Name	     Callin  Link Auth	IPMI Msg   Channel Priv Limit
9   usertest      true    false      false      NO ACCESS

Test non matching passwords.

$./ipmitool  user set password 9
Password for user 9:
Password for user 9:
Passwords do not match or are longer than 20

Test password greater than 20 chars.

$./ipmitool  user set password 9
Password for user 9:
ipmitool: password input error: No buffer space available

Test setting empty password ("")

$./ipmitool  user set password 9
Password for user 9:
Password for user 9:
Set User Password command successful (user 9)

Test set matching password (testpw)

$./ipmitool  user set password 9
Password for user 9:
Password for user 9:
Set User Password command successful (user 9)

Test correct password.

$./ipmitool  user test 9 16
Password for user 9:
Success

Test incorrect Password

$./ipmitool  user test 9 16
Password for user 9:
Failure: password incorrect

[mbap]

Tested on Fedora 33 over the lanplus interface to a Dell PowerEdge r640.

[mbap]

Sorry for the long delay.

[AlexanderAmelkin]

Sorry for the long delay.

Thank you for your time. I will take a look at it as soon as I have some spare time too.

[filipjares]

Hello everybody, please forgive my question if it is inappropriate.

I encountered the bug documented in #229 independently and created a simple patch myself. Then I found this pull request.

I can see that this pull request has more ambitions than merely fixing the bug itself and it also intends to improve the code.

I can see there are failed checks in this pull request, but their logs are not available any more.
Can I help with this pull request? Or is it just waiting for review? Does it make sense to create an alternative/simpler pull request or would that just create an unnecessary overhead?

[AlexanderAmelkin]

@filipjares, it looks to me that the failed builds are caused by some changes in GitHub Actions infrastructure, as they just started failing on new PRs without me merging anything that failed them. If you're willing to investigate that, you're more than welcome.

[mbap]

Anything I can do to help move this along?