{"action":"created","issue":{"url":"https://api.github.com/repos/strongswan/strongswan/issues/109","repository_url":"https://api.github.com/repos/strongswan/strongswan","labels_url":"https://api.github.com/repos/strongswan/strongswan/issues/109/labels{/name}","comments_url":"https://api.github.com/repos/strongswan/strongswan/issues/109/comments","events_url":"https://api.github.com/repos/strongswan/strongswan/issues/109/events","html_url":"https://github.com/strongswan/strongswan/pull/109","id":344799298,"node_id":"MDExOlB1bGxSZXF1ZXN0MjA0MTA3MzI3","number":109,"title":"Add Botan plugin to libstrongswan","user":{"login":"securitykernel","id":8866185,"node_id":"MDQ6VXNlcjg4NjYxODU=","avatar_url":"https://avatars2.githubusercontent.com/u/8866185?v=4","gravatar_id":"","url":"https://api.github.com/users/securitykernel","html_url":"https://github.com/securitykernel","followers_url":"https://api.github.com/users/securitykernel/followers","following_url":"https://api.github.com/users/securitykernel/following{/other_user}","gists_url":"https://api.github.com/users/securitykernel/gists{/gist_id}","starred_url":"https://api.github.com/users/securitykernel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/securitykernel/subscriptions","organizations_url":"https://api.github.com/users/securitykernel/orgs","repos_url":"https://api.github.com/users/securitykernel/repos","events_url":"https://api.github.com/users/securitykernel/events{/privacy}","received_events_url":"https://api.github.com/users/securitykernel/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":12,"created_at":"2018-07-26T11:09:58Z","updated_at":"2018-08-10T16:04:35Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/strongswan/strongswan/pulls/109","html_url":"https://github.com/strongswan/strongswan/pull/109","diff_url":"https://github.com/strongswan/strongswan/pull/109.diff","patch_url":"https://github.com/strongswan/strongswan/pull/109.patch"},"body":"Adds a plugin for the [Botan](https://botan.randombit.net) crypto library to libstrongswan. Botan is a C++ library, buth with C wrapper functions (\"ffi\"). \r\n\r\nThe plugin implements the following interfaces:\r\n\r\n* crypter\r\n* dh\r\n* ecdh\r\n* ec\r\n* gcm\r\n* hasher\r\n* hmac\r\n* rng\r\n* rsa\r\n\r\nThe plugin current only supports algorithms that are recommended by the BSI in its technical guidelines, e.g., for hashing SHA-2, but not MD5. We also added SHA-1, as it seems to be a hard requirement for strongswan.\r\n\r\nWe had to remove tests for generating 768 bit RSA keys, as Botan only allows >=1,024 bit RSA keys to be generated (everything <1,024 bits is considered too weak).\r\n\r\n/cc @randombit"},"comment":{"url":"https://api.github.com/repos/strongswan/strongswan/issues/comments/412128769","html_url":"https://github.com/strongswan/strongswan/pull/109#issuecomment-412128769","issue_url":"https://api.github.com/repos/strongswan/strongswan/issues/109","id":412128769,"node_id":"MDEyOklzc3VlQ29tbWVudDQxMjEyODc2OQ==","user":{"login":"tobiasbrunner","id":457648,"node_id":"MDQ6VXNlcjQ1NzY0OA==","avatar_url":"https://avatars2.githubusercontent.com/u/457648?v=4","gravatar_id":"","url":"https://api.github.com/users/tobiasbrunner","html_url":"https://github.com/tobiasbrunner","followers_url":"https://api.github.com/users/tobiasbrunner/followers","following_url":"https://api.github.com/users/tobiasbrunner/following{/other_user}","gists_url":"https://api.github.com/users/tobiasbrunner/gists{/gist_id}","starred_url":"https://api.github.com/users/tobiasbrunner/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tobiasbrunner/subscriptions","organizations_url":"https://api.github.com/users/tobiasbrunner/orgs","repos_url":"https://api.github.com/users/tobiasbrunner/repos","events_url":"https://api.github.com/users/tobiasbrunner/events{/privacy}","received_events_url":"https://api.github.com/users/tobiasbrunner/received_events","type":"User","site_admin":false},"created_at":"2018-08-10T16:04:35Z","updated_at":"2018-08-10T16:04:35Z","author_association":"MEMBER","body":"> When generating RSA and ECDSA keys you create \"user\" type botan_rng_t instances. However, in the rng_t implementation you mapped RNG_TRUE, which we usually use for private keys, to a \"system\" type botan_rng_t. So shouldn't the latter also be used when generating private keys?\r\n> \r\n> In this case, yes.\r\n\r\nThe question is whether it makes that much of a difference in the end. \"system\" seems to default to `/dev/urandom`, whereas in our _random_ plugin we use `/dev/random` for `RNG_TRUE` (in practice that probably makes not that much of a difference, though, unless the system entropy is low, I guess). And the DRBG created with \"user\" is seeded by the \"system\" RNG too by default (although strictly limited to 256 bits entropy for the initial seed, but that probably also doesn't matter much in practice). But I guess using \"system\" would be more consistent, so I'll change that.\r\n\r\n> Actually this was our first shot for the implementation, but our appliance QA run failed with an error in this area. I cannot remember though what the error messages were. Maybe a race condition? Does strongswan require the rng to be threadsafe? Because neither Botan's System_RNG nor HMAC_DRBG are threadsafe by default. You have to wrap them in a Serialized_RNG object.\r\n\r\nInteresting. Could you try to find out what the issue was? Because to me it makes more sense to bind `botan_rng_t` instances to the lifetime of `rng_t` instances (in particular for DRBGs).\r\n\r\nI don't think they have to be thread-safe in the sense that multiple threads will operate on a single `rng_t` instance concurrently. But there are at all times multiple instances on which different threads operate (some might be used by multiple threads because they are held in shared objects, but not concurrently). But those are pretty much exclusively \"user\" instances, which should be independent anyway (except for the initial seeding, but if you create a new instance with each call to  `allocate/get_bytes()` there are probably even more chances for races).\r\n\r\n> I just pushed a change to allow x509 to be optional for ffi, if not enabled all functions just return not implemented error code.\r\n\r\nPerfect, thanks.\r\n\r\n> If you decide you want to start using the X509 support within the plugin and need additional FFI functions just let me know.\r\n\r\nGreat, thanks. Although it's not our first priority (we'll probably focus on wrapping additional algorithms e.g. curve25519/ed25519 or chacha20poly1305).\r\n\r\n> For your purposes though, perhaps you just need to make libs test?\r\n\r\nYou're absolutely right, didn't realize there are separate make targets for these. This and disabling _x509_ reduced the build time of the library from ~360 to ~230 seconds (full build was ~410 seconds).\r\n\r\n> For userspace, I just add an option to FFI so \"user-threadsafe\" requests a serialized RNG. This might help.\r\n\r\nNice, but as mentioned above, we'll hopefully not need it. However, this is missing a `#if defined(BOTAN_TARGET_OS_HAS_THREADS)` wrapper around it (our build without _threads_ OS feature currently fails).\r\n"}}
{"actor":{"display_login":"tobiasbrunner"}}