[Pkg-freeipa-devel] [Git][freeipa-team/certmonger][master] 53 commits: Improve logging in SCEP helper

Timo Aaltonen gitlab at salsa.debian.org
Thu Jul 30 17:48:07 BST 2020 


Timo Aaltonen pushed to branch master at FreeIPA packaging / certmonger


Commits:
f9342db6 by Rob Crittenden at 2020-04-14T11:50:17-04:00
Improve logging in SCEP helper

Always check return value of cm_pkcs7_verify_signed() and return
a unique error message.

Change log level from 1 to 0 for all errors in scep.c and pkcs7.c
so they appear by default.

Centralize logging across scep.c and pkcs7.c to reduce code
duplication.

Check the return code to cm_pkcs7_verify_signed in all cases.

Add the last available message, if any, to the error returned
via stdout to certmonger as a hint to what is going on.

- - - - -
58cc1f26 by Rob Crittenden at 2020-04-14T11:50:17-04:00
Add verbose option to SCEP CA if requested in add-scep-ca

This option was silently dropped from the helper arguments even
if requested on the add-scep-ca CLI and was only passed to the
dbus helper.

Add as many -v as requested though the scep helper only logs at
most at level 1.

- - - - -
3881b8a9 by Your Name at 2020-04-15T18:39:55-04:00
Cleanup the SCEP helper curl and talloc contexts when finished

The talloc context was freed in only a few cases and the curl
context was never freed.

- - - - -
71f5fbdb by Rob Crittenden at 2020-04-17T10:16:46-04:00
Re-order the way the SCEP signing and CA certs are collected

Put cacert into the ca store, the racert at the top of the
othercerts list. Then we parse certs, placing all ca certs
we find into the ca store, and all other certs we find after
the racert.

Variables are renamed to match the cm_pkcs7_parse() and
cm_pkcs7_verify_signed() calls.

A special case for IPA (dogtag) was added because dogtag
uses its CA cert to sign the PKCS7 so it is both an RA cert
and a CA cert. If a self-signed CA is detected and no other
certs are provided then the CA is treated as the RA.

https://bugzilla.redhat.com/show_bug.cgi?id=1808052

Graham Leggett did the majority of the work on this patch.

- - - - -
230b640e by Rob Crittenden at 2020-04-17T16:57:44-04:00
Add new option to allow overriding the detected SCEP CA chain

The -R option was doing double-duty for the SCEP CA.

1. It was required if the SCEP URL used TLS
2. It override the CA certificate downloaded from the SCEP server

If the chains were different then validating the SCEP responses would
fail.

https://bugzilla.redhat.com/show_bug.cgi?id=1808613

- - - - -
01b6db9d by Rob Crittenden at 2020-04-24T13:30:59-04:00
template_profile, issuer and MS certificat template are single-value

These can only be single valued so trying to retrieve them as
multi-value will always return NULL. Use the single API instead
to correctly retrieve and display the values.

- - - - -
2e617297 by Rob Crittenden at 2020-04-28T15:03:48-04:00
Fix broken -N option, helper argument list

There was an extra NULL value which caused it to not work.

A %s was missing from the argument format string.

- - - - -
f78f6eb0 by Rob Crittenden at 2020-05-01T16:02:20-04:00
Ensure that files read in have a trailing new-line

In SCEP when retrieving the CA chain the certificates passed in
on the command-line (RA agent and CA cert) area printed along with
the contents of what was retrieved remotely.

If one of the filesystem certificates lacks a newline then the
output will be jumbled like:

-----END CERTIFICATE----------BEGIN CERTIFICATE-----\n

https://bugzilla.redhat.com/show_bug.cgi?id=1814976

- - - - -
39ce89ec by Rob Crittenden at 2020-05-08T11:43:37-04:00
Adapt to the new behavior of disconnect in dbus-broker

The dbus server was replaced in Fedora-29 to use dbus-broker.

This server does not allow reconnects on a disconnect. certmonger
was crashing as a result.

The only way to directly pass a message between the dbus queue and
the main tevent handle is via a signal. So when a disconnect is
detected send a SIGTERM to getpid() in order to force a graceful
shutdown.

https://bugzilla.redhat.com/show_bug.cgi?id=1687698

- - - - -
525a9b4f by Rob Crittenden at 2020-05-14T13:31:41-04:00
Add long command-line options to man pages

The man pages almost universally only documented the short
options even though the long options were all defined in
the popt configuration.

Also do a bit of minor bit of reformatting and added a lint
option. I'm not going to require mandoc as a requirement as
the linting is pretty minor at the moment but it's better than
nothing.

https://bugzilla.redhat.com/show_bug.cgi?id=1782838

- - - - -
6bf7aef2 by Rob Crittenden at 2020-05-14T13:31:41-04:00
Add long options to command-line help

The command-line help mostly consisted of only the short options.
Add the long-option and clean up some of the output.

https://bugzilla.redhat.com/show_bug.cgi?id=1782838

- - - - -
5c21bcbc by Rob Crittenden at 2020-06-01T13:28:17-04:00
Include &message=CA-IDENT with GetCACaps and GetCACert requests

The guttman spec is quite unclear on this and in the GetCACaps
section doesn't mention &message at all. It only appears in the
generic GET requests section 4.1

The nourse spec is clearer and requires &message=CA-IDENT on
GetCACaps requests.

AD 2012 R2 servers also require message on GetCACert requests.

This reverts much of 60a4db5796b0575ca2cc9f1af4ecb3fdc6359242

https://bugzilla.redhat.com/show_bug.cgi?id=1839181
https://pagure.io/certmonger/issue/103

- - - - -
41a7946b by Stanislav Levin at 2020-06-09T11:28:25+03:00
configure: Fix detection of supported NSS db type

For now configure always reports about the lack of support for
any NSS db:
checking if NSS supports "sql:" databases... no
checking if NSS supports "dbm:" databases... no

Actually, there is the linking issue, but AC_TRY_RUN is not
designed to detect this.

The recipe is the obeying the proper linking order:
$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS

Fixes: https://pagure.io/certmonger/issue/153
Signed-off-by: Stanislav Levin <slev at altlinux.org>

- - - - -
1fa94e53 by Fraser Tweedale at 2020-06-18T13:36:13+10:00
dogtag: use POST for profileProcess requests

An upcoming change to Dogtag requires HTTP POST method for
profileProcess operations.  Update certmonger to use POST for these
operations.

Fixes: https://pagure.io/freeipa/issue/8373

- - - - -
3668f5e0 by Stanislav Levin at 2020-06-24T18:05:23-04:00
tests: Split the NSS db specific tests

If NSS is configured with NSS_DISABLE_DBM then Certmonger's tests
which are related to DBM fail. The legacy NSS db type(DBM) will be
eventually disabled. Thus, Certmonger should handle this.

NSS db specific tests are placed under the corresponding
HAVE_SQL_NSSDB and HAVE_DBM_NSSDB sections.

Fixes: https://pagure.io/certmonger/issue/155
Signed-off-by: Stanislav Levin <slev at altlinux.org>

- - - - -
1bba4075 by Stanislav Levin at 2020-06-24T18:05:23-04:00
tests: Parametrize 025-casave with NSS db types

This test fails against NSS having disabled DBM.

Fixes: https://pagure.io/certmonger/issue/155
Signed-off-by: Stanislav Levin <slev at altlinux.org>

- - - - -
3c74b97a by Stanislav Levin at 2020-06-24T18:05:23-04:00
make: Add missing files to clean up and to distribute

Fixes: https://pagure.io/certmonger/issue/155
Signed-off-by: Stanislav Levin <slev at altlinux.org>

- - - - -
1e61e111 by Rob Crittenden at 2020-06-24T18:19:44-04:00
Handle an uninitialized token when adding CA certs to an NSS db

The getcert -a option will add the CA chain to an NSS database.

It will create this database if it doesn't already exist. For
sqlite database it is created with an uninitialized PIN. Catch
this condition and set an empty PIN.

Related to: https://pagure.io/certmonger/issue/155

- - - - -
ad15bc64 by Rob Crittenden at 2020-06-25T17:25:24-04:00
Fix memory leak

Free t allocated in cm_store_base64_to_bin() if PK11_HashBuf()
fails.

- - - - -
28147cb3 by Rob Crittenden at 2020-06-25T17:25:25-04:00
Fix uninitialized value

- - - - -
6fdf6f3e by Rob Crittenden at 2020-06-25T17:25:25-04:00
Add all enumeration values to prefs test

- - - - -
2175884b by Rob Crittenden at 2020-06-25T17:25:25-04:00
Add missing return value declaration to ca_get_config_file_path()

- - - - -
cac3f50a by Rob Crittenden at 2020-06-25T17:25:25-04:00
Drop const from csr and csre declaration

- - - - -
e1d8de9d by Rob Crittenden at 2020-06-25T17:25:25-04:00
Check that sent_nonce is not NULL to avoid a potential NULL compare

Theoretically if sent_nonce is NULL then the length would be 0
but don't assume this is the case.

Discovered by clang.

- - - - -
dc7e3331 by Rob Crittenden at 2020-06-25T17:25:25-04:00
Make a Kerberos ccache if one doesn't already exist

This was embedding the logic into a conditional to set a variable
that isn't used beyond this point.

Discovered by clang.

- - - - -
cde4c3bf by Rob Crittenden at 2020-06-25T17:25:25-04:00
Fix memory leak determining the dogtag operation

This code scans through the state cookie to determine what operation
it is executing: review, fetch or retrieve. The intermediate values
used were not freed.

Discovered by clang.

- - - - -
fe84c918 by Rob Crittenden at 2020-06-25T17:25:25-04:00
Free the csr value after generating the request

The value of the csr is being embedded into the request URI.
It isn't used again.

Discovered by clang.

- - - - -
fbcf03dd by Rob Crittenden at 2020-06-25T17:25:25-04:00
Free the submit options on exit

A number -o name=value pairs can be passed in. Free the memory
allocated to store those name/value pairs on exit.

Discovered by clang.

- - - - -
9f6693da by Rob Crittenden at 2020-06-25T17:25:25-04:00
Free the approval options on exit

A number -O name=value pairs can be passed in. Free the memory
allocated to store those name/value pairs on exit.

Discovered by clang.

- - - - -
f3d248bb by Rob Crittenden at 2020-06-25T17:25:25-04:00
Free profile, issuer and/or csr on incorrect usage

The values were allocated prior to exit, do proper cleanup.

Discovered by clang.

- - - - -
62ef421b by Rob Crittenden at 2020-06-25T17:25:25-04:00
Break after finding a defaultNamingContext value

There can only be a single defaultNamingContext so stop processing
entries if one is found. This should theoretically never hit because
there should be only one entry but just in case this will prevent
leaking memory.

Discovered by clang.

- - - - -
92796b89 by Rob Crittenden at 2020-06-25T17:25:25-04:00
Fix memory leak

defaults is allocated by cm_submit_d_xml_defaults() so free the
value when we are done with it.

Discovered by clang.

- - - - -
d087c995 by Rob Crittenden at 2020-06-25T17:25:25-04:00
Fix memory leak

skey is duplicated, free it when done with it.

Discovered by clang.

- - - - -
c0567cf3 by Rob Crittenden at 2020-06-25T17:25:25-04:00
Return NULL if realloc of CSR fails in cm_submit_u_from_file

- - - - -
7c20afd2 by Rob Crittenden at 2020-06-25T17:25:25-04:00
Fix memory leak of old_keyfile in cm_certsave_o_main

- - - - -
09200f43 by Rob Crittenden at 2020-06-25T17:25:25-04:00
Check for NSS_Shutdown and context failures and exit

This is not likely since a successful Init/Shutdown combination
just happened but safety first.

- - - - -
c279d711 by Rob Crittenden at 2020-06-25T17:25:25-04:00
Avoid a modulo by zero

range (tweight) could be 0 so check for that before getting the
mod of r.

- - - - -
7d65601f by Rob Crittenden at 2020-06-25T17:25:25-04:00
Avoid memory overrun when adding NULL byte, fix memory leak

- - - - -
c1287bec by Rob Crittenden at 2020-06-25T17:25:25-04:00
More aoptions/soptions fixups

- - - - -
aee33d31 by Rob Crittenden at 2020-06-25T17:25:25-04:00
Include NSPR header so PR_PR_ErrorToString() is defined

- - - - -
6a6962ca by Rob Crittenden at 2020-06-25T17:25:25-04:00
Fix memory leak, always free buf

- - - - -
d5fd17d8 by Rob Crittenden at 2020-06-25T17:25:25-04:00
Free XML objects on errors

- - - - -
d2fde991 by Rob Crittenden at 2020-06-25T17:25:25-04:00
Terminate string passed to cm_pkcs7_envelope_ias in tests

- - - - -
d40c22c4 by Rob Crittenden at 2020-06-25T17:25:25-04:00
Avoid NULL dereference if strchr() returns NULL

- - - - -
63d54706 by Rob Crittenden at 2020-06-25T17:25:25-04:00
Don't allow unitialized value in a conditional

If newcert is NULL then error is undefined (probably 0) which could
lead to the code thinking the certificate imported.

If the certificate wasn't decoded then set error to SECFailure

- - - - -
d3c47090 by Rob Crittenden at 2020-06-26T16:03:28-04:00
Change linkage for cm_submit_u_from_file test in fromfile.c

I tried some crazy linkage but it was segfaulting for no
apparent reason. Including the file directly fixes it and still
tests that \n is appended to read files and that is sufficient.

- - - - -
a643fc58 by Rob Crittenden at 2020-06-26T16:07:30-04:00
Tag 0.79.10

Signed-off-by: Rob Crittenden <rcritten at redhat.com>

- - - - -
26a67f4b by Timo Aaltonen at 2020-06-30T13:36:06+03:00
rules: Use /run instead of /var/run.

- - - - -
49a9cc35 by Rob Crittenden at 2020-06-30T11:33:43-04:00
Revert part of 7d65601f4238e65078592b3486e47ad564f00134

The free() was causing a sigabrt and was in effect freeing
the return value.

https://pagure.io/certmonger/issue/161

- - - - -
14f75bf8 by Rob Crittenden at 2020-06-30T13:17:25-04:00
Tag 0.79.11

Signed-off-by: Rob Crittenden <rcritten at redhat.com>

- - - - -
0e5c991c by Timo Aaltonen at 2020-06-30T23:29:49+03:00
Merge branch 'upstream'

- - - - -
13cba96e by Timo Aaltonen at 2020-06-30T23:30:14+03:00
bump the version

- - - - -
5d8489f5 by Timo Aaltonen at 2020-07-30T15:20:56+03:00
releasing package certmonger version 0.79.11-1

- - - - -


30 changed files:

- certmonger.spec
- configure.ac
- debian/changelog
- debian/rules
- src/Makefile.am
- src/casave.c
- src/certmaster-getcert.1.in
- src/certmonger-certmaster-submit.8.in
- src/certmonger-dogtag-ipa-renew-agent-submit.8.in
- src/certmonger-dogtag-submit.8.in
- src/certmonger-ipa-submit.8.in
- src/certmonger-local-submit.8.in
- src/certmonger-scep-submit.8.in
- src/certmonger.8.in
- src/certmonger.conf.5.in
- src/certread-n.c
- src/certsave-n.c
- src/certsave-o.c
- src/csrgen-o.c
- src/dogtag.c
- src/getcert-add-ca.1.in
- src/getcert-add-scep-ca.1.in
- src/getcert-list-cas.1.in
- src/getcert-list.1.in
- src/getcert-modify-ca.1.in
- src/getcert-refresh-ca.1.in
- src/getcert-refresh.1.in
- src/getcert-rekey.1.in
- src/getcert-remove-ca.1.in
- src/getcert-request.1.in


The diff was not included because it is too large.


View it on GitLab: https://salsa.debian.org/freeipa-team/certmonger/-/compare/8fc294b8eec0391b645b66f01c6dc3fe9bb6feeb...5d8489f577d95d5fd191e4678598927ebc8b36f0

-- 
View it on GitLab: https://salsa.debian.org/freeipa-team/certmonger/-/compare/8fc294b8eec0391b645b66f01c6dc3fe9bb6feeb...5d8489f577d95d5fd191e4678598927ebc8b36f0
You're receiving this email because of your account on salsa.debian.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/pkg-freeipa-devel/attachments/20200730/211a8a93/attachment-0001.html>

More information about the Pkg-freeipa-devel mailing list