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

Importing a ca chain only imports the first certificate #5162

Closed
Firstyear opened this issue Feb 11, 2022 · 8 comments · Fixed by #5164
Closed

Importing a ca chain only imports the first certificate #5162

Firstyear opened this issue Feb 11, 2022 · 8 comments · Fixed by #5164
Milestone

Comments

@Firstyear
Copy link
Contributor

A ca chain pem file is just a series of pem certs concatinated. NSS of course, when presented with a chain file will only import the first certificate, and will silently ignore the rest.

This can create user confusion who expect to be able to import a chain file.

While we can't fix NSS, we CAN fix dsctl instance tls import commands such that when a chain file is presented we can inform the user they need to split it and import the components individually.

@Firstyear Firstyear added the needs triage The issue will be triaged during scrum label Feb 11, 2022
@mreynolds389
Copy link
Contributor

Related downstream bug: https://bugzilla.redhat.com/show_bug.cgi?id=1878808

Firstyear added a commit to Firstyear/389-ds-base that referenced this issue Feb 11, 2022
Bug Description: Nss can't import pem chain files which can
confuse users why they have missing certificates when they try
to import a chain.

Fix Description: Error out on chain files in any of the import
paths since they are ambiguous.

fixes: 389ds#5162

Author: William Brown <william@blackhats.net.au>

Review by: ???
Firstyear added a commit that referenced this issue Mar 3, 2022
Bug Description: Nss can't import pem chain files which can
confuse users why they have missing certificates when they try
to import a chain.

Fix Description: Error out on chain files in any of the import
paths since they are ambiguous.

fixes: #5162

Author: William Brown <william@blackhats.net.au>

Review by: @droideck
Firstyear added a commit to Firstyear/389-ds-base that referenced this issue Mar 3, 2022
Bug Description: Nss can't import pem chain files which can
confuse users why they have missing certificates when they try
to import a chain.

Fix Description: Error out on chain files in any of the import
paths since they are ambiguous.

fixes: 389ds#5162

Author: William Brown <william@blackhats.net.au>

Review by: @droideck
mreynolds389 pushed a commit that referenced this issue Apr 14, 2022
Bug Description: Nss can't import pem chain files which can
confuse users why they have missing certificates when they try
to import a chain.

Fix Description: Error out on chain files in any of the import
paths since they are ambiguous.

fixes: #5162

Author: William Brown <william@blackhats.net.au>

Review by: @droideck
mreynolds389 added a commit to mreynolds389/389-ds-base that referenced this issue Nov 15, 2022
Description:  Parse PEM file and add each CA cert separately.
              Fixed some PEP8 errors.

relates: 389ds#5162

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue Nov 15, 2022
Description:  Parse PEM file and add each CA cert separately.
              Fixed some PEP8 errors.

relates: #5162

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue Nov 15, 2022
Description:  Parse PEM file and add each CA cert separately.
              Fixed some PEP8 errors.

relates: #5162

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue Nov 15, 2022
Description:  Parse PEM file and add each CA cert separately.
              Fixed some PEP8 errors.

relates: #5162

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue Nov 15, 2022
Description:  Parse PEM file and add each CA cert separately.
              Fixed some PEP8 errors.

relates: #5162

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue Nov 15, 2022
Description:  Parse PEM file and add each CA cert separately.
              Fixed some PEP8 errors.

relates: #5162

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue Nov 15, 2022
Description:  Parse PEM file and add each CA cert separately.
              Fixed some PEP8 errors.

relates: #5162

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

Allow CA bundles to be imported

78ee3a5..4a1e3bd 389-ds-base-2.2 -> 389-ds-base-2.2
b5e5afb..007e215 389-ds-base-2.1 -> 389-ds-base-2.1
ef2ec82..98aa09d 389-ds-base-2.0 -> 389-ds-base-2.0
ce0bda6..6f77f66 389-ds-base-1.4.4 -> 389-ds-base-1.4.4
fd904d5..1aaed99 389-ds-base-1.4.3 -> 389-ds-base-1.4.3

