Change kuksa-client argument names

Aligning implementation in kuksa-client with proposal in #584.
May affect applications using kuksa-client.
Also removes support for client certificates as mutual authentication is not supported
by Databroker

.github/workflows/kuksa-client.yml

@@ -11,7 +11,7 @@
 # * SPDX-License-Identifier: Apache-2.0
 # ********************************************************************************/
 
-name: kuksa_client_docker
+name: kuksa_client
 
 on:
   push:
@@ -96,7 +96,7 @@ jobs:
         labels: ${{ steps.meta.outputs.labels }}
 
 
-  dbc2val-test:
+  kuksa-client-test:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout kuksa.val

kuksa-client/README.md

@@ -24,7 +24,7 @@ If you want to install from sources instead see [Building and running a local ve
 After you have installed the kuksa-client package via pip you can run the test client CLI directly by executing:
 
 ```console
-$ kuksa-client
+$ kuksa-client 
 ```
 
 With default CLI arguments, the client will try to connect to a local VISS server e.g. `kuksa-val-server`.
@@ -67,7 +67,7 @@ KUKSA.val Server and KUKSA.val Databroker use different token formats.
 
 A gRPC connection to KUKSA.val Databroker is started by specifying address and port for the Databroker and giving
 `--protocol grpc` as argument.
-KUKSA.val Client use TLS by default, it only run in insecure mode if `--insecure` is given as argument.
+KUKSA.val Client use TLS by default, it only run in insecure mode if `--no-tls` is given as argument.
 By default the KUKSA.val example Root CA and Client keys are used, but client keys have no effect as mutual authentication is not supported by KUKSA.val Databroker or KUKSA.val Server.
 
 ```
@@ -77,23 +77,15 @@ By default the KUKSA.val example Root CA and Client keys are used, but client ke
 This call with all parameters specified give same effect:
 
 ```
-~/kuksa.val/kuksa-client$ kuksa-client --ip localhost --port 55555 --protocol grpc --certificate ../kuksa_certificates/Client.pem --keyfile ../kuksa_certificates/Client.key --cacertificate ./kuksa_certificates/CA.pem
-```
-
-There is actually no reason to specify client key and certificate, as mutual authentication is not supported in KUKSA.val Databroker,
-so the command can be simplified like this:
-
-
-```
-~/kuksa.val/kuksa-client$ kuksa-client --ip localhost --port 55555 --protocol grpc --cacertificate ./kuksa_certificates/CA.pem
+~/kuksa.val/kuksa-client$ kuksa-client --ip localhost --port 55555 --protocol grpc  --tls-ca-cert ./kuksa_certificates/CA.pem
 ```
 
 The example server protocol list 127.0.0.1 as an alternative name, but the TLS-client currently used does not accept it,
 instead a valid server name must be given as argument.
 Currently `Server` and `localhost`are valid names from the example certificates.
 
 ```
-~/kuksa.val/kuksa-client$ kuksa-client --ip 127.0.0.1 --port 55555 --protocol grpc  --cacertificate ../kuksa_certificates/CA.pem --tls-server-name Server
+~/kuksa.val/kuksa-client$ kuksa-client --ip 127.0.0.1 --port 55555 --protocol grpc  --tls-ca-cert ../kuksa_certificates/CA.pem --tls-server-name Server
 ```
 
 ### Connecting to KUKSA.val Server
@@ -109,7 +101,7 @@ that is the only difference compared to connecting to KUKSA.val Databroker.
 This corresponds to this call:
 
 ```
-kuksa-client --ip 127.0.0.1 --port 8090 --protocol ws --cacertificate ./kuksa_certificates/CA.pem
+kuksa-client --ip 127.0.0.1 --port 8090 --protocol ws --tls-ca-cert ./kuksa_certificates/CA.pem
 ```
 
 ### Authorizing against KUKSA.val Server

kuksa-client/docs/examples/threaded.md

@@ -11,19 +11,19 @@ Be also aware that this API returns JSON responses whose schema may vary from on
 - `ip` server/databroker hostname or IP address, default: "127.0.0.1"
 - `port` server/databroker port, default: 8090
 - `protocol` protocol used to interact with server/databroker ("ws" or "grpc"), default: "ws"
-- `insecure` whether the communication should be unencrypted or not, default: `False`
-- `cacertificate` root certificate path, default: "../kuksa_certificates/CA.pem"
-- `certificate` client certificate path, default: "../kuksa_certificates/Client.pem"
-- `key` client private key path, default: "../kuksa_certificates/Client.key"
+- `no_tls` whether the communication should be unencrypted or not, default: `False`
+- `tls_ca_cert` root certificate path, default: "../kuksa_certificates/CA.pem"
+- `tls_cert` client certificate path, default: "../kuksa_certificates/Client.pem"
+- `tls_private_key` client private key path, default: "../kuksa_certificates/Client.key"
 
 ```python
 # An empty configuration dictionary will use the aforementioned default values:
 config = {}
-# Here is what a databroker configuration would look like:
+# Here is what a databroker configuration with an insecure connection would look like:
 config = {
     'port': 55555,
     'protocol': 'grpc',
-    'insecure': True,  # databroker does not yet support encryption
+    'no_tls': True, 
 }
 client = kuksa_client.KuksaClientThread(config)
 ```

kuksa-client/kuksa_client/__main__.py

@@ -145,7 +145,7 @@ def subscriptionIdCompleter(self, text, line, begidx, endidx):
     ap_getServerAddr = argparse.ArgumentParser()
     ap_connect = argparse.ArgumentParser()
     ap_connect.add_argument(
-        '-i', "--insecure", default=False, action="store_true", help='Connect in insecure mode')
+        '-n', "--no-tls", default=False, action="store_true", help='Connect in insecure mode (not using TLS)')
     ap_disconnect = argparse.ArgumentParser()
     ap_authorize = argparse.ArgumentParser()
     tokenfile_completer_method = functools.partial(Cmd.path_complete,
@@ -252,9 +252,9 @@ def subscriptionIdCompleter(self, text, line, begidx, endidx):
         "Json", help="Json tree to update VSS", completer_method=jsonfile_completer_method)
 
     # Constructor, request names after protocol to avoid errors
-    def __init__(self, server_ip=None, server_port=None, server_protocol=None, *, insecure=False, token_or_tokenfile=None,
-                 certificate=None, keyfile=None,
-                 cacertificate=None, tls_server_name=None):
+    def __init__(self, server_ip=None, server_port=None, server_protocol=None, *, no_tls=False, token_or_tokenfile=None,
+                 tls_cert=None, tls_private_key=None,
+                 tls_ca_cert=None, tls_server_name=None):
         super().__init__(
             persistent_history_file=".vssclient_history", persistent_history_length=100, allow_cli_args=False,
         )
@@ -270,10 +270,10 @@ def __init__(self, server_ip=None, server_port=None, server_protocol=None, *, in
         self.subscribeIds = set()
         self.commThread = None
         self.token_or_tokenfile = token_or_tokenfile
-        self.insecure = insecure
-        self.certificate = certificate
-        self.keyfile = keyfile
-        self.cacertificate = cacertificate
+        self.no_tls = no_tls
+        self.tls_cert = tls_cert
+        self.tls_private_key = tls_private_key
+        self.tls_ca_cert = tls_ca_cert
         self.tls_server_name = tls_server_name
 
         print("Welcome to Kuksa Client version " + str(_metadata.__version__))
@@ -494,19 +494,19 @@ def connect(self):
                 self.commThread = None
         config = {'ip': self.serverIP,
                   'port': self.serverPort,
-                  'insecure': self.insecure,
+                  'no_tls': self.no_tls,
                   'protocol': self.serverProtocol
                   }
 
         # Configs should only be added if they actually have a value
         if self.token_or_tokenfile:
             config['token_or_tokenfile'] = self.token_or_tokenfile
-        if self.certificate:
-            config['certificate'] = self.certificate
-        if self.keyfile:
-            config['keyfile'] = self.keyfile
-        if self.cacertificate:
-            config['cacertificate'] = self.cacertificate
+        if self.tls_cert:
+            config['tls_cert'] = self.tls_cert
+        if self.tls_private_key:
+            config['tls_private_key'] = self.tls_private_key
+        if self.tls_ca_cert:
+            config['tls_ca_cert'] = self.tls_ca_cert
         if self.tls_server_name:
             config['tls_server_name'] = self.tls_server_name
 
@@ -529,7 +529,7 @@ def connect(self):
     @with_category(COMM_SETUP_COMMANDS)
     @with_argparser(ap_connect)
     def do_connect(self, args):
-        self.insecure = args.insecure
+        self.no_tls = args.no_tls
         self.connect()
 
     @with_category(COMM_SETUP_COMMANDS)
@@ -603,25 +603,19 @@ def main():
         choices=SUPPORTED_SERVER_PROTOCOLS,
         default=DEFAULT_SERVER_PROTOCOL,
     )
-    parser.add_argument('--insecure', default=False,
-                        action='store_true', help='Connect in insecure mode')
+    parser.add_argument('--no-tls', default=False,
+                        action='store_true', help='Connect in insecure mode (not using TLS)')
     parser.add_argument(
         '--logging-config', default=os.path.join(scriptDir, 'logging.ini'), help="Path to logging configuration file",
     )
     parser.add_argument(
         '--token_or_tokenfile', default=None, help="JWT token or path to a JWT token file (.token)",
     )
 
-    # Add TLS arguments
-    # Note: Databroker does not yet support mutual authentication, so no need to use two first arguments
+    # Add TLS arguments - note we do not provide command line arguments for setting client certificates
+    # (tls-cert and tls-private-key) as neither Broker nor Server supports mutual authentication
     parser.add_argument(
-        '--certificate', default=None, help="Client cert file(.pem), only needed for mutual authentication",
-    )
-    parser.add_argument(
-        '--keyfile', default=None, help="Client private key file (.key), only needed for mutual authentication",
-    )
-    parser.add_argument(
-        '--cacertificate', default=None, help="Client root cert file (.pem), needed unless insecure mode used",
+        '--tls-ca-cert', default=None, help="Client root cert file (.pem), needed unless insecure mode used",
     )
     # Observations for Python
     # Connecting to "localhost" works well, subjectAltName seems to suffice
@@ -636,9 +630,8 @@ def main():
     logging.config.fileConfig(args.logging_config)
 
     clientApp = TestClient(args.ip, args.port, args.protocol,
-                           insecure = args.insecure, token_or_tokenfile = args.token_or_tokenfile,
-                           certificate = args.certificate, keyfile = args.keyfile,
-                           cacertificate = args.cacertificate, tls_server_name = args.tls_server_name)
+                           no_tls = args.no_tls, token_or_tokenfile = args.token_or_tokenfile,
+                           tls_ca_cert = args.tls_ca_cert, tls_server_name = args.tls_server_name)
     try:
         # We exit the loop when the user types "quit" or hits Ctrl-D.
         clientApp.cmdloop()

kuksa-client/kuksa_client/cli_backend/__init__.py

@@ -27,16 +27,16 @@ def __init__(self, config):
         self.serverIP = config.get('ip', "127.0.0.1")
         self.serverPort = config.get('port', 8090)
         try:
-            self.insecure = config.getboolean('insecure', False)
+            self.no_tls = config.getboolean('no_tls', False)
         except AttributeError:
-            self.insecure = config.get('insecure', False)
-        self.default_cert_path = pathlib.Path(kuksa_certificates.__path__[0])
-        self.cacertificate = config.get(
-            'cacertificate', str(self.default_cert_path / 'CA.pem'))
-        self.certificate = config.get('certificate', str(
+            self.no_tls = config.get('no_tls', False)
+        self.default_cert_path = pathlib.Path(kuksa_certificates.__path__[0])        
+        self.tls_cert = config.get('tls_cert', str(
             self.default_cert_path / 'Client.pem'))
-        self.keyfile = config.get('keyfile', str(
+        self.tls_private_key = config.get('tls_private_key', str(
             self.default_cert_path / 'Client.key'))
+        self.tls_ca_cert = config.get(
+            'tls_ca_cert', str(self.default_cert_path / 'CA.pem'))
         self.tls_server_name = config.get('tls_server_name', "")
         self.token_or_tokenfile = config.get('token_or_tokenfile', str(
             self.default_cert_path / 'jwt/all-read-write.json.token'))

kuksa-client/kuksa_client/cli_backend/grpc.py

@@ -54,9 +54,9 @@ def default(self, obj):
 class Backend(cli_backend.Backend):
     def __init__(self, config):
         super().__init__(config)
-        self.cacertificate = pathlib.Path(self.cacertificate)
-        self.keyfile = pathlib.Path(self.keyfile)
-        self.certificate = pathlib.Path(self.certificate)
+        self.tls_cert = pathlib.Path(self.tls_cert)
+        self.tls_private_key = pathlib.Path(self.tls_private_key)
+        self.tls_ca_cert = pathlib.Path(self.tls_ca_cert)
         if self.token_or_tokenfile is not None:
             if os.path.isfile(self.token_or_tokenfile):
                 self.token_or_tokenfile = pathlib.Path(self.token_or_tokenfile)
@@ -258,18 +258,20 @@ async def _grpcHandler(self, vss_client: kuksa_client.grpc.aio.VSSClient):
 
     # Main loop for handling gRPC communication
     async def mainLoop(self):
-        if self.insecure:
+        if self.no_tls:
 
-            async with kuksa_client.grpc.aio.VSSClient(self.serverIP, self.serverPort, token=self.token) as vss_client:
-                print("gRPC channel connected.")
+            async with kuksa_client.grpc.aio.VSSClient(self.serverIP, self.serverPort, no_tls=self.no_tls, 
+                                                       token=self.token) as vss_client:
+                print("Insecure gRPC channel connected.")
                 await self._grpcHandler(vss_client)
         else:
             async with kuksa_client.grpc.aio.VSSClient(
                 self.serverIP,
                 self.serverPort,
-                root_certificates=self.cacertificate,
-                private_key=self.keyfile,
-                certificate_chain=self.certificate,
+                no_tls=self.no_tls,
+                tls_cert=self.tls_cert,
+                tls_private_key=self.tls_private_key,
+                tls_ca_cert=self.tls_ca_cert,
                 tls_server_name=self.tls_server_name,
                 token=self.token
             ) as vss_client:

kuksa-client/kuksa_client/cli_backend/ws.py

@@ -229,13 +229,13 @@ def checkConnection(self):
         return self.wsConnected
 
     async def connect(self):
-        if not self.insecure:
+        if not self.no_tls:
             context = ssl.create_default_context()
             context.load_cert_chain(
-                certfile=self.certificate, keyfile=self.keyfile)
-            context.load_verify_locations(cafile=self.cacertificate)
+                certfile=self.tls_cert, keyfile=self.tls_private_key)
+            context.load_verify_locations(cafile=self.tls_ca_cert)
             # We want host name to match
-            # For example certificates we use subjectAltName to make it match for Server, localahost and 127.0.0.1
+            # For example certificates we use subjectAltName to make it match for Server, localhost and 127.0.0.1
             # If using your own certificates make sure that name is included or use tls_server_name work-around
             context.check_hostname = True
             try:

kuksa-client/kuksa_client/grpc/__init__.py

@@ -23,6 +23,7 @@
 import http
 import logging
 import re
+import sys
 from typing import Any
 from typing import Collection
 from typing import Dict
@@ -487,9 +488,10 @@ def __init__(
         host: str,
         port: int,
         token: Optional[str] = None,
-        root_certificates: Optional[Path] = None,
-        private_key: Optional[Path] = None,
-        certificate_chain: Optional[Path] = None,
+        no_tls: bool = False,  # Always use TLS unless other explicitly requested
+        tls_cert: Optional[Path] = None,
+        tls_private_key: Optional[Path] = None,
+        tls_ca_cert: Optional[Path] = None,
         ensure_startup_connection: bool = True,
         connected: bool = False,
         tls_server_name: Optional[str] = None
@@ -498,30 +500,33 @@ def __init__(
 
         self.authorization_header = self.get_authorization_header(token)
         self.target_host = f'{host}:{port}'
-        self.root_certificates = root_certificates
-        self.private_key = private_key
-        self.certificate_chain = certificate_chain
+        self.no_tls = no_tls
+        self.tls_cert = tls_cert
+        self.tls_private_key = tls_private_key
+        self.tls_ca_cert = tls_ca_cert
         self.tls_server_name = tls_server_name
         self.ensure_startup_connection = ensure_startup_connection
         self.connected = connected
         self.client_stub = None
 
     def _load_creds(self) -> Optional[grpc.ChannelCredentials]:
-        if self.root_certificates:
-            logger.info(f"Using TLS with Root CA from {self.root_certificates}")
-            root_certificates = self.root_certificates.read_bytes()
-            if self.private_key and self.certificate_chain:
-                private_key = self.private_key.read_bytes()
-                certificate_chain = self.certificate_chain.read_bytes()
-                logger.info(f"Using private client key {self.private_key} "
-                                 f"and chain/certificate {self.certificate_chain}")
-                return grpc.ssl_channel_credentials(root_certificates, private_key, certificate_chain)
+        if self.tls_ca_cert:
+            logger.info(f"Using TLS with Root CA from {self.tls_ca_cert}")
+            tls_ca_cert = self.tls_ca_cert.read_bytes()
+
+            if self.tls_private_key and self.tls_cert:
+                # This means that client can be authenticated if server wants it
+                # As of today KUKSA.val databroker does not support that
+                private_key = self.tls_private_key.read_bytes()
+                certificate_chain = self.tls_cert.read_bytes()
+                logger.debug(f"Using private client key {self.tls_private_key} "
+                                 f"and chain/certificate {self.tls_cert}")
+                return grpc.ssl_channel_credentials(tls_ca_cert, private_key, certificate_chain)
             else:
-                logger.info(f"No client certificates provided, mutual TLS not supported!")
-                return grpc.ssl_channel_credentials(root_certificates)
-        logger.info(f"No Root CA present, it will not be posible to use a secure connection!")
+                logger.debug(f"No client certificates provided, mutual TLS not supported!")
+                return grpc.ssl_channel_credentials(tls_ca_cert)
+        logger.info("No Root CA present, it will not be possible to use a secure connection!")
         return None
-
 
     def _prepare_get_request(self, entries: Iterable[EntryRequest]) -> val_pb2.GetRequest:
         req = val_pb2.GetRequest(entries=[])
@@ -645,7 +650,14 @@ def connect(self, target_host=None):
         if target_host is None:
             target_host = self.target_host
 
-        if creds is not None:
+        if self.no_tls:
+            logger.info("Establishing insecure channel")
+            channel = grpc.insecure_channel(target_host)
+        else:
+            if creds is None:
+                logger.error("TLS requested but not credentials provided. "
+                             "Use --no-tls if you want an insecure connection.")
+                sys.exit(-1)
             logger.info("Establishing secure channel")
             if self.tls_server_name:
                 logger.info(f"Using TLS server name {self.tls_server_name}")
@@ -654,9 +666,6 @@ def connect(self, target_host=None):
             else:
                 logger.debug(f"Not providing explicit TLS server name")
                 channel = grpc.secure_channel(target_host, creds)
-        else:
-            logger.info("Establishing insecure channel")
-            channel = grpc.insecure_channel(target_host)
 
         self.channel = self.exit_stack.enter_context(channel)
         self.client_stub = val_pb2_grpc.VALStub(self.channel)