Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TXT record encryption AES key length check is broken #4975

Open
meyfa opened this issue Dec 25, 2024 · 1 comment · May be fixed by #4980
Open

TXT record encryption AES key length check is broken #4975

meyfa opened this issue Dec 25, 2024 · 1 comment · May be fixed by #4980
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@meyfa
Copy link

meyfa commented Dec 25, 2024

This was previously reported in #3992 but is still relevant (can still be reproduced).

What happened:

external-dns pod was crashing because of "the AES Encryption key must have a length of 32 bytes" after following the documentation, using a url-base64 encoding for the key.

Instead, using a 32 character-long string "works", meaning that the key is successfully taken by external-dns but it then fails with other errors related to old TXT records not being encrypted.

Moreover, 32 characters are not 32 bytes of entropy, because people will likely use only alphanumerical characters. As such, it should be discouraged to just update the documentation to use 32 characters.

What you expected to happen:

external-dns should pick the key, decode it using base64 and use it to encrypt/decrypt TXT records

How to reproduce it (as minimally and precisely as possible):

Use the following arguments, which should be valid according to the documentation but actually prevent external-dns from starting:

--txt-encrypt-enabled
--txt-encrypt-aes-key=ZPitL0NGVQBZbTD6DwXJzD8RiStSazzYXQsdUowLURY=

On the other hand, this configuration works:

--txt-encrypt-enabled
--txt-encrypt-aes-key=01234567890123456789012345678901

Anything else we need to know?:

Environment:

  • External-DNS version (use external-dns --version): v0.15.1
  • DNS provider: Cloudflare

cc @bennesp @lnhrdt from the previous issue

@meyfa meyfa added the kind/bug Categorizes issue or PR as related to a bug. label Dec 25, 2024
@ivankatliarchuk
Copy link
Contributor

ivankatliarchuk commented Dec 28, 2024

A string is a read-only, immutable, slice of arbitrary bytes at the lowest level.

By arbitrary, we mean that the bytes can be of any format. When a character value is stored in a string, its byte-at-a-time representation is stored. Go does not know or require that the bytes represent any particular encoding e.g. ASCII, UTF-8, UTF-32 etc- they are just bytes.

So in example that is not working ZPitL0NGVQBZbTD6DwXJzD8RiStSazzYXQsdUowLURY= there are 44 bytes

[90 80 105 116 76 48 78 71 86 81 66 90 98 84 68 54 68 119 88 74 122 68 56 82 105 83 116 83 97 122 122 89 88 81 115 100 85 111 119 76 85 82 89 61]

In second working example 01234567890123456789012345678901 there are only 32 bytes, which is correct. 32 bytes x 8 = 256bits, which is a requirement for AES-256

[48 49 50 51 52 53 54 55 56 57 48 49 50 51 52 53 54 55 56 57 48 49 50 51 52 53 54 55 56 57 48 49]

But agree technically there is a bug. According to documentation the aes key is The 32-byte AES-256-GCM encryption key must be specified in URL-safe base64 form

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants