Adds MbedTLS support to Cytoplasm #54

lda · 2024-09-15T09:19:09Z

lda commented

2024-09-15 09:19:09 +00:00

Only issue with this IMO is that requiring the presence of CYTO_TLS_CA whenever a program may want to use TLS doesn't sound all that great, so maybe it should check off common paths when unset before complaining.

Only issue with this IMO is that requiring the presence of `CYTO_TLS_CA` whenever a program may want to use TLS doesn't sound all that great, so maybe it should check off common paths when unset before complaining.

lda added 2 commits 2024-09-15 09:19:10 +00:00

[ADD] Add MbedTLS support c1d59b2c6f

[META] Denote the presence of Mbed in the README ee58167928

lda added 1 commit 2024-09-15 11:08:15 +00:00

Merge branch 'master' into add-mbed ffc11ea8ba

lda added 2 commits 2024-09-15 11:09:44 +00:00

[FIX] Remove dummy functions 69f8e47ea2

Merge branch 'add-mbed' of https://git.telodendria.io/lda/Cytoplasm into add-mbed f9702cf0ef

jordan reviewed 2024-09-15 22:24:43 +00:00

src/Tls/TlsMbedTLS.c Outdated

					
				@ -0,0 +96,4 @@

				    }

				    /* Add a source of entropy if possible(using the CYTO_TLS_SEED env).

				     * Note that we ignore the error code. */

				    seed = getenv("CYTO_TLS_SEED");

jordan commented

2024-09-15 22:24:43 +00:00

Forgive me for not understanding MbedTLS that well, but why does this need to be sourced from the environment? Can't we randomly generate a seed? What's the use case for using the same seed more than once?

lda commented

2024-09-16 07:49:11 +00:00

It doesn't have to in most cases(MbedTLS will try to use things like /dev/random if possible) but considering how POSIX doesn't really mandate such sources to exist in the first place (and that I really wouldn't use something like the Rand API for cryptographic tasks), and I think having an optional source of extra randomness, if possible, may be worth it.

What's the use case for using the same seed more than once?
MbdeTLS tends to change the seed anyways with whatever other sources it can grab too.

It *doesn't* have to in most cases(MbedTLS will try to use things like `/dev/random` if possible) but considering how POSIX doesn't really mandate such sources to exist in the first place (and that I really *wouldn't* use something like the `Rand` API for cryptographic tasks), and I think having an optional source of extra randomness, if possible, may be worth it. > What's the use case for using the same seed more than once? MbdeTLS tends to change the seed anyways with whatever other sources it can grab too.

jordan commented

2024-09-16 17:11:42 +00:00

(and that I really wouldn't use something like the Rand API for cryptographic tasks)

I agree, and that's the real problem here. Ideally, we should use the Rand API to generate the MbedTLS seed. The fact that we can't because Rand isn't cryptographically secure is a problem of its own that, in my opinion, should be fixed first before we start relying on environment variables.

I'd like Cytoplasm to very much be link-and-forget. It shouldn't have any special runtime dependencies like this. I don't even know how we'd trust the user to set the seed properly. If the system doesn't have some source of entropy that we can draw from, then I don't know how the user could generate a seed. Because if they can do it on the machine we're running on, then so can we, and so should we. Otherwise, they'd have to generate it on a different machine and then copy it over to the machine executing this Cytoplasm code? That seems pretty impractical, particularly because every time Cytoplasm is started, (or every time this function is called?) the seed needs to be freshly generated.

> (and that I really wouldn't use something like the Rand API for cryptographic tasks) I agree, and that's the real problem here. Ideally, we *should* use the Rand API to generate the MbedTLS seed. The fact that we can't because Rand isn't cryptographically secure is a problem of its own that, in my opinion, should be fixed first before we start relying on environment variables. I'd like Cytoplasm to very much be link-and-forget. It shouldn't have any special runtime dependencies like this. I don't even know how we'd trust the user to set the seed properly. If the system doesn't have some source of entropy that we can draw from, then I don't know how the user *could* generate a seed. Because if they can do it on the machine we're running on, then so can we, and so should we. Otherwise, they'd have to generate it on a different machine and then copy it over to the machine executing this Cytoplasm code? That seems pretty impractical, particularly because every time Cytoplasm is started, (or every time this function is called?) the seed needs to be freshly generated.

jordan reviewed 2024-09-15 22:25:34 +00:00

src/Tls/TlsMbedTLS.c Outdated

					
				@ -0,0 +117,4 @@

				    }

				    /* Setup key verification */

				    cafile = getenv("CYTO_TLS_CA");

jordan commented

2024-09-15 22:25:34 +00:00

Again, why should this come from the environment? Is there a way we can somehow just use the system's CA file?

lda commented

2024-09-16 21:28:22 +00:00

Just made it check for known certificate paths(though I've kept the environment, in case any users may still want to use a custom directory/are really unlucky in their OS setups)

jordan commented

2024-09-15 22:28:08 +00:00

We should definitely try to use the system's CA store when possible. I don't know if there's a standard location for this, but on all the systems I've used, it's been /etc/ssl/cert.pem.

Then again, if the system doesn't have a CA store, I'm not really sue what we'll do there.

Either way, this is otherwise a nice looking pull request. I think our design for supporting multiple TLS implementations is great, it's super easy to add new implementations.

We should definitely try to use the system's CA store when possible. I don't know if there's a standard location for this, but on all the systems I've used, it's been `/etc/ssl/cert.pem`. Then again, if the system doesn't *have* a CA store, I'm not really sue what we'll do there. Either way, this is otherwise a nice looking pull request. I think our design for supporting multiple TLS implementations is great, it's super easy to add new implementations.

lda added 2 commits 2024-09-16 21:26:35 +00:00

[MOD] Try to probe for certificate stores dfcdef51fe

Merge branch 'add-mbed' of https://git.telodendria.io/lda/Cytoplasm into add-mbed b28cd422fb

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions