From 4576565fd1c49026772a5169de7604a83a573d65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dag-Erling=20Sm=C3=B8rgrav?= Date: Wed, 25 Apr 2018 18:23:21 +0200 Subject: [PATCH 1/5] Make rand_bytes() work more like read(2). --- include/cryb/rand.h | 2 +- lib/oath/cryb_oath_key_create.c | 4 +++- lib/rand/cryb_rand_bytes.c | 17 ++++------------- t/t_cxx.cc | 2 ++ 4 files changed, 10 insertions(+), 15 deletions(-) diff --git a/include/cryb/rand.h b/include/cryb/rand.h index dd3864f..5a90e40 100644 --- a/include/cryb/rand.h +++ b/include/cryb/rand.h @@ -39,7 +39,7 @@ CRYB_BEGIN const char *cryb_rand_version(void); #define rand_bytes cryb_rand_bytes -int rand_bytes(uint8_t *, size_t); +ssize_t rand_bytes(uint8_t *, size_t); CRYB_END diff --git a/lib/oath/cryb_oath_key_create.c b/lib/oath/cryb_oath_key_create.c index 8301162..9e5a79b 100644 --- a/lib/oath/cryb_oath_key_create.c +++ b/lib/oath/cryb_oath_key_create.c @@ -29,6 +29,8 @@ #include "cryb/impl.h" +#include + #include #include @@ -87,7 +89,7 @@ oath_key_create(const char *label, /* generate key data if necessary */ if (keydata == NULL) { - if (rand_bytes((uint8_t *)keybuf, keylen) != 1) + if (rand_bytes((uint8_t *)keybuf, keylen) != (ssize_t)keylen) return (NULL); keydata = keybuf; } diff --git a/lib/rand/cryb_rand_bytes.c b/lib/rand/cryb_rand_bytes.c index 83f6d44..8f9da66 100644 --- a/lib/rand/cryb_rand_bytes.c +++ b/lib/rand/cryb_rand_bytes.c @@ -41,24 +41,15 @@ * Working placeholder until we come up with a proper API and start adding * more methods. */ -int +ssize_t rand_bytes(uint8_t *buf, size_t len) { ssize_t rlen; - int fd, serrno; + int fd; if ((fd = open("/dev/random", O_RDONLY)) < 0) return (-1); - if ((rlen = read(fd, buf, len)) < 0) { - serrno = errno; - close(fd); - errno = serrno; - return (-1); - } + rlen = read(fd, buf, len); close(fd); - if (rlen != (ssize_t)len) { - errno = EIO; - return (-1); - } - return (0); + return (rlen); } diff --git a/t/t_cxx.cc b/t/t_cxx.cc index 83d3799..49e5c1a 100644 --- a/t/t_cxx.cc +++ b/t/t_cxx.cc @@ -29,6 +29,8 @@ #include "cryb/impl.h" +#include + /* gcc's is broken */ #include #include From 54c67f337abb13a8f4d68df62c81d383ca16f2cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dag-Erling=20Sm=C3=B8rgrav?= Date: Wed, 25 Apr 2018 23:42:48 +0200 Subject: [PATCH 2/5] Add partial support for issuer parameter. We can read the issuer from a URI (as a separate parameter, not as a prefix to the label) and store it, but not yet output it. That will be implemented in a future rewrite of oath_key_to_uri(). --- include/cryb/oath.h | 4 ++-- include/cryb/oath_constants.h | 10 ++++++++-- include/cryb/oath_types.h | 4 ++++ lib/oath/cryb_oath_key_create.c | 14 ++++++++++++-- lib/oath/cryb_oath_key_dummy.c | 2 ++ lib/oath/cryb_oath_key_from_uri.c | 6 +++++- 6 files changed, 33 insertions(+), 7 deletions(-) diff --git a/include/cryb/oath.h b/include/cryb/oath.h index 3d8f6d8..52c82fb 100644 --- a/include/cryb/oath.h +++ b/include/cryb/oath.h @@ -54,8 +54,8 @@ const char *cryb_oath_version(void); #define oath_mode_value cryb_oath_mode_value struct oath_key *oath_key_alloc(void); -struct oath_key *oath_key_create(const char *, enum oath_mode, - enum oath_hash, const char *, size_t); +struct oath_key *oath_key_create(const char *, const char *, + enum oath_mode, enum oath_hash, const char *, size_t); void oath_key_free(struct oath_key *); struct oath_key *oath_key_from_uri(const char *); struct oath_key *oath_key_from_file(const char *); diff --git a/include/cryb/oath_constants.h b/include/cryb/oath_constants.h index 09634ef..7782429 100644 --- a/include/cryb/oath_constants.h +++ b/include/cryb/oath_constants.h @@ -76,15 +76,21 @@ enum oath_hash { */ #define OATH_MAX_KEYLEN 64 +/* + * Maximum issuer length in characters, including terminating NUL. + */ +#define OATH_MAX_ISSUERLEN 64 + /* * Maximum label length in characters, including terminating NUL. */ #define OATH_MAX_LABELLEN 64 /* - * Label to use for dummy keys + * Issuer and label to use for dummy keys. */ -#define OATH_DUMMY_LABEL "oath-dummy@cryb.to" +#define OATH_DUMMY_ISSUER "cryb-oath" +#define OATH_DUMMY_LABEL "dummy@cryb.to" CRYB_END diff --git a/include/cryb/oath_types.h b/include/cryb/oath_types.h index b895f1f..fa12a62 100644 --- a/include/cryb/oath_types.h +++ b/include/cryb/oath_types.h @@ -55,6 +55,10 @@ struct oath_key { /* hash algorithm */ enum oath_hash hash; + /* issuer */ + size_t issuerlen; /* bytes incl. NUL */ + char issuer[OATH_MAX_ISSUERLEN]; + /* label */ size_t labellen; /* bytes incl. NUL */ char label[OATH_MAX_LABELLEN]; diff --git a/lib/oath/cryb_oath_key_create.c b/lib/oath/cryb_oath_key_create.c index 9e5a79b..0b0f5de 100644 --- a/lib/oath/cryb_oath_key_create.c +++ b/lib/oath/cryb_oath_key_create.c @@ -44,13 +44,18 @@ */ struct oath_key * -oath_key_create(const char *label, +oath_key_create(const char *issuer, const char *label, enum oath_mode mode, enum oath_hash hash, const char *keydata, size_t keylen) { char keybuf[OATH_MAX_KEYLEN]; struct oath_key *key; - int labellen; + int issuerlen, labellen; + + /* check issuer */ + if (issuer == NULL || + (issuerlen = strlen(issuer)) >= OATH_MAX_ISSUERLEN) + return (NULL); /* check label */ if (label == NULL || @@ -98,6 +103,11 @@ oath_key_create(const char *label, if ((key = oath_key_alloc()) == NULL) return (NULL); + /* issuer */ + memcpy(key->issuer, issuer, issuerlen); + key->issuer[issuerlen] = 0; + key->issuerlen = issuerlen; + /* label */ memcpy(key->label, label, labellen); key->label[labellen] = 0; diff --git a/lib/oath/cryb_oath_key_dummy.c b/lib/oath/cryb_oath_key_dummy.c index f7422d3..6dbccd0 100644 --- a/lib/oath/cryb_oath_key_dummy.c +++ b/lib/oath/cryb_oath_key_dummy.c @@ -53,6 +53,8 @@ oath_key_dummy(enum oath_mode mode, enum oath_hash hash, unsigned int digits) key->counter = 0; key->timestep = 30; key->hash = hash; + memcpy(key->issuer, OATH_DUMMY_ISSUER, sizeof OATH_DUMMY_ISSUER); + key->issuerlen = sizeof OATH_DUMMY_ISSUER - 1; memcpy(key->label, OATH_DUMMY_LABEL, sizeof OATH_DUMMY_LABEL); key->labellen = sizeof OATH_DUMMY_LABEL - 1; key->keylen = sizeof key->key; diff --git a/lib/oath/cryb_oath_key_from_uri.c b/lib/oath/cryb_oath_key_from_uri.c index 6b1b538..cf371e3 100644 --- a/lib/oath/cryb_oath_key_from_uri.c +++ b/lib/oath/cryb_oath_key_from_uri.c @@ -36,6 +36,7 @@ #include #include #include +#include #include /* @@ -157,7 +158,10 @@ oath_key_from_uri(const char *uri) goto invalid; key->timestep = n; } else if (strcmp("issuer", name) == 0) { - // noop for now + key->issuerlen = strlcpy(key->issuer, value, + sizeof key->issuer); + if (key->issuerlen > sizeof key->issuer) + goto invalid; } else { goto invalid; } From 48fc358df738bd01df986afefdcb52e5c10dfbc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dag-Erling=20Sm=C3=B8rgrav?= Date: Thu, 26 Apr 2018 00:07:55 +0200 Subject: [PATCH 3/5] Major cleanup and API overhaul. - The API has been redesigned so the caller is now responsible for allocating storage. - A few more macros and typedefs have been added to clean up the namespace. - Key parameter validation has been strengthened. --- include/cryb/oath.h | 22 ++- include/cryb/oath_constants.h | 25 ++- include/cryb/oath_hotp.h | 4 +- include/cryb/oath_totp.h | 4 +- include/cryb/oath_types.h | 14 +- lib/oath/Makefile.am | 4 +- lib/oath/cryb_oath_hotp.c | 4 +- lib/oath/cryb_oath_key_alloc.c | 80 --------- lib/oath/cryb_oath_key_create.c | 152 +++++------------- ...ath_key_free.c => cryb_oath_key_destroy.c} | 33 +--- lib/oath/cryb_oath_key_dummy.c | 52 +++--- lib/oath/cryb_oath_key_from_uri.c | 51 ++---- lib/oath/cryb_oath_key_to_uri.c | 2 +- lib/oath/cryb_oath_mode.c | 6 +- lib/oath/cryb_oath_totp.c | 4 +- 15 files changed, 126 insertions(+), 331 deletions(-) delete mode 100644 lib/oath/cryb_oath_key_alloc.c rename lib/oath/{cryb_oath_key_free.c => cryb_oath_key_destroy.c} (76%) diff --git a/include/cryb/oath.h b/include/cryb/oath.h index 52c82fb..4ed954c 100644 --- a/include/cryb/oath.h +++ b/include/cryb/oath.h @@ -44,27 +44,23 @@ CRYB_BEGIN const char *cryb_oath_version(void); -#define oath_key_alloc cryb_oath_key_alloc #define oath_key_create cryb_oath_key_create +#define oath_key_destroy cryb_oath_key_destroy #define oath_key_dummy cryb_oath_key_dummy -#define oath_key_free cryb_oath_key_free #define oath_key_from_uri cryb_oath_key_from_uri #define oath_key_to_uri cryb_oath_key_to_uri #define oath_mode_name cryb_oath_mode_name #define oath_mode_value cryb_oath_mode_value -struct oath_key *oath_key_alloc(void); -struct oath_key *oath_key_create(const char *, const char *, - enum oath_mode, enum oath_hash, const char *, size_t); -void oath_key_free(struct oath_key *); -struct oath_key *oath_key_from_uri(const char *); -struct oath_key *oath_key_from_file(const char *); -char *oath_key_to_uri(const struct oath_key *); +int oath_key_create(oath_key *, oath_mode, oath_hash, unsigned int, + const char *, const char *, const char *, size_t); +void oath_key_destroy(oath_key *); +int oath_key_dummy(oath_key *, oath_mode, oath_hash, unsigned int); +int oath_key_from_uri(oath_key *, const char *); +char *oath_key_to_uri(const oath_key *); -struct oath_key *oath_key_dummy(enum oath_mode, enum oath_hash, unsigned int); - -const char *oath_mode_name(enum oath_mode); -enum oath_mode oath_mode_value(const char *); +const char *oath_mode_name(oath_mode); +oath_mode oath_mode_value(const char *); CRYB_END diff --git a/include/cryb/oath_constants.h b/include/cryb/oath_constants.h index 7782429..d487fc0 100644 --- a/include/cryb/oath_constants.h +++ b/include/cryb/oath_constants.h @@ -36,28 +36,38 @@ CRYB_BEGIN +#define oath_mode cryb_oath_mode +#define oath_hash cryb_oath_hash + /* * OATH modes */ -enum oath_mode { +typedef enum { om_undef, /* not set / default */ om_hotp, /* RFC 4226 HOTP */ om_totp, /* RFC 6238 TOTP */ om_ocra, /* RFC 6287 OCRA */ om_max -}; +} oath_mode; /* * Hash functions */ -enum oath_hash { +typedef enum { oh_undef, /* not set / default */ oh_md5, /* RFC 1321 MD5 */ oh_sha1, /* FIPS 180 SHA-1 */ oh_sha256, /* FIPS 180 SHA-256 */ oh_sha512, /* FIPS 180 SHA-512 */ oh_max -}; +} oath_hash; + +/* + * Minimum and default number of digits as per RFC 4226. + */ +#define OATH_MIN_DIGITS 6 +#define OATH_DEF_DIGITS 6 +#define OATH_MAX_DIGITS 9 /* * Default time step for TOTP: 30 seconds. @@ -71,9 +81,12 @@ enum oath_hash { #define OATH_MAX_TIMESTEP 600 /* - * Maximum key length in bytes. HMAC has a 64-byte block size; if the key - * K is longer than that, HMAC derives a new key K' = H(K). + * Minimum, default and maximum key lengths in bytes as per RFC 4226. + * HMAC has a 64-byte block size; if the key K is longer than that, HMAC + * derives a new key K' = H(K). */ +#define OATH_MIN_KEYLEN 16 +#define OATH_DEF_KEYLEN 20 #define OATH_MAX_KEYLEN 64 /* diff --git a/include/cryb/oath_hotp.h b/include/cryb/oath_hotp.h index c7aeb55..08de5ea 100644 --- a/include/cryb/oath_hotp.h +++ b/include/cryb/oath_hotp.h @@ -45,8 +45,8 @@ CRYB_BEGIN #define oath_hotp_match cryb_oath_hotp_match unsigned int oath_hotp(const uint8_t *, size_t, uint64_t, unsigned int); -unsigned int oath_hotp_current(struct oath_key *); -int oath_hotp_match(struct oath_key *, unsigned int, int); +unsigned int oath_hotp_current(oath_key *); +int oath_hotp_match(oath_key *, unsigned int, int); CRYB_END diff --git a/include/cryb/oath_totp.h b/include/cryb/oath_totp.h index 1c2d253..87f1a12 100644 --- a/include/cryb/oath_totp.h +++ b/include/cryb/oath_totp.h @@ -45,8 +45,8 @@ CRYB_BEGIN #define oath_totp_match cryb_oath_totp_match unsigned int oath_totp(const uint8_t *, size_t, unsigned int); -unsigned int oath_totp_current(const struct oath_key *); -int oath_totp_match(struct oath_key *, unsigned int, int); +unsigned int oath_totp_current(const oath_key *); +int oath_totp_match(oath_key *, unsigned int, int); CRYB_END diff --git a/include/cryb/oath_types.h b/include/cryb/oath_types.h index fa12a62..5641a4e 100644 --- a/include/cryb/oath_types.h +++ b/include/cryb/oath_types.h @@ -36,12 +36,15 @@ CRYB_BEGIN +#define oath_key cryb_oath_key + /* * OATH key and associated parameters */ -struct oath_key { +typedef struct { /* mode and parameters */ - enum oath_mode mode; + oath_mode mode; + oath_hash hash; unsigned int digits; uint64_t counter; /* HOTP only */ unsigned int timestep; /* TOTP only - in seconds */ @@ -49,11 +52,6 @@ struct oath_key { /* housekeeping */ unsigned int dummy:1; /* dummy key, always fail */ - unsigned int mapped:1; /* allocated with mmap() */ - unsigned int locked:1; /* locked / wired with madvise() */ - - /* hash algorithm */ - enum oath_hash hash; /* issuer */ size_t issuerlen; /* bytes incl. NUL */ @@ -66,7 +64,7 @@ struct oath_key { /* key */ size_t keylen; /* bytes */ uint8_t key[OATH_MAX_KEYLEN]; -}; +} oath_key; CRYB_END diff --git a/lib/oath/Makefile.am b/lib/oath/Makefile.am index d1ba66d..8be52ad 100644 --- a/lib/oath/Makefile.am +++ b/lib/oath/Makefile.am @@ -5,11 +5,11 @@ lib_LTLIBRARIES = libcryb-oath.la libcryb_oath_la_SOURCES = \ cryb_oath_hotp.c \ cryb_oath_totp.c \ - cryb_oath_key_alloc.c \ + \ cryb_oath_key_create.c \ + cryb_oath_key_destroy.c \ cryb_oath_key_dummy.c \ cryb_oath_key_from_uri.c \ - cryb_oath_key_free.c \ cryb_oath_key_to_uri.c \ cryb_oath_mode.c \ \ diff --git a/lib/oath/cryb_oath_hotp.c b/lib/oath/cryb_oath_hotp.c index b477471..7e4fb04 100644 --- a/lib/oath/cryb_oath_hotp.c +++ b/lib/oath/cryb_oath_hotp.c @@ -85,7 +85,7 @@ oath_hotp(const uint8_t *K, size_t Klen, uint64_t seq, unsigned int Digit) * Computes the current code for the given key and advances the counter. */ unsigned int -oath_hotp_current(struct oath_key *k) +oath_hotp_current(oath_key *k) { unsigned int code; @@ -106,7 +106,7 @@ oath_hotp_current(struct oath_key *k) * error occurred. Also advances the counter if there was a match. */ int -oath_hotp_match(struct oath_key *k, unsigned int response, int window) +oath_hotp_match(oath_key *k, unsigned int response, int window) { unsigned int code; diff --git a/lib/oath/cryb_oath_key_alloc.c b/lib/oath/cryb_oath_key_alloc.c deleted file mode 100644 index e29783b..0000000 --- a/lib/oath/cryb_oath_key_alloc.c +++ /dev/null @@ -1,80 +0,0 @@ -/*- - * Copyright (c) 2013 The University of Oslo - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * 3. The name of the author may not be used to endorse or promote - * products derived from this software without specific prior written - * permission. - * - * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - */ - -#include "cryb/impl.h" - -#include - -#include -#include -#include - -#include - -/* - * OATH - * - * Allocates an OATH key structure - */ - -struct oath_key * -oath_key_alloc(void) -{ - struct oath_key *key; - int prot, flags; - - prot = PROT_READ|PROT_WRITE; - flags = MAP_ANON; -#ifdef MAP_NOCORE - flags |= MAP_NOCORE; -#endif - if ((key = mmap(NULL, sizeof *key, prot, flags, -1, 0)) != NULL) { - memset(key, 0, sizeof *key); - key->mapped = 1; - if (mlock(key, sizeof *key) == 0) - key->locked = 1; - } else { - /* openpam_log(PAM_LOG_ERROR, "mmap(): %m"); */ - if ((key = calloc(sizeof *key, 1)) == NULL) { - /* openpam_log(PAM_LOG_ERROR, "malloc(): %m") */; - } - } - return (key); -} - -/** - * The =oath_key_alloc function allocates and initializes an OATH key - * structure. - * - * Keys allocated with =oath_key_alloc must be freed using =oath_key_free. - * - * >oath_key_free - * - * AUTHOR UIO - */ diff --git a/lib/oath/cryb_oath_key_create.c b/lib/oath/cryb_oath_key_create.c index 0b0f5de..bfc7019 100644 --- a/lib/oath/cryb_oath_key_create.c +++ b/lib/oath/cryb_oath_key_create.c @@ -36,149 +36,79 @@ #include #include +#include /* - * OATH - * - * Creates an OATH key with the specified parameters + * Initialize an OATH key with the specified parameters. */ - -struct oath_key * -oath_key_create(const char *issuer, const char *label, - enum oath_mode mode, enum oath_hash hash, +int +oath_key_create(oath_key *ok, + oath_mode mode, oath_hash hash, unsigned int digits, + const char *issuer, const char *label, const char *keydata, size_t keylen) { - char keybuf[OATH_MAX_KEYLEN]; - struct oath_key *key; - int issuerlen, labellen; + + memset(ok, 0, sizeof *ok); /* check issuer */ if (issuer == NULL || - (issuerlen = strlen(issuer)) >= OATH_MAX_ISSUERLEN) - return (NULL); + strlcpy(ok->issuer, issuer, sizeof ok->issuer) >= sizeof ok->issuer) + goto fail; /* check label */ if (label == NULL || - (labellen = strlen(label)) >= OATH_MAX_LABELLEN) - return (NULL); - - /* check key length */ - if (keylen > OATH_MAX_KEYLEN || - (keydata != NULL && keylen == 0)) - return (NULL); - if (keylen == 0) - keylen = 20; + strlcpy(ok->label, label, sizeof ok->label) >= sizeof ok->label) + goto fail; /* check mode */ switch (mode) { - case om_hotp: case om_totp: + ok->timestep = 30; + /* fall through */ + case om_hotp: + ok->mode = mode; break; default: - return (NULL); + goto fail; } /* check hash */ switch (hash) { case oh_undef: hash = oh_sha1; - break; + /* fall through */ case oh_md5: case oh_sha1: case oh_sha256: case oh_sha512: + ok->hash = hash; break; default: - return (NULL); + goto fail; } - /* generate key data if necessary */ - if (keydata == NULL) { - if (rand_bytes((uint8_t *)keybuf, keylen) != (ssize_t)keylen) - return (NULL); - keydata = keybuf; - } + /* check digits */ + if (digits == 0) + digits = OATH_DEF_DIGITS; + if (digits < OATH_MIN_DIGITS || digits > OATH_MAX_DIGITS) + goto fail; + ok->digits = digits; - /* allocate */ - if ((key = oath_key_alloc()) == NULL) - return (NULL); + /* check key length */ + if (keydata == NULL && keylen == 0) + keylen = OATH_DEF_KEYLEN; + if (keylen < OATH_MIN_KEYLEN || keylen > OATH_MAX_KEYLEN) + goto fail; + ok->keylen = keylen; - /* issuer */ - memcpy(key->issuer, issuer, issuerlen); - key->issuer[issuerlen] = 0; - key->issuerlen = issuerlen; + /* copy or generate key */ + if (keydata != NULL) + memcpy(ok->key, keydata, ok->keylen); + else if (rand_bytes(ok->key, ok->keylen) != (ssize_t)ok->keylen) + goto fail; - /* label */ - memcpy(key->label, label, labellen); - key->label[labellen] = 0; - key->labellen = labellen; - - /* mode and hash */ - key->mode = mode; - key->hash = hash; - - /* default parameters */ - key->digits = 6; - if (key->mode == om_totp) - key->timestep = 30; - - /* key */ - memcpy(key->key, keydata, keylen); - key->keylen = keylen; - - return (key); + return (0); +fail: + memset(ok, 0, sizeof *ok); + return (-1); } - -/** - * The =oath_key_create function allocates and initializes an OATH key - * structure with the specified parameters. - * - * The =label parameter must point to a string describing the key. - * - * The =mode parameter indicates the OTP algorithm to use: - * - * ;om_hotp: - * RFC 4226 HOTP - * ;om_totp: - * RFC 6238 TOTP - * - * The =hash parameter indicates which hash algorithm to use: - * - * ;oh_md5: - * RFC 1321 MD5 - * ;oh_sha1: - * RFC 3174 SHA-1 - * ;oh_sha256: - * RFC 6234 SHA-256 - * ;oh_sha512: - * RFC 6234 SHA-512 - * - * If =hash is ;oh_undef, the default algorithm (SHA-1) is used. - * - * The =keydata parameter should point to a buffer containing the raw key - * to use. - * If =keydata is NULL, a key will be randomly generated. - * Note that the strength of the generated key is dependent on the - * strength of the operating system's pseudo-random number generator. - * - * The =keylen parameter specifies the length of the provided (or - * generated) key in bytes. - * Note that some OATH HOTP / TOTP implementations do not support key - * lengths that are not a multiple of 20 bits (5 bytes). - * If =keydata is NULL and =keylen is 0, a hardcoded default of 160 bits - * (20 bytes) is used. - * - * The following key parameters are set to hardcoded default values and - * can be changed after key creation: - * - * - For HOTP keys, the initial counter value is set to 0. - * - For TOTP keys, the timestep is set to 30 seconds. - * - For both HOTP and TOTP keys, the number of digits is set to 6. - * - * Keys created with =oath_key_create must be freed using =oath_key_free. - * - * >oath_key_alloc - * >oath_key_free - * - * AUTHOR UIO - */ diff --git a/lib/oath/cryb_oath_key_free.c b/lib/oath/cryb_oath_key_destroy.c similarity index 76% rename from lib/oath/cryb_oath_key_free.c rename to lib/oath/cryb_oath_key_destroy.c index 92160de..362c81e 100644 --- a/lib/oath/cryb_oath_key_free.c +++ b/lib/oath/cryb_oath_key_destroy.c @@ -1,5 +1,6 @@ /*- * Copyright (c) 2013 The University of Oslo + * Copyright (c) 2018 Dag-Erling Smørgrav * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -35,38 +36,16 @@ #include #include +#include #include /* - * OATH - * - * Wipes and frees an OATH key structure + * Wipes an OATH key structure. */ - void -oath_key_free(struct oath_key *key) +oath_key_destroy(oath_key *key) { - int mapped, locked; - if (key != NULL) { - mapped = key->mapped; - locked = key->locked; - memset(key, 0, sizeof *key); - if (mapped) { - if (locked) - munlock(key, sizeof *key); - munmap(key, sizeof *key); - } else { - free(key); - } - } + if (key != NULL) + memset_s(key, 0, sizeof *key, sizeof *key); } - -/** - * The =oath_key_free function wipes and frees an OATH key structure which - * was previously allocated using the =oath_key_alloc function. - * - * >oath_key_alloc - * - * AUTHOR UIO - */ diff --git a/lib/oath/cryb_oath_key_dummy.c b/lib/oath/cryb_oath_key_dummy.c index 6dbccd0..31b81f2 100644 --- a/lib/oath/cryb_oath_key_dummy.c +++ b/lib/oath/cryb_oath_key_dummy.c @@ -33,43 +33,27 @@ #include #include +#include /* - * OATH - * - * Creates a dummy OATH key structure + * Initialize an OATH key with dummy parameters. */ - -struct oath_key * -oath_key_dummy(enum oath_mode mode, enum oath_hash hash, unsigned int digits) +int +oath_key_dummy(oath_key *ok, oath_mode mode, oath_hash hash, + unsigned int digits) { - struct oath_key *key; - if ((key = oath_key_alloc()) == NULL) - return (NULL); - key->dummy = 1; - key->mode = mode; - key->digits = digits; - key->counter = 0; - key->timestep = 30; - key->hash = hash; - memcpy(key->issuer, OATH_DUMMY_ISSUER, sizeof OATH_DUMMY_ISSUER); - key->issuerlen = sizeof OATH_DUMMY_ISSUER - 1; - memcpy(key->label, OATH_DUMMY_LABEL, sizeof OATH_DUMMY_LABEL); - key->labellen = sizeof OATH_DUMMY_LABEL - 1; - key->keylen = sizeof key->key; - return (key); + memset(ok, 0, sizeof *ok); + ok->dummy = 1; + ok->mode = mode; + ok->hash = hash; + ok->digits = digits; + ok->timestep = 30; + ok->labellen = + strlcpy(ok->issuer, OATH_DUMMY_LABEL, sizeof ok->label); + ok->issuerlen = + strlcpy(ok->issuer, OATH_DUMMY_ISSUER, sizeof ok->issuer); + ok->keylen = sizeof ok->key; + memset(ok->key, 0xff, ok->keylen); + return (0); } - -/** - * The =oath_key_dummy function allocates and initializes a dummy OATH key - * structure. - * Authentication attempts using a dummy key will always fail. - * - * Keys allocated with =oath_key_dummy must be freed using =oath_key_free. - * - * >oath_key_alloc - * >oath_key_free - * - * AUTHOR UIO - */ diff --git a/lib/oath/cryb_oath_key_from_uri.c b/lib/oath/cryb_oath_key_from_uri.c index cf371e3..dd9b558 100644 --- a/lib/oath/cryb_oath_key_from_uri.c +++ b/lib/oath/cryb_oath_key_from_uri.c @@ -1,6 +1,6 @@ /*- * Copyright (c) 2013-2016 The University of Oslo - * Copyright (c) 2016-2017 Dag-Erling Smørgrav + * Copyright (c) 2016-2018 Dag-Erling Smørgrav * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -33,30 +33,27 @@ #include #include +#include #include #include #include #include -#include /* - * OATH + * Initializes an OATH key with the parameters from a Google otpauth URI. * - * Creates an OATH key from a Google otpauth URI + * https://github.com/google/google-authenticator/wiki/Key-Uri-Format */ - -struct oath_key * -oath_key_from_uri(const char *uri) +int +oath_key_from_uri(oath_key *key, const char *uri) { char name[64], value[256]; size_t namelen, valuelen; - struct oath_key *key; const char *p, *q, *r; uintmax_t n; char *e; - if ((key = oath_key_alloc()) == NULL) - return (NULL); + memset(key, 0, sizeof *key); /* check method */ p = uri; @@ -109,7 +106,8 @@ oath_key_from_uri(const char *uri) /* dupe */ goto invalid; key->keylen = sizeof key->key; - if (base32_decode(value, valuelen, key->key, &key->keylen) != 0) + if (base32_decode(value, valuelen, key->key, + &key->keylen) != 0) goto invalid; } else if (strcmp("algorithm", name) == 0) { if (key->hash != oh_undef) @@ -189,8 +187,7 @@ oath_key_from_uri(const char *uri) key->lastused = 0; } else { /* unreachable */ - oath_key_free(key); - return (NULL); + goto invalid; } if (key->hash == oh_undef) key->hash = oh_sha1; @@ -198,31 +195,9 @@ oath_key_from_uri(const char *uri) key->digits = 6; if (key->keylen == 0) goto invalid; - return (key); + return (0); invalid: - // openpam_log(PAM_LOG_NOTICE, "invalid OATH URI: %s", uri); - oath_key_free(key); - return (NULL); + memset(key, 0, sizeof *key); + return (-1); } - -/** - * The =oath_key_from_uri function parses a Google otpauth URI into a key - * structure. - * - * The =uri parameter points to a NUL-terminated string containing the - * URI. - * - * Keys created with =oath_key_from_uri must be freed using - * =oath_key_free. - * - * >oath_key_alloc - * >oath_key_free - * >oath_key_to_uri - * - * REFERENCES - * - * https://code.google.com/p/google-authenticator/wiki/KeyUriFormat - * - * AUTHOR UIO - */ diff --git a/lib/oath/cryb_oath_key_to_uri.c b/lib/oath/cryb_oath_key_to_uri.c index 50e831d..b5b4b50 100644 --- a/lib/oath/cryb_oath_key_to_uri.c +++ b/lib/oath/cryb_oath_key_to_uri.c @@ -40,7 +40,7 @@ #include char * -oath_key_to_uri(const struct oath_key *key) +oath_key_to_uri(const oath_key *key) { const char *hash; char *tmp, *uri; diff --git a/lib/oath/cryb_oath_mode.c b/lib/oath/cryb_oath_mode.c index 2fb194b..a0cc109 100644 --- a/lib/oath/cryb_oath_mode.c +++ b/lib/oath/cryb_oath_mode.c @@ -43,10 +43,10 @@ static const char *oath_mode_names[om_max] = { /* * Returns the enum value that corresponds to an OATH mode name */ -enum oath_mode +oath_mode oath_mode_value(const char *str) { - enum oath_mode om; + oath_mode om; for (om = 0; om < om_max; ++om) { if (oath_mode_names[om] != NULL && @@ -61,7 +61,7 @@ oath_mode_value(const char *str) * Returns the name of an OATH mode given its enum value */ const char * -oath_mode_name(enum oath_mode om) +oath_mode_name(oath_mode om) { if (om < om_max) diff --git a/lib/oath/cryb_oath_totp.c b/lib/oath/cryb_oath_totp.c index cf5f9b2..3d21d29 100644 --- a/lib/oath/cryb_oath_totp.c +++ b/lib/oath/cryb_oath_totp.c @@ -48,7 +48,7 @@ oath_totp(const uint8_t *K, size_t Klen, unsigned int Digit) } unsigned int -oath_totp_current(const struct oath_key *k) +oath_totp_current(const oath_key *k) { unsigned int code; uint64_t seq; @@ -70,7 +70,7 @@ oath_totp_current(const struct oath_key *k) * error occurred. */ int -oath_totp_match(struct oath_key *k, unsigned int response, int window) +oath_totp_match(oath_key *k, unsigned int response, int window) { unsigned int code; uint64_t seq; From bd4b5c246ee23c6d7dcc3c3f82dd737346fe16f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dag-Erling=20Sm=C3=B8rgrav?= Date: Thu, 26 Apr 2018 02:41:47 +0200 Subject: [PATCH 4/5] Complete rewrite of oath_key_to_uri(). - The API has changed so that the function now writes its output into a caller-provided buffer, in a style similar to libcryb-enc. - All parameter values are now correctly percent-encoded. - The issuer parameter is now supported. --- include/cryb/oath.h | 2 +- lib/oath/cryb_oath_key_to_uri.c | 148 +++++++++++++++++++++++--------- 2 files changed, 110 insertions(+), 40 deletions(-) diff --git a/include/cryb/oath.h b/include/cryb/oath.h index 4ed954c..6248caa 100644 --- a/include/cryb/oath.h +++ b/include/cryb/oath.h @@ -57,7 +57,7 @@ int oath_key_create(oath_key *, oath_mode, oath_hash, unsigned int, void oath_key_destroy(oath_key *); int oath_key_dummy(oath_key *, oath_mode, oath_hash, unsigned int); int oath_key_from_uri(oath_key *, const char *); -char *oath_key_to_uri(const oath_key *); +int oath_key_to_uri(const oath_key *, char *, size_t *); const char *oath_mode_name(oath_mode); oath_mode oath_mode_value(const char *); diff --git a/lib/oath/cryb_oath_key_to_uri.c b/lib/oath/cryb_oath_key_to_uri.c index b5b4b50..65a3d9b 100644 --- a/lib/oath/cryb_oath_key_to_uri.c +++ b/lib/oath/cryb_oath_key_to_uri.c @@ -29,24 +29,96 @@ #include "cryb/impl.h" -#include - -#include -#include -#include +#include +#include +#include +#include #include -#include #include -char * -oath_key_to_uri(const oath_key *key) +static inline void +append_char(char *buf, size_t size, size_t *pos, int ch) { - const char *hash; - char *tmp, *uri; - size_t kslen, urilen; - switch (key->hash) { + if (*pos + 1 < size) + buf[*pos] = ch; + (*pos)++; + if (*pos < size) + buf[*pos] = '\0'; +} + +static inline void +append_str(char *buf, size_t size, size_t *pos, const char *str) +{ + + while (*str != '\0') { + if (*pos + 1 < size) + buf[*pos] = *str++; + (*pos)++; + } + if (*pos < size) + buf[*pos] = '\0'; +} + +static inline void +append_num(char *buf, size_t size, size_t *pos, uintmax_t num) +{ + char numbuf[32], *p; + + p = numbuf + sizeof numbuf - 1; + *p-- = '\0'; + do { + *p-- = '0' + num % 10; + num /= 10; + } while (num > 0); + append_str(buf, size, pos, p + 1); +} + +static inline void +append_percent(char *buf, size_t size, size_t *pos, const char *str) +{ + size_t res; + + res = *pos < size ? size - *pos : 0; + percent_encode(str, SIZE_MAX, buf + *pos, &res); + *pos += res - 1; +} + +static inline void +append_base32(char *buf, size_t size, size_t *pos, + const uint8_t *data, size_t len) +{ + size_t res; + + res = *pos < size ? size - *pos : 0; + base32_encode(data, len, buf + *pos, &res); + *pos += res - 1; +} + +int +oath_key_to_uri(const oath_key *ok, char *buf, size_t *size) +{ + const char *mode, *hash; + size_t pos; + + pos = 0; + append_str(buf, *size, &pos, "otpauth://"); + switch (ok->mode) { + case om_hotp: + mode = "hotp"; + break; + case om_totp: + mode = "totp"; + break; + default: + return (0); + } + append_str(buf, *size, &pos, mode); + append_char(buf, *size, &pos, '/'); + append_percent(buf, *size, &pos, ok->label); + append_str(buf, *size, &pos, "?algorithm="); + switch (ok->hash) { case oh_sha1: hash = "SHA1"; break; @@ -60,36 +132,34 @@ oath_key_to_uri(const oath_key *key) hash = "MD5"; break; default: - return (NULL); + return (0); } - - /* XXX the label and secret should be URI-encoded */ - if (key->mode == om_hotp) { - urilen = asprintf(&uri, "otpauth://%s/%s?" - "algorithm=%s&digits=%d&counter=%ju&secret=", - "hotp", key->label, hash, key->digits, - (uintmax_t)key->counter); - } else if (key->mode == om_totp) { - urilen = asprintf(&uri, "otpauth://%s/%s?" - "algorithm=%s&digits=%d&period=%u&lastused=%ju&secret=", - "totp", key->label, hash, key->digits, key->timestep, - (uintmax_t)key->lastused); + append_str(buf, *size, &pos, hash); + append_str(buf, *size, &pos, "&digits="); + append_num(buf, *size, &pos, ok->digits); + if (ok->mode == om_hotp) { + append_str(buf, *size, &pos, "&counter="); + append_num(buf, *size, &pos, (uintmax_t)ok->counter); + } else if (ok->mode == om_totp) { + append_str(buf, *size, &pos, "&period="); + append_num(buf, *size, &pos, (uintmax_t)ok->timestep); + append_str(buf, *size, &pos, "&lastused="); + append_num(buf, *size, &pos, (uintmax_t)ok->lastused); } else { - /* unreachable */ - return (NULL); + return (0); } - - /* compute length of base32-encoded key and append it */ - kslen = base32_enclen(key->keylen) + 1; - if ((tmp = realloc(uri, urilen + kslen)) == NULL) { - free(uri); - return (NULL); + if (ok->issuerlen > 0) { + append_str(buf, *size, &pos, "&issuer="); + append_percent(buf, *size, &pos, ok->issuer); } - uri = tmp; - if (base32_encode(key->key, key->keylen, uri + urilen, &kslen) != 0) { - free(uri); - return (NULL); + append_str(buf, *size, &pos, "&secret="); + append_base32(buf, *size, &pos, ok->key, ok->keylen); + pos++; // terminating NUL + if (pos > *size) { + *size = pos; + errno = ENOSPC; + return (-1); } - - return (uri); + *size = pos; + return (0); } From 31f2831b67eb906ad5e26bbe9086c21137ea7884 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dag-Erling=20Sm=C3=B8rgrav?= Date: Thu, 26 Apr 2018 03:09:51 +0200 Subject: [PATCH 5/5] Fix a few key URI validation issues. --- lib/oath/cryb_oath_key_from_uri.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/oath/cryb_oath_key_from_uri.c b/lib/oath/cryb_oath_key_from_uri.c index dd9b558..4fdd33d 100644 --- a/lib/oath/cryb_oath_key_from_uri.c +++ b/lib/oath/cryb_oath_key_from_uri.c @@ -127,10 +127,11 @@ oath_key_from_uri(oath_key *key, const char *uri) if (key->digits != 0) /* dupe */ goto invalid; - /* only 6 or 8 */ - if (valuelen != 1 || (*value != '6' && *value != '8')) + n = strtoumax(value, &e, 10); + if (e == value || *e != '\0' || + n < OATH_MIN_DIGITS || n > OATH_MAX_DIGITS) goto invalid; - key->digits = *q - '0'; + key->digits = n; } else if (strcmp("counter", name) == 0) { if (key->counter != UINT64_MAX) /* dupe */ @@ -158,7 +159,7 @@ oath_key_from_uri(oath_key *key, const char *uri) } else if (strcmp("issuer", name) == 0) { key->issuerlen = strlcpy(key->issuer, value, sizeof key->issuer); - if (key->issuerlen > sizeof key->issuer) + if (key->issuerlen >= sizeof key->issuer) goto invalid; } else { goto invalid;