CP-50518: Add binding for crypt_r to ocaml/auth

[contificate]

This change adds a C stub for the crypt_r function and exposes it from the pam library:

val crypt_r : key:string -> setting:string -> string option

The intention of introducing this function is to use it for hashing passwords in a future authentication cache implementation: the fact we already link and use libcrypt.so simplifies export compliance (initial work used PBKDF2).

Some basic testing is also provided as quicktests. On a recently provisioned machine with these changes, all of the tests pass:

The testing format benefits from being a quick test because it must be tested on a target machine (as the behaviour of crypt_r varies between implementations and is up to the implementation used on hosts). The algorithms we are probably going to use (SHA-256 and SHA-512) seems to actually be a GNU extension. Other implementations are not guaranteed to provide the same functionality.

There is opportunity for writing more tests so, if anyone has any suggestions, please comment.
There is some commentary within the commit messages.

---

Longer term, I think we should go about addressing the export compliance issues so we can use PBKDF2 instead (from mirage). For example, Python notably deprecated its crypt module (which binds the same function) because of reasons akin to those implementation-specific issues above (see here).

[last-genius]

I understand that crypto stuff is carefully watched over in this project, but would it not be better to provide a "safer" interface on the OCaml side? i.e. crypt_r would wrap the provided salt like: "$6$" ^ salt before calling into the stub, forcing the usage of better algorithms, and allowing easier transition from one to the other (if need be) in the future. Alternatively, it could take some (polymorphic?) variant of SHA_256 | SHA_512 and convert the salt to the required string.

At the moment, given that there is no documentation for crypto_r introduced here (and man pages seem not to talk about setting types at all), it'd be all too easy for some usage to be less safe than other usages of this function.

We are already testing "$6$" is present in the tests, so we could use it here too, concerns of portability are the same.

UPD: This would require providing some alternative facilities of checking if two strings hash the same, to not prefix the given salt with something, but to take it as it is, for the following to work:

  h  = crypt_r(k, s)
  h' = crypt_r(k', h)
  equal = h == h'

[contificate]

I understand that crypto stuff is carefully watched over in this project, but would it not be better to provide a "safer" interface on the OCaml side? i.e. crypt_r would wrap the provided salt like: "$6$" ^ salt before calling into the stub, forcing the usage of better algorithms, and allowing easier transition from one to the other (if need be) in the future. Alternatively, it could take some (polymorphic?) variant of SHA_256 | SHA_512 and convert the salt to the required string.

At the moment, given that there is no documentation for crypto_r introduced here (and man pages seem not to talk about setting types at all), it'd be all too easy for some usage to be less safe than other usages of this function.

We are already testing "$6$" is present in the tests, so we could use it here too, concerns of portability are the same.

Yes, a thin wrapper around the stub (and then hiding the stub) that provides a safer API would be a good follow-up. This was discussed already but currently the supposed use case is in one place (authentication cache, as it simplifies export compliance): it is not recommended to use this elsewhere.

That said, I still think the stub itself needs to be tested directly as done here: it exercises expectations that we don't want to change if anyone ever links a different implementation when installing to a host.

[last-genius]

If it's not recommended to use it elsewhere, could we call it unsafe_crypt_r 🦀?

[contificate]

UPD: This would require providing some alternative facilities of checking if two strings hash the same, to not prefix the given salt with something, but to take it as it is, for the following to work:

  h  = crypt_r(k, s)
  h' = crypt_r(k', h)
  equal = h == h'

It wouldn't require alternative facilities, but it would be better to have a specific function (as is common elsewhere, e.g. here).

The above pseudocode would still be the valid usage pattern as it's how crypt_r works. So, a safer API could use the same pattern:

let h = Pam.crypt ~algo:SHA256 ~key:k ~salt:s in
let h' = Pam.crypt ~algo:SHA256 ~key:k' ~salt:h in
h = h'

I do agree, though, that a safer API would just expose a simpler function (as linked above, e.g. password_equal) to avoid misuse (like specifying the wrong algo).

[contificate]

Converted to a draft as we reconsider how this should be exposed. I will try to use alerting attributes to dissuade casual usage. I may attempt to simplify the multiple hashing test, as it probably doesn't achieve the important invariant (despite working).

[contificate]

For those not following this PR, the latest force updates make the following changes:

• crypt_r is renamed to unsafe_crypt_r and annotated with an alert to discourage casual usage.
• The "test many threads" test is changed to compute the expected outcomes sequentially before spawning any of the threads to ensure the "golden" sample is not corrupted. The main thread now busy waits for the test threads to identify themselves as having been spawned, before broadcasting them to start. There is a slight loophole in that Condition.wait (within the test thread) could return (without having been signalled) and test the condition to be true before the main thread broadcasts the same. I expect this is unlikely to happen and that the tick thread's time slice would soon enough yield to the main thread to signal the rest to catch up. Could probably guard the test threads with a separate predicate from the main thread, which is only made true once the main thread's predicate is satisfied.

[contificate]

Thanks for the reviews. I will mark this as ready for further review once I have written a short (safer) API that wraps the unsafe stub: along with introducing a test that demonstrates their correspondence.

[contificate]

Did a clean run on a host with the recent test additions.
There is opportunity to refine the contributions of this PR at a later date, but I'd like to stage it.