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

Broken import of CA certs #5768

Closed
lolllpop opened this issue May 12, 2023 · 2 comments
Closed

Broken import of CA certs #5768

lolllpop opened this issue May 12, 2023 · 2 comments
Assignees
Labels
CLI CLI tools Container lib389 Involves lib389 librabry

Comments

@lolllpop
Copy link

Issue Description
Importing CA certs from /data/tls/ca/*.crt into docker container results in error message, container does not start.

Package Version and Platform:

  • Platform: docker / podman 4.4.1
  • Package and version: docker.io/389ds/dirsrv:2.4

Steps to Reproduce

  1. setup folder containing the certs, all in PEM format
ssl/
├── ca/
│   ├── ca-inter.crt
│   └── ca-root.crt
├── server.crt
└── server.key
  1. start docker container
podman run --replace --name 389ds \
  --volume ./ssl:/data/tls \
  --env DS_DM_PASSWORD=xxx123 \
  --env DS_SUFFIX_NAME=dc=my,dc=org \
  docker.io/389ds/dirsrv:2.4
  1. Error message importing a CA cert
INFO: Checking for PEM TLS files ...
INFO: Found -> ['ca-root.crt', 'ca-inter.crt']
INFO: Have /data/tls/server.key -> True
INFO: Have /data/tls/server.crt -> True
INFO: Have /data/tls/ca -> True
INFO: Have /data/config/pwdfile.txt -> True
INFO: TLS PEM requirements met - configuring NSSDB ...
DEBUG: Allocate local instance <class 'lib389.DirSrv'> with None
DEBUG: del_cert cmd: /usr/bin/certutil -D -d /data/config -n Server-Cert -f /data/config/pwdfile.txt
INFO: Enrolling -> /data/tls/ca/ca-root.crt
Traceback (most recent call last):
  File "/usr/lib/dirsrv/dscontainer", line 467, in <module>
    begin_magic()
  File "/usr/lib/dirsrv/dscontainer", line 300, in begin_magic
    _begin_setup_pem_tls()
  File "/usr/lib/dirsrv/dscontainer", line 141, in _begin_setup_pem_tls
    tls.add_cert(nickname=ca_path, input_file=ca_path)
  File "/usr/lib/python3.10/site-packages/lib389/nss_ssl.py", line 1187, in add_cert
    raise ValueError(f"Certificate ({nickname}) is not a server certificate")
ValueError: Certificate (/data/tls/ca/ca-root.crt) is not a server certificate

Expected results
CA certs get imported and used, e.g. for replication via LDAPs later on.

Additional context
dscontainer looks for CA files only at /data/tls/ca/*.crt and stores their filenames in variable cas. It tries to add them using tls.add_cert, but calls this function without parameter ca=True. Accordingly, the CA certs get treated as server certs in function add_cert, but the check cert_is_ca identifies them correctly CA certs, and above error message is raised.

From 389-ds-base/src/lib389/cli/dscontainer, line 139

 # Import the ca's
    for ca_path in [os.path.join(CONTAINER_TLS_SERVER_CADIR, ca) for ca in cas]:
        log.info("Enrolling -> %s" % ca_path)
        tls.add_cert(nickname=ca_path, input_file=ca_path)
        tls.edit_cert_trust(ca_path, "C,,")

From 389-ds-base/src/lib389/lib389 /nss_ssl.py, line 1165

    def add_cert(self, nickname, input_file, ca=False):
        """Add server or CA cert
        """

        # Verify input_file exists
        if not os.path.exists(input_file):
            raise ValueError("The certificate file ({}) does not exist".format(input_file))

        pem_file = True
        if not input_file.lower().endswith(".pem"):
            pem_file = False
        else:
            self._assert_not_chain(input_file)

        if ca:
            # Verify this is a CA cert
            if not cert_is_ca(input_file):
                raise ValueError(f"Certificate ({nickname}) is not a CA certificate")
            trust_flags = "CT,,"
        else:
            # Verify this is a server cert
            if cert_is_ca(input_file):
                raise ValueError(f"Certificate ({nickname}) is not a server certificate")
            trust_flags = ",,"

The cert_is_ca verification was introduced by commit c69f269 three month ago. Using an older dirsrv 2.2, the ca certs still get imported:

podman run --replace --name 389ds \
  --volume ./ssl:/data/tls \
  --env DS_DM_PASSWORD=xxx123 \
  --env DS_SUFFIX_NAME=dc=my,dc=org \
  docker.io/389ds/dirsrv:2.2
INFO: Checking for PEM TLS files ...
INFO: Found -> ['ca-root.crt', 'ca-inter.crt']
INFO: Have /data/tls/server.key -> True
INFO: Have /data/tls/server.crt -> True
INFO: Have /data/tls/ca -> True
INFO: Have /data/config/pwdfile.txt -> True
INFO: TLS PEM requirements met - configuring NSSDB ...
DEBUG: Allocate local instance <class 'lib389.DirSrv'> with None
DEBUG: del_cert cmd: /usr/bin/certutil -D -d /data/config -n Server-Cert -f /data/config/pwdfile.txt
INFO: Enrolling -> /data/tls/ca/ca-root.crt
DEBUG: add_cert cmd: /usr/bin/certutil -A -d /data/config -n /data/tls/ca/ca-root.crt -t ,, -i /data/tls/ca/ca-root.crt -a -f /data/config/pwdfile.txt
DEBUG: edit_cert_trust cmd: /usr/bin/certutil -M -d /data/config -n /data/tls/ca/ca-root.crt -t C,, -f /data/config/pwdfile.txt
INFO: Enrolling -> /data/tls/ca/ca-inter.crt
DEBUG: add_cert cmd: /usr/bin/certutil -A -d /data/config -n /data/tls/ca/ca-inter.crt -t ,, -i /data/tls/ca/ca-inter.crt -a -f /data/config/pwdfile.txt
DEBUG: edit_cert_trust cmd: /usr/bin/certutil -M -d /data/config -n /data/tls/ca/ca-inter.crt -t C,, -f /data/config/pwdfile.txt
DEBUG: Importing key and cert -> /data/tls/server.key, /data/tls/server.crt
DEBUG: nss cmd: openssl pkcs12 -export -in /data/tls/server.crt -inkey /data/tls/server.key -out /data/config/temp_server_key_cert.p12 -name Server-Cert -passout pass: -aes128
DEBUG: del_cert cmd: /usr/bin/certutil -D -d /data/config -n Server-Cert -f /data/config/pwdfile.txt
DEBUG: nss cmd: pk12util -v -i /data/config/temp_server_key_cert.p12 -d /data/config -k /data/config/pwdfile.txt -W ''
INFO: TLS PEM configuration complete.
INFO: Starting 389-ds-container ...
@lolllpop lolllpop added the needs triage The issue will be triaged during scrum label May 12, 2023
@mreynolds389
Copy link
Contributor

mreynolds389 commented May 15, 2023

Correct, function add_cert() needs to set ca=True (this flag was added recently). Fix is in the works...

@mreynolds389 mreynolds389 self-assigned this May 15, 2023
@mreynolds389 mreynolds389 added lib389 Involves lib389 librabry CLI CLI tools Container and removed needs triage The issue will be triaged during scrum labels May 15, 2023
mreynolds389 added a commit to mreynolds389/389-ds-base that referenced this issue May 18, 2023
Description:

The certificate type checks for CA/server break if there are no certificate
extensions set (use openssl in that case to gather the info instead).
dscontainter needed to be updated for new cert checks, and UI adding certs
improvements.

relates: 389ds#5768

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue May 18, 2023
Description:

The certificate type checks for CA/server break if there are no certificate
extensions set (use openssl in that case to gather the info instead).
dscontainter needed to be updated for new cert checks, and UI adding certs
improvements.

relates: #5768

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue May 18, 2023
Description:

The certificate type checks for CA/server break if there are no certificate
extensions set (use openssl in that case to gather the info instead).
dscontainter needed to be updated for new cert checks, and UI adding certs
improvements.

relates: #5768

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue May 18, 2023
Description:

The certificate type checks for CA/server break if there are no certificate
extensions set (use openssl in that case to gather the info instead).
dscontainter needed to be updated for new cert checks, and UI adding certs
improvements.

relates: #5768

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue May 18, 2023
Description:

The certificate type checks for CA/server break if there are no certificate
extensions set (use openssl in that case to gather the info instead).
dscontainter needed to be updated for new cert checks, and UI adding certs
improvements.

relates: #5768

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue May 18, 2023
Description:

The certificate type checks for CA/server break if there are no certificate
extensions set (use openssl in that case to gather the info instead).
dscontainter needed to be updated for new cert checks, and UI adding certs
improvements.

relates: #5768

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue May 18, 2023
Description:

The certificate type checks for CA/server break if there are no certificate
extensions set (use openssl in that case to gather the info instead).
dscontainter needed to be updated for new cert checks, and UI adding certs
improvements.

relates: #5768

Reviewed by: spichugi(Thanks!)
@mreynolds389
Copy link
Contributor

dfa4e81..c0076d0 389-ds-base-2.3 -> 389-ds-base-2.3
5a96da0..9e8b4b1 389-ds-base-2.2 -> 389-ds-base-2.2
c95231b..231ebd4 389-ds-base-2.1 -> 389-ds-base-2.1
77cb171..c0b8a21 389-ds-base-2.0 -> 389-ds-base-2.0
6aed0ea..cc53fd8 389-ds-base-1.4.3 -> 389-ds-base-1.4.3

lab-at-nohl pushed a commit to lab-at-nohl/cockpit-389-ds-containerproxy that referenced this issue May 9, 2024
Description:

The certificate type checks for CA/server break if there are no certificate
extensions set (use openssl in that case to gather the info instead).
dscontainter needed to be updated for new cert checks, and UI adding certs
improvements.

relates: 389ds/389-ds-base#5768

Reviewed by: spichugi(Thanks!)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI CLI tools Container lib389 Involves lib389 librabry
Projects
None yet
Development

No branches or pull requests

2 participants