Skip to content

Commit

Permalink
Implement oldest-pass
Browse files Browse the repository at this point in the history
* Instead of a single set of validation canons we need them for each ARC
  instance.
* `arc_add_canon()`'s deduplication of header canons was wrong, it
  didn't account for `hdrlist` and the hash includes the signature header
  itself so these are always different.
* Body canons, which are more expensive, can easily be deduplicated
  and weren't.
* `arc_state` was referenced a couple of times when `arc_cstate` was
  meant, in a way that made the checks always false in normal operation.
  The check that wasn't redundant was wrong, though, so two bugs canceled
  each other out.
  • Loading branch information
flowerysong committed Oct 25, 2024
1 parent 9d864f7 commit 8d9f63a
Show file tree
Hide file tree
Showing 8 changed files with 218 additions and 134 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ All notable changes to this project will be documented in this file.
## Unreleased

### Added
- `oldest-pass` processing per [RFC 8617 section 5.2](https://datatracker.ietf.org/doc/html/rfc8617#section-5.2).
- libopenarc - `arc_chain_oldest_pass()`
- milter - `AuthResIP` configuration option.
- milter - `RequireSafeKeys` configuration option.

Expand All @@ -22,6 +24,8 @@ All notable changes to this project will be documented in this file.
are excluded from the AMS, as required by RFC 8617.
- libopenarc - ARC headers are returned with a space before the header value.
- libopenarc - String arguments are marked as `const` where applicable.
- milter - `Authentication-Results` and `ARC-Authentication-Results` include
`header.oldest-pass` when appropriate.

### Fixed
- libopenarc - Seals on failed chains only cover the latest ARC header set,
Expand All @@ -32,6 +36,8 @@ All notable changes to this project will be documented in this file.
- libopenarc - The installed pkg-config file is more correct.
- libopenarc - U-labels (domain labels encoded as UTF-8) are allowed in `d=`
and `s=` tags.
- libopenarc - `arc_eom()` propagates internal errors like memory allocation
failure instead of marking the chain as failed.
- milter - Use after free.
- milter - Unlikely division by zero.
- milter - Small memory leak during config loading.
Expand Down
23 changes: 16 additions & 7 deletions libopenarc/arc-canon.c
Original file line number Diff line number Diff line change
Expand Up @@ -684,12 +684,16 @@ arc_add_canon(ARC_MESSAGE *msg,

assert(hashtype == ARC_HASHTYPE_SHA1 || hashtype == ARC_HASHTYPE_SHA256);

if (type == ARC_CANONTYPE_HEADER)
/* Body canons can be shared if the parameters match. Header canons could
* theoretically be partially shared if the `h` tags match, but it would be
* complex so we don't currently do it. */
if (type == ARC_CANONTYPE_BODY)
{
for (cur = msg->arc_canonhead; cur != NULL; cur = cur->canon_next)
{
if (cur->canon_type != ARC_CANONTYPE_HEADER ||
cur->canon_hashtype != hashtype || cur->canon_length != length)
if (cur->canon_type != ARC_CANONTYPE_BODY ||
cur->canon_canon != canon || cur->canon_hashtype != hashtype ||
cur->canon_length != length)
{
continue;
}
Expand Down Expand Up @@ -1947,6 +1951,7 @@ arc_canon_getsealhash(ARC_MESSAGE *msg, int setnum, void **sh, size_t *shlen)
**
** Parameters:
** msg -- ARC message from which to get completed hashes
** setnum -- which seal's hashes to get
** hh -- pointer to header hash buffer (returned)
** hhlen -- bytes used at hh (returned)
** bh -- pointer to body hash buffer (returned)
Expand All @@ -1958,8 +1963,12 @@ arc_canon_getsealhash(ARC_MESSAGE *msg, int setnum, void **sh, size_t *shlen)
*/

ARC_STAT
arc_canon_gethashes(
ARC_MESSAGE *msg, void **hh, size_t *hhlen, void **bh, size_t *bhlen)
arc_canon_gethashes(ARC_MESSAGE *msg,
int setnum,
void **hh,
size_t *hhlen,
void **bh,
size_t *bhlen)
{
ARC_STAT status;
struct arc_canon *hdc;
Expand All @@ -1969,8 +1978,8 @@ arc_canon_gethashes(
size_t hdlen;
size_t bdlen;

hdc = msg->arc_valid_hdrcanon;
bdc = msg->arc_valid_bodycanon;
hdc = msg->arc_hdrcanons[setnum - 1];
bdc = msg->arc_bodycanons[setnum - 1];

status = arc_canon_getfinal(hdc, &hd, &hdlen);
if (status != ARC_STAT_OK)
Expand Down
2 changes: 1 addition & 1 deletion libopenarc/arc-canon.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ extern void arc_canon_cleanup(ARC_MESSAGE *);
extern ARC_STAT arc_canon_closebody(ARC_MESSAGE *);
extern ARC_STAT arc_canon_getfinal(ARC_CANON *, unsigned char **, size_t *);
extern ARC_STAT arc_canon_gethashes(
ARC_MESSAGE *, void **, size_t *, void **, size_t *);
ARC_MESSAGE *, int, void **, size_t *, void **, size_t *);
extern ARC_STAT arc_canon_getsealhash(ARC_MESSAGE *, int, void **, size_t *);
extern ARC_STAT arc_canon_header_string(
struct arc_dstring *, arc_canon_t, const char *, size_t, bool);
Expand Down
5 changes: 3 additions & 2 deletions libopenarc/arc-types.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ struct arc_msghandle
bool arc_infail;
int arc_dnssec_key;
int arc_signalg;
int arc_oldest_pass;
unsigned int arc_mode;
unsigned int arc_nsets;
unsigned int arc_margin;
Expand Down Expand Up @@ -158,9 +159,9 @@ struct arc_msghandle
struct arc_dstring *arc_hdrbuf;
struct arc_canon *arc_sealcanon;
struct arc_canon **arc_sealcanons;
struct arc_canon *arc_valid_hdrcanon;
struct arc_canon **arc_hdrcanons;
struct arc_canon **arc_bodycanons;
struct arc_canon *arc_sign_hdrcanon;
struct arc_canon *arc_valid_bodycanon;
struct arc_canon *arc_sign_bodycanon;
struct arc_canon *arc_canonhead;
struct arc_canon *arc_canontail;
Expand Down
Loading

0 comments on commit 8d9f63a

Please sign in to comment.