Unfriendly new behavior in cert cli flags

[sabedevops]

ziti version: >= v0.28.0

Problem Description:

https://github.com/openziti/ziti/releases/tag/v0.28.0

version v0.28.0 changed the meaning of the ziti flag -c and and introduced a --ca to replace it. This introduces a confusing user-facing behavior, because -c was repurposed for --client-cert. This results in a situation where the old cli invocation is semantically valid, in that it's still receiving a cert path, but fails with an unhelpful error. In the case of an old invocation of ziti edge login ... -c /my/cert/path:

error: unable to retrieve server certificate authority from https://127.0.0.1:8443: pkcs7: input data is empty

Expected Behavior:

The user should be made aware of the new semantics of the flag, and receive helpful aid in migrating to the new semantics.

One option is to verify that the cert passed to the client for -c and --ca are client and CA certificates accordingly. A python example to illustrate a potential method:

"""Enforces NOT CA"""
from cryptography import x509
from cryptography.hazmat.backends import default_backend

def get_certificate_extensions(cert_path):
    """Gets certificate extensions"""
    with open(cert_path, "rb") as cert_file:
        cert_data = cert_file.read()

    cert = x509.load_pem_x509_certificate(cert_data, default_backend())

    constraints = None
    usage = None

    for extension in cert.extensions:
        if extension.oid == x509.OID_BASIC_CONSTRAINTS:
            constraints = extension.value
        elif extension.oid == x509.OID_KEY_USAGE:
            usage = extension.value

    return constraints, usage

def main():
    """Main"""
    cert_path = "./ca.pem"
    basic_constraints, key_usage = get_certificate_extensions(cert_path)

    if ((hasattr(basic_constraints, 'ca') and basic_constraints.ca)
       or (hasattr(key_usage, 'crl_sign') and key_usage.crl_sign)
       or (hasattr(key_usage, 'key_cert_sign') and key_usage.key_cert_sign)):

        print("Cert is a CA, not a client certificate.")
        print("Did you mean to use '--ca'?")

if __name__ == '__main__':
    main()

[qrkourier]

Hey Steven, Thanks for explaining your suggestion of a decision point in the ziti edge login code that would infer the intended use of the -c based on the type of certificate.

I'm concerned that a squishy behavior like this new decision point would be smellier than the pain it might avoid.

Are you thinking it's justifiable in this particular case on principle, e.g., because CLIs shouldn't break, or to help out someone that's run into a heap of pain?

With the info I have, it seems best to let the version bump speak for itself in this case, i.e., 0.28 is presumed to have breaking changes for 0.27 users.

[qrkourier]

Related: https://github.com/openziti/ziti/pull/1079/files#r1161707075

[sabedevops]

The concern is as stated. This semantics difference results in a confusing outcome for the user. There is no clear change in the interface that lets the user know they should be doing something differently. It accepts the same input type with different semantics and no clear indication as to what is going on. The flags should've been renamed but not repurposed.

In other words, I believe it would've been better to let the cli fail when "-c" was specified to alert the user as an alternative solution.