Fix base64 decoding on platforms where char is unsigned

[eastagiletracker]

This PR proposes fixing base64 decoding on platforms where char is unsigned, which corrupts DNSKEY, CDNSKEY, RRSIG and OPENPGPKEY rdata on AArch64 (fixes #70). We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/387. You can sign in with your GitHub ID to claim ownership of the project.

The defect

base64_decode_value() in libmy/b64_decode.c keeps its decoding table in a plain char array and signals "not a base64 alphabet octet" by returning a negative sentinel (-1, and -2 for =). On AArch64, ppc64le and s390x, where char is unsigned by default, those sentinels come back as 255 and 254. The four while (fragment < 0) loops in base64_decode_block() therefore never skip anything: padding and the whitespace that separates presentation-format key chunks are folded into the output as if they were real base64 digits. wdns_str_to_rdata() (wdns/str_to_rdata_ubuf.c:93) is the direct caller, so every base64-encoded rdata type comes back longer and silently wrong — the len 140 != 134 and trailing > byte in the report.

While confirming that, I also found a one-past-the-end read in the same bounds check: value_in > decoding_size admits value_in == 80 on an 80-element table. The character { hits it, and on this machine it read back 65, so { decodes as a valid base64 value instead of being rejected. That one is not platform-dependent.

The change

decoding[] becomes signed char, the table offset is computed in an int from (unsigned char) value_in, fragment becomes an int, and the bounds check uses >=. Behavior on signed-char platforms is unchanged: for octets >= 0x80 the old code went negative before the comparison and returned -1, and the new code produces an offset >= 80 and returns -1 as well.

Reproduction at current HEAD

AArch64's ABI is unsigned char, so -funsigned-char reproduces it on any host. On master (c308fe8):

$ ./autogen.sh && ./configure CFLAGS="-g -O2 -funsigned-char" && make && make check
FAIL: t/test-str_to_rdata
FAIL: t/test-rdata
# TOTAL: 7
# PASS:  5
# FAIL:  2

That is the same 5-pass/2-fail split as the report, with the same failing cases:

FAIL 1: input="256 3 5 AQPSKmynfzW4kyBv015MUG2DeIQ3 Cbl+BBZH4b/0PY1kxkmvHjcZc8no ..." IN DNSKEY len 140 != 134
FAIL 5: input="AQIDBAUGBwg=" IN OPENPGPKEY len 9 != 8 res=success value="\x01\x02\x03\x04\x05\x06\x07\x08>" != "\x01\x02\x03\x04\x05\x06\x07\x08"

Verification

A new t/test-b64_decode covers the sentinels, = padding and embedded whitespace, following the pattern t/test-fast_inet_ntop uses for libmy code. It is red on the unpatched tree in both configurations — 11 failures under -funsigned-char, and 1 failure in a default build from the { out-of-bounds read:

FAIL 8: base64_decode_value(0x3d) = 254 != -2
FAIL 14: base64_decode_value(0x7b) = 65 != -1
FAIL 3: base64_decode_block("ZGVh ZGJl ZWY=") len 10 != 8 value="dea\xfd\x91\x89\x97\xf6Vc"

With the patch applied, both configurations are fully green and nothing that passed before regressed:

default CFLAGS   : # TOTAL: 8  # PASS: 8  # FAIL: 0   (baseline before the patch: 7/7)
-funsigned-char  : # TOTAL: 8  # PASS: 8  # FAIL: 0   (baseline before the patch: 5/7)

The real consumer path is covered by your own suite: t/test-str_to_rdata and t/test-rdata round-trip DNSKEY, CDNSKEY, RRSIG and OPENPGPKEY through wdns_str_to_rdata(), and both go from failing to passing under -funsigned-char.

No public signature, dependency or build setting changed. I left ChangeLog alone to avoid colliding with #78.

How this was managed

We imported your issues and pull requests into a live agile board — 78 stories and 1 label — and worked this change on the story for issue #70: https://eastagiletracker.com/projects/387/stories/260343. The board itself is at https://eastagiletracker.com/projects/387.

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.

---

Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com