[SSSD] [PATCH] p11: enable ocsp checks

Sumit Bose sbose at redhat.com
Thu Nov 12 12:32:13 UTC 2015


On Tue, Nov 10, 2015 at 12:40:23PM +0100, Lukas Slebodnik wrote:
> 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 at 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.

Thank you for testing the build and your comments. I hope you don't mind
that I didn't took you patch but shuffled to code a bit around to
remove the circular dependency between libsss_util and libsss_cert. The
patch now passes the debian test in CI for me.

You suggested on irc to split out the calls which need DHASH_LIBS from
util.c. I think this is a good idea, would you like to open a ticket for
this or shall I? For the main SSSD daemon it does not matter much since
most of them need nearly everything form libsss_util anyway. But I think
it would make sense to trim down the list of loaded libraries for the
child processes especially the ones which have the SETUID or SETGID bit
set. I'm not an expert here and I'm not sure if this might be
exploitable somehow but I think the less code is in the address space of
the process the less code is there to get exploited.

> 
> 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

Thanks, I included the changes.
> 
> 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-----"

New version attached.

bye,
Sumit
-------------- next part --------------
From 3ff6cf406fa40e0b621794013e18afaabc071986 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at 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                          |  2 ++
 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       | 25 +++++++++++++++++++-
 src/tests/cmocka/test_cert_utils.c   |  2 +-
 src/tests/cmocka/test_pam_srv.c      | 41 +++++++++++++++++++++++++-------
 src/util/cert.h                      |  1 +
 src/util/cert/libcrypto/cert.c       |  1 +
 src/util/cert/nss/cert.c             | 15 +++++++++++-
 src/util/util.c                      | 46 ++++++++++++++++++++++++++++++++++++
 src/util/util.h                      |  3 +++
 18 files changed, 209 insertions(+), 22 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index a0abb8fb3a9cc19cfabad8066c6c65d78e78d206..2c3baa788d7411a66afb55e42565c6095a81cc6a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3156,6 +3156,7 @@ 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) \
@@ -3165,6 +3166,7 @@ p11_child_CFLAGS = \
 p11_child_LDADD = \
     libsss_debug.la \
     $(TALLOC_LIBS) \
+    $(DHASH_LIBS) \
     $(POPT_LIBS) \
     $(NSS_LIBS) \
     libsss_crypt.la \
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;
+        }
+    }
 
     if (mode == OP_AUTH && pin_mode == PIN_STDIN) {
         ret = p11c_recv_data(main_ctx, STDIN_FILENO, &pin);
@@ -623,7 +644,7 @@ int main(int argc, const char *argv[])
         }
     }
 
-    ret = do_work(main_ctx, nss_db, slot_name_in, mode, pin, &cert,
+    ret = do_work(main_ctx, nss_db, slot_name_in, mode, pin, do_ocsp, &cert,
                   &token_name_out);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "do_work failed.\n");
diff --git a/src/responder/pam/pamsrv.h b/src/responder/pam/pamsrv.h
index 64a7d85735ac3eb20e4504899916a49a280d4959..b44e1c3378817aa169fa1fe8995bdb223f593cb0 100644
--- a/src/responder/pam/pamsrv.h
+++ b/src/responder/pam/pamsrv.h
@@ -87,6 +87,7 @@ struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx,
                                        int child_debug_fd,
                                        const char *nss_db,
                                        time_t timeout,
+                                       const char *verify_opts,
                                        struct pam_data *pd);
 errno_t pam_check_cert_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
                             char **cert, char **token_name);
diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
index 80095cc0bf1c4c92dd04344c069b666ab76a3714..b9fd353259834910e85b05f8e5aa20c9c5674501 100644
--- a/src/responder/pam/pamsrv_cmd.c
+++ b/src/responder/pam/pamsrv_cmd.c
@@ -1032,6 +1032,7 @@ static errno_t check_cert(TALLOC_CTX *mctx,
 {
     int p11_child_timeout;
     const int P11_CHILD_TIMEOUT_DEFAULT = 10;
+    char *cert_verification_opts;
     errno_t ret;
     struct tevent_req *req;
 
@@ -1046,8 +1047,19 @@ static errno_t check_cert(TALLOC_CTX *mctx,
         return ret;
     }
 
+    ret = confdb_get_string(pctx->rctx->cdb, mctx, CONFDB_MONITOR_CONF_ENTRY,
+                            CONFDB_MONITOR_CERT_VERIFICATION, NULL,
+                            &cert_verification_opts);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              "Failed to read certificate_verification from confdb: [%d]: %s\n",
+              ret, sss_strerror(ret));
+        return ret;
+    }
+
     req = pam_check_cert_send(mctx, ev, pctx->p11_child_debug_fd,
-                              pctx->nss_db, p11_child_timeout, pd);
+                              pctx->nss_db, p11_child_timeout,
+                              cert_verification_opts, pd);
     if (req == NULL) {
         DEBUG(SSSDBG_OP_FAILURE, "pam_check_cert_send failed.\n");
         return ENOMEM;
diff --git a/src/responder/pam/pamsrv_p11.c b/src/responder/pam/pamsrv_p11.c
index afb28fd529245975efbf34a04624adb3f09704f2..58310a2530287fc6d08a7195c8e879f96dcc5403 100644
--- a/src/responder/pam/pamsrv_p11.c
+++ b/src/responder/pam/pamsrv_p11.c
@@ -236,6 +236,7 @@ struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx,
                                        int child_debug_fd,
                                        const char *nss_db,
                                        time_t timeout,
+                                       const char *verify_opts,
                                        struct pam_data *pd)
 {
     errno_t ret;
@@ -246,9 +247,10 @@ struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx,
     struct timeval tv;
     int pipefd_to_child[2];
     int pipefd_from_child[2];
-    const char *extra_args[5] = {NULL, NULL, NULL, NULL, NULL};
+    const char *extra_args[7] = { NULL };
     uint8_t *write_buf = NULL;
     size_t write_buf_len = 0;
+    size_t arg_c;
 
     req = tevent_req_create(mem_ctx, &state, struct pam_check_cert_state);
     if (req == NULL) {
@@ -262,16 +264,21 @@ struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx,
     }
 
     /* extra_args are added in revers order */
-    extra_args[1] = "--nssdb";
-    extra_args[0] = nss_db;
+    arg_c = 0;
+    extra_args[arg_c++] = nss_db;
+    extra_args[arg_c++] = "--nssdb";
+    if (verify_opts != NULL) {
+        extra_args[arg_c++] = verify_opts;
+        extra_args[arg_c++] = "--verify";
+    }
     if (pd->cmd == SSS_PAM_AUTHENTICATE) {
-        extra_args[2] = "--auth";
+        extra_args[arg_c++] = "--auth";
         switch (sss_authtok_get_type(pd->authtok)) {
         case SSS_AUTHTOK_TYPE_SC_PIN:
-            extra_args[3] = "--pin";
+            extra_args[arg_c++] = "--pin";
             break;
         case SSS_AUTHTOK_TYPE_SC_KEYPAD:
-            extra_args[3] = "--keypad";
+            extra_args[arg_c++] = "--keypad";
             break;
         default:
             DEBUG(SSSDBG_OP_FAILURE, "Unsupported authtok type.\n");
@@ -279,7 +286,7 @@ struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx,
             goto done;
         }
     } else if (pd->cmd == SSS_PAM_PREAUTH) {
-        extra_args[2] = "--pre";
+        extra_args[arg_c++] = "--pre";
     } else {
         DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected PAM command [%d}.\n", pd->cmd);
         ret = EINVAL;
diff --git a/src/responder/ssh/sshsrv_cmd.c b/src/responder/ssh/sshsrv_cmd.c
index 5f5487035b298dcc540de246017731b94139a413..af385fde8b382da513dbac29f496462a90c4c269 100644
--- a/src/responder/ssh/sshsrv_cmd.c
+++ b/src/responder/ssh/sshsrv_cmd.c
@@ -797,6 +797,8 @@ static errno_t decode_and_add_base64_data(struct ssh_cmd_ctx *cmd_ctx,
     int ret;
     size_t d;
     TALLOC_CTX *tmp_ctx;
+    char *cert_verification_opts;
+    bool do_ocsp = true;
 
     if (el == NULL) {
         DEBUG(SSSDBG_TRACE_ALL, "Mssing element, nothing to do.\n");
@@ -811,9 +813,30 @@ static errno_t decode_and_add_base64_data(struct ssh_cmd_ctx *cmd_ctx,
 
     for (d = 0; d < el->num_values; d++) {
         if (cert_data) {
+
+            ret = confdb_get_string(cctx->rctx->cdb, tmp_ctx,
+                                    CONFDB_MONITOR_CONF_ENTRY,
+                                    CONFDB_MONITOR_CERT_VERIFICATION, NULL,
+                                    &cert_verification_opts);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_CRIT_FAILURE,
+                      "Failed to read p11_child_timeout from confdb: [%d] %s\n",
+                      ret, sss_strerror(ret));
+                return ret;
+            }
+
+            if (cert_verification_opts != NULL) {
+                ret = parse_cert_verify_opts(cert_verification_opts, &do_ocsp);
+                if (ret != EOK) {
+                    DEBUG(SSSDBG_FATAL_FAILURE,
+                          "Failed to parse verifiy option.\n");
+                    return ret;
+                }
+            }
+
             ret = cert_to_ssh_key(tmp_ctx, ssh_ctx->ca_db,
                                   el->values[d].data, el->values[d].length,
-                                  &key, &key_len);
+                                  do_ocsp, &key, &key_len);
             if (ret != EOK) {
                 DEBUG(SSSDBG_OP_FAILURE, "cert_to_ssh_key failed.\n");
                 return ret;
diff --git a/src/tests/cmocka/test_cert_utils.c b/src/tests/cmocka/test_cert_utils.c
index 7bd8cf2344003421e9ec84dc5e1b2305a861ab38..658391d147154d7381d4f1ab74f199453fcab352 100644
--- a/src/tests/cmocka/test_cert_utils.c
+++ b/src/tests/cmocka/test_cert_utils.c
@@ -356,7 +356,7 @@ void test_cert_to_ssh_key(void **state)
     assert_non_null(exp_key);
 
     ret = cert_to_ssh_key(ts, "sql:" ABS_SRC_DIR "/src/tests/cmocka/p11_nssdb",
-                          der, der_size, &key, &key_size);
+                          der, der_size, false, &key, &key_size);
     assert_int_equal(ret, EOK);
     assert_int_equal(key_size, exp_key_size);
     assert_memory_equal(key, exp_key, exp_key_size);
diff --git a/src/tests/cmocka/test_pam_srv.c b/src/tests/cmocka/test_pam_srv.c
index f3fbf907a02205e2da06e30f433048624e8e2103..75411feeefb02cfbb9d5eb19b1082f6445d92a66 100644
--- a/src/tests/cmocka/test_pam_srv.c
+++ b/src/tests/cmocka/test_pam_srv.c
@@ -188,26 +188,38 @@ struct pam_ctx *mock_pctx(TALLOC_CTX *mem_ctx)
     return pctx;
 }
 
-static int add_pam_params(struct sss_test_conf_param pam_params[],
-                          struct confdb_ctx *cdb)
+static int add_confdb_params(struct sss_test_conf_param params[],
+                             struct confdb_ctx *cdb, const char *section)
 {
     const char *val[2];
     int ret;
 
     val[1] = NULL;
 
-    for (int i = 0; pam_params[i].key; i++) {
-        val[0] = pam_params[i].value;
-        ret = confdb_add_param(cdb, true, CONFDB_PAM_CONF_ENTRY,
-                               pam_params[i].key, val);
+    for (int i = 0; params[i].key; i++) {
+        val[0] = params[i].value;
+        ret = confdb_add_param(cdb, true, section, params[i].key, val);
         assert_int_equal(ret, EOK);
     }
 
     return EOK;
 }
 
+static int add_pam_params(struct sss_test_conf_param pam_params[],
+                          struct confdb_ctx *cdb)
+{
+    return add_confdb_params(pam_params, cdb, CONFDB_PAM_CONF_ENTRY);
+}
+
+static int add_monitor_params(struct sss_test_conf_param monitor_params[],
+                              struct confdb_ctx *cdb)
+{
+    return add_confdb_params(monitor_params, cdb, CONFDB_MONITOR_CONF_ENTRY);
+}
+
 void test_pam_setup(struct sss_test_conf_param dom_params[],
                     struct sss_test_conf_param pam_params[],
+                    struct sss_test_conf_param monitor_params[],
                     void **state)
 {
     errno_t ret;
@@ -243,6 +255,9 @@ void test_pam_setup(struct sss_test_conf_param dom_params[],
     ret = add_pam_params(pam_params, pam_test_ctx->rctx->cdb);
     assert_int_equal(ret, EOK);
 
+    ret = add_monitor_params(monitor_params, pam_test_ctx->rctx->cdb);
+    assert_int_equal(ret, EOK);
+
     /* Create client context */
     pam_test_ctx->cctx = mock_cctx(pam_test_ctx, pam_test_ctx->rctx);
     assert_non_null(pam_test_ctx->cctx);
@@ -299,7 +314,12 @@ static int pam_test_setup(void **state)
         { NULL, NULL },             /* Sentinel */
     };
 
-    test_pam_setup(dom_params, pam_params, state);
+    struct sss_test_conf_param monitor_params[] = {
+        { "certificate_verification", "no_ocsp"},
+        { NULL, NULL },             /* Sentinel */
+    };
+
+    test_pam_setup(dom_params, pam_params, monitor_params, state);
 
     pam_test_setup_common();
     return 0;
@@ -319,7 +339,12 @@ static int pam_cached_test_setup(void **state)
         { NULL, NULL },             /* Sentinel */
     };
 
-    test_pam_setup(dom_params, pam_params, state);
+    struct sss_test_conf_param monitor_params[] = {
+        { "certificate_verification", "no_ocsp"},
+        { NULL, NULL },             /* Sentinel */
+    };
+
+    test_pam_setup(dom_params, pam_params, monitor_params, state);
 
     pam_test_setup_common();
     return 0;
diff --git a/src/util/cert.h b/src/util/cert.h
index edbafc492a1ed42ad616d0bf2fae882046711746..c8c42548745baaed5a39addde911d4d6e9305462 100644
--- a/src/util/cert.h
+++ b/src/util/cert.h
@@ -47,5 +47,6 @@ errno_t bin_to_ldap_filter_value(TALLOC_CTX *mem_ctx,
 
 errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db,
                         const uint8_t *der_blob, size_t der_size,
+                        bool do_ocsp,
                         uint8_t **key, size_t *key_size);
 #endif /* __CERT_H__ */
diff --git a/src/util/cert/libcrypto/cert.c b/src/util/cert/libcrypto/cert.c
index 01f9554b990d6a139bb9a1d8d558c1c3f6bb745c..4e2dbe70cafcb31e9ca4cc232dacb0cc176b0dc0 100644
--- a/src/util/cert/libcrypto/cert.c
+++ b/src/util/cert/libcrypto/cert.c
@@ -172,6 +172,7 @@ done:
 
 errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db,
                         const uint8_t *der_blob, size_t der_size,
+                        bool do_ocsp,
                         uint8_t **key, size_t *key_size)
 {
     int ret;
diff --git a/src/util/cert/nss/cert.c b/src/util/cert/nss/cert.c
index 1ada35b6321b9e0f3daf87b3412cf7a124cbf2c7..fbd063cf5f0c0100dab32f8ff14339b80979039a 100644
--- a/src/util/cert/nss/cert.c
+++ b/src/util/cert/nss/cert.c
@@ -18,16 +18,19 @@
    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>
 #include <base64.h>
 #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-----"
@@ -220,6 +223,7 @@ done:
 
 errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db,
                         const uint8_t *der_blob, size_t der_size,
+                        bool do_ocsp,
                         uint8_t **key, size_t *key_size)
 {
     CERTCertDBHandle *handle;
@@ -255,6 +259,15 @@ errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db,
 
     handle = CERT_GetDefaultCertDB();
 
+    if (do_ocsp) {
+        rv = CERT_EnableOCSPChecking(handle);
+        if (rv != SECSuccess) {
+            DEBUG(SSSDBG_OP_FAILURE, "CERT_EnableOCSPChecking failed: [%d].\n",
+                                     PR_GetError());
+            return EIO;
+        }
+    }
+
     der_item.len = der_size;
     der_item.data = discard_const(der_blob);
 
diff --git a/src/util/util.c b/src/util/util.c
index 11924a3718e55352a73848ea422a7568d42f564a..8c0899b06b0f3648fd47e65400d8da919dde4f95 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -1106,3 +1106,49 @@ errno_t sss_unique_filename(TALLOC_CTX *owner, char *path_tmpl)
 
     return ret;
 }
+
+errno_t parse_cert_verify_opts(const char *verify_opts, bool *do_ocsp)
+{
+    int ret;
+    TALLOC_CTX *tmp_ctx;
+    char **opts;
+    size_t c;
+
+    if (verify_opts == NULL) {
+        *do_ocsp = true;
+
+        return EOK;
+    }
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n");
+        return ENOMEM;
+    }
+
+    ret = split_on_separator(tmp_ctx, verify_opts, ',', true, true, &opts,
+                             NULL);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, "split_on_separator failed.\n");
+        goto done;
+    }
+
+    for (c = 0; opts[c] != NULL; c++) {
+        if (strcasecmp(opts[c], "no_ocsp") == 0) {
+            DEBUG(SSSDBG_TRACE_ALL,
+                  "Found 'no_ocsp' option, disabling OCSP.\n");
+            *do_ocsp = false;
+        } else {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  "Unsupported certificate verification option [%s], " \
+                  "skipping.\n", opts[c]);
+        }
+    }
+
+    ret = EOK;
+
+done:
+    talloc_free(tmp_ctx);
+
+    return ret;
+}
diff --git a/src/util/util.h b/src/util/util.h
index 3693a5e9f16737e966a250741c2dc1d32662c4fd..e1245bb0fbab742dd58522f1892c66d08a45b59c 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -424,6 +424,9 @@ int split_on_separator(TALLOC_CTX *mem_ctx, const char *str,
 
 char **parse_args(const char *str);
 
+errno_t parse_cert_verify_opts(const char *verify_opts, bool *do_ocsp);
+
+
 errno_t sss_hash_create(TALLOC_CTX *mem_ctx,
                         unsigned long count,
                         hash_table_t **tbl);
-- 
2.1.0



More information about the sssd-devel mailing list