Skip to content

Wildcard domains in SAN are not matching #462

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

Closed
eldis80 opened this issue Mar 6, 2025 · 11 comments
Closed

Wildcard domains in SAN are not matching #462

eldis80 opened this issue Mar 6, 2025 · 11 comments
Labels
bug Something isn't working patch available

Comments

@eldis80
Copy link

eldis80 commented Mar 6, 2025

Our connections started giving an error for the certificate to hostname mismatch after updating to version 3.0.0. It seems that the new version doesn't allow matching wildcards in the Subject Alternative Names.

This code is probably the cause and seems to be matching with exact values only: https://github.com/oracle/python-oracledb/blob/main/src/oracledb/impl/thin/crypto.pyx#L67-L72

However, Oracle Cloud's ADBs present a wildcard certificate that was verified and accepted with previous versions of python-oracledb.

Our ADB instance has address in the form <instaceid>.adb.eu-amsterdam-1.oraclecloud.com and the certificate has SANs: adb.eu-amsterdam-1.oraclecloud.com, *.adb.eu-amsterdam-1.oraclecloud.com, *.adb.eu-amsterdam-1.oraclevcn.com

  1. What versions are you using?

3.0.0

Database is the Oracle Autonomous Database (ADB) in Oracle Cloud

  1. Is it an error or a hang or a crash?

Error

  1. What error(s) or behavior you are seeing?

DPY-6006: The name on the server certificate does not match the expected value: "<id>.adb.eu-amsterdam-1.oraclecloud.com

  1. Does your application call init_oracle_client()?

I'm not sure.

  1. Include a runnable Python script that shows the problem.

N/A

@eldis80 eldis80 added the bug Something isn't working label Mar 6, 2025
@anthony-tuininga
Copy link
Member

Thanks for the report. Previous versions of python-oracledb used the Python implementation so the new implementation in the driver is clearly lacking. This will be corrected!

@eldis80
Copy link
Author

eldis80 commented Mar 6, 2025

Thanks.

I think the original issue (#415) that led to the change might have been caused by their use of underscore (_) in the hostname - at least according to the printed error. Their code snippet actually uses a dash but the error message has an underscore in the hostname.

OpenSSL will deem those as invalid as discussed here: openssl/openssl#12566 . It's a bit debatable should this project allow non-standard hostnames for SSL/TLS hostname verification or not.

I think that by default OpenSSL will only consider SANs if they are available. I don't think there is an option to set the OpenSSL's X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT flag via Python's SSLContext.

@anthony-tuininga
Copy link
Member

If you are able to test this yourself, that would be helpful, as I don't have such a setup myself:

diff --git a/src/oracledb/impl/thin/crypto.pyx b/src/oracledb/impl/thin/crypto.pyx
index b9d7c9c6..8cc5bffe 100644
--- a/src/oracledb/impl/thin/crypto.pyx
+++ b/src/oracledb/impl/thin/crypto.pyx
@@ -68,7 +68,8 @@ def check_server_dn(sock, expected_dn, expected_name):
                 x509.oid.ExtensionOID.SUBJECT_ALTERNATIVE_NAME
             )
             for name in ext.value.get_values_for_type(x509.DNSName):
-                if name == expected_name:
+                if name == expected_name or name.startswith("*") \
+                        and expected_name.endswith(name[1:]):
                     return
         except x509.ExtensionNotFound:
             pass

Does this also need to be done for the COMMON name? Or just the SUBJECT ALTERNATIVE name?

@eldis80
Copy link
Author

eldis80 commented Mar 6, 2025

I think that code would work in our case but it will match multiple labels (subdomains) and usually the wildcard would match only the leftmost label.

It's like setting X509_CHECK_FLAG_MULTI_LABEL_WILDCARDS in OpenSSL and I'm not sure if it's according to the standard.

I think the standard RFC 6125 also says that the SANs should be checked first and to check the Subject only in case there's no SAN of the correct type. But, yes, CN can also contain a wildcard but nowadays all the matching should be done via SANs. Self-signed certificates might not follow the standards that well.

I'm no expert in this area and I'm just googling for this stuff and trying to learn how it should go. 😀

And, of course, your implementation can allow for some deviation from the standard to not break existing setups.

@anthony-tuininga
Copy link
Member

