Skip to content
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

load_certificate can't load certificate created with non-matching hostname and CN #55

Open
lawndoc opened this issue Nov 15, 2021 · 4 comments
Assignees
Labels
bug Something isn't working documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@lawndoc
Copy link
Member

lawndoc commented Nov 15, 2021

If you issue a certificate with non-matching hostname and common_name values, you cannot use load_certificate() to fetch the certificate for later use. You end up with one of the following errors:

Using the hostname as the argument value:

ownca.exceptions.OwnCAInconsistentData: Initialized CN name does not match with current existent common_name: <hostname>

Using the CN as the argument value:

ownca.exceptions.OwnCAInvalidCertificate: The certificate does not exist for 'example.com'.

Example code that will reproduce the issue:

from ownca import CertificateAuthority
from ownca.exceptions import OwnCAInconsistentData, OwnCAInvalidCertificate

CA_CERT_DIR = <enter your own for testing>

ca = CertificateAuthority(ca_storage=CA_CERT_DIR, common_name="ownca")

serverCert = ca.issue_certificate(hostname="frontend",
                                  maximum_days=825,
                                  common_name="example.com",
                                  dns_names=[],
                                  oids=certOids,
                                  public_exponent=65537,
                                  key_size=2048)

print("-- load_certificate(<hostname>) --")
try: 
    sameCert = ca.load_certificate("frontend")
except OwnCAInconsistentData as e:
    print(e)

print("-- load_certificate(<common_name>) --")
try: 
    sameCert = ca.load_certificate("example.com")
except OwnCAInvalidCertificate as e:
    print(e)
@kairoaraujo kairoaraujo added bug Something isn't working documentation Improvements or additions to documentation good first issue Good for newcomers labels Nov 16, 2021
@kairoaraujo
Copy link
Member

Hi @lawndoc, thank you for reporting the issue.

I see that the documentation is unclear, and the code does not protect it.

The common_name and hostname should be used as the primary identification of the certificate.
Different DNS names should go to dns_names.

ca.issue_certificate(
    "frontend",
     maximum_days=825,
     dns_names=["example.com"],
     oids=certOids,
     public_exponent=65537,
     key_size=2048
)

However, I would like to see if this would cover your use case. Otherwise, I could plan a better fix for it.

@kairoaraujo kairoaraujo self-assigned this Nov 16, 2021
@lawndoc
Copy link
Member Author

lawndoc commented Nov 17, 2021

Should common_name and hostname always be the same? If so, I think the code should be modified to enforce that. If not, it should probably allow for that in load_certificate. What do you think?

@kairoaraujo
Copy link
Member

Yes, the common_name (CN) and hostname should be the same.
For example, when we think about the SSL Certificates, we should approach them as a single thing.

https://knowledge.digicert.com/solution/SO7239.html
https://support.dnsimple.com/articles/what-is-common-name/

So we need to apply the changes in the code and deprecate hostname IMO.
Also, make the documentation more clear about using the dns_names

@lawndoc
Copy link
Member Author

lawndoc commented Nov 18, 2021

That sounds like a good solution to me. Not that there are thousands of users this would affect right now, but do you think there is a clean way to deprecate hostname without hurting backward-compatibility between versions? Or is it better to rip off the band-aid and replace instances of hostname with common_name?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants