Amos Jeffries <[email protected]> writes:
> The attached patch implements a proposed API/ABI extension adding
> support for RFC 4648 section 5 "Base 64 Encoding with URL and Filename
> Safe Alphabet"
Thanks. It makes sense to me to add support for this in one way or the
other.
> External code simply calls the init() function relevant to the
> alphabet it is needing to encode/decode with. The library internally
> uses the context to select which lookup table to use for later base64
> function calls.
Passing the alphabet to the init function and recording it in the context
is nice in some ways, but it breaks the ABI (since the size of the
context struct is part of the ABI). Now, maybe we have to break the ABI
for 3.1 anyway, to introduce versioned symbols. I don't yet know for
sure if an soname change is necessary or not.
The alternative to storing this in the context, is to introduce some
general encoding/decoding functions which support arbitrary alphabets and
take the needed table as an additional argument. Then base64_* and
base64url_* would be simple wrapper functions passing the right table.
> The base64_encode_raw() and base64_encode_group() functions which do
> not use contexts are left untouched for now.
Both functions are undocumented, so we can consider changing them if
neeed. As for encode_group, that seems to be used only in the openpgp code
which is not really in a working state. So we can probably ignore that one.
But base64_encode_raw is used by the main encoding function,
base64_encode_update. So to support other alphabets one either needs to
pass some kind of alphabet argument to base64_encode_raw, or introduce a
new function with such an argument.
Does anyone know if applications are using base64_encode_raw, despite
it's status as undocumented?
> +/* which alphabet to use */
> +#define BASE64_ALPHABET 0
> +#define BASE64URL_ALPHABET 1
I think I'd prefer to avoid an enumeration of available alphabets. And
in the API you sketch, applications will never use these constants,
right?
> /* Base64 encoding */
>
> /* Maximum length of output for base64_encode_update. NOTE: Doesn't
> @@ -73,11 +79,17 @@ struct base64_encode_ctx
> {
> unsigned word; /* Leftover bits */
> unsigned bits; /* Number of bits, always 0, 2, or 4. */
> + unsigned alphabet; /* which alphabet to use for encoding */
> };
Assuming we go with adding stuff to the context struct, I'd prefer to
add a pointer directly to the alphabet (and for decoding, a pointer
directly to the lookup table). So we don't need any switch on an
alphabet enum.
> diff --git a/base64-decode.c b/base64-decode.c
> index f622baa..fbaf54f 100644
> --- a/base64-decode.c
> +++ b/base64-decode.c
> @@ -43,7 +43,7 @@
> #define TABLE_END -3
>
> static const signed char
> -decode_table[0x100] =
> +default_decode_table[0x100] =
> {
> /* White space is HT, VT, FF, CR, LF and SPC */
> -1, -1, -1, -1, -1, -1, -1, -1, -1, -2, -2, -2, -2, -2, -1, -1,
> @@ -64,10 +64,40 @@ decode_table[0x100] =
> -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> };
>
> +static const signed char
> +urlextended_decode_table[0x100] =
> +{
> + /* White space is HT, VT, FF, CR, LF and SPC */
> + -1, -1, -1, -1, -1, -1, -1, -1, -1, -2, -2, -2, -2, -2, -1, -1,
> + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> + -2, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 62, -1, -1,
> + 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, -1, -1, -1, -3, -1, -1,
> + -1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14,
> + 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, -1, -1, -1, -1, 63,
> + -1, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40,
> + 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, -1, -1, -1, -1, -1,
> + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> +};
> +
I think the base64url decoding functions, and its table, should be put
in its own object file. Maybe static linking is obscure these days, but
it's desirable that if one uses only the standard base64 alphabet, one
shouldn't get dependencies causing the linker to drag in tables for
other alphabets.
> void
> base64_decode_init(struct base64_decode_ctx *ctx)
> {
> ctx->word = ctx->bits = ctx->padding = 0;
> + ctx->alphabet = BASE64_ALPHABET;
> +}
> +
> +void
> +base64url_decode_init(struct base64_decode_ctx *ctx)
> +{
> + ctx->word = ctx->bits = ctx->padding = 0;
> + ctx->alphabet = BASE64URL_ALPHABET;
> }
And then the assignment of ctx->alphabet would be
ctx->table = base64_decode_table;
and
ctx->table = base64url_decode_table;
in the respective init functions.
> int
> @@ -76,8 +106,11 @@ base64_decode_single(struct base64_decode_ctx *ctx,
> uint8_t src)
> {
> int data;
> -
> - data = decode_table[src];
> +
> + if (ctx->alphabet == BASE64URL_ALPHABET)
> + data = urlextended_decode_table[src];
> + else
> + data = default_decode_table[src];
And this piece of code would then simplify to
data = ctx->table[src];
> @@ -155,6 +171,7 @@ base64_encode_single(struct base64_encode_ctx *ctx,
> unsigned done = 0;
> unsigned word = ctx->word << 8 | src;
> unsigned bits = ctx->bits + 8;
> + const uint8_t *encode_table = (ctx->alphabet == BASE64URL_ALPHABET ?
> urlextended_encode_table : default_encode_table);
>
> while (bits >= 6)
> {
I think this works, but only because the ENCODE macro which refers to
the name "encode_table" now gets a local variable, instead of the global
table in the old code. At a casual look, the local variable appears
unused. I think it's easier to understand if this dependency where made
more explicit.
Finally, are there other alphabets that are in use? You have
mentioned, earlier, the alphabet used for crypt(3) (where is that
documented???), and iirc, SRP files also use some unusual alphabet for
base64.
Maybe we should make it reasonably easy to let applications supply
custom alphabets? This should be almost trivial for encoding, while for
decoding, we would either need to document the format of the lookup
table, or provide a function to generate a decoding table for a given
alphabet.
Best regards,
/Niels
--
Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26.
Internet email is subject to wholesale government surveillance.
_______________________________________________
nettle-bugs mailing list
[email protected]
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
