Repository navigation
Add an experimental TLS FFI module with a policy handle - #5941
Conversation
cc7f60c to
ce32c42
Compare
reneme
left a comment
There was a problem hiding this comment.
Looks like a good start to me. Thanks. Some inline comments for discussion.
| def __copy__(self): | ||
| raise TypeError('TLSPolicy objects cannot be copied') | ||
|
|
||
| def __deepcopy__(self, _memo): | ||
| raise TypeError('TLSPolicy objects cannot be copied') |
There was a problem hiding this comment.
Technically, they could be copied because, internally, they are just a shared pointer after all. We'd need to provide a botan_tls_policy_dup() FFI binding for that, I guess. That might be useful when trying to juggle multiple TLS handles in the future.
At the moment, I'd suggest to just keep it in mind and revisit it later as needed.
There was a problem hiding this comment.
Agreed, and noted for later. Since the handle is a shared_ptr internally, a botan_tls_policy_dup() would be cheap, and the same would apply to the credentials and session manager handles. I will leave the Python objects non-copyable for now and add duplication when there is a concrete need for it.
|
@randombit Would you like to take a look at this one as well, or is reneme's approval enough to merge it? The next PR (the credentials and session manager handles) is ready locally (well, almost) and is stacked on this branch. I would open it once this one is merged, or earlier as a draft if you prefer to see it now. |
|
Looks good but re shared_ptr let's fix this now - see #5959 So I'd suggest merging 5959 first, rebase this to use the new native shared_ptr support, then continue |
|
Sounds good. I will rebase onto #5959 once it is merged and switch the policy handle to the new shared_ptr struct. The follow-up PRs will use it from the start. |
|
5959 merged now |
c9d669e to
840328a
Compare
|
@moritzschmitt lgtm but please squash the commits |
840328a to
6e0d27e
Compare
|
Squashed into one commit; the tree is unchanged from the version CI ran on. |
|
The two failures are the windows-11-arm MSVC runners, and I have no idea what's going on, because both jobs passed on the identical tree a couple of hours earlier. The only thing I've noticed is that MSVC 19.51 was picked up in the meantime (before, it was 19.44). I just love software engineering. |
6e0d27e to
048e694
Compare
First step towards a C API for TLS (GH randombit#2492): the ffi_tls module skeleton with lifecycle Experimental, runtime version functions in ffi.h, the botan_tls_policy_t handle, tests, Python binding and docs.
048e694 to
8741d76
Compare
|
@randombit Squashed and rebased onto current master; CI is green now. The earlier arm64 failures were the runner-image change (#5960 and #5962 took care of them). Ready from my side. |
This is the first, deliberately small step towards a C API for TLS (#2492), following the outline discussed there. It settles the conventions that the later parts (credentials, session managers, channels) will build on, without any of them yet.
ffi_tls(src/lib/ffi/ffi_tls) with its own public headerbotan/ffi_tls.hand version stampFFI_TLS, laid out likejack/ffi-tls. The module haslifecycle -> "Experimental"as agreed on the issue, so it is only built with--enable-modules=ffi_tlsor--enable-experimental-features(the CI script passes the latter on every target, so CI covers it) and its functions may still change. As far as I can tell it is the first module using that lifecycle value.botan_ffi_tls_api_version()andbotan_ffi_tls_supports_api()inffi.h, as injack/ffi-tls, so applications can detect the module at runtime; they return 0 resp. -1 when it is not built. The existing FFI version stamp is not bumped (per the remark on Expose SAN/IAN otherName entries via the FFI GeneralName API #5903 that this is better left to release time).botan_tls_policy_t, an opaque handle wrapping ashared_ptr<const TLS::Policy>(via a small wrapper struct asffi_tpm2.cppdoes), so that channels can later co-own the policy and the handle may be destroyed right after use. Four functions:botan_tls_policy_init(policy, name)with the stock policies"default","strict"and"bsi_tr_02102_2"(NULL selects the default, an unknown name returnsBAD_PARAMETER; Suite B left out as suggested),botan_tls_policy_init_from_text()forText_Policy(malformed text returnsINVALID_INPUT; values are checked byText_Policyonly when consulted, which the header documents),botan_tls_policy_view_text()(Policy::to_string()through a view function) andbotan_tls_policy_destroy().src/tests/test_ffi_tls.cpp:ffi_tls_versionruns in every FFI build and checks the version functions againstBOTAN_HAS_FFI_TLS;ffi_tls_policycovers the stock policies, text policies and the error paths.TLSPolicyandffi_tls_api_version()inbotan3.py(the latter returns 0 for libraries without the module or predating it), a test that skips when the module is absent, and a section inpython.rst.ffi.rst.Checked locally on macOS (Apple clang) with the full test suite and the Python tests, with gcc 13
-std=c89on the header, clang-tidy, clang-format, pylint and ruff, and a Sphinx build with-W. Nonews.rstentry yet; happy to add one now or with the first PR that makes TLS usable, whichever you prefer.