mreynolds389 added a commit to mreynolds389/389-ds-base that referenced this issue Nov 15, 2022
Description:  Incorrectly added "required=True" to positional arg nickname
when adding ca-cert.

relates: 389ds#5162

Reviewed by: ?
mreynolds389 added a commit that referenced this issue Nov 15, 2022
Description:  Incorrectly added "required=True" to positional arg nickname
when adding ca-cert.

relates: #5162

Reviewed by: ?
mreynolds389 added a commit that referenced this issue Nov 15, 2022
Description:  Incorrectly added "required=True" to positional arg nickname
when adding ca-cert.

relates: #5162

Reviewed by: ?
mreynolds389 added a commit that referenced this issue Nov 15, 2022
Description:  Incorrectly added "required=True" to positional arg nickname
when adding ca-cert.

relates: #5162

Reviewed by: ?
mreynolds389 added a commit that referenced this issue Nov 15, 2022
Description:  Incorrectly added "required=True" to positional arg nickname
when adding ca-cert.

relates: #5162

Reviewed by: ?
mreynolds389 added a commit that referenced this issue Nov 15, 2022
Description:  Incorrectly added "required=True" to positional arg nickname
when adding ca-cert.

relates: #5162

Reviewed by: ?
mreynolds389 added a commit that referenced this issue Nov 15, 2022
Description:  Incorrectly added "required=True" to positional arg nickname
when adding ca-cert.

relates: #5162

Reviewed by: ?
@mreynolds389
Copy link
Contributor

Fix regression

dde3fc6..69235d8 389-ds-base-2.2 -> 389-ds-base-2.2
c9c7a01..7abaa06 389-ds-base-2.1 -> 389-ds-base-2.1
98aa09d..12bca39 389-ds-base-2.0 -> 389-ds-base-2.0
6f77f66..8cf6cfd 389-ds-base-1.4.4 -> 389-ds-base-1.4.4
1aaed99..7f311bb 389-ds-base-1.4.3 -> 389-ds-base-1.4.3

@aivanov389
Copy link

aivanov389 commented Nov 22, 2022

after this commit (at least, in the version 2.0, commit 98aa09d) the file security.py uses the function "log.info()". The result is that it is impossible to set some (maybe even any?) security property:


[root@ldap-model Admin]# /usr/sbin/dsconf model security set --tls-protocol-min="TLS1.2"
Error: name 'log' is not defined


[root@ldap-model Admin]# /usr/sbin/dsconf slapd-model security set --allow-weak-dh-param yes
Error: name 'log' is not defined

It's probably a mistype in the argument name of the functions "def _security_generic_get(inst, basedn, logs, args, attrs_map):" and "def _security_generic_set(inst, basedn, logs, args, attrs_map):". The argument name "logs" should be replaced by "log"?

def _security_generic_get(inst, basedn, **logs**, args, attrs_map):
    result = {}
    for attr, props in attrs_map.items():
        val = props.cls(inst).get_attr_val_utf8(props.attr)
        if val is None:
            val = ""
        result[props.attr.lower()] = val
    if args.json:
        print(json.dumps({'type': 'list', 'items': result}, indent=4))
    else:
        print('\n'.join([f'{attr}: {value or ""}' for attr, value in result.items()]))


def _security_generic_set(inst, basedn, **logs**, args, attrs_map):
    for attr, props in attrs_map.items():
        arg = getattr(args, attr.replace('-', '_'))
        if arg is None:
            continue
        dsobj = props.cls(inst)
        if arg != "":
            dsobj.replace(props.attr, arg)
        else:
            dsobj.remove_all(props.attr)
    log.info(f"Successfully updated security configuration ({props.attr})")

@progier389 progier389 added this to the 1.4.3 milestone Nov 22, 2022
@progier389 progier389 removed the needs triage The issue will be triaged during scrum label Nov 22, 2022
@progier389
Copy link
Contributor

