Skip to content

Conversation

@arckoor
Copy link
Contributor

@arckoor arckoor commented May 20, 2025

Adds bindings for creating X.509 certificates.

@arckoor
Copy link
Contributor Author

arckoor commented May 20, 2025

@randombit @reneme could you take a look and tell me what you think? Still WIP, almost no tests, but the python binding is usable-ish

@coveralls
Copy link

coveralls commented May 20, 2025

Coverage Status

coverage: 90.666% (+0.002%) from 90.664%
when pulling 32b12bb on arckoor:x509-ffi
into 7fe3a73 on randombit:master.

@randombit randombit requested review from Copilot and randombit May 22, 2025 12:14

This comment was marked as outdated.

@arckoor
Copy link
Contributor Author

arckoor commented May 22, 2025

The normal copilot comments I already mentioned myself, funnily enough the low-confidence suppressed ones are valid issues 🤔

@arckoor
Copy link
Contributor Author

arckoor commented Jun 23, 2025

Won't compile, needs #4890

@arckoor arckoor requested a review from Copilot June 23, 2025 16:36

This comment was marked as outdated.

@arckoor arckoor force-pushed the x509-ffi branch 2 times, most recently from d8647ab to 74c900d Compare July 14, 2025 09:28
@arckoor arckoor requested a review from Copilot July 14, 2025 09:52

This comment was marked as outdated.

@arckoor arckoor marked this pull request as ready for review July 14, 2025 14:13
@arckoor
Copy link
Contributor Author

arckoor commented Jul 14, 2025

@randombit I think this is ready to take a look at. The python binding definitely still needs some attention, and I'm not sure how much more you want me to document. But all in all it works.

@randombit
Copy link
Owner

@arckoor At this point I've just looked over the declarations in ffi.h but this already should give you something to go on

Comment on lines 2474 to 2541
def revoke(self, rng, ca_cert, ca_key, issue_time, next_update, revoked, reason, hash_fn=None, padding=None):
crl = X509CRL()
c_revoked = len(revoked) * c_void_p
arr_revoked = c_revoked()
for i, cert in enumerate(revoked):
arr_revoked[i] = cert.handle_()
revoked_len = c_size_t(len(revoked))

_DLL.botan_x509_crl_update(
byref(crl.handle_()),
self.__obj,
rng.handle_(),
ca_cert.handle_(),
ca_key.handle_(),
issue_time,
next_update,
arr_revoked,
revoked_len,
X509CRLReason.to_bits(reason),
_ctype_str(hash_fn),
_ctype_str(padding)
)
return crl
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't love that the python binding has 0 type hints. You just have to know this method does not modify in place, while a def revoke(...) -> X509CRL would make that very clear.
If you're interested, I would be up for finally getting some type hints into the binding.

@arckoor arckoor requested a review from randombit August 11, 2025 15:09
@arckoor arckoor force-pushed the x509-ffi branch 4 times, most recently from faf135d to 32b12bb Compare August 12, 2025 10:17
@arckoor arckoor requested a review from Copilot August 12, 2025 10:18
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds comprehensive Foreign Function Interface (FFI) bindings for X.509 certificate and PKCS#10 request creation. The changes introduce a new CertificateParametersBuilder class that provides a modern, builder-pattern API for constructing X.509 certificates and certificate signing requests, replacing the older X509_Cert_Options approach. Additionally, the PR includes extensive support for RPKI (Resource Public Key Infrastructure) extensions through new FFI bindings for IPAddressBlocks and ASBlocks certificate extensions.

  • Introduces CertificateParametersBuilder class with a fluent builder interface for certificate creation
  • Adds comprehensive FFI bindings for certificate creation, PKCS#10 requests, and CRL management
  • Implements RPKI extension support with IPAddressBlocks and ASBlocks certificate extensions

Reviewed Changes

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

Show a summary per file
File Description
src/tests/test_x509_rpki.cpp Updates test infrastructure to use new CertificateParametersBuilder API instead of legacy X509_Cert_Options
src/tests/test_ffi.cpp Adds comprehensive test coverage for new FFI certificate creation and RPKI extension APIs
src/python/botan3.py Implements Python bindings for new certificate creation APIs and RPKI extensions
src/lib/x509/x509_builder.h/cpp Introduces new CertificateParametersBuilder class with fluent interface for certificate construction
src/lib/x509/x509self.cpp Refactors legacy certificate creation functions to use new builder internally
src/lib/ffi/ffi_cert.cpp Implements FFI bindings for certificate creation, PKCS#10 requests, and CRL operations
src/lib/ffi/ffi_cert_ext.cpp Implements FFI bindings for RPKI certificate extensions (IPAddressBlocks and ASBlocks)

@arckoor
Copy link
Contributor Author

arckoor commented Nov 11, 2025

I'll wait until #5095 is merged, and then I'm probably going to pick this back up in a bunch of smaller PRs (CRLs, more getters for the Certs, and finally creation)
I think right now this thing is just way to big to reasonably review and merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants