On (10/11/15 11:36), Sumit Bose wrote:
Hi,
this patch switches on the Online Certificate Status Protocol (OCSP)
checks while validation the certificate. This is done by calling
CERT_EnableOCSPChecking() before doing the validation. The main part of
the patch makes this configurable.
Since I expect that certificate validation will need more tuning option
in future I didn't add a new option of switch OCSP off and on but a more
generic one called 'certificate_verification' which accepts a comma
separated list of parameters. Currently only 'no_ocsp' is supported.
Currently this option is tested indirectly because I generated the test
certificates with IPA they contain the OCSP data. But since the OCSP
check, which is now on by default, requires on-line access to the
referenced OCSP server the OCSP check must be disabled for the
unit-test.
bye,
Sumit
Just few "build-related" comments.
From 64701f85a3c76ce420b974b6fd94186007e53257 Mon Sep 17 00:00:00
2001
From: Sumit Bose <sbose(a)redhat.com>
Date: Thu, 5 Nov 2015 18:20:27 +0100
Subject: [PATCH] p11: enable ocsp checks
This patch enables the Online Certificate Status Protocol in NSS and
adds an option to disable it if needed. To make further tuning of
certificate verification more easy it is not an option on its own but an
option to the new certificate_verification configuration option.
Resolves
https://fedorahosted.org/sssd/ticket/2812
---
Makefile.am | 5 ++--
src/confdb/confdb.h | 1 +
src/config/SSSDConfig/__init__.py.in | 1 +
src/config/SSSDConfigTest.py | 3 ++-
src/config/etc/sssd.api.conf | 1 +
src/man/sssd.conf.5.xml | 28 ++++++++++++++++++++++
src/p11_child/p11_child_nss.c | 25 ++++++++++++++++++--
src/responder/pam/pamsrv.h | 1 +
src/responder/pam/pamsrv_cmd.c | 14 ++++++++++-
src/responder/pam/pamsrv_p11.c | 21 ++++++++++------
src/responder/ssh/sshsrv_cmd.c | 15 +++++++++++-
src/tests/cmocka/test_cert_utils.c | 2 +-
src/tests/cmocka/test_pam_srv.c | 34 ++++++++++++++++++++------
src/util/cert.h | 3 +++
src/util/cert/cert_common.c | 46 ++++++++++++++++++++++++++++++++++++
src/util/cert/libcrypto/cert.c | 1 +
src/util/cert/nss/cert.c | 20 ++++++++++++++++
17 files changed, 199 insertions(+), 22 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index a0abb8fb3a9cc19cfabad8066c6c65d78e78d206..ee97d095e07f6d485232a1afd87f9e4057688f22
100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2579,8 +2579,7 @@ test_cert_utils_LDADD = \
$(CRYPTO_LIBS) \
libsss_debug.la \
libsss_test_common.la \
- libsss_cert.la \
- libsss_crypt.la \
+ libsss_util.la \
$(NULL)
libss_cert and libsss_crypt chould not be removed.
Otherwise I cannot build the test.
src/tests/cmocka/test_cert_utils-test_cert_utils.o: In function
`test_sss_cert_der_to_pem':
/dev/shm/gcc/../sssd/src/tests/cmocka/test_cert_utils.c:193: undefined reference to
`sss_cert_der_to_pem'
/dev/shm/gcc/../sssd/src/tests/cmocka/test_cert_utils.c:196: undefined reference to
`sss_cert_der_to_pem'
src/tests/cmocka/test_cert_utils-test_cert_utils.o: In function
`test_sss_cert_pem_to_der':
/dev/shm/gcc/../sssd/src/tests/cmocka/test_cert_utils.c:213: undefined reference to
`sss_cert_pem_to_der'
/dev/shm/gcc/../sssd/src/tests/cmocka/test_cert_utils.c:216: undefined reference to
`sss_cert_pem_to_der'
src/tests/cmocka/test_cert_utils-test_cert_utils.o: In function
`test_sss_cert_derb64_to_pem':
/dev/shm/gcc/../sssd/src/tests/cmocka/test_cert_utils.c:232: undefined reference to
`sss_cert_derb64_to_pem'
/dev/shm/gcc/../sssd/src/tests/cmocka/test_cert_utils.c:235: undefined reference to
`sss_cert_derb64_to_pem'
src/tests/cmocka/test_cert_utils-test_cert_utils.o: In function
`test_sss_cert_pem_to_derb64':
/dev/shm/gcc/../sssd/src/tests/cmocka/test_cert_utils.c:250: undefined reference to
`sss_cert_pem_to_derb64'
/dev/shm/gcc/../sssd/src/tests/cmocka/test_cert_utils.c:253: undefined reference to
`sss_cert_pem_to_derb64'
src/tests/cmocka/test_cert_utils-test_cert_utils.o: In function
`test_bin_to_ldap_filter_value':
/dev/shm/gcc/../sssd/src/tests/cmocka/test_cert_utils.c:279: undefined reference to
`bin_to_ldap_filter_value'
/dev/shm/gcc/../sssd/src/tests/cmocka/test_cert_utils.c:283: undefined reference to
`bin_to_ldap_filter_value'
src/tests/cmocka/test_cert_utils-test_cert_utils.o: In function
`test_sss_cert_derb64_to_ldap_filter':
/dev/shm/gcc/../sssd/src/tests/cmocka/test_cert_utils.c:300: undefined reference to
`sss_cert_derb64_to_ldap_filter'
/dev/shm/gcc/../sssd/src/tests/cmocka/test_cert_utils.c:303: undefined reference to
`sss_cert_derb64_to_ldap_filter'
src/tests/cmocka/test_cert_utils-test_cert_utils.o: In function
`test_cert_to_ssh_key':
/dev/shm/gcc/../sssd/src/tests/cmocka/test_cert_utils.c:352: undefined reference to
`sss_base64_decode'
/dev/shm/gcc/../sssd/src/tests/cmocka/test_cert_utils.c:355: undefined reference to
`sss_base64_decode'
/dev/shm/gcc/../sssd/src/tests/cmocka/test_cert_utils.c:358: undefined reference to
`cert_to_ssh_key'
src/tests/cmocka/test_cert_utils-test_cert_utils.o: In function `main':
/dev/shm/gcc/../sssd/src/tests/cmocka/test_cert_utils.c:422: undefined reference to
`nspr_nss_cleanup'
collect2: error: ld returned 1 exit status
Makefile:10360: recipe for target 'test_cert_utils' failed
the root of proble was caused by following change in this patch.
test_data_provider_be_SOURCES = \
@@ -3156,6 +3155,7 @@ proxy_child_LDADD = \
p11_child_SOURCES = \
src/p11_child/p11_child_nss.c \
src/util/atomic_io.c \
+ src/util/util.c \
This file was added here because the function
split_on_separator
is used in file src/util/cert/cert_common.c (parse_cert_verify_opts)
But the file src/util/cert/cert_common.c is part of libsss_cert.so
and not part of p11_child.
$(NULL)
p11_child_CFLAGS = \
$(AM_CFLAGS) \
@@ -3168,6 +3168,7 @@ p11_child_LDADD = \
$(POPT_LIBS) \
$(NSS_LIBS) \
libsss_crypt.la \
+ libsss_cert.la \
$(NULL)
Following diff should fix it.
DHASH_LIBS are required because functions from libdhash
are used in src/util/util.c.
index ee97d09..7a9468d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -777,6 +777,7 @@ pkglib_LTLIBRARIES += libsss_cert.la
libsss_cert_la_SOURCES = \
$(SSS_CERT_SOURCES) \
+ src/util/util.c \
$(NULL)
libsss_cert_la_CFLAGS = \
$(AM_CFLAGS) \
@@ -785,6 +786,7 @@ libsss_cert_la_CFLAGS = \
libsss_cert_la_LIBADD = \
$(SSS_CERT_LIBS) \
$(TALLOC_LIBS) \
+ $(DHASH_LIBS) \
libsss_crypt.la \
libsss_debug.la \
$(NULL)
@@ -2579,7 +2581,8 @@ test_cert_utils_LDADD = \
$(CRYPTO_LIBS) \
libsss_debug.la \
libsss_test_common.la \
- libsss_util.la \
+ libsss_cert.la \
+ libsss_crypt.la \
$(NULL)
test_data_provider_be_SOURCES = \
@@ -3155,7 +3158,6 @@ proxy_child_LDADD = \
p11_child_SOURCES = \
src/p11_child/p11_child_nss.c \
src/util/atomic_io.c \
- src/util/util.c \
$(NULL)
p11_child_CFLAGS = \
$(AM_CFLAGS) \
diff --git a/src/util/cert/nss/cert.c b/src/util/cert/nss/cert.c
memberof_la_SOURCES = \
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 37b5fd7c7629e2618a1699e3ffd58110171db605..c6a5e3f61d8bfd045eb2699d0f5e279cb7d89f86
100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -71,6 +71,7 @@
#define CONFDB_MONITOR_DEFAULT_DOMAIN "default_domain_suffix"
#define CONFDB_MONITOR_OVERRIDE_SPACE "override_space"
#define CONFDB_MONITOR_USER_RUNAS "user"
+#define CONFDB_MONITOR_CERT_VERIFICATION "certificate_verification"
/* Both monitor and domains */
#define CONFDB_NAME_REGEX "re_expression"
diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index bf61c402796122050fa43cf41128faec4771c5d2..2cb857013fe4bddfd2e79e589d3ba9721dc3ca4f
100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -60,6 +60,7 @@ option_strings = {
'krb5_rcache_dir' : _('Directory on the filesystem where SSSD should
store Kerberos replay cache files.'),
'default_domain_suffix' : _('Domain to add to names without a domain
component.'),
'user' : _('The user to drop privileges to'),
+ 'certificate_verification' : _('Tune certificate verification'),
# [nss]
'enum_cache_timeout' : _('Enumeration cache timeout length
(seconds)'),
diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py
index 45562214da5d227b45914abbcb298e043048adf5..8277d1ee4198f342b5cff50f8d44c973fce4a29b
100755
--- a/src/config/SSSDConfigTest.py
+++ b/src/config/SSSDConfigTest.py
@@ -307,7 +307,8 @@ class SSSDConfigTestSSSDService(unittest.TestCase):
'reconnection_retries',
'fd_limit',
'client_idle_timeout',
- 'description']
+ 'description',
+ 'certificate_verification']
self.assertTrue(type(options) == dict,
"Options should be a dictionary")
diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf
index 72abb8b3f427e9bcf3cf3dcaa67aaf4e3e50226c..b6a396a75e564355d0828fa24858337eb06ff4bf
100644
--- a/src/config/etc/sssd.api.conf
+++ b/src/config/etc/sssd.api.conf
@@ -25,6 +25,7 @@ full_name_format = str, None, false
krb5_rcache_dir = str, None, false
user = str, None, false
default_domain_suffix = str, None, false
+certificate_verification = str, None, false
[nss]
# Name service
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index 573f421a7d885d28d5dbc03423e6c6dd84d7b8fd..05dcb4555e02da9e94c76baee65f1d7168f8026b
100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -365,6 +365,34 @@
</para>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term>certificate_verification (string)</term>
+ <listitem>
+ <para>
+ With this parameter the certificate verification
+ can be tuned with a comma separated list of
+ options. Supported options are:
+ <variablelist>
+ <varlistentry>
+ <term>no_ocsp</term>
+ <listitem>
+ <para>Disables Online Certificate Status
+ Protocol (OCSP) checks. This might be
+ needed if the OCSP servers defined in
+ the certificate are not reachable from
+ the client.</para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </para>
+ <para>
+ Unknown options are reported but ignored.
+ </para>
+ <para>
+ Default: not set
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</para>
</refsect2>
diff --git a/src/p11_child/p11_child_nss.c b/src/p11_child/p11_child_nss.c
index 22e449d547c64c94a3ee9e29939377c98443b47d..de5494d03fbec2f66a882521638e94942e964f90
100644
--- a/src/p11_child/p11_child_nss.c
+++ b/src/p11_child/p11_child_nss.c
@@ -38,10 +38,12 @@
#include <keyhi.h>
#include <pk11pub.h>
#include <prerror.h>
+#include <ocsp.h>
#include "util/child_common.h"
#include "providers/dp_backend.h"
#include "util/crypto/sss_crypto.h"
+#include "util/cert.h"
enum op_mode {
OP_NONE,
@@ -68,7 +70,7 @@ static char *password_passthrough(PK11SlotInfo *slot, PRBool retry, void
*arg)
int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, const char *slot_name_in,
- enum op_mode mode, const char *pin, char **cert,
+ enum op_mode mode, const char *pin, bool do_ocsp, char **cert,
char **token_name_out)
{
int ret;
@@ -261,6 +263,14 @@ int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, const char
*slot_name_in,
return EIO;
}
+ if (do_ocsp) {
+ rv = CERT_EnableOCSPChecking(handle);
+ if (rv != SECSuccess) {
+ DEBUG(SSSDBG_OP_FAILURE, "CERT_EnableOCSPChecking failed:
[%d].\n",
+ PR_GetError());
+ return EIO;
+ }
+ }
found_cert = NULL;
DEBUG(SSSDBG_TRACE_ALL, "Filtered certificates:\n");
@@ -456,6 +466,8 @@ int main(int argc, const char *argv[])
char *slot_name_in = NULL;
char *token_name_out = NULL;
char *nss_db = NULL;
+ bool do_ocsp = true;
+ char *verify_opts = NULL;
struct poptOption long_options[] = {
POPT_AUTOHELP
@@ -475,6 +487,8 @@ int main(int argc, const char *argv[])
{"pin", 0, POPT_ARG_NONE, NULL, 'i', _("Expect PIN on
stdin"), NULL},
{"keypad", 0, POPT_ARG_NONE, NULL, 'k', _("Expect PIN on
keypad"),
NULL},
+ {"verify", 0, POPT_ARG_STRING, &verify_opts, 0 , _("Tune
validation"),
+ NULL},
{"nssdb", 0, POPT_ARG_STRING, &nss_db, 0, _("NSS DB to
use"),
NULL},
POPT_TABLEEND
@@ -614,6 +628,13 @@ int main(int argc, const char *argv[])
}
talloc_steal(main_ctx, debug_prg_name);
+ if (verify_opts != NULL) {
+ ret = parse_cert_verify_opts(verify_opts, &do_ocsp);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_FATAL_FAILURE, "Failed to parse verifiy option.\n");
+ goto fail;
+ }
+ }
This change caused a warning
CC src/util/cert/nss/libsss_cert_la-cert.lo
../sssd/src/util/cert/nss/cert.c: In function ‘cert_to_ssh_key’:
../sssd/src/util/cert/nss/cert.c:246:15: error: implicit declaration of function
‘parse_cert_verify_opts’ [-Werror=implicit-function-declaration]
ret = parse_cert_verify_opts(verify_opts, &do_ocsp);
^
cc1: all warnings being treated as errors
You forgot to include "util/cert.h" to this file.
BTW if you don't mind you can also remove util/util.h from this file.
It is not used. It might have been included only for config.h
diff --git a/src/util/cert/nss/cert.c b/src/util/cert/nss/cert.c
index ab3113d..f95a87b 100644
--- a/src/util/cert/nss/cert.c
+++ b/src/util/cert/nss/cert.c
@@ -18,7 +18,7 @@
along with this program. If not, see <
http://www.gnu.org/licenses/>.
*/
-#include "util/util.h"
+#include "config.h"
#include <nss.h>
#include <cert.h>
@@ -26,9 +26,11 @@
#include <key.h>
#include <prerror.h>
#include <ocsp.h>
+#include <talloc.h>
#include "util/crypto/sss_crypto.h"
#include "util/crypto/nss/nss_util.h"
+#include "util/cert.h"
#define NS_CERT_HEADER "-----BEGIN CERTIFICATE-----"
#define NS_CERT_TRAILER "-----END CERTIFICATE-----"
LS