progier389 commented Nov 22, 2022

The 'log' typo is now fixed. See issue 5539

droideck pushed a commit to droideck/389-ds-base that referenced this issue Feb 16, 2023
Description:  Verify that when importing a certificate that is the correct
              type.  Also cleanup temporary certs that are created when
              processing a bundle of certs in a PEM file.

relates: 389ds#5162

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit to mreynolds389/389-ds-base that referenced this issue Feb 17, 2023
Description:  Verify that when importing a certificate that is the correct
              type.  Also cleanup temporary certs that are created when
              processing a bundle of certs in a PEM file.

relates: 389ds#5162

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue Feb 17, 2023
Description:  Verify that when importing a certificate that is the correct
              type.  Also cleanup temporary certs that are created when
              processing a bundle of certs in a PEM file.

relates: #5162

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue Feb 17, 2023
Description:  Verify that when importing a certificate that is the correct
              type.  Also cleanup temporary certs that are created when
              processing a bundle of certs in a PEM file.

relates: #5162

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue Feb 17, 2023
Description:  Verify that when importing a certificate that is the correct
              type.  Also cleanup temporary certs that are created when
              processing a bundle of certs in a PEM file.

relates: #5162

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue Feb 17, 2023
Description:  Verify that when importing a certificate that is the correct
              type.  Also cleanup temporary certs that are created when
              processing a bundle of certs in a PEM file.

relates: #5162

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue Feb 17, 2023
Description:  Verify that when importing a certificate that is the correct
              type.  Also cleanup temporary certs that are created when
              processing a bundle of certs in a PEM file.

relates: #5162

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

d992302..c6613be 389-ds-base-2.2 -> 389-ds-base-2.2
fdc7a57..5f51502 389-ds-base-2.1 -> 389-ds-base-2.1
9a71f2b..332e487 389-ds-base-2.0 -> 389-ds-base-2.0
a8cb9f4..b85508b 389-ds-base-1.4.3 -> 389-ds-base-1.4.3

mreynolds389 added a commit to mreynolds389/389-ds-base that referenced this issue Feb 22, 2023
Description:  With recent changes to certificate validation the error message
has changed and the CI needs to be updated.

relates: 389ds#5162

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue Feb 22, 2023
Description:  With recent changes to certificate validation the error message
has changed and the CI needs to be updated.

relates: #5162

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue Feb 22, 2023
Description:  With recent changes to certificate validation the error message
has changed and the CI needs to be updated.

relates: #5162

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue Feb 22, 2023
Description:  With recent changes to certificate validation the error message
has changed and the CI needs to be updated.

relates: #5162

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue Feb 22, 2023
Description:  With recent changes to certificate validation the error message
has changed and the CI needs to be updated.

relates: #5162

Reviewed by: spichugi(Thanks!)
mreynolds389 added a commit that referenced this issue Feb 22, 2023
Description:  With recent changes to certificate validation the error message
has changed and the CI needs to be updated.

relates: #5162

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

Fix CI test

064a3f7..764a306 389-ds-base-2.2 -> 389-ds-base-2.2
055c14d..9476a78 389-ds-base-2.1 -> 389-ds-base-2.1
f8aca8e..0ffe4ab 389-ds-base-2.0 -> 389-ds-base-2.0
35b76e0..e10f1c6 389-ds-base-1.4.3 -> 389-ds-base-1.4.3

progier389 added a commit that referenced this issue Sep 6, 2023
Partial backport of PR #5525 towards 1.4.3
contains the changes about SELinux messages and some code cleanup to avoid warnings.

Issue: #5162 (Originally about about a cockpit improvement but it also contains BZ [2149967] fix
@progier389
Copy link
Contributor

ba7ad9a4f..ee1e525b0 389-ds-base-1.4.3 -> 389-ds-base-1.4.3
Because BZ 2149967 part was not included in 1.4.3 branch unlike main and 2.x branches

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants