libxcrypt icon indicating copy to clipboard operation
libxcrypt copied to clipboard

[RFC] crypt.conf documentation

Open zackw opened this issue 6 years ago • 16 comments

Addressing issue #4, I've written up a bunch of documentation for a hypothetical new feature in which the set of hashing methods that may be used, and the default cost parameter for each, are configurable at runtime via a file /etc/security/crypt.conf. I have not actually implemented the new feature and I don't know when I will have time to do it, but I thought it might be a good idea to invite feedback on the documentation first. It will probably be easier to read if you check out the branch and then use man -l to read the formatted pages. Start with crypt.conf.5.

This is not intended to be merged until the feature is actually implemented.

zackw avatar Aug 21 '18 15:08 zackw

I like this, even if it is quite ambitious. :-)

rfc1036 avatar Aug 21 '18 23:08 rfc1036

Debian maintainer here.

I think that this kind of scheme (the file in /usr is ignored if there is one in /etc) would be worse in this case, because then administrators who create the /etc file to modify a specific setting would not be notified of possibly important changes in the /usr file in future releases.

Debian and Ubuntu have well established methods to preserve and manage changes to files in /etc/, and RPM systems at least warn at package installation time if a configuration file has changed.

rfc1036 avatar Aug 24 '18 14:08 rfc1036

@rfc1036 I've never said not to issue a warning on package installation / upgrade. I'm just saying there should be an option for a sane, distro specific preset.

In the case of rpm the file in /etc would be marked as %config(noreplace) and would issue a warning and create a .rpmnew-file in the same loaction, if the file in the new package differs.

This would inform the admin properly, but leave the old config in effect as long as the file in /etc isn't updated. This ensures there is no fallout on running systems.

besser82 avatar Aug 24 '18 15:08 besser82

In this case, I was imagining that there would be no file installed by our “make install”; the library would just use its compiled-in defaults. However, package postinstall scripts could run crypt-tune-costs automatically. Since that would produce a file customized for the specific machine it was run on, /etc seems like the proper home for it.

zackw avatar Aug 24 '18 16:08 zackw

/etc/security is a bad choice, as this is used for PAM configuration files. Putting other stuff there is very confusing.

And putting the configuration file in /usr: nearly every distribution, if not even all, who support atomic updates, put the distribution specific configuration file in /usr: /usr/etc, /usr/share/default, /usr/lib/basesystem, and a lot of more. The reason is, that /etc is not accessible on such systems during upgrade. There are three ways how they use this: put a symlink in /etc pointing to /usr, which the admin can replace with a copy with his changes. Do a three-way-merge at next reboot. Not really reliable. Or, preferred, use something like systemd is doing: /usr contains the distribution config file, and /etc only contains the changes from the admin. So distributions can put easily new options in /usr without the risk that the configuration breaks after update.

thkukuk avatar Aug 30 '18 07:08 thkukuk

@thkukuk Where in /etc should a machine-specific crypt.conf go, then? Just loose at top level?

zackw avatar Sep 08 '18 15:09 zackw

I'd say it should be configurable below /etc. Depending on the distribution there are different dirs for single, additional config files:

  • Debian-ish: /etc/default/crypt.conf
  • redhat-ish: /etc/sysconfig/crypt.conf

besser82 avatar Sep 08 '18 16:09 besser82

On Sat, Sep 08, Björn Esser wrote:

I'd say it should be configurable below /etc. Depending on the distribution there are different dirs for single, additional config files:

• Debian-ish: /etc/default/crypt.conf • redhat-ish: /etc/sysconfig/crypt.conf

Both, /etc/default and /etc/sysconfig, contain by definition environment files, which are sourced bei init scripts or systemd services and contain options for the daemons. This don't sound like the correct directory.

On SUSE (which is also conform with FHS) we put such single configuration files still directly in /etc, subdirectories should only be used or created, if a services has multiple configuration files. What's wrong with that? That's where admin normally looks for such files.

-- Thorsten Kukuk, Distinguished Engineer, Senior Architect SLES & CaaSP SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nuernberg, Germany GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

thkukuk avatar Sep 09 '18 07:09 thkukuk

As the Debian maintainer I agree: I would just use /etc/crypt.conf and this way we would use the same name among different distributions.

/etc/defaults/ is mostly hacks to work around daemons which lack configuration files, and I feel that nowadays it is vaguely deprecated in Debian.

rfc1036 avatar Sep 09 '18 11:09 rfc1036

This PR introduces crypt.conf names for the various hash types, and we'll be stuck supporting those names. Let's be more careful in how we choose them. Instead of what's currently proposed:

sha512
sha256
sha1
md5_sun
md5_bsd
des_bsd
des_big
des
nthash

I strongly recommend the following:

sha512crypt
sha256crypt
sha1crypt
sunmd5
md5crypt
bsdicrypt
bigcrypt
descrypt
nt

This is the naming currently used in JtR. Well, for "nthash" vs. "nt" I don't feel strongly, but for the rest it's important that we do not invent new names nor proliferate the confusion between e.g. sha512crypt and raw SHA-512 nor between e.g. descrypt and DES. I understand the desire for consistent naming based on the underlying cryptographic primitive, but we can't do that consistently anyway (e.g., bcrypt is better known as bcrypt, and not as Blowfish), it's confusing to many users who are not familiar with how very different these high-level algorithms are from their underlying cryptographic primitives, and in the end the high-level algorithms around whatever primitive typically matter more than the choice of primitive. I had originally made the same mistake in JtR in calling things "des" and "md5" and "bf", and that was confusing people. These have since (a few years ago) been renamed to "descrypt" and "md5crypt" and "bcrypt", etc.

Perhaps we should also edit the sub-headings in the man page to use this or similar naming, but we can revise those any time without worrying about backwards compatibility. So only fixing the crypt.conf names as above is a must before merging this PR.

solardiz avatar Sep 20 '18 08:09 solardiz

@solardiz Thanks for the comprehensive list of JtR's names. I will make this change next time I cycle around to working on this project.

zackw avatar Sep 20 '18 10:09 zackw

Codecov Report

Merging #26 into develop will decrease coverage by 0.11%. The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #26      +/-   ##
===========================================
- Coverage    96.43%   96.31%   -0.12%     
===========================================
  Files           32       32              
  Lines         3112     3097      -15     
===========================================
- Hits          3001     2983      -18     
- Misses         111      114       +3
Impacted Files Coverage Δ
lib/crypt-sunmd5.c
lib/crypt-scrypt.c
lib/crypt-nthash.c
lib/crypt-bcrypt.c
lib/alg-hmac-sha1.c
lib/crypt-des-obsolete.c
lib/alg-yescrypt-common.c
lib/alg-sha256.c
lib/crypt.c
lib/crypt-common.c
... and 54 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update db944d1...ea20f5c. Read the comment docs.

codecov-io avatar Oct 19 '18 11:10 codecov-io

@zackw You may squash or edit any of my commits as needed.

besser82 avatar Oct 19 '18 12:10 besser82

@zackw FYI, I've rebased this onto recent develop.

besser82 avatar Oct 28 '18 22:10 besser82

@zackw Rebased against recent develop.

besser82 avatar Nov 11 '18 17:11 besser82

I don't have time to work on this project myself in the near future, but, looking at #117, #130, etc. I think it should be a high priority for anyone interested in hacking on libxcrypt.

zackw avatar Jun 10 '21 14:06 zackw