Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TLS argument handling (renaming and clean-up) #584

Open
erikbosch opened this issue Jun 26, 2023 · 3 comments
Open

TLS argument handling (renaming and clean-up) #584

erikbosch opened this issue Jun 26, 2023 · 3 comments
Labels
Databroker Issues related to databroker (core) documentation Improvements or additions to documentation

Comments

@erikbosch
Copy link
Contributor

erikbosch commented Jun 26, 2023

This is a proposal on how KUKSA.val applications/libraries in the long run shall handle TLS. The background is that names as of today are a bit inconsistent. I would like us to discuss if we want to do any changes in this areas to make names more aligned

Server/Databroker

  • Default config (default config file and/or default arguments as stored in GitHub) shall NOT be usable.
  • To use an insecure connection an argument --insecure or similar must be added
  • To use a secure connection Server key/pem must be given.
  • Mutual authentication not supported

Clients

  • Remove code/documentation from feeder/provider implementations as mutual authentication is currently not supported, but keep it in kuksa-client library.
  • Remove "insecure", for clients we can use/try an insecure connection if there are no certs
  • For clients an insecure connection by default is ok, will anyway be rejected by server/broker if not supported.
  • (Alternatively we can say that all clients should have a no-tls argument as well)

Proposed names

Argument Comment Server Server Default Client Client Default Repository Example File
no-tls If set an insecure connection shall be used Yes False No N/A -
tls-cert Path to client/server certificate/chain Yes None Yes* None Server.pem/Client.pem
tls-private-key Path to client/server private key Yes None Yes* None Server.key/Client.pem
tls-ca-cert Path to root CA certificate No N/A Yes None CA.pem
tls-server-name Name of server to be used by client in server host name validation No N/A Yes None -

The kuksa-client library supports client certificates, at least partially, but not described in user facing documentation!

Proposed Changes

  • Align naming ( do not care about background compatibility on master)
  • Remove all support of client certs in user facing functionality - we anyway will not support it in the next years
  • Remove insecure from clients, no needed
@SebastianSchildt
Copy link
Contributor

Looks good reasonable from my side, I am just not sure whether we should also have an explicit --insecure on client side.
A client could try a TLS connection as default, given e.g. a "real" certificate is used (e.g. consider connecting to a demo instance in the cloud via DNS name, where a correct letsencrypt cert is set up), or even locally if the system root certs are configured in a way suitable for a deployment. So I would argue, for consistency also have an explicit --insecure/--no-tls on client side

erikbosch added a commit to boschglobal/kuksa.val that referenced this issue Jun 28, 2023
Aligning implementation in kuksa-client with proposal in eclipse#584.
May affect applications using kuksa-client
erikbosch added a commit to boschglobal/kuksa.val that referenced this issue Jun 29, 2023
Aligning implementation in kuksa-client with proposal in eclipse#584.
May affect applications using kuksa-client.
Also removes support for client certificates as mutual authentication is not supported
by Databroker
erikbosch added a commit to boschglobal/kuksa.val that referenced this issue Jun 30, 2023
Aligning implementation in kuksa-client with proposal in eclipse#584.
May affect applications using kuksa-client.
Also removes support for client certificates as mutual authentication is not supported
by Databroker
@erikbosch erikbosch changed the title TLS argument renaming and clean-up TLS argument handling (renaming and clean-up) Jul 6, 2023
@erikbosch
Copy link
Contributor Author

Note, a similar discussion is needed for authorization. Today (in Databroker) you opt-in to authorization by --jwt-public-key <file>, if you do not use it you get no authorization. Maybe we want to introduce a --no-authorization argument and use logic like this:

if "--no-authorization":
    <disable authorization>
else if "--jwt-public-key":
   <use given key for token validation>
else:
   <give error "You must use either use --no-authorization or --jwt-public-key">
   exit -1

@erikbosch
Copy link
Contributor Author

This topic was discussed at a Community call. Participants seemed to agree that it is OK that default configuration isn't usable, that you either must specify tokens/cert or explicitly request an insecure solution. I suggest that this can be implemented after v0.4.0. so that existing integrations can use v0.4.0 rather than latest if they want the "old" behavior.

@lukasmittag lukasmittag added documentation Improvements or additions to documentation Databroker Issues related to databroker (core) labels Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Databroker Issues related to databroker (core) documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants