Skip to content

Commit

Permalink
Change kuksa-client argument names
Browse files Browse the repository at this point in the history
Aligning implementation in kuksa-client with proposal in #584.
May affect applications using kuksa-client
  • Loading branch information
erikbosch committed Jun 28, 2023
1 parent 9ab495b commit c5fbd73
Show file tree
Hide file tree
Showing 13 changed files with 100 additions and 131 deletions.
18 changes: 5 additions & 13 deletions kuksa-client/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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.

```
Expand All @@ -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
Expand All @@ -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
Expand Down
10 changes: 4 additions & 6 deletions kuksa-client/docs/examples/threaded.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,17 @@ 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"

```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)
```
Expand Down
43 changes: 14 additions & 29 deletions kuksa-client/kuksa_client/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -252,9 +252,8 @@ 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_ca_cert=None, tls_server_name=None):
super().__init__(
persistent_history_file=".vssclient_history", persistent_history_length=100, allow_cli_args=False,
)
Expand All @@ -270,10 +269,8 @@ 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_ca_cert = tls_ca_cert
self.tls_server_name = tls_server_name

print("Welcome to Kuksa Client version " + str(_metadata.__version__))
Expand Down Expand Up @@ -494,19 +491,15 @@ 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_ca_cert:
config['tls_ca_cert'] = self.tls_ca_cert
if self.tls_server_name:
config['tls_server_name'] = self.tls_server_name

Expand All @@ -529,7 +522,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)
Expand Down Expand Up @@ -603,8 +596,8 @@ 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",
)
Expand All @@ -613,15 +606,8 @@ def main():
)

# Add TLS arguments
# Note: Databroker does not yet support mutual authentication, so no need to use two first arguments
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
Expand All @@ -636,9 +622,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()
Expand Down
12 changes: 4 additions & 8 deletions kuksa-client/kuksa_client/cli_backend/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,12 @@ 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.no_tls = config.get('no_tls', 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.default_cert_path / 'Client.pem'))
self.keyfile = config.get('keyfile', 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'))
Expand Down
16 changes: 7 additions & 9 deletions kuksa-client/kuksa_client/cli_backend/grpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,7 @@ 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_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)
Expand Down Expand Up @@ -258,18 +256,18 @@ 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_ca_cert=self.tls_ca_cert,
tls_server_name=self.tls_server_name,
token=self.token
) as vss_client:
Expand Down
8 changes: 3 additions & 5 deletions kuksa-client/kuksa_client/cli_backend/ws.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,13 +229,11 @@ 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)
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:
Expand Down
42 changes: 18 additions & 24 deletions kuksa-client/kuksa_client/grpc/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import http
import logging
import re
import sys
from typing import Any
from typing import Collection
from typing import Dict
Expand Down Expand Up @@ -487,9 +488,8 @@ 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_ca_cert: Optional[Path] = None,
ensure_startup_connection: bool = True,
connected: bool = False,
tls_server_name: Optional[str] = None
Expand All @@ -498,30 +498,20 @@ 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_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)
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!")
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()
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=[])
Expand Down Expand Up @@ -645,7 +635,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}")
Expand All @@ -654,9 +651,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)
Expand Down
15 changes: 11 additions & 4 deletions kuksa-client/kuksa_client/grpc/aio.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import asyncio
import contextlib
import logging
import sys
from typing import AsyncIterator
from typing import Callable
from typing import Collection
Expand Down Expand Up @@ -67,7 +68,15 @@ async def connect(self, target_host=None):
creds = self._load_creds()
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.aio.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}")
Expand All @@ -76,9 +85,7 @@ async def connect(self, target_host=None):
else:
logger.debug(f"Not providing explicit TLS server name")
channel = grpc.aio.secure_channel(target_host, creds)
else:
logger.info("Establishing insecure channel")
channel = grpc.aio.insecure_channel(target_host)

self.channel = await self.exit_stack.enter_async_context(channel)
self.client_stub = val_pb2_grpc.VALStub(self.channel)
self.connected = True
Expand Down
Loading

0 comments on commit c5fbd73

Please sign in to comment.