cloudflare / cloudflare/boringtun
Library C API design issues
- Dominant language
- Rust
- Stars
- 7.2k
- Forks
- 532
- PR merge metrics
- No merged PRs in 30d
Description
Hello. I am writing [python ctypes-based bindings](https://github.com/KOLANICH-libs/BoringTUN.py) to BoringTUN and I don't understand well the C API it provides and why it was designed that way. I may badly misunderstand what BoringTUN is about, but it seems that the library C API has issues and needs a redesign.
1. `new_tunnel` takes the keys in the form of base64, while `x25519_key` type exists. IMHO it should take the keys in the form of pointers to `x25519_key`. This would allow avoid converting to and from base64 multiple times.
2. `x25519_key_to_base64`, `x25519_key_to_hex` and `x25519_key_to_str_free` are IMHO out of scope of this lib. Base64 is the well-known encoding and it has nothing to do with following the crypto protocol.
3. Why do we need `check_base64_encoded_x25519_key` ? If it only checks if the string prrovided is valid base64 and if it matches the length of a keys, I guess we don't need it at all. If it does any other checks, why not to do it on already parsed key?
4. It is not very clear what is expected in `const uint8_t *src`. Do I guess right that an IP packet is expected? Can arbitrary payloads be used?
5. It is not very clear what is returned into `uint8_t *dst`. Do I guess right that it is an UDP packet?
6. What is the size `uint32_t dst_size`? `/// Size of dst should be at least src.len() + 32, and no less than 148 bytes.` Why don't we have a function computing the needed size from `uint32_t src_size`?
7. This API looks like an implementation of only the Noise protocol. If so, why not to put it into a separate shared library? The size of the shared library is already large, if we cannot use some part of it, we probably should not have that part on our machine.
8. BTW there should be better error reporting. I should be able to know what exactly is wrong.
9. If I understand right, this library also implements parsing of IP packets and processing of them depending on the config. But parsing IP packets is not cool, often the software have packets fields like IP addless in preparsed form, because most of userspace progrems rely on kernel's TCP/IP & UDP stacks. Should also the API be provided where payloads of packets and IP addresses are the arguments?
It is also proposed to drop support of the old API and in the lib and instead have an additional lib translating the old API into the new one.
Contributor guide
Assessment
This issue has not been assessed yet.