Skip to content

Remove TLS between daemon and cells#443

Closed
izissise wants to merge 3 commits intoaurae-runtime:mainfrom
Wuageorg:poc-no-tls-daemon-nested
Closed

Remove TLS between daemon and cells#443
izissise wants to merge 3 commits intoaurae-runtime:mainfrom
Wuageorg:poc-no-tls-daemon-nested

Conversation

@izissise
Copy link
Copy Markdown
Contributor

@izissise izissise commented Mar 7, 2023

Because the daemon should not have access to the client key,
it should not be able to reuse the client auth to communicate with
the nested_auraed. Here we disable tls for proto message between
the daemon and the cell.

This not really intended to be merged as is, but mostly to trigger the discussion
on what are the security requirements for the communication between daemon and cells.

This pr is on top of #442

@krisnova
Copy link
Copy Markdown
Contributor

krisnova commented Mar 9, 2023

This is a fantastic conversation to be having. Thank you for bringing this up. I see two options forward (feel free to make suggestions if there is an option I am not considering that you feel is viable!).

  1. Disable TLS and have 1:1 open connections
  2. Generate new TLS material and pass a fresh client certificate down to the nested cell somehow.

My first thought on 1 is that it is potentially a security hole for the project, but is probably a perfectly reasonable place to start.

I think my decision on the topic is just that, to start with an open and un-encrypted connection between the host daemon and the nested cell. We can make improvements on the connection in future PRs. Maybe it makes sense to do some sort of lower level access control that only allows connections from a given socket or something. Unsure at the moment.

If you have an opinion or suggestion on if we should generate TLS client material as described in 2. please open an issue on that topic specifically and we can introduce that change as we make a decision as a project.

@izissise izissise force-pushed the poc-no-tls-daemon-nested branch from fe85980 to 89ba8e8 Compare March 11, 2023 19:32
@izissise
Copy link
Copy Markdown
Contributor Author

Awesome!, thanks for the response!
I'll do the following then:

  • Make the current pr code more clear and we can merge once everything is right
  • Open an issue for 2, I might have some idea (memfd?)

@izissise izissise force-pushed the poc-no-tls-daemon-nested branch 2 times, most recently from b547e45 to 85658df Compare March 13, 2023 19:35
@izissise izissise changed the title WIP: no tls between daemon and cells No tls between daemon and cells Mar 14, 2023
@izissise izissise changed the title No tls between daemon and cells Remove TLS between daemon and cells Mar 14, 2023
In order for relative path in options to work, we also remove
current_dir("/") for the forked client

Make ClientConfig output warnings on stderr
Because the deamon should not have access to the client key,
it should not be able to reuse the client auth to communicate with
the nested_auraed. Here we disable tls for proto message between
the daemon and the cell.
@izissise izissise force-pushed the poc-no-tls-daemon-nested branch from 85658df to 21302d6 Compare March 17, 2023 10:58
@izissise
Copy link
Copy Markdown
Contributor Author

Rebased on main

@krisnova
Copy link
Copy Markdown
Contributor

Rebased in #462

@krisnova krisnova closed this Mar 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants