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

Errors from certutil are not propagated #4736

Closed
vashirov opened this issue Apr 23, 2021 · 5 comments
Closed

Errors from certutil are not propagated #4736

vashirov opened this issue Apr 23, 2021 · 5 comments
Assignees
Labels
CLI CLI tools lib389 Involves lib389 librabry priority_high need urgent fix / highly valuable / easy to fix
Milestone

Comments

@vashirov
Copy link
Member

Issue Description
When an error occurs with certutil, stderr output from it is not printed:

[root@fedora ~]# dsctl localhost tls generate-server-cert-csr -s "bad" 
Error: Command '['/usr/bin/certutil', '-R', '--keyUsage', 'digitalSignature,nonRepudiation,keyEncipherment,dataEncipherment', '--nsCertType', 'sslClient,sslServer', '--extKeyUsage', 'clientAuth,serverAuth', '-s', 'bad', '-8', 'fedora', '-g', '4096', '-d', '/etc/dirsrv/slapd-localhost', '-z', '/etc/dirsrv/slapd-localhost/noise.txt', '-f', '/etc/dirsrv/slapd-localhost/pwdfile.txt', '-a', '-o', '/etc/dirsrv/slapd-localhost/Server-Cert.csr']' returned non-zero exit status 255.

It just states that the exit status is 255, but no certutil output, which in this case is

certutil -s: improperly formatted name: "bad"

In lib389/nss_ssl.py we have many places with statements like this that show this behaviour:

        result = ensure_str(check_output(cmd, stderr=subprocess.STDOUT))
        self.log.debug("nss output: %s", result)

Package Version and Platform:

  • Platform: Fedora
  • Package and version: 389-ds-base-2.0.3-3.fc34.x86_64

Steps to Reproduce
Steps to reproduce the behavior:

  1. dsctl localhost tls generate-server-cert-csr -s "bad"

Expected results
Instead of a python traceback, a clear error message should be printed.

@vashirov vashirov added lib389 Involves lib389 librabry CLI CLI tools needs triage The issue will be triaged during scrum labels Apr 23, 2021
@tbordaz tbordaz added priority_high need urgent fix / highly valuable / easy to fix and removed needs triage The issue will be triaged during scrum labels Apr 29, 2021
@tbordaz tbordaz added this to the 1.4.3 milestone Apr 29, 2021
@tbordaz tbordaz added the Need BZ The ticket needs to be cloned to a BZ label Jun 16, 2021
@tbordaz
Copy link
Contributor

tbordaz commented Jun 16, 2021

Create a BZ RHDS

@mreynolds389 mreynolds389 self-assigned this Aug 2, 2021
@mreynolds389 mreynolds389 removed the Need BZ The ticket needs to be cloned to a BZ label Aug 2, 2021
@mreynolds389
Copy link
Contributor

mreynolds389 added a commit to mreynolds389/389-ds-base that referenced this issue Aug 4, 2021
Description:  Errors from certutil are not returned to the client, and
only a generic failure code is returned.  The actual error text should be
returned to the client since it has meaning.  Just catch all the
exception and return the output as a ValueError.

relates: 389ds#4736

Reviewed by: firstyear (Thanks!)
mreynolds389 added a commit that referenced this issue Aug 4, 2021
Description:  Errors from certutil are not returned to the client, and
only a generic failure code is returned.  The actual error text should be
returned to the client since it has meaning.  Just catch all the
exception and return the output as a ValueError.

relates: #4736

Reviewed by: firstyear (Thanks!)
mreynolds389 added a commit that referenced this issue Aug 4, 2021
Description:  Errors from certutil are not returned to the client, and
only a generic failure code is returned.  The actual error text should be
returned to the client since it has meaning.  Just catch all the
exception and return the output as a ValueError.

relates: #4736

Reviewed by: firstyear (Thanks!)
mreynolds389 added a commit that referenced this issue Aug 4, 2021
Description:  Errors from certutil are not returned to the client, and
only a generic failure code is returned.  The actual error text should be
returned to the client since it has meaning.  Just catch all the
exception and return the output as a ValueError.

relates: #4736

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

4bd1c94..b99236a 389-ds-base-1.4.4 -> 389-ds-base-1.4.4
a68decb..c0dd585 389-ds-base-1.4.3 -> 389-ds-base-1.4.3

@vashirov
Copy link
Member Author

Here certutil is called twice:

result = ensure_str(check_output(cmd, stderr=subprocess.STDOUT))
try:
result = ensure_str(check_output(cmd, stderr=subprocess.STDOUT))

On the second call it asks for the password again, because it thinks it is a db upgrade.

@vashirov vashirov reopened this Aug 10, 2021
mreynolds389 added a commit that referenced this issue Aug 10, 2021
Description: A regression in the previous commit accidentally called
certutil twice which triggered the CLI to prompt for the NSS database
password.  This broke CI tests, etc.

relates: #4736

Reviewed by: mreynolds (one line commit rule)
mreynolds389 added a commit that referenced this issue Aug 10, 2021
Description: A regression in the previous commit accidentally called
certutil twice which triggered the CLI to prompt for the NSS database
password.  This broke CI tests, etc.

relates: #4736

Reviewed by: mreynolds (one line commit rule)
mreynolds389 added a commit that referenced this issue Aug 10, 2021
Description: A regression in the previous commit accidentally called
certutil twice which triggered the CLI to prompt for the NSS database
password.  This broke CI tests, etc.

relates: #4736

Reviewed by: mreynolds (one line commit rule)
mreynolds389 added a commit that referenced this issue Aug 10, 2021
Description: A regression in the previous commit accidentally called
certutil twice which triggered the CLI to prompt for the NSS database
password.  This broke CI tests, etc.

relates: #4736

Reviewed by: mreynolds (one line commit rule)
@mreynolds389
Copy link
Contributor

Yeah that was it, thanks @vashirov !!!!

f4f255d..0208b72 master -> master
1bbef77..c7a6796 389-ds-base-2.0 -> 389-ds-base-2.0
b99236a..07b44fe 389-ds-base-1.4.4 -> 389-ds-base-1.4.4
c0dd585..1314143 389-ds-base-1.4.3 -> 389-ds-base-1.4.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI CLI tools lib389 Involves lib389 librabry priority_high need urgent fix / highly valuable / easy to fix
Projects
None yet
Development

No branches or pull requests

3 participants