open-ils.auth.login fails when password contains percent sign

[Jeff Davis]

If a user's password contains a percent sign (%), open-ils.auth.login will always fail. Old-style authentication with init/complete works just fine.

[Jeff Davis (jdavis-sitka)]

This bug is annoying to test manually, so I've added a live test for it in working branch user/jeffdavis/lp1830642-password-percent (using the existing test for open-ils.auth.login):

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jeffdavis/lp1830642-password-percent

The updated live test authenticates a user via both init/complete and open-ils.auth.login; currently the latter test fails when the password contains a percent sign, but init/complete succeeds. The system is restored to its initial state at the end, so you can safely run the test multiple times to verify the issue and to test any potential bugfix.

I haven't figured out what's causing the problem so I don't have a fix. If anyone feels like tackling this bug, please feel free to do so. :)

[Bill Erickson (berick)]

I suspect this is caused by the OpenSRF utils.c md5sum() function accepting variable length arguments. A quick glance suggests no existing OpenSRF or Evergreen code takes advantage of this, so I propose we just change the md5sum() function definition.

Confirming...

[Bill Erickson (berick)]

Issue and fix confirmed. OpenSRF branch pushed:

https://git.evergreen-ils.org/?p=working/OpenSRF.git;a=shortlog;h=refs/heads/user/berick/lp1830642-md5sum-no-var-args

Thanks for the tests, Jeff! Very helpful.

[Galen Charlton (gmc)]

I believe the patch as it breaks ABI compatibility. For the sake of maintenance releases, is there any reason not to (for rel_3_1 and rel_3_0) keep the function notionally variadic but remove the invocation of VA_LIST_TO_STRING, similar to commit 6414c25?

[Jeff Davis (jdavis-sitka)]

The commit mentioned by Galen was a fix for bug 1702978. I don't know anything about ABI compatibility so I can't comment on Galen's suggestion.

I am marking this bug as Medium priority since there is a workaround (don't use % in passwords).

[Galen Charlton (gmc)]

I've committed Bill's patch for inclusion in 3.2.x. I suspect that a variant patch is still needed to maintain ABI compatibility for OpenSRF 3.1.x.

[Galen Charlton (gmc)]

I've pushed the Evergreen tests to master in Evergreen for inclusion in 3.4-rc. Thanks, Jeff!

[Galen Charlton (gmc)]

As Evergreen has moved past OpenSRF 3.1, I'm not expecting a release of 3.1.2 and am consequently removing the target.