crypt(3) style password support for lua loader

[david_crossfamilyweb.com]

Summary

allow lua-loader to use crypt(3) style passwords

Test Plan

test with no password (done)
tested with plaintext (done)
tested with SHA512 password (done)
tested with invalid password (done)
tested with valid password(done)

[kevans]

To be frank, stand/lua/hashses.lua makes me a bit uncomfortable... we have sha256/sha512 in libsa if GELI is enabled, I wonder how expensive it would be to re-scope those out to libsa out of the GELI option and break those out to Lua. Less worried about the latter part being too chunky than the former...

[david_crossfamilyweb.com]

Uncomfortable how? This is a very straightforward implementation of SHA256/SHA512 modeled after existing LUA SHA2 libraries. I only reimplemented because:

1. existing ones were based on lua <5.4 and relied on the bit32 library which is not in LUA54
2. were vastly overcomplicated and not BSD licensed (ones that worked on every version of LUA and included multiple different sub-implementations optimized for a vast number of possible LUA configurations.)
3. were incapable of being used in this context (they would return hex encoded values, where the crypt(3) algorithms require the raw binary to feed into subsequent stages

Exposing SHA2 from to loader in this way would be complicated that would require either:

1. exposing complex 'context' objects to the underlying scripting language, since crypt(3) builds up the hashes with multiple updates, with those needing to be constructed/initialized, returned, and passed to subsequent calls for update and finalization
   • or -
2. rewriting the crypt(3) routines to concatenate their entire requests together into a single call such that it could be wrapped into a single shaX() that would return the binary result. I guess technically the crypt(3) implementation could unenode the hex to get back the binary.. gross. Regardless this would mean fairly substantial rewrite of the crypt implementation, and constructing kilobyte strings to sling the data around in the interpreter.

Honestly the crypt(3) implementation frightens me much worse, that is a grotesquely overly complicated algorithm.

If we were to export anything to the loader(8) level the right choice would be libcrypt; I looked at doing that originally but figured this would much less invasive to loader and easier to get in since it was entirely in the interpreter level.

But, what exactly is uncomfortable? This is used ONLY for doing the password hash, nothing is being validated/signed against this, there's no network access, it only even comes into play to match a provided password that someone who already had root access had to set.
It is *explicitly* extensively tested (I tested every sequence of 0s from none to 65536 and compared to sha256/sha512) offline (I didn't include those)
It is *implicitly* extensively tested (I included all provided test vectors of crypt for $5$ and $6$), (these are included in the source) and they all passed, which puts the sha libraries through a rather thorough torture test themselves.

If the concern here is a trojan, then I'd be targeting a *VERY* narrow group where I'd already have access to their preboot environment, and I'd have to get them to abandon existing plaintext passwords.. and even if we moved the SHAX routines to C, I'd still have the crypt(3) layer which didn't seem to be a concern and is much easier to hide something in given its complexity.

[kevans]

In D41509#945841, @david_crossfamilyweb.com wrote:

Uncomfortable how? This is a very straightforward implementation of SHA256/SHA512 modeled after existing LUA SHA2 libraries. I only reimplemented because:

The problem is that now the implementation will exist, and thus it will get used for uses that we aren't anticipating and it becomes crypto code that actually should be audited.

1. existing ones were based on lua <5.4 and relied on the bit32 library which is not in LUA54
2. were vastly overcomplicated and not BSD licensed (ones that worked on every version of LUA and included multiple different sub-implementations optimized for a vast number of possible LUA configurations.)
3. were incapable of being used in this context (they would return hex encoded values, where the crypt(3) algorithms require the raw binary to feed into subsequent stages

Exposing SHA2 from to loader in this way would be complicated that would require either:

1. exposing complex 'context' objects to the underlying scripting language, since crypt(3) builds up the hashes with multiple updates, with those needing to be constructed/initialized, returned, and passed to subsequent calls for update and finalization
   • or -
2. rewriting the crypt(3) routines to concatenate their entire requests together into a single call such that it could be wrapped into a single shaX() that would return the binary result. I guess technically the crypt(3) implementation could unenode the hex to get back the binary.. gross. Regardless this would mean fairly substantial rewrite of the crypt implementation, and constructing kilobyte strings to sling the data around in the interpreter.

Honestly the crypt(3) implementation frightens me much worse, that is a grotesquely overly complicated algorithm.

If we were to export anything to the loader(8) level the right choice would be libcrypt; I looked at doing that originally but figured this would much less invasive to loader and easier to get in since it was entirely in the interpreter level.

I don't know where you get the idea that you'd need crypt(3), to be honest. In my first comment, I noted that we already have sha256/sha512 available in libsa that could be broken out relatively easily (same API that's seen in libmd). You don't even need the complex context you suggested, you can still do closures just fine in C and keep your consumers as-is. I'd even be willing to write said replacement if you'd use it, but I need to check how much size we gain in non-default loader configurations if we move sha256/sha512 out from behind the GELI knob.

But, what exactly is uncomfortable? This is used ONLY for doing the password hash, nothing is being validated/signed against this, there's no network access, it only even comes into play to match a provided password that someone who already had root access had to set.
It is *explicitly* extensively tested (I tested every sequence of 0s from none to 65536 and compared to sha256/sha512) offline (I didn't include those)
It is *implicitly* extensively tested (I included all provided test vectors of crypt for $5$ and $6$), (these are included in the source) and they all passed, which puts the sha libraries through a rather thorough torture test themselves.

Already generally answered above, but to re-iterate: for your current use it's perfectly fine, but as soon as it's made available it becomes a public API that could actually get used for more sensitive applications that we didn't anticipate or want.

[delphij]

Why do we even have support of loader password other than unlocking the GELI provider in the first place? Or in other words, what was the threat model it's trying to protect the user from?

I think password.lua should only support entering passphrases for unlocking encrypted providers, and absolutely nothing beyond that. Storing passphrase material in the unencrypted boot storage is calling all kinds of problems and I can't see real security benefits of doing over using e.g. GELI.

[kevans]

In D41509#945887, @delphij wrote:

Why do we even have support of loader password other than unlocking the GELI provider in the first place? Or in other words, what was the threat model it's trying to protect the user from?

No idea where it originated, I just ported it from 4th.

I think password.lua should only support entering passphrases for unlocking encrypted providers, and absolutely nothing beyond that. Storing passphrase material in the unencrypted boot storage is calling all kinds of problems and I can't see real security benefits of doing over using e.g. GELI.

IMO we can't easily get rid of these; one is to prevent boot entirely if you don't have the password, the other is to just prevent user from getting to the loader menu. At least for bootlock_password, the most obvious application I think is to prevent a local user from pushing the system into single-user mode assuming they don't have any other access to the box.

[david_crossfamilyweb.com]

In D41509#945887, @delphij wrote:

Why do we even have support of loader password other than unlocking the GELI provider in the first place? Or in other words, what was the threat model it's trying to protect the user from?

I think password.lua should only support entering passphrases for unlocking encrypted providers, and absolutely nothing beyond that. Storing passphrase material in the unencrypted boot storage is calling all kinds of problems and I can't see real security benefits of doing over using e.g. GELI.

These have existed for decades and pretty much every boot loader and every link in the booting process has optional passwords in place as a check against random people interfering with the boot process. BIOS has it, openboot has it, grub has it.

With these in place there is no bypassing/circumventing the boot process without physical disassembly of the hardware (which at that point GELI won't save you either since they can trojan the loader, install keyloggers, etc); this is a *significant* increase in complexity of attack and mitigating threats.

Without it all they need is any kind of console access (even just a serial port).

Agreed 100% on the plaintext password potion however, and this is seeking to mitigate that portion.

[david_crossfamilyweb.com]

In D41509#945936, @kevans wrote:

In D41509#945887, @delphij wrote:

Why do we even have support of loader password other than unlocking the GELI provider in the first place? Or in other words, what was the threat model it's trying to protect the user from?

No idea where it originated, I just ported it from 4th.

I think password.lua should only support entering passphrases for unlocking encrypted providers, and absolutely nothing beyond that. Storing passphrase material in the unencrypted boot storage is calling all kinds of problems and I can't see real security benefits of doing over using e.g. GELI.

IMO we can't easily get rid of these; one is to prevent boot entirely if you don't have the password, the other is to just prevent user from getting to the loader menu. At least for bootlock_password, the most obvious application I think is to prevent a local user from pushing the system into single-user mode assuming they don't have any other access to the box.

Minor, bootlock_password prevents any booting, password is the one that blocks menu access. :)

[david_crossfamilyweb.com]

In D41509#945885, @kevans wrote:

In D41509#945841, @david_crossfamilyweb.com wrote:

Uncomfortable how? This is a very straightforward implementation of SHA256/SHA512 modeled after existing LUA SHA2 libraries. I only reimplemented because:

The problem is that now the implementation will exist, and thus it will get used for uses that we aren't anticipating and it becomes crypto code that actually should be audited.

1. existing ones were based on lua <5.4 and relied on the bit32 library which is not in LUA54
2. were vastly overcomplicated and not BSD licensed (ones that worked on every version of LUA and included multiple different sub-implementations optimized for a vast number of possible LUA configurations.)
3. were incapable of being used in this context (they would return hex encoded values, where the crypt(3) algorithms require the raw binary to feed into subsequent stages

Exposing SHA2 from to loader in this way would be complicated that would require either:

1. exposing complex 'context' objects to the underlying scripting language, since crypt(3) builds up the hashes with multiple updates, with those needing to be constructed/initialized, returned, and passed to subsequent calls for update and finalization
   • or -
2. rewriting the crypt(3) routines to concatenate their entire requests together into a single call such that it could be wrapped into a single shaX() that would return the binary result. I guess technically the crypt(3) implementation could unenode the hex to get back the binary.. gross. Regardless this would mean fairly substantial rewrite of the crypt implementation, and constructing kilobyte strings to sling the data around in the interpreter.

Honestly the crypt(3) implementation frightens me much worse, that is a grotesquely overly complicated algorithm.

If we were to export anything to the loader(8) level the right choice would be libcrypt; I looked at doing that originally but figured this would much less invasive to loader and easier to get in since it was entirely in the interpreter level.

I don't know where you get the idea that you'd need crypt(3), to be honest. In my first comment, I noted that we already have sha256/sha512 available in libsa that could be broken out relatively easily (same API that's seen in libmd). You don't even need the complex context you suggested, you can still do closures just fine in C and keep your consumers as-is. I'd even be willing to write said replacement if you'd use it, but I need to check how much size we gain in non-default loader configurations if we move sha256/sha512 out from behind the GELI knob.

But, what exactly is uncomfortable? This is used ONLY for doing the password hash, nothing is being validated/signed against this, there's no network access, it only even comes into play to match a provided password that someone who already had root access had to set.
It is *explicitly* extensively tested (I tested every sequence of 0s from none to 65536 and compared to sha256/sha512) offline (I didn't include those)
It is *implicitly* extensively tested (I included all provided test vectors of crypt for $5$ and $6$), (these are included in the source) and they all passed, which puts the sha libraries through a rather thorough torture test themselves.

Already generally answered above, but to re-iterate: for your current use it's perfectly fine, but as soon as it's made available it becomes a public API that could actually get used for more sensitive applications that we didn't anticipate or want.

Ok, this is fine, but if we're going to go through the effort of exporting C routines to the lua environment I'd STRONGLY advocate just exposing crypt(3). For numerous reasons:

1. its signature and use is FAR simpler, strings in, strings out. no contexts to deal with
2. its code is fairly complicated, and lack of duplication here is good
3. it will evolve naturally with the rest of the system, as any future password providers are added.
4. it ALREADY supports more providers, I had intended over time to add DES, blowfish, and md5.. which would have had the same concerns about re-implementing crypto and auditing as above.

Additionally, all of this would need to be made non-dependent on various compile options as password setting is core functionality in loader; having its behavior depend on what would otherwise be non-obvious dependencies on feature will cause people to lock people out of their systems, and with the GOAL of this being that if you don't have the password then the only fix requires physical disassembly that is an excessive failure mode. (Note, I have been bitten by this type of non-obvious dependency before causing great grief.

[kevans]

(Thanks for the correction in the earlier comment)

In D41509#945956, @david_crossfamilyweb.com wrote:

In D41509#945885, @kevans wrote:

In D41509#945841, @david_crossfamilyweb.com wrote:

Uncomfortable how? This is a very straightforward implementation of SHA256/SHA512 modeled after existing LUA SHA2 libraries. I only reimplemented because:

The problem is that now the implementation will exist, and thus it will get used for uses that we aren't anticipating and it becomes crypto code that actually should be audited.

1. existing ones were based on lua <5.4 and relied on the bit32 library which is not in LUA54
2. were vastly overcomplicated and not BSD licensed (ones that worked on every version of LUA and included multiple different sub-implementations optimized for a vast number of possible LUA configurations.)
3. were incapable of being used in this context (they would return hex encoded values, where the crypt(3) algorithms require the raw binary to feed into subsequent stages

Exposing SHA2 from to loader in this way would be complicated that would require either:

1. exposing complex 'context' objects to the underlying scripting language, since crypt(3) builds up the hashes with multiple updates, with those needing to be constructed/initialized, returned, and passed to subsequent calls for update and finalization
   • or -
2. rewriting the crypt(3) routines to concatenate their entire requests together into a single call such that it could be wrapped into a single shaX() that would return the binary result. I guess technically the crypt(3) implementation could unenode the hex to get back the binary.. gross. Regardless this would mean fairly substantial rewrite of the crypt implementation, and constructing kilobyte strings to sling the data around in the interpreter.

Honestly the crypt(3) implementation frightens me much worse, that is a grotesquely overly complicated algorithm.

If we were to export anything to the loader(8) level the right choice would be libcrypt; I looked at doing that originally but figured this would much less invasive to loader and easier to get in since it was entirely in the interpreter level.

I don't know where you get the idea that you'd need crypt(3), to be honest. In my first comment, I noted that we already have sha256/sha512 available in libsa that could be broken out relatively easily (same API that's seen in libmd). You don't even need the complex context you suggested, you can still do closures just fine in C and keep your consumers as-is. I'd even be willing to write said replacement if you'd use it, but I need to check how much size we gain in non-default loader configurations if we move sha256/sha512 out from behind the GELI knob.

But, what exactly is uncomfortable? This is used ONLY for doing the password hash, nothing is being validated/signed against this, there's no network access, it only even comes into play to match a provided password that someone who already had root access had to set.
It is *explicitly* extensively tested (I tested every sequence of 0s from none to 65536 and compared to sha256/sha512) offline (I didn't include those)
It is *implicitly* extensively tested (I included all provided test vectors of crypt for $5$ and $6$), (these are included in the source) and they all passed, which puts the sha libraries through a rather thorough torture test themselves.

Already generally answered above, but to re-iterate: for your current use it's perfectly fine, but as soon as it's made available it becomes a public API that could actually get used for more sensitive applications that we didn't anticipate or want.

Ok, this is fine, but if we're going to go through the effort of exporting C routines to the lua environment I'd STRONGLY advocate just exposing crypt(3). For numerous reasons:

Unfortunately I don't think that's really feasible, particularly not without a build knob to turn it off. Keep in mind that biosloader is heavily constrained (under 512k-ish total) and we're already pushing the envelope on that, enough so that various downstreams do actually have to turn various things off to fit everything they need in loader (this is why I was wondering about the size requirements of moving sha256/512 of the GELI knob).

[jhb]

In D41509#945967, @kevans wrote:

Ok, this is fine, but if we're going to go through the effort of exporting C routines to the lua environment I'd STRONGLY advocate just exposing crypt(3). For numerous reasons:

Unfortunately I don't think that's really feasible, particularly not without a build knob to turn it off. Keep in mind that biosloader is heavily constrained (under 512k-ish total) and we're already pushing the envelope on that, enough so that various downstreams do actually have to turn various things off to fit everything they need in loader (this is why I was wondering about the size requirements of moving sha256/512 of the GELI knob).

But that just means we don't support non-plaintext passwords for the BIOS loader, right? It would basically mean that we would not be adding this new feature for that case (which is becoming more and more obsolete) but would be adding the new feature everywhere else. That tradeoff is probably fine. Exporting just crypt(3) also I think narrows your concerns about other people deciding to use SHA hashes in other places.

[imp]

In D41509#947260, @jhb wrote:

But that just means we don't support non-plaintext passwords for the BIOS loader, right?

We already subset for different things, and this is a big enough new feature that it falls under my blanket 'no new features for the BIOS boot loader, unless they are (a) tiny and (b) hard to otherwise exclude' rule that I'm hoping to follow in 14. We can't just kill BIOS booting for some time now (16 maybe if we're very very lucky), so we have to manage its size and excluding new features from being included with it, especially ones that carry a big cost, is one way to do that.

How to do that for this case, thought, I'm unsure of. I think that lua can tolerate code that has calls to the crypto stuff (for those saying it should be in C not lua) as long as those calls aren't actually called, and we can gate the calls in lua base on how we're booting...

After all, this commit also follows the 'no new forth features [*]' rule without invoking the [*] to explain why it needed to be included.

As for the rest, I've not studied this in enough detail to have more of an opinion.

btw, lua's math.* aren't included because float/double math isn't included and I thought there's very little there that doesn't use it. If my thoughts are wrong, of course we can reconder.

[david_crossfamilyweb.com]

In D41509#947261, @imp wrote:

btw, lua's math.* aren't included because float/double math isn't included and I thought there's very little there that doesn't use it. If my thoughts are wrong, of course we can reconder.

That was mostly tongue in cheek, as I went through the process of thoroughly testing this on console via lua54 and making sure it worked knowing well it could lock me out of my system to on first boot have min/max not be there was a humorous moment.

Definitely glad that I rewrote the sha256/512 routines from scratch then, many/most of the public LUA implementations share a pedigree that derived at runtime the SHA256/512 magic numbers from first principles (first N prime numbers, cube roots of, etc). that would have been a VERY painful unraveling for me.

But there is no need to include math.min/math.max for such a TRIVIAL use-case as this.

[david_crossfamilyweb.com]

In D41509#947260, @jhb wrote:

In D41509#945967, @kevans wrote:

Ok, this is fine, but if we're going to go through the effort of exporting C routines to the lua environment I'd STRONGLY advocate just exposing crypt(3). For numerous reasons:

Unfortunately I don't think that's really feasible, particularly not without a build knob to turn it off. Keep in mind that biosloader is heavily constrained (under 512k-ish total) and we're already pushing the envelope on that, enough so that various downstreams do actually have to turn various things off to fit everything they need in loader (this is why I was wondering about the size requirements of moving sha256/512 of the GELI knob).

But that just means we don't support non-plaintext passwords for the BIOS loader, right? It would basically mean that we would not be adding this new feature for that case (which is becoming more and more obsolete) but would be adding the new feature everywhere else. That tradeoff is probably fine. Exporting just crypt(3) also I think narrows your concerns about other people deciding to use SHA hashes in other places.

Exporting 'just crypt(3)' is a lot; I wasn't aware of the 512k bios limit before this. crypt(3) pulls in sha512, sha256, md5, blowfish, des, etc,etc. That will add a lot.

[david_crossfamilyweb.com]

Thinking about this, I can make a couple of changes that could make this more palatable:

1. move hashes into crypt and make it non-exported.
2. add more validations to sha routines (all of the NIST test vectors)
3. rename them to call out they are internal crypt only methods.. maybe even going so far as to call them hash256 and hash512

Additionally, I am strongly considering instead of relying on password matching $5$ or $6$, namespacing it as 'crypt:$5$' or 'crypt:$6$' in case we add other methods; I looked at pbkdf2 but one of the reasons I went with crypt(3) support is that it is so readily available on a base system "openssl passwd -6" will generate this for you, or you can pull out of master.passwd

[kevans]

In D41509#947276, @david_crossfamilyweb.com wrote:

Thinking about this, I can make a couple of changes that could make this more palatable:

1. move hashes into crypt and make it non-exported.

I think I'd be happy enough with just this...

2. add more validations to sha routines (all of the NIST test vectors)

... but it's hard to say "no" to that, I'd say it's a good goal but don't let that block you from making forward progress; especially if the scope's reduced such that they really can't be used for anything else.

3. rename them to call out they are internal crypt only methods.. maybe even going so far as to call them hash256 and hash512

IMO, I'd just make them local and drop a comment at the top that they're not to be exported without further discussion.

Additionally, I am strongly considering instead of relying on password matching $5$ or $6$, namespacing it as 'crypt:$5$' or 'crypt:$6$' in case we add other methods; I looked at pbkdf2 but one of the reasons I went with crypt(3) support is that it is so readily available on a base system "openssl passwd -6" will generate this for you, or you can pull out of master.passwd

I like this idea as well... mostly because that doesn't force someone that wants a legitimate password starting with these tokens to use the hashed forms. A crypt: prefix is less likely to come up naturally, I would think.

Thanks for being willing to entertain alternatives to the initial implementation; I'd like to try and find a path forward that doesn't terribly demotivate you, and I think what you've outlined aligns with that goal.

[david_crossfamilyweb.com]

Updated diff with all comments addressed, specifically:

1. hashes.lua removed and moved into crypt.lua with no reexport
2. tests refactored
3. tests for NIST vectors for SHA256/512 added (and verified)
4. added "crypt:" prefix on password

[david_crossfamilyweb.com]

5. refactored tests to make them easier to add to.

[david_crossfamilyweb.com]

6. Updated documentation

(sorry for spam, I kept forgetting things, last one I promise)

[kevans]

Nothing too concerning, mostly some style complaints but as a general rule I'd like to keep luacheck happy for the times it does actually identify legitimate issues.

stand/defaults/loader.conf.5

184

New sentence should begin on a new line; ditto below

stand/lua/crypt.lua

2

This file needs a copyright/license header. I would also recommend grabbing devel/lua-luacheck (pkg lua54-luacheck) and running src/tools/boot/lua-lint.sh -- it has a number of complaints... most of them are overly long lines (wrapping at 120 is most of them; ideally we'd wrap where-ever reasonable between ~75-120 columns using the same wrapping rules as style(9); i.e., operator goes on the preceding line and space-indentation after tabbed to the same level. I'll describe some of the other ones below.

160

For loop vars that you won't use, naming them _ will make it clear to both luacheck and the reader that we're effectively discarding teh result.

175

This line should go away altogether or moved to just below endrange and initialized to endrange + 1 (with the below assignment dropped).

252

I think we can just drop this now that use is limited to the same file.

531

algorithm / salthash / hashed should all be local

stand/lua/password.lua

95

nil is the default, this assignment should simply be dropped entirely

96

Space after each of these commas

[david_crossfamilyweb.com]

Updated with all PR comments, save 1.

Kept the "hashes" hash with the sha256/sha512 elements; I recognize that this is local HERE for now, but I am going to keep this locally separate and distinct since n terms of pure lua implementations of SHA256/SHA512 (and in the future other hashes) that are BSD licensed, pure LUA, and NOT using math libraries to derive hash values; keeping this as close to my development format will ease future updates keeping diff sizes down and reduced opportunities for errors

[imp]

I've created a hash module that has some of the sha routines. I don't have big issues connecting those.

[david_crossfamilyweb.com]

In D41509#1022312, @imp wrote:

I've created a hash module that has some of the sha routines. I don't have big issues connecting those.

Which module, and how would I connect it?

I am assuming this is a compiled module/native code? I remember in earlier discussions there were concerns about file size and the MBR loader capped at something like 512K?

[david_crossfamilyweb.com]

Could this be imported as is, and then when the hash modules are in and exported it could be switched over?

I would love to have this in for 14.1; I keep having to patch my local copies, and when I forget I can get locked out since my passwords won't match.

[david_crossfamilyweb.com]

updated diff -U 99999

[imp]

I added lua bindings for the Sha hash functions. Can those be used instead of the lua implementation?

[david_crossfamilyweb.com]

I can work on that and validate it; checking quickly it looks like the version exported is compatible (the big issue with other implementations was being given the binary form of the hash to work with, your export seems to do that).

However only sha256 seems to be available there; sha512 is the default for password hashing. I only had done the sha256 since the structure was so similar to sha512 in both the hash mechanism and crypt(3) mechanism that it was easy, but SHA512 was the primary target.

[imp]

In D41509#1054253, @david_crossfamilyweb.com wrote:

I can work on that and validate it; checking quickly it looks like the version exported is compatible (the big issue with other implementations was being given the binary form of the hash to work with, your export seems to do that).

However only sha256 seems to be available there; sha512 is the default for password hashing. I only had done the sha256 since the structure was so similar to sha512 in both the hash mechanism and crypt(3) mechanism that it was easy, but SHA512 was the primary target.

I can add it easily enough. I just didn't need it for the thing I was working on.

[david_crossfamilyweb.com]

In D41509#1054254, @imp wrote:

In D41509#1054253, @david_crossfamilyweb.com wrote:

I can work on that and validate it; checking quickly it looks like the version exported is compatible (the big issue with other implementations was being given the binary form of the hash to work with, your export seems to do that).

However only sha256 seems to be available there; sha512 is the default for password hashing. I only had done the sha256 since the structure was so similar to sha512 in both the hash mechanism and crypt(3) mechanism that it was easy, but SHA512 was the primary target.

I can add it easily enough. I just didn't need it for the thing I was working on.

I see that SHA256 is in the 14.1 branch; I have test vectors for the sha256 crypt(3) in the code, this will be exceptionally easy to validate

[david_crossfamilyweb.com]

In D41509#1054254, @imp wrote:

In D41509#1054253, @david_crossfamilyweb.com wrote:

I can work on that and validate it; checking quickly it looks like the version exported is compatible (the big issue with other implementations was being given the binary form of the hash to work with, your export seems to do that).

However only sha256 seems to be available there; sha512 is the default for password hashing. I only had done the sha256 since the structure was so similar to sha512 in both the hash mechanism and crypt(3) mechanism that it was easy, but SHA512 was the primary target.

I can add it easily enough. I just didn't need it for the thing I was working on.

Could I get a quick pointer as to accessing these in the code?

I've tried 'require("hash")' and 'loader.sha256' and even 'loader.hash.sha256') (tried modeling these after the gfx module which seems at a high level to be implemented the same way: LUA_COMPILE_SET() to an _init function that luaL_require the module,and then accessing it via the loader hash, but no matter what I try I get a nil dereferene

[david_crossfamilyweb.com]

In D41509#1054842, @david_crossfamilyweb.com wrote:

In D41509#1054254, @imp wrote:

In D41509#1054253, @david_crossfamilyweb.com wrote:

I can work on that and validate it; checking quickly it looks like the version exported is compatible (the big issue with other implementations was being given the binary form of the hash to work with, your export seems to do that).

However only sha256 seems to be available there; sha512 is the default for password hashing. I only had done the sha256 since the structure was so similar to sha512 in both the hash mechanism and crypt(3) mechanism that it was easy, but SHA512 was the primary target.

I can add it easily enough. I just didn't need it for the thing I was working on.

Could I get a quick pointer as to accessing these in the code?

I've tried 'require("hash")' and 'loader.sha256' and even 'loader.hash.sha256') (tried modeling these after the gfx module which seems at a high level to be implemented the same way: LUA_COMPILE_SET() to an _init function that luaL_require the module,and then accessing it via the loader hash, but no matter what I try I get a nil dereferene

Nevermind, I think I got it; it seems it wasn't fully added to the loader lua environment; the LUA_COMPILE_SET() reference was in the kboot directory, which is not even compiled with standard loader; I added the luaopen_hash() to the lua interpreter init where it pulled in other lua bindings; there were a few other small changes I needed to make as well, adding luaopen_hash to lutil.h..

And then I had assumed that sha256() returned a structure with local attributes in it vs needing to pass in the table; but at least the sha256 code is working. should have sha512 by later this evening.

[imp]

Great! I'd forgotten I'd only included it in kboot since that's the only place I'd tested it.
I'll include it elsewhere by default. IIRC, the lua glue code is only about 200 extra bytes of code, so we can have it in the BIOS loader. We're not that close to the edge on it. Normally, I'd say no new features there, but this is a reasonable exception. The big ones that will be hard in the future is there will be no new ZFS compression or crypto supported there.

[david_crossfamilyweb.com]

In D41509#1055136, @imp wrote:

Great! I'd forgotten I'd only included it in kboot since that's the only place I'd tested it.
I'll include it elsewhere by default. IIRC, the lua glue code is only about 200 extra bytes of code, so we can have it in the BIOS loader. We're not that close to the edge on it. Normally, I'd say no new features there, but this is a reasonable exception. The big ones that will be hard in the future is there will be no new ZFS compression or crypto supported there.

Could you wait to when I get the next version of the diff up, and then correct how I did it there vs what you have?
I'd like to see if we can get this all in in one swoop; vs waiting for your changes, and then having to wait so i can get those to make mine; at this point I think I am at my last compile, I've added it to the lua interp initialization, i've added SHA512 and I've refactored my crypt implementation to use the C exported sha routines.

(also, i fixed my issue with not doing OOP correctly in lua;I previously had 'ctx = hash.sha256()' and then 'ctx.update(ctx, data)' and it was bothering me that I had to pass ctx in (this is what I was referring to in the previous message), but after going through a lot of the LUA C-Api documents to figure out how to do what I wanted to, I realized the problem was in my lua code, not the C-API interface. ctx:update(data) and it is fixed!

[david_crossfamilyweb.com]

As promised, updated, including modifications to export sha512 from C, updated lua crypt implementation to use exported C routines, removed the now unneeded pure lua implementation, I ran all validation suites on my computer and this is passing.

Updated man pages.

I am now going to take this diff and apply it and recompile and make sure I didn't miss anything; but I figure get the review wheels going in the interim

[david_crossfamilyweb.com]

Synced with latest freebsd/main for diff generation, no other changes

[imp]

Thanks for the rebase. I have one minor housekeeping issue here, but otherwise defer to kevans' reviews (I can't tell if they are folded in or not, but they don't appear to be).

stand/lua/crypt.lua

3

"BSD-2-Clause" the the proper thing here.
You can also drop the rest of the boiler plate if you want.
The -FreeBSD version was deprecated sometime ago by the SPDX folks.

[david_crossfamilyweb.com]

Updated the diff per requested copyright changes.

My intent was that I had implemented all of kevans' reivew changes already, lua-lint passes cleanly-ish. the only issues are about the 'hash' accesses, but those mirror 'gfx' in other lua files.

Note, I also compared compiled file sizes against 14.1 distribution sizes as follows:
File 14.1 size new size
loader 503808 507904
loader.efi 660480 662528

It looks like this adds about 2-4k to the size.

[kevans]

In D41509#1055376, @david_crossfamilyweb.com wrote:

Updated the diff per requested copyright changes.

My intent was that I had implemented all of kevans' reivew changes already, lua-lint passes cleanly-ish. the only issues are about the 'hash' accesses, but those mirror 'gfx' in other lua files.

I've updated the linter exception list in 6df36deb8bba31, thanks for pointing that out -- it's good to push the noise out of the way.

[david_crossfamilyweb.com]

Should note I just got done testing this with a clean 14.1 src tree that I applied the git diff to with no other changes; tested sha256 password and sha512 password, additionally un-commented both test vector suites and all passed.

(wow the old pure LUA implementation would take well over 10 minutes to pass, this passes in under a minute)

[imp]

lib/flua/libhash/lhash.c

141

Extra newline?

stand/common/interp_lua.c

96

Kboot removal?

[david_crossfamilyweb.com]

Updated to remove the extra newline

Not sure what you mean or to do with kboot; this is something I have no experience, understanding, or even really the ability to test/validate; could this be a second/unrelated change to this? I don't even really understand the usecase (but I do have some guesses)

[david_crossfamilyweb.com]

Is there anything I can do here?

I think the last part is the kboot component; does that need to go with this or can it be separate/later/independent?

If it has to go together, I can attempt that piece as well, I'll just need some pointers as to how to use it (compile it, test it,etc).

Eager to get this (and the other 2 PRs) in, and not lose momentum.