Add getters for RFC 3779 extensions to FFI

[arckoor]

Adds some getters for the RFC 3779 extensions to the FFI. The FFI structs are not declared with BOTAN_FFI_DECLARE_STRUCT to allow them to contain an extra m_writable bool. I plan to add methods to build these extensions from scratch once e.g. #4996 is finalized and the FFI can create certificates from scratch, and I want to disallow writing to extension objects that have been obtained from an existing certificate (and can thus not be changed without creating a new certificate). This does prohibit copying the extension from an old cert to a new cert while also adjusting it, but I think this is better solved with a _duplicate method instead of allowing writes to all extension objects.

[arckoor]

@reneme if possible I'd like to redeem your offer to assist with something like this :p
I'll write the rst doc with some more general explanations of what the expected call flow is when the code itself is finalized

[Copilot]

Pull request overview

This PR extends Botan’s FFI and Python bindings to expose RFC 3779 X.509 certificate extensions (IP Address Blocks and AS Blocks), along with a Python regression test that exercises the new getters.

Changes:

• Add new FFI handle types and getter functions for RFC 3779 IP Address Blocks and AS Blocks extensions.
• Add Python wrapper classes/methods on X509Cert to access these extensions.
• Add Python tests validating behavior for missing extensions and expected parsed values.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
src/lib/ffi/ffi.h	Declares new public C FFI APIs for RFC 3779 extension access.
src/lib/ffi/ffi_cert_ext.h	Adds internal FFI struct wrappers for the new extension handle types.
src/lib/ffi/ffi_cert_ext.cpp	Implements creation/destruction and getters for IP/AS blocks extensions.
src/lib/ffi/info.txt	Registers the new internal header in the FFI module’s header list.
src/python/botan3.py	Binds new FFI functions and adds Python-facing extension accessors.
src/scripts/test_python.py	Adds Python tests covering new extension getters and error cases.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coveralls]

coverage: 89.547% (+0.02%) from 89.529%
when pulling 2b3e572 on arckoor:x509-ffi-ext
into 507bbbf on randombit:master.

[arckoor]

@reneme could you take a look?

[randombit]

Sorry I did not get to the review here in time.

I'm not so confident of the overall shape of this. Having the getter being in term of the extension rather than the certificate is quite unlike the rest of the interface. It would make sense to have the extension as a standalone type when we reach the point of actually being able to create certificates. But even then - there isn't that much value (that I can see) in being able to read the values out of an extension just created, you would just want some setters for that. I think it's better if these were all just functions on botan_x509_cert_t

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

[randombit]

Thanks looks good - I think my main concern at this point is the implementation doesn't write to the output parameters in certain error cases, which is fine, but the doc comments should call this out specifically that the values are not written at all if an error occurs. I left notes on some but there are probably others eg min/max

[arckoor]

I added a disclaimer for each function that in case of an error "<everything that is usually modified> is not modified". For botan_x509_ext_ip_addr_blocks_get_family that is technically a lie, since if you pass the wrong family, e.g. ipv6 = 1 when accessing a V4 family, has_safi and safi get set, after which it errors when trying to access the family. Not sure if that is a huge issue though.

[randombit]

Some more comments. Mostly just quesions on the docs, plus one API change request which I think should be pretty easy to handle.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/lib/ffi/ffi_cert_ext.cpp:162

• Same local-index issue as ..._get_family(): for ipv6==0, an i that actually refers to an IPv6 family can slip past the index >= addr_blocks.size() check and then yields BOTAN_FFI_ERROR_BAD_PARAMETER from the type-checking helper, rather than a clean BOTAN_FFI_ERROR_OUT_OF_RANGE.

      const size_t index = ipv6 == 0 ? i : ext->v4_count() + i;
      const auto& addr_blocks = ext->addr_blocks();
      if(index >= addr_blocks.size()) {
         return BOTAN_FFI_ERROR_OUT_OF_RANGE;
      }
      const auto& addr_choice = addr_blocks.at(index).addr_choice();

[arckoor]

I suppose instead of checking for index >= addr_blocks.size() this could check like so:

size_t limit = ipv6 == 0 ? ext->v4_count() : ext->v6_count();
if (i >= limit) {
    return BOTAN_FFI_ERROR_OUT_OF_RANGE;
}

IMO checking for >= addr_blocks.size() is more easily read as what it actually does, making sure no out of bounds index happens. Checking for the counts directly seems kind of arbitrary, but I don't mind either

[randombit]

I'd be inclined for checking the index against the v4/v6 count directly (your suggested version is better than copilots), esp as it prevents a (unlikely, definitely application bug) footgun where v4_count + i overflows. The at(index) makes it clear that worst case it's still a checked index.

[arckoor]

Second commit only for readability, will squash before merge.
I ended up tracking the counts in the extension itself, might be interesting for regular C++ consumers as well, and this way it doesn't duplicate the logic as much (also avoids counting based on afi alone, and instead is type-backed).
The one unresolved copilot comment is worth of consideration, if you prefer that way of doing it :)

The CI analysis failure is unrelated I think, ruff seems very unhappy all of a sudden, probably got a version bump

[randombit]

LGTM! Just needs a squash and then can merge