Geezus.
I'm finding lots of problems with the error reports:
- Some error messages are getting logged with what appears to be the
wrong severity, variant (normal vs libcrypto) or type (val vs op).
Also, the line between val and op is sometimes blurry.
- Some error messages are extremely ambiguous, which makes them useless.
It's hard to fix them because they tend to be caused by library utils
that either refuse to spit details, or export them through
undocumented, unreliable and/or inconsistent means.
- Another consequence of the generic errors is that it's hard to tell
the ENOMEMs apart, which sucks because we're supposed to handle them
differently.
- Some error messages aren't printing the offending function arguments,
which will make them hard to debug when they happen.
I'm anticipating another redesign of the framework, but I'm also trying
very hard not to do any new major rewrites before the next release.
#include <openssl/err.h>
#include <openssl/evp.h>
#include "alloc.h"
-#include "log.h"
-
-/**
- * Converts error from libcrypto representation to this project's
- * representation.
- */
-static int
-error_ul2i(unsigned long error)
-{
- /* I'm assuming int has at least 32 bits. Don't mess with the sign. */
- int interror = error & 0x7FFFFFFFul;
- return interror ? interror : -EINVAL;
-}
/*
* Reference: openbsd/src/usr.bin/openssl/enc.c
*
* @in: The BIO that will stream the base64 encoded string you want to decode.
* @out: Buffer where this function will write the decoded string.
- * @has_nl: Indicate if the encoded string has newline char.
+ * @has_nl: Encoded string has newline char?
* @out_len: Total allocated size of @out. It's supposed to be the result of
* EVP_DECODE_LENGTH(<size of the encoded string>).
* @out_written: This function will write the actual number of decoded bytes
* here.
*
- * Returns error status. (Nonzero = error code, zero = success)
+ * Returns true on success, false on failure. If the caller wants to print
+ * errors, do it with the crypto functions. If not, remember to clean
+ * libcrypto's error queue somehow.
*
- * If this returns error, do visit ERR_print_errors(), but also print an
- * additional error message anyway. Functions such as BIO_new() don't always
- * register a libcrypto stack error.
+ * TODO (fine) Callers always do a bunch of boilerplate; refactor.
*/
-int
+bool
base64_decode(BIO *in, unsigned char *out, bool has_nl, size_t out_len,
size_t *out_written)
{
BIO *b64;
size_t offset = 0;
int written = 0;
- unsigned long error;
+ bool success = false;
/*
* BTW: The libcrypto API is perplexing.
* populate the error stack.
*/
b64 = BIO_new(BIO_f_base64());
- if (b64 == NULL) {
- error = ERR_peek_last_error();
- if (error)
- return error_ul2i(error);
- enomem_panic();
- }
+ if (b64 == NULL)
+ return false;
/*
* BIO_push() can technically fail through BIO_ctrl(), but it ignores
* the error. This will not cause it to revert the push, so we have to
* do it ourselves.
+ *
+ * BTW: I'm assigning the result of BIO_push() to @in (instead of @b64
+ * or, more logically, throwing it away) because the sample reference in
+ * enc.c does it that way.
+ * But the writer of enc.c probably overcomplicated things.
+ * It shouldn't make a difference. We don't need @in anymore; just
+ * assume both @b64 and @in now point to the same BIO, which is @b64.
+ */
+ in = BIO_push(b64, in);
+
+ /*
* Should we ignore this error? BIO_ctrl(BIO_CTRL_PUSH) performs some
* "internal, used to signify change" thing, whose importance is
* undefined due to BIO_ctrl()'s callback spaghetti.
- * I'm not risking it.
+ * Let's be strict, I guess.
*/
- in = BIO_push(b64, in);
- if (!has_nl)
- BIO_set_flags(in, BIO_FLAGS_BASE64_NO_NL);
-
- error = ERR_peek_last_error();
- if (error)
+ if (ERR_peek_last_error() != 0)
goto end;
+ if (!has_nl)
+ BIO_set_flags(in, BIO_FLAGS_BASE64_NO_NL); /* Cannot fail */
+
do {
/*
* Do not move this after BIO_read().
* imply error, and which ruins the counter.
*/
offset += written;
- /*
- * According to the documentation, the first argument should
- * be b64, not in.
- * But this is how it's written in enc.c.
- * It doesn't seem to make a difference either way.
- */
written = BIO_read(in, out + offset, out_len - offset);
} while (written > 0);
/* BIO_read() can fail. It does not return status. */
- error = ERR_peek_last_error();
+ if (ERR_peek_last_error() != 0)
+ goto end;
+
*out_written = offset;
+ success = true;
end:
/*
/* Returns 0 on failure, but that's only if b64 is NULL. Meaningless. */
BIO_free(b64);
- return error ? error_ul2i(error) : 0;
+ return success;
}
/*
* Return 0 on success, or the error code if something went wrong. Don't forget
* to free @result after a successful decoding.
*/
-int
+bool
base64url_decode(char const *str_encoded, unsigned char **result,
size_t *result_len)
{
BIO *encoded; /* base64 encoded. */
char *str_copy;
size_t encoded_len, alloc_size, dec_len;
- int error, pad, i;
+ int pad, i;
/*
* Apparently there isn't a base64url decoder, and there isn't
/* Now decode as regular base64 */
encoded = BIO_new_mem_buf(str_copy, -1);
- if (encoded == NULL) {
- error = -EINVAL;
+ if (encoded == NULL)
goto free_copy;
- }
alloc_size = EVP_DECODE_LENGTH(strlen(str_copy));
*result = pzalloc(alloc_size + 1);
- error = base64_decode(encoded, *result, false, alloc_size, &dec_len);
- if (error)
+ if (!base64_decode(encoded, *result, false, alloc_size, &dec_len))
goto free_all;
- if (dec_len == 0) {
- error = -EINVAL;
+ if (dec_len == 0)
goto free_all;
- }
+
*result_len = dec_len;
free(str_copy);
BIO_free(encoded);
- return 0;
+ return true;
+
free_all:
free(*result);
BIO_free(encoded);
free_copy:
free(str_copy);
- return error;
+ return false;
}
static char *
/*
* Encode @in (with size @in_len) as base64url without trailing pad, and
* allocate at @result.
+ *
+ * TODO (SLURM, RK) From the way this function keeps being called in pairs and
+ * failing too late, it would appear the code should be caching the encoded
+ * result during construction.
*/
-int
+bool
base64url_encode(unsigned char const *in, int in_len, char **result)
{
BIO *b64, *mem;
BUF_MEM *mem_buf;
- int error;
ERR_clear_error();
mem = BIO_new(BIO_s_mem());
- if (mem == NULL) {
- error = ERR_peek_last_error();
- goto peeked;
- }
+ if (mem == NULL)
+ return false;
b64 = BIO_new(BIO_f_base64());
if (b64 == NULL) {
- error = ERR_peek_last_error();
- goto free_mem;
+ BIO_free(mem);
+ return false;
}
+
+ /*
+ * TODO (SLURM, RK) WHY IS THERE NO ERROR HANDLING HERE
+ * ARGGGGGGGGGGGGGGGGGGGGHHHHHHHHHHHHHHHHHHHHH
+ */
mem = BIO_push(b64, mem);
BIO_set_flags(b64, BIO_FLAGS_BASE64_NO_NL);
-
BIO_write(b64, in, in_len);
BIO_flush(b64);
BIO_get_mem_ptr(mem, &mem_buf);
*result = to_base64url(mem_buf->data, mem_buf->length);
BIO_free_all(b64);
- return 0;
-
-free_mem:
- BIO_free_all(b64);
-peeked:
- if (error)
- return error_ul2i(error);
- enomem_panic();
+ return true;
}
#include <sys/types.h>
#include <unistd.h>
-int base64_decode(BIO *, unsigned char *, bool, size_t, size_t *);
-int base64url_decode(char const *, unsigned char **, size_t *);
+bool base64_decode(BIO *, unsigned char *, bool, size_t, size_t *);
+bool base64url_decode(char const *, unsigned char **, size_t *);
-int base64url_encode(unsigned char const *, int, char **);
+bool base64url_encode(unsigned char const *, int, char **);
#endif /* SRC_BASE64_H_ */
/* Get the working dir, the daemon will use (and free) it later */
pwd = getcwd(NULL, 0);
- if (pwd == NULL)
- enomem_panic();
+ if (pwd == NULL) {
+ error = errno;
+ if (error == ENOMEM)
+ enomem_panic();
+ pr_op_err("Cannot get current directory: %s", strerror(error));
+ return error;
+ }
pid = fork();
if (pid < 0) {
}
}
-static void
+static int
http_easy_init(struct http_handler *handler, curl_off_t ims)
{
CURL *result;
result = curl_easy_init();
if (result == NULL)
- enomem_panic();
+ return pr_val_err(
+ "curl_easy_init() returned NULL; no error message given."
+ );
setopt_str(result, CURLOPT_USERAGENT, config_get_http_user_agent());
}
handler->curl = result;
+ return 0;
}
static void
long http_code;
int error;
- http_easy_init(&handler, ims);
+ error = http_easy_init(&handler, ims);
+ if (error)
+ return error;
handler.errbuf[0] = 0;
setopt_str(handler.curl, CURLOPT_URL, src);
return crypto_err(&val_config, pr_val_err);
}
-/* FIXME open call hierarchy */
__dead void
enomem_panic(void)
{
*/
tm = ASN1_TIME_adj(NULL, t, 0, 60);
if (tm == NULL)
- return pr_val_err("Crypto function ASN1_TIME_adj() returned error");
+ return val_crypto_err("ASN1_TIME_adj() returned NULL.");
clone = X509_CRL_dup(original_crl);
if (clone == NULL) {
ASN1_STRING_free(tm);
- enomem_panic();
+ return val_crypto_err("X509_CRL_dup() returned NULL.");
}
X509_CRL_set1_nextUpdate(clone, tm);
int error;
*pp = rpp_create();
- if (*pp == NULL)
- enomem_panic();
tal = tal_get_file_name(validation_tal(state_retrieve()));
{
BIO *encoded; /* base64 encoded. */
char *tmp;
- size_t alloc_size;
+ size_t size;
int error;
- alloc_size = get_spki_alloc_size(lfile);
- tal->spki = pmalloc(alloc_size);
+ size = get_spki_alloc_size(lfile);
+ tal->spki = pmalloc(size);
tmp = NULL;
error = base64_sanitize(lfile, &tmp);
- if (error) {
- free(tal->spki);
- return error;
- }
+ if (error)
+ goto revert_spki;
encoded = BIO_new_mem_buf(tmp, -1);
if (encoded == NULL) {
- free(tal->spki);
- free(tmp);
- return op_crypto_err("BIO_new_mem_buf() returned NULL");
+ error = op_crypto_err("BIO_new_mem_buf() returned NULL.");
+ goto revert_tmp;
}
- error = base64_decode(encoded, tal->spki, true, alloc_size,
- &tal->spki_len);
- if (error)
- free(tal->spki);
+ if (!base64_decode(encoded, tal->spki, true, size, &tal->spki_len)) {
+ error = op_crypto_err("Cannot decode SPKI.");
+ goto revert_encoded;
+ }
free(tmp);
BIO_free(encoded);
+ return 0;
+
+revert_encoded:
+ BIO_free(encoded);
+revert_tmp:
+ free(tmp);
+revert_spki:
+ free(tal->spki);
return error;
}
{
FILE *out = arg;
char *buf1, *buf2;
- int error;
- error = base64url_encode(key->ski, RK_SKI_LEN, &buf1);
- if (error)
- return error;
+ if (!base64url_encode(key->ski, RK_SKI_LEN, &buf1)) {
+ op_crypto_err("Cannot encode SKI.");
+ return 0; /* Skip it, I guess */
+ }
- error = base64url_encode(key->spk, RK_SPKI_LEN, &buf2);
- if (error)
- goto free1;
+ if (!base64url_encode(key->spk, RK_SPKI_LEN, &buf2)) {
+ op_crypto_err("Cannot encode SPK.");
+ free(buf1);
+ return 0; /* Skip it, I guess */
+ }
fprintf(out, "AS%u,%s,%s\n", key->as, buf1, buf2);
free(buf2);
-free1:
free(buf1);
- return error;
+ return 0;
}
/* Print as base64url strings without trailing pad */
JSON_OUT *json_out = arg;
FILE *out;
char *buf1, *buf2;
- int error;
- error = base64url_encode(key->ski, RK_SKI_LEN, &buf1);
- if (error)
- return error;
+ if (!base64url_encode(key->ski, RK_SKI_LEN, &buf1)) {
+ op_crypto_err("Cannot encode SKI.");
+ return 0; /* Skip it, I guess */
+ }
- error = base64url_encode(key->spk, RK_SPKI_LEN, &buf2);
- if (error)
- goto free1;
+ if (!base64url_encode(key->spk, RK_SPKI_LEN, &buf2)) {
+ op_crypto_err("Cannot encode SPK.");
+ free(buf1);
+ return 0; /* Skip it, I guess */
+ }
out = json_out->file;
if (!json_out->first)
buf2);
free(buf2);
-free1:
free(buf1);
json_out->first = false;
- return error;
+ return 0;
}
static int
}
}
- if (resources->ip4s == NULL) {
+ if (resources->ip4s == NULL)
resources->ip4s = res4_create();
- if (resources->ip4s == NULL)
- enomem_panic();
- }
error = res4_add_prefix(resources->ip4s, &prefix);
if (error) {
}
}
- if (resources->ip6s == NULL) {
+ if (resources->ip6s == NULL)
resources->ip6s = res6_create();
- if (resources->ip6s == NULL)
- enomem_panic();
- }
error = res6_add_prefix(resources->ip6s, &prefix);
if (error) {
}
}
- if (resources->ip4s == NULL) {
+ if (resources->ip4s == NULL)
resources->ip4s = res4_create();
- if (resources->ip4s == NULL)
- enomem_panic();
- }
error = res4_add_range(resources->ip4s, &range);
if (error) {
}
}
- if (resources->ip6s == NULL) {
+ if (resources->ip6s == NULL)
resources->ip6s = res6_create();
- if (resources->ip6s == NULL)
- enomem_panic();
- }
error = res6_add_range(resources->ip6s, &range);
if (error) {
}
}
- if (resources->asns == NULL) {
+ if (resources->asns == NULL)
resources->asns = rasn_create();
- if (resources->asns == NULL)
- enomem_panic();
- }
error = rasn_add(resources->asns, asns);
if (error){
alloc_size = EVP_DECODE_LENGTH(strlen(content));
result = pmalloc(alloc_size);
- error = base64_decode(encoded, result, true, alloc_size, &result_len);
- if (error)
+ if (!base64_decode(encoded, result, true, alloc_size, &result_len)) {
+ error = val_crypto_err("Cannot decode publish tag's base64.");
goto release_result;
+ }
free(sanitized);
BIO_free(encoded);
(*out) = result;
(*out_len) = result_len;
return 0;
+
release_result:
free(result);
BIO_free(encoded);
deltas[i].serial.str, max->str);
if (!BN_sub(target_slot, deltas[i].serial.num, min))
- enomem_panic();
+ return val_crypto_err("BN_sub() returned error.");
_target_slot = BN_get_word(target_slot);
if (i == _target_slot)
return 0;
max_serial = ¬if->session.serial;
min_serial = BN_dup(max_serial->num);
if (min_serial == NULL)
- enomem_panic();
+ return val_crypto_err("BN_dup() returned NULL.");
if (!BN_sub_word(min_serial, deltas->len - 1)) {
error = pr_val_err("Could not subtract %s - %zu; unknown cause.",
notif->session.serial.str, deltas->len - 1);
goto stop;
case ENOMEM:
enomem_panic();
- /* Fall through */
case EAGAIN:
goto retry;
case EFAULT:
{
char *pad = " ";
char *buf;
- int error;
pr_op_info(" {");
if (bgpsec->data_flag & SLURM_COM_FLAG_ASN)
if (bgpsec->data_flag & SLURM_BGPS_FLAG_SKI) {
do {
- error = base64url_encode(bgpsec->ski, RK_SKI_LEN, &buf);
- if (error) {
+ if (!base64url_encode(bgpsec->ski, RK_SKI_LEN, &buf)) {
+ op_crypto_err("Cannot encode SKI.");
pr_op_info("%s SKI: <error encoding value>",
pad);
break;
if (bgpsec->data_flag & SLURM_BGPS_FLAG_ROUTER_KEY) {
do {
- error = base64url_encode(bgpsec->router_public_key,
- RK_SPKI_LEN, &buf);
- if (error) {
+ if (!base64url_encode(bgpsec->router_public_key,
+ RK_SPKI_LEN, &buf)) {
+ op_crypto_err("Cannot encode routerPublicKey.");
pr_op_info("%s Router public key: <error encoding value>",
pad);
break;
return error;
if (str_encoded == NULL)
return is_assertion
- ? pr_op_err("SLURM assertion %s is required", SKI)
+ ? pr_op_err("SLURM assertion " SKI " is required")
: 0; /* Optional for filters */
error = validate_base64url_encoded(str_encoded);
if (error)
return error;
- error = base64url_decode(str_encoded, &result->ski, &ski_len);
- if (error)
- return error;
+ if (!base64url_decode(str_encoded, &result->ski, &ski_len))
+ return op_crypto_err("The " SKI " could not be decoded.");
/* Validate that's at least 20 octects long */
if (ski_len != RK_SKI_LEN) {
set_router_pub_key(json_t *object, bool is_assertion,
struct slurm_bgpsec *result, size_t *members_loaded)
{
- char const *str_encoded;
+ char const *encoded;
size_t spk_len;
int error;
- error = json_get_str(object, ROUTER_PUBLIC_KEY, &str_encoded);
+ error = json_get_str(object, ROUTER_PUBLIC_KEY, &encoded);
if (error < 0)
return error;
: 0;
/* Assertions */
- if (str_encoded == NULL)
- return pr_op_err("SLURM assertion %s is required",
- ROUTER_PUBLIC_KEY);
+ if (encoded == NULL)
+ return pr_op_err("SLURM assertion " ROUTER_PUBLIC_KEY " is required.");
- error = validate_base64url_encoded(str_encoded);
+ error = validate_base64url_encoded(encoded);
if (error)
return error;
- error = base64url_decode(str_encoded, &result->router_public_key,
- &spk_len);
- if (error)
- return pr_op_err("'%s' couldn't be decoded", str_encoded);
+ if (!base64url_decode(encoded, &result->router_public_key, &spk_len))
+ return op_crypto_err("The " ROUTER_PUBLIC_KEY " could not be decoded.");
/*
* Validate that "is the full ASN.1 DER encoding of the
bio = BIO_new(BIO_s_mem());
if (bio == NULL)
- enomem_panic();
+ return val_crypto_err("Cannot create a BIO.");
if (BN_print(bio, bn) == 0) {
BIO_free(bio);
notif.deltas = *deltas;
notif.session.serial.num = BN_create();
if (!BN_set_word(notif.session.serial.num, max_serial))
- enomem_panic();
+ ck_abort_msg("BN_set_word() returned zero.");
notif.session.serial.str = (unsigned char *) max_serial_str;
error = sort_deltas(¬if);
while ((cursor = va_arg(vl, unsigned long)) != END) {
delta.serial.num = BN_create();
if (!BN_set_word(delta.serial.num, cursor))
- enomem_panic();
+ ck_abort_msg("BN_set_word() returned zero.");
notification_deltas_add(deltas, &delta);
}
va_end(vl);