Thanks for the explanation. I'll see what can be done internally. You can disable the check, of course, by setting the ssl_server_dn_match flag to False during the call to connect().

@anthony-tuininga
Copy link
Member

I have a new patch that matches what is done with JDBC and should cover your scenario. It also checks the SANs first. This will be tested internally first, but if you are able to verify it, too, that would be appreciated:

diff --git a/src/oracledb/impl/thin/crypto.pyx b/src/oracledb/impl/thin/crypto.pyx
index b9d7c9c6..8275dc02 100644
--- a/src/oracledb/impl/thin/crypto.pyx
+++ b/src/oracledb/impl/thin/crypto.pyx
@@ -42,6 +42,45 @@ except Exception as e:
 DN_REGEX = '(?:^|,\s?)(?:(?P<name>[A-Z]+)=(?P<val>"(?:[^"]|"")+"|[^,]+))+'
 PEM_WALLET_FILE_NAME = "ewallet.pem"
 
+def _name_matches(name_to_check, cert_name):
+    """
+    Returns a boolean indicating if the name to check matches with the
+    certificate name. The certificate name may contain a wildcard (*)
+    character.
+    """
+
+    # check for a full match (case insensitive)
+    cert_name = cert_name.lower()
+    name_to_check = name_to_check.lower()
+    if name_to_check == cert_name:
+        return True
+
+    # ensure that more than one label exists in both the name to check and the
+    # certificate name
+    check_pos = name_to_check.find(".")
+    cert_pos = cert_name.find(".")
+    if check_pos <= 0 or cert_pos <= 0:
+        return False
+
+    # ensure that the right hand labels all match
+    if name_to_check[check_pos:] != cert_name[cert_pos:]:
+        return False
+
+    # match wildcards, if applicable
+    cert_label = cert_name[:cert_pos]
+    check_label = name_to_check[:check_pos]
+    if cert_label == "*":
+        return True
+    elif cert_label.startswith("*"):
+        return check_label.endswith(cert_label[1:])
+    elif cert_label.endswith("*"):
+        return check_label.startswith(cert_label[:-1])
+    wildcard_pos = cert_name.find("*")
+    if wildcard_pos < 0:
+        return False
+    return check_label.startswith(cert_label[:wildcard_pos]) \
+            and check_label.endswith(cert_label[wildcard_pos + 1:])
+
 
 def check_server_dn(sock, expected_dn, expected_name):
     """
@@ -58,20 +97,20 @@ def check_server_dn(sock, expected_dn, expected_name):
             errors._raise_err(errors.ERR_INVALID_SERVER_CERT_DN,
                               expected_dn=expected_dn)
     else:
-        for name in cert.subject.get_attributes_for_oid(
-            x509.oid.NameOID.COMMON_NAME
-        ):
-            if name.value == expected_name:
-                return
         try:
             ext = cert.extensions.get_extension_for_oid(
                 x509.oid.ExtensionOID.SUBJECT_ALTERNATIVE_NAME
             )
             for name in ext.value.get_values_for_type(x509.DNSName):
-                if name == expected_name:
+                if _name_matches(expected_name, name):
                     return
         except x509.ExtensionNotFound:
             pass
+        for name in cert.subject.get_attributes_for_oid(
+            x509.oid.NameOID.COMMON_NAME
+        ):
+            if _name_matches(expected_name, name.value):
+                return
         errors._raise_err(errors.ERR_INVALID_SERVER_NAME,
                           expected_name=expected_name)
 

@eldis80
Copy link
Author

eldis80 commented Mar 7, 2025

I don't know how to test that in our environment but the logic seems fine to me.

@cjbj
Copy link
Member

cjbj commented Mar 7, 2025

@eldis80 once we check it a bit, we can push it to github and initiate a development build.

@anthony-tuininga
Copy link
Member

I have pushed a patch that corrects this issue and have initated a build from which you can download pre-built development wheels once it completes. You can also build from source if you prefer. If you can test your scenario and confirm the patch works as expected, that would be appreciated!

@eldis80
Copy link
Author

eldis80 commented Mar 18, 2025

I downloaded and installed from the build artifact (v3.1.0b1) and the certificate presented by ADB is accepted with this version.

@anthony-tuininga
Copy link
Member

This was included in python-oracledb 3.1.0 which was just released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working patch available
Projects
None yet
Development

No branches or pull requests

3 participants