PR#50735: Issue 50734 - lib389 creates non-SSCA cert DBs with misleading README.txt - 389-ds-base

389-ds-base

#50735 Issue 50734 - lib389 creates non-SSCA cert DBs with misleading README.txt

Closed 3 years ago by spichugi. Opened 4 years ago by mhonek.

mhonek/389-ds-base fix-non-ssca-readme-creation into master

Issue 50734 - lib389 creates non-SSCA cert DBs with misleading README.txt

Matus Honek • 4 years ago

ff75058

src/lib389/lib389/instance/setup.py

file modified

+1 -1

		`@@ -822,7 +822,7 @@`
		`assert_c(ds_instance.exists(), "Instance failed to install, does not exist when expected")`

		`# Create a certificate database.`
		`- tlsdb = NssSsl(dbpath=slapd['cert_dir'])`
		`+ tlsdb = NssSsl(dirsrv=ds_instance, dbpath=slapd['cert_dir'])`
		`if not tlsdb._db_exists():`
		`tlsdb.reinit()`

src/lib389/lib389/nss_ssl.py

file modified

+7 -6

		`@@ -151,18 +151,19 @@`
		`except FileExistsError:`
		`pass`

		`- # Write a README to let people know what this is`
		`- readme_file = '%s/%s' % (self._certdb, 'README.txt')`
		`- if not os.path.exists(readme_file):`
		`- with open(readme_file, 'w') as f:`
		`- f.write("""`
		`+ if self.dirsrv is None:`
		`+ # Write a README to let people know what this is`
		`+ readme_file = '%s/%s' % (self._certdb, 'README.txt')`
		`+ if not os.path.exists(readme_file):`
		`+ with open(readme_file, 'w') as f:`
		`+ f.write("""`
		`SSCA - Simple Self-Signed Certificate Authority`

		`This is part of the 389 Directory Server project's lib389 toolkit. It`
		`creates a simple, standalone certificate authority for testing and`
		`development purposes. It's suitable for evaluation and testing purposes`
		`only.`
		`- """)`
		`+ """)`

		`# In the future we may add the needed option to avoid writing the pin`
		`# files.`

mhonek commented 4 years ago

Bug Description:
NssSsl always creates README.txt which describes the purpose of SSCA, even
when creating only an instance-specific certificate database.

Fix Description:
Create the README.txt only when creating cert DB for a specified DS instance.

Fixes https://pagure.io/389-ds-base/issue/50734

Author: Matus Honek mhonek@redhat.com

Review by: ???

mreynolds commented 4 years ago

ack

firstyear commented 4 years ago

I'm not sure what your concern here is. The statement is true, we self-sign certs out of box and they really are only for evaluation.

I'd rather have the text updated to communicate better if you have some kind of concern here.

mhonek commented 4 years ago

I'm not sure what your concern here is. The statement is true, we self-sign certs out of box and they really are only for evaluation.
I'd rather have the text updated to communicate better if you have some kind of concern here.

My concern is that an instance's DB is not an SSCA. Only the one in /etc/dirsrv/ssca is. And also that in case of an instance the directory contains whole lot of other files than just the cert DB's ones - should we describe them in the README.txt, too?

How should we rather handle this README.txt in the context of an instance? Update a presence of the file (with a different contents, of course) based on the state-dependent fact of whether the cert DB contains some self-signed cert signed by only our SSCA? -- I believe it is not feasible, not worth the effort. Another possible implementation would be to state within the README that the cert DB could contain a SSCA-signed cert, which is more confusing than useful.

If we would like to warn a user of using SSCA signed cert then that should be done in other places, like dsconf security warning, ns-slapd startup warning; dscreate should state it at the creation time loudly if it does not, yet.

firstyear commented 4 years ago

ahh okay, I clearly missed that detail. Apparently I forgot that ssca uses a seperate database despite the fact I wrote that code ....

In that case there is nothing wrong with this then. I think that dsctl healthcheck is a better place to build on the back of @mreynolds' recent work if we want to alert to ssca usage. Dscreate I think is the wrong place, there is a LOT of information thrown at the user in the logs, so we shouldn't put it there unless we had dscreate emit a post-install checklist or something that admins should follow?

Anyway, in that case, ack from me.

rebased onto ff75058

4 years ago

Pull-Request has been merged by mhonek

4 years ago

mhonek commented 4 years ago

@firstyear Your idea for healthcheck: #50759.