Hacker News Reader

Comment by creata | Hacker News Reader

I don't think so. Even in a language with a good type system like Rust, somebody could have made this API:

    /// Hash the phrase with the given setting.
    fn crypt(phrase: &[u8], setting: &[u8]) -> [u8; CRYPT_OUTPUT_SIZE];

Then, in a later version, `crypt` needs to become fallible, so they need to signal errors somehow. But because they really want to preserve backward compatibility, they can't change the output type to `Result<..., ...>`. And so they end up doing the same thing that was done in libcrypt - they output some error value and hackily make sure it's not equal to the `setting` string, under the mistaken assumption that the output of `crypt` would only ever be compared to the `setting` string. And then someone creates the exact same bug.

Maybe I'm wrong, but it looks to me like the exact same incentives and means to screw things up exist in C and in Rust for this particular bug.

View on HN

IMO type safety requires both language features and a mindset where you take efforts to keep things type safe. Your example has only the former.

For example, in a Rust web app, you can choose to use String for encoded URLs, unencoded URL fragments, escaped html, unescaped HTML, usernames, etc. Or you can choose to use newtypes and carefully design all APIs to only take and return the correct newtypes.

You are looking at it from the wrong direction.

The problem is not that you can write bad stuff in rust, the problem is that this particular usage pattern can't be sanely coded in C. This is a public API that should inform users and enforce certain properties. A Results type does exactly that, you would have to go way out of your way to force invalid stuff through that type alone.

I don't think so. In both Rust and C, the solution is the same - make the function explicitly fallible, instead of trying to preserve backward compatibility. In C, the function would probably just look like this, returning zero on success and negative on failure:

    int crypt2(const char *phrase, const char *setting, char *out);

You can call such a signature "insane" if you want, and that's fine, but it would have averted the bug that is the topic of discussion.

what you are suggesting is a form of type safety. There are still degrees of type safety even in C, and this bug resulted from deciding that backwards compatibility is more important than type safety, which I think we agree on.

And C has effectively deprecated bad APIs over the years. Just not this one.

It's a perfectly sane signature, especially coupled with an API surface that uses the return channel for errors generally.

It would not have averted the bug. The bug is that out is the value "*" even when setting is also "*".

It would have averted the bug in a program which checks the int return value for error.

But we had that already: the null value, different from any valid string. That had to be done away with because programmers couldn't be relied upon to check for it.

Those same programmers are not going to check an integer value which doesn't even threaten them with a segfault.

(However, there is a GCC attribute that can be given to functions such that a diagnostic is produced if their value is not used: that could be profitably used with the above API.)

This isn't good either. Crypt is a pure computation that can't fail for arbitrary reasons. The former implementation could fail if the salt was invalid, which is something you could guarantee. Now it also fails if the password is invalid. That's new behaviour and no matter which way you wrap it up it has the potential to be incompatible. They should have added the maximum length recommendation to the man page, and let DoS attacks continue until the calling software was updated, not the crypt library.

OP suggests it was always fallible, they switched to a sentinel to mitigate callers dereferencing null, which they wouldn't have with Option/Result. https://git.musl-libc.org/cgit/musl/commit/?id=b3c4cc12

But in your version, you can't preserve backwards compatibility, making an infallible function fallible is a breaking change. For something comparable to libc you might want to (eg Rust std still has mem::uninitialized), but even with security issues, I feel like people would be more inclined to deprecate for a new function (or panic, which does have its own problems).

A good type system also means things are written with better forwards compatibility, and can be safely changed later. The Rust equivalent to `crypt` likely wouldn't exist: a low-level library would just provide hash functions, leaving callers to mitigate DOS etc; a broader library might return some opaque handle with equality/serialization, which you couldn't try to compare to a passwordless user; a high-level library would parse the account store itself. None of these really need to change in incompatible ways, they have all the information to add features and fixes at their own level of abstraction.

Yes, even in Lean you can just refuse to prove any properties.

But if you have a stronger and more expressive type system, you are more inclined to use it, instead of looking for silly workaround like in C.

You wouldn't be concerned about breaking backward compatibility with rust though, would you? A recompile would be needed anyway and the change would be detected and enforced. The rationale for the bad API here (two types in one variable) was that users were ignoring NULL - that's the part which wouldn't happen in rust.

Maybe he meant binary compatibility.

They would have to make it crypt2.

Reply on news.ycombinator.com