search

opendatahub-io / caikit-nlp-client

A grpc and http client library for caikit-nlp
Apache License 2.0
3 stars 6 forks source link

grpc client: fix verify=None for some cases #81

Closed dtrifiro closed 11 months ago

dtrifiro commented 11 months ago

verify=False used ssl.get_server_certificate to get the server certificates. Unfortunately this does not provide a server hostname (TLS SNI), causing the wrong certificates to be fetched when connecting to an host using name-based virtual hosting. This can be solved by providing server_hostname when wrapping the socket for TLS.

fixes #80

dtrifiro commented 11 months ago

@bdattona this should solve all the issues we had :partying_face: :mountain:

codecov[bot] commented 11 months ago

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (58ff319) 93.08% compared to head (6f53766) 93.61%.

Files Patch % Lines
tests/test_utils.py 96.87% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #81 +/- ## ========================================== + Coverage 93.08% 93.61% +0.52% ========================================== Files 13 15 +2 Lines 709 752 +43 Branches 151 157 +6 ========================================== + Hits 660 704 +44 + Misses 33 32 -1 Partials 16 16 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dtrifiro commented 11 months ago

Here's the a script that highlights the differences between the two approaches

import ssl
import socket
import subprocess

host, port = (  # on rosa
    "caikit-tgis-example-isvc-predictor-mytest.apps.rosa.vajain-test.cusw.p3.openshiftapps.com",
    443,
)
# host, port = ( # on internal network
#     "flan-t5-small-caikit-predictor-kserve-grpc.apps.ods-qe-02.rhods.ccitredhat.com",
#     443,
# )

def use_raw_socket():
    print("=== Getting cert using raw socket")
    context = ssl.SSLContext(ssl.PROTOCOL_TLS)
    # context = ssl._create_unverified_context(cert_reqs=ssl.CERT_NONE)

    with socket.create_connection((host, port)) as sock:
        with context.wrap_socket(sock, server_hostname=host) as ssock:
            der_cert = ssock.getpeercert(True)

    pem_cert = ssl.DER_cert_to_PEM_cert(der_cert)

    name = "cert.pem"
    with open(name, "wb") as fh:
        fh.write(pem_cert.encode())
    print(f"=== saved {name}")

    res = subprocess.check_output(f"openssl x509 -text -in {name}".split(" "))
    name_txt = name.replace(".pem", ".txt")
    print(res.decode())
    with open(name_txt, "wb") as fh:
        fh.write(res)
    print(f"=== saved {name_txt}")

def use_ssl_get_server_cert():
    print("=== Getting cert using ssl.get_server_cert")

    cert = ssl.get_server_certificate((host, port))

    name = "cert-ssl_get_server_cert.pem"
    with open(name, "wb") as fh:
        fh.write(cert.encode())
    print(f"=== saved {name}")

    res = subprocess.check_output(f"openssl x509 -text -in {name}".split(" "))
    print(res.decode())
    name_txt = name.replace(".pem", ".txt")
    with open(name_txt, "wb") as fh:
        fh.write(res)
    print(f"=== saved {name_txt}")

if __name__ == "__main__":
    use_raw_socket()
    use_ssl_get_server_cert()

To view differences: vimdiff cert.txt cert-ssl_get_server_cert.txt

- Diff between presented certificates (some details snipped) ```diff 4c4,5 < Serial Number: 3085371815171645655 (0x2ad17151a34ee0d7) --- > Serial Number: > 04:86:8f:29:a6:c2:9e:1b:1d:97:3a:3a:58:f0:62:69:2a:b9 6c7 < Issuer: O = opendatahub-self-signed, CN = *.apps.rosa.vajain-test.cusw.p3.openshiftapps.com --- > Issuer: C = US, O = Let's Encrypt, CN = R3 8,10c9,11 < Not Before: Dec 1 07:51:52 2023 GMT < Not After : Nov 30 07:51:52 2024 GMT < Subject: O = opendatahub-self-signed, CN = *.apps.rosa.vajain-test.cusw.p3.openshiftapps.com --- > Not Before: Nov 30 12:41:54 2023 GMT > Not After : Feb 28 12:41:53 2024 GMT > Subject: CN = *.apps.rosa.vajain-test.cusw.p3.openshiftapps.com 15,32c16,33 --- 36c37 < Digital Signature, Key Encipherment, Certificate Sign --- > Digital Signature, Key Encipherment 38c39 < TLS Web Server Authentication --- > TLS Web Server Authentication, TLS Web Client Authentication 40c41 < CA:TRUE --- > CA:FALSE 42c43,48 < 4E:96:DE:3A:C8:4E:D7:D5:91:D2:B4:0B:0A:1E:E0:68:52:91:DD:A2 --- > A6:CF:49:4D:9E:B1:F9:F5:44:53:03:40:8F:F3:50:1D:01:C5:84:40 > X509v3 Authority Key Identifier: > 14:2E:B3:17:B7:58:56:CB:AE:50:09:40:E6:1F:AF:9D:8B:14:C2:C6 > Authority Information Access: > OCSP - URI:http://r3.o.lencr.org > CA Issuers - URI:http://r3.i.lencr.org/ 44c50,77 < DNS:apps.rosa.vajain-test.cusw.p3.openshiftapps.com, DNS:*.apps.rosa.vajain-test.cusw.p3.openshiftapps.com, DNS:localhost --- > DNS:*.apps.rosa.vajain-test.cusw.p3.openshiftapps.com > X509v3 Certificate Policies: > Policy: 2.23.140.1.2.1 > CT Precertificate SCTs: > Signed Certificate Timestamp: > Version : v1 (0x0) > Log ID : 48:B0:E3:6B:DA:A6:47:34:0F:E5:6A:02:FA:9D:30:EB: > 1C:52:01:CB:56:DD:2C:81:D9:BB:BF:AB:39:D8:84:73 > Timestamp : Nov 30 13:41:54.602 2023 GMT > Extensions: none > Signature : ecdsa-with-SHA256 > 30:45:02:21:00:D7:97:D3:5B:A9:3B:70:0E:CC:1C:01: > C3:C9:DF:91:10:97:0D:65:6C:4B:EC:82:25:5A:9C:2F: > 34:0F:0A:2C:66:02:20:58:F1:52:C1:84:15:F3:86:DF: > 0B:C3:7E:C4:97:1F:1F:87:BE:5F:63:FE:A7:A9:1C:97: > 34:40:59:48:D3:A7:B9 > Signed Certificate Timestamp: > Version : v1 (0x0) > Log ID : EE:CD:D0:64:D5:DB:1A:CE:C5:5C:B7:9D:B4:CD:13:A2: > 32:87:46:7C:BC:EC:DE:C3:51:48:59:46:71:1F:B5:9B > Timestamp : Nov 30 13:41:54.616 2023 GMT > Extensions: none > Signature : ecdsa-with-SHA256 > 30:44:02:20:6B:44:7C:30:03:00:67:85:B4:7B:2A:D2: > 74:79:F6:03:CA:B4:A0:0A:7D:31:91:09:39:26:38:E3: > E6:BB:1D:15:02:20:47:DF:9E:64:62:66:6C:6B:2D:9A: > B0:BE:F0:9F:E6:0B:5D:E4:6B:5C:09:BC:2B:7B:14:FD: > C0:AE:57:55:5B:8B ```
dtrifiro commented 11 months ago

A small note: this fix is only required for python <= 3.9, as more recent versions support TLS SNI with no extra configuration required.