Changeset View
Standalone View
lib/libcrypt/crypt.c
Context not available. | |||||
* | * | ||||
* Copyright (c) 1999 Mark Murray | * Copyright (c) 1999 Mark Murray | ||||
* Copyright (c) 2014 Dag-Erling Smørgrav | * Copyright (c) 2014 Dag-Erling Smørgrav | ||||
* Copyright (c) 2015 Derek Marcotte | |||||
* All rights reserved. | * All rights reserved. | ||||
* | * | ||||
* Redistribution and use in source and binary forms, with or without | * Redistribution and use in source and binary forms, with or without | ||||
Context not available. | |||||
__FBSDID("$FreeBSD$"); | __FBSDID("$FreeBSD$"); | ||||
#include <sys/types.h> | #include <sys/types.h> | ||||
#include <sys/param.h> | |||||
#include <errno.h> | |||||
#include <libutil.h> | #include <libutil.h> | ||||
#include <regex.h> | |||||
#include <stdbool.h> | |||||
#include <stdlib.h> | |||||
#include <string.h> | #include <string.h> | ||||
#include <unistd.h> | #include <unistd.h> | ||||
Context not available. | |||||
/* | /* | ||||
* List of supported crypt(3) formats. | * List of supported crypt(3) formats. | ||||
* | * | ||||
* The default algorithm is the last entry in the list (second-to-last | * Ordered from most probable to least probable[1], for the find algorithm to | ||||
* array element since the last is a sentinel). The reason for placing | * preform a little better in some cases. Generally, order is not important. | ||||
* the default last rather than first is that DES needs to be at the | * | ||||
* bottom for the algorithm guessing logic in crypt(3) to work correctly, | * 1. as guessed by a random person | ||||
* and it needs to be the default for backward compatibility. | * | ||||
*/ | */ | ||||
static const struct crypt_format { | static const struct crypt_format { | ||||
const char *name; | const char *name; | ||||
int (*func)(const char *, const char *, char *); | int (*func)(const char *, const char *, char *); | ||||
const char *magic; | const char *magic; | ||||
const char *const default_format; | |||||
const char *const format_regex; | |||||
const uint8_t salt_bytes; | |||||
const bool salt_trailing_sign; /* Do we tack on a $ at the end of the salt */ | |||||
} crypt_formats[] = { | } crypt_formats[] = { | ||||
{ "md5", crypt_md5, "$1$" }, | { "md5", crypt_md5, "$1$", "$1$", "^\\$1\\$$", 8, true }, | ||||
{ "sha512", crypt_sha512, "$6$", "$6$", "^\\$6\\$(rounds=[0-9]{0,9}\\$)?$", 16, true }, | |||||
#ifdef HAS_BLOWFISH | #ifdef HAS_BLOWFISH | ||||
{ "blf", crypt_blowfish, "$2" }, | { "blf", crypt_blowfish, "$2", "$2b$04$", "^\\$2[ab]?\\$[0-9]{2}\\$$", 22 /* 16 * 1.333 */, false }, | ||||
#endif | #endif | ||||
{ "nth", crypt_nthash, "$3$" }, | |||||
{ "sha256", crypt_sha256, "$5$" }, | |||||
{ "sha512", crypt_sha512, "$6$" }, | |||||
#ifdef HAS_DES | #ifdef HAS_DES | ||||
{ "des", crypt_des, "_" }, | { "des", crypt_des, NULL, "", NULL, 2, false }, | ||||
{ "des-ext", crypt_des, "_", "_..T.", "^_[A-Za-z0-9./]{4}$", 4, false }, | |||||
#endif | #endif | ||||
{ "nth", crypt_nthash, "$3$", "$3$", "^\\$3\\$$", 0, false }, | |||||
/* sentinel */ | { "sha256", crypt_sha256, "$5$", "$5$", "^\\$5\\$(rounds=[0-9]{0,9}\\$)?$", 16, true }, | ||||
{ NULL, NULL, NULL } | |||||
/* Sentinel */ | |||||
{ NULL, NULL, NULL, NULL, NULL, 0, NULL } | |||||
}; | }; | ||||
static const struct crypt_format *crypt_format = | #ifdef HAS_DES | ||||
&crypt_formats[(sizeof crypt_formats / sizeof *crypt_formats) - 2]; | /* Must be des if system has des. */ | ||||
delphij: Why? | |||||
static char default_format[256] = "des"; | |||||
#else | |||||
static char default_format[256] = "sha512"; | |||||
#endif | |||||
/* Local-scope only. */ | |||||
static const struct crypt_format *crypt_find_format(const char *); | |||||
static bool crypt_validate_format_regex(const char *, const char *); | |||||
static bool crypt_format_is_modular(const char*); | |||||
#define DES_SALT_ALPHABET \ | #define DES_SALT_ALPHABET \ | ||||
"./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" | "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" | ||||
/* | |||||
* Fill a buffer with a new salt conforming to a particular crypt format | |||||
Not Done Inline ActionsWhy is this API exposed to user? ("Do not add new functionality unless an implementor cannot complete a real application without it.") There are multiple style and coding issues, by the way, please fix. delphij: Why is this API exposed to user? ("Do not add new functionality unless an implementor cannot… | |||||
Not Done Inline ActionsI will take care of the style (and strn*) items, please excuse. With respect to the API, I think there's a good discussion to be had here. It would seem that having the salt generation (and knowledge of what an appropriate salt is for the entire suite of algorithms supported by crypt) pushed into the consumer would be needlessly complex. It ossifies algorithm support all over the place. This is not the only approach however, and perhaps we can take a page with the work done by the OpenBSD project in this space since the original patch was submitted: https://www.tedunangst.com/flak/post/retiring-crypt i.e. crypt_newhash and crypt_checkpass specifically. Also, there's some old old discussion available on the mailing lists as well, if that interests you. I've linked into the thread here: https://lists.freebsd.org/pipermail/freebsd-current/2014-March/048848.html 482254ac_razorfever.net: I will take care of the style (and strn*) items, please excuse.
With respect to the API, I… | |||||
Not Done Inline ActionsThanks for the context. Is there some reason not to have the crypt functions to generate a salt on demand instead (e.g. when the passed salt is too short, which is something that we already will check, and the individual crypt function actually knows better)? It worried me because with your new API, the callers are now required to do more work (allocating memory, check for the result, etc.). delphij: Thanks for the context. Is there some reason not to have the crypt functions to generate a… | |||||
Not Done Inline ActionsI'm not sure, it sounds like a good idea. I feel like having crypt do that automatically would be pretty surprising though, there's no precedent? (There's also no precedent for crypt_makesalt, but there are a few crypt_gensalt implementations.) With regard to the more work, perhaps that's also a benefit:
I think the format validation is a strong case for this. The crypt_newhash approach also allows to signal an invalid format, but doesn't differentiate a short buffer with an invalid format. What do you think? 482254ac_razorfever.net: I'm not sure, it sounds like a good idea. I feel like having `crypt` do that automatically… | |||||
Not Done Inline ActionsAny thoughts on a sensible way to move this portion forward? I would submit a crypt_newhash approach similar to that of OpenBSD, if that was agreed to be a sensible way to proceed. 482254ac_razorfever.net: Any thoughts on a sensible way to move this portion forward?
I would submit a crypt_newhash… | |||||
* | |||||
* We're breaking the API convention established by crypt_set_format (return 0 | |||||
* on success) * because it might be nice to behave like the rest of C | |||||
* libraries, rather than the one deprecated function. | |||||
Not Done Inline Actionswas the addition of the semicolon intentional? this is usually left off and it looks like it may cause a compile error. jmg: was the addition of the semicolon intentional? this is usually left off and it looks like it… | |||||
* | |||||
*/ | |||||
int | |||||
crypt_makesalt(char *out, const char *format, size_t *outlen) | |||||
{ | |||||
const struct crypt_format *cf; | |||||
uint8_t rand_buf[4]; | |||||
Done Inline ActionsYou are using only 3 bytes, why 4 here? delphij: You are using only 3 bytes, why 4 here? | |||||
size_t reqsz; | |||||
const char *prefix; | |||||
size_t prefix_len; | |||||
/* Diff really is a size, but keeping it api compatible with | |||||
* b64_from_24bit. Only up to 4 bytes anyways, shouldn't be a problem, | |||||
* right? | |||||
*/ | |||||
int diff; | |||||
unsigned int i; | |||||
/* Find the appropriate format entry. */ | |||||
cf = crypt_find_format(format); | |||||
if (cf == NULL) | |||||
return (EINVAL); | |||||
/* Calculate required output size. */ | |||||
if (crypt_format_is_modular(format)) { | |||||
prefix = format; | |||||
} else { | |||||
prefix = cf->default_format; | |||||
} | |||||
prefix_len = strlen(prefix); | |||||
reqsz = prefix_len + (size_t) cf->salt_bytes; | |||||
if (cf->salt_trailing_sign) | |||||
reqsz++; | |||||
/* Trailing '\0' */ | |||||
reqsz++; | |||||
/* Does the output buffer have enough. */ | |||||
if (reqsz > *outlen) { | |||||
*outlen = reqsz; | |||||
return (ENOMEM); | |||||
} | |||||
/* Start building our output. strncpy will fill the remaining buffer | |||||
* with zeros. We know we have enough due to reqsz calulation above. | |||||
*/ | |||||
strncpy(out, prefix, *outlen); | |||||
Done Inline ActionsPlease refactor the code, use strl* instead of strn*. delphij: Please refactor the code, use strl* instead of strn*. | |||||
out += prefix_len; | |||||
for (i = 0; i < cf->salt_bytes; i += 4 ) { | |||||
arc4random_buf(rand_buf, 3); | |||||
diff = MIN(cf->salt_bytes - i, 4); | |||||
b64_from_24bit(rand_buf[2], rand_buf[1], rand_buf[0], diff, &out); | |||||
Done Inline ActionsCould you please change the code to use a separate base64 buffer and use strlcat instead of operating on the output buffer directly? delphij: Could you please change the code to use a separate base64 buffer and use strlcat instead of… | |||||
} | |||||
/* Cleanup random buffer. */ | |||||
explicit_bzero(rand_buf, sizeof(rand_buf) ); | |||||
if (cf->salt_trailing_sign) { | |||||
out[0] = '$'; | |||||
Done Inline ActionsJust use strlcat, the trailing nul is not actually written, by the way. delphij: Just use strlcat, the trailing nul is not actually written, by the way. | |||||
out++; | |||||
Done Inline ActionsWhy? delphij: Why? | |||||
} | |||||
/* Don't need to add trailing '\0', strncpy above will have set it | |||||
* already. | |||||
*/ | |||||
return (0); | |||||
} | |||||
/* | /* | ||||
* Returns the name of the currently selected format. | * Returns the name of the currently selected format. | ||||
*/ | */ | ||||
Context not available. | |||||
crypt_get_format(void) | crypt_get_format(void) | ||||
{ | { | ||||
return (crypt_format->name); | return (default_format); | ||||
} | } | ||||
/* | /* | ||||
Context not available. | |||||
int | int | ||||
crypt_set_format(const char *format) | crypt_set_format(const char *format) | ||||
{ | { | ||||
const struct crypt_format *cf; | |||||
for (cf = crypt_formats; cf->name != NULL; ++cf) { | if (crypt_find_format(format) == NULL) { | ||||
if (strcasecmp(cf->name, format) == 0) { | return (0); | ||||
crypt_format = cf; | } | ||||
return (1); | |||||
strncpy(default_format, format, sizeof(default_format) ); | |||||
delphijUnsubmitted Done Inline ActionsThis introduced a security vulnerability. delphij: This introduced a security vulnerability. | |||||
return (1); | |||||
} | |||||
/* | |||||
* is the crypt format a recognized as valid | |||||
*/ | |||||
static bool | |||||
crypt_format_validate_regex(const char* regex, const char *format) | |||||
{ | |||||
regex_t regex_c; | |||||
int res = 0; | |||||
/* We could cache these, but they are simple, and this function won't be | |||||
* called often. | |||||
*/ | |||||
if (regcomp(®ex_c, regex, REG_EXTENDED) != 0) { | |||||
return (false); | |||||
} | |||||
res = regexec(®ex_c, format, 0, NULL, 0); | |||||
regfree(®ex_c); | |||||
return (!res); | |||||
} | |||||
/* | |||||
* Is the crypt format a fancy-dancy modular format. | |||||
*/ | |||||
static bool | |||||
crypt_format_is_modular(const char* format) | |||||
{ | |||||
/* We'll treat 'new des' as modular, because they can set 24 bits of | |||||
* count via salt. | |||||
*/ | |||||
return (format[0] == '$' || format[0] == '_'); | |||||
} | |||||
/* | |||||
* Lookup our format in our internal table for a matching crypt_format | |||||
* structure. | |||||
*/ | |||||
static const struct crypt_format * | |||||
crypt_find_format(const char *format) | |||||
{ | |||||
const struct crypt_format *cf; | |||||
if (strcmp(format, "default") == 0 ) { | |||||
format = default_format; | |||||
} | |||||
if (crypt_format_is_modular(format) ) { | |||||
/* modular crypt magic lookup, force full syntax */ | |||||
for (cf = crypt_formats; cf->name != NULL; ++cf) { | |||||
if (cf->magic != NULL && | |||||
strstr(format, cf->magic) == format && | |||||
crypt_format_validate_regex(cf->format_regex, format) ) { | |||||
return (cf); | |||||
} | |||||
} | |||||
} else { | |||||
/* name lookup */ | |||||
for (cf = crypt_formats; cf->name != NULL; ++cf) { | |||||
if (strcasecmp(cf->name, format) == 0) { | |||||
return (cf); | |||||
} | |||||
} | } | ||||
} | } | ||||
return (0); | |||||
return (NULL); | |||||
} | } | ||||
/* | /* | ||||
Context not available. | |||||
#ifdef HAS_DES | #ifdef HAS_DES | ||||
int len; | int len; | ||||
#endif | #endif | ||||
/* Use the magic in the salt for lookup. */ | |||||
for (cf = crypt_formats; cf->name != NULL; ++cf) | for (cf = crypt_formats; cf->name != NULL; ++cf) | ||||
if (cf->magic != NULL && strstr(salt, cf->magic) == salt) { | if (cf->magic != NULL && strstr(salt, cf->magic) == salt) { | ||||
func = cf->func; | func = cf->func; | ||||
goto match; | goto match; | ||||
} | } | ||||
#ifdef HAS_DES | #ifdef HAS_DES | ||||
/* Check if it's standard des. */ | |||||
len = strlen(salt); | len = strlen(salt); | ||||
if ((len == 13 || len == 2) && strspn(salt, DES_SALT_ALPHABET) == len) { | if ((len == 13 || len == 2) && strspn(salt, DES_SALT_ALPHABET) == len) { | ||||
func = crypt_des; | func = crypt_des; | ||||
goto match; | goto match; | ||||
} | } | ||||
#endif | #endif | ||||
func = crypt_format->func; | cf = crypt_find_format(default_format); | ||||
func = cf->func; | |||||
match: | match: | ||||
if (func(passwd, salt, data->__buf) != 0) | if (func(passwd, salt, data->__buf) != 0) | ||||
return (NULL); | return (NULL); | ||||
Context not available. |
Why?