Skip to content
Permalink
Browse files Browse the repository at this point in the history
certmap: sanitize LDAP search filter
The sss_certmap_get_search_filter() will now sanitize the values read
from the certificates before adding them to a search filter. To be able
to get the plain values as well sss_certmap_expand_mapping_rule() is
added.

Resolves:
#5135

Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
  • Loading branch information
sumit-bose authored and pbrezina committed Jul 24, 2020
1 parent 41a60c6 commit a2b9a84
Show file tree
Hide file tree
Showing 8 changed files with 272 additions and 108 deletions.
2 changes: 1 addition & 1 deletion Makefile.am
Expand Up @@ -2163,7 +2163,7 @@ libsss_certmap_la_LIBADD = \
$(NULL)
libsss_certmap_la_LDFLAGS = \
-Wl,--version-script,$(srcdir)/src/lib/certmap/sss_certmap.exports \
-version-info 1:0:1
-version-info 2:0:2

if HAVE_NSS
libsss_certmap_la_SOURCES += \
Expand Down
42 changes: 37 additions & 5 deletions src/lib/certmap/sss_certmap.c
Expand Up @@ -441,10 +441,12 @@ static int expand_san(struct sss_certmap_ctx *ctx,
static int expand_template(struct sss_certmap_ctx *ctx,
struct parsed_template *parsed_template,
struct sss_cert_content *cert_content,
bool sanitize,
char **expanded)
{
int ret;
char *exp = NULL;
char *exp_sanitized = NULL;

if (strcmp("issuer_dn", parsed_template->name) == 0) {
ret = rdn_list_2_dn_str(ctx, parsed_template->conversion,
Expand All @@ -455,6 +457,8 @@ static int expand_template(struct sss_certmap_ctx *ctx,
} else if (strncmp("subject_", parsed_template->name, 8) == 0) {
ret = expand_san(ctx, parsed_template, cert_content->san_list, &exp);
} else if (strcmp("cert", parsed_template->name) == 0) {
/* cert blob is already sanitized */
sanitize = false;
ret = expand_cert(ctx, parsed_template, cert_content, &exp);
} else {
CM_DEBUG(ctx, "Unsupported template name.");
Expand All @@ -471,6 +475,16 @@ static int expand_template(struct sss_certmap_ctx *ctx,
goto done;
}

if (sanitize) {
ret = sss_filter_sanitize(ctx, exp, &exp_sanitized);
if (ret != EOK) {
CM_DEBUG(ctx, "Failed to sanitize expanded template.");
goto done;
}
talloc_free(exp);
exp = exp_sanitized;
}

ret = 0;

done:
Expand All @@ -485,7 +499,7 @@ static int expand_template(struct sss_certmap_ctx *ctx,

static int get_filter(struct sss_certmap_ctx *ctx,
struct ldap_mapping_rule *parsed_mapping_rule,
struct sss_cert_content *cert_content,
struct sss_cert_content *cert_content, bool sanitize,
char **filter)
{
struct ldap_mapping_rule_comp *comp;
Expand All @@ -503,7 +517,7 @@ static int get_filter(struct sss_certmap_ctx *ctx,
result = talloc_strdup_append(result, comp->val);
} else if (comp->type == comp_template) {
ret = expand_template(ctx, comp->parsed_template, cert_content,
&expanded);
sanitize, &expanded);
if (ret != 0) {
CM_DEBUG(ctx, "Failed to expanded template.");
goto done;
Expand Down Expand Up @@ -791,8 +805,9 @@ int sss_certmap_match_cert(struct sss_certmap_ctx *ctx,
return ret;
}

int sss_certmap_get_search_filter(struct sss_certmap_ctx *ctx,
static int expand_mapping_rule_ex(struct sss_certmap_ctx *ctx,
const uint8_t *der_cert, size_t der_size,
bool sanitize,
char **_filter, char ***_domains)
{
int ret;
Expand All @@ -819,7 +834,8 @@ int sss_certmap_get_search_filter(struct sss_certmap_ctx *ctx,
return EINVAL;
}

ret = get_filter(ctx, ctx->default_mapping_rule, cert_content, &filter);
ret = get_filter(ctx, ctx->default_mapping_rule, cert_content, sanitize,
&filter);
goto done;
}

Expand All @@ -829,7 +845,7 @@ int sss_certmap_get_search_filter(struct sss_certmap_ctx *ctx,
if (ret == 0) {
/* match */
ret = get_filter(ctx, r->parsed_mapping_rule, cert_content,
&filter);
sanitize, &filter);
if (ret != 0) {
CM_DEBUG(ctx, "Failed to get filter");
goto done;
Expand Down Expand Up @@ -873,6 +889,22 @@ int sss_certmap_get_search_filter(struct sss_certmap_ctx *ctx,
return ret;
}

int sss_certmap_get_search_filter(struct sss_certmap_ctx *ctx,
const uint8_t *der_cert, size_t der_size,
char **_filter, char ***_domains)
{
return expand_mapping_rule_ex(ctx, der_cert, der_size, true,
_filter, _domains);
}

int sss_certmap_expand_mapping_rule(struct sss_certmap_ctx *ctx,
const uint8_t *der_cert, size_t der_size,
char **_expanded, char ***_domains)
{
return expand_mapping_rule_ex(ctx, der_cert, der_size, false,
_expanded, _domains);
}

int sss_certmap_init(TALLOC_CTX *mem_ctx,
sss_certmap_ext_debug *debug, void *debug_priv,
struct sss_certmap_ctx **ctx)
Expand Down
5 changes: 5 additions & 0 deletions src/lib/certmap/sss_certmap.exports
Expand Up @@ -16,3 +16,8 @@ SSS_CERTMAP_0.1 {
global:
sss_certmap_display_cert_content;
} SSS_CERTMAP_0.0;

SSS_CERTMAP_0.2 {
global:
sss_certmap_expand_mapping_rule;
} SSS_CERTMAP_0.1;
35 changes: 30 additions & 5 deletions src/lib/certmap/sss_certmap.h
Expand Up @@ -103,7 +103,7 @@ int sss_certmap_add_rule(struct sss_certmap_ctx *ctx,
*
* @param[in] ctx certmap context previously initialized with
* @ref sss_certmap_init
* @param[in] der_cert binary blog with the DER encoded certificate
* @param[in] der_cert binary blob with the DER encoded certificate
* @param[in] der_size size of the certificate blob
*
* @return
Expand All @@ -119,10 +119,11 @@ int sss_certmap_match_cert(struct sss_certmap_ctx *ctx,
*
* @param[in] ctx certmap context previously initialized with
* @ref sss_certmap_init
* @param[in] der_cert binary blog with the DER encoded certificate
* @param[in] der_cert binary blob with the DER encoded certificate
* @param[in] der_size size of the certificate blob
* @param[out] filter LDAP filter string, caller should free the data by
* calling sss_certmap_free_filter_and_domains
* @param[out] filter LDAP filter string, expanded templates are sanitized,
* caller should free the data by calling
* sss_certmap_free_filter_and_domains
* @param[out] domains NULL-terminated array of strings with the domains the
* rule applies, caller should free the data by calling
* sss_certmap_free_filter_and_domains
Expand All @@ -136,8 +137,32 @@ int sss_certmap_get_search_filter(struct sss_certmap_ctx *ctx,
const uint8_t *der_cert, size_t der_size,
char **filter, char ***domains);

/**
* @brief Expand the mapping rule by replacing the templates
*
* @param[in] ctx certmap context previously initialized with
* @ref sss_certmap_init
* @param[in] der_cert binary blob with the DER encoded certificate
* @param[in] der_size size of the certificate blob
* @param[out] expanded expanded mapping rule, templates are filled in
* verbatim in contrast to sss_certmap_get_search_filter,
* caller should free the data by
* calling sss_certmap_free_filter_and_domains
* @param[out] domains NULL-terminated array of strings with the domains the
* rule applies, caller should free the data by calling
* sss_certmap_free_filter_and_domains
*
* @return
* - 0: certificate matches a rule
* - ENOENT: certificate does not match
* - EINVAL: internal error
*/
int sss_certmap_expand_mapping_rule(struct sss_certmap_ctx *ctx,
const uint8_t *der_cert, size_t der_size,
char **_expanded, char ***_domains);
/**
* @brief Free data returned by @ref sss_certmap_get_search_filter
* and @ref sss_certmap_expand_mapping_rule
*
* @param[in] filter LDAP filter strings returned by
* sss_certmap_get_search_filter
Expand All @@ -150,7 +175,7 @@ void sss_certmap_free_filter_and_domains(char *filter, char **domains);
* @brief Get a string with the content of the certificate used by the library
*
* @param[in] mem_ctx Talloc memory context, may be NULL
* @param[in] der_cert binary blog with the DER encoded certificate
* @param[in] der_cert binary blob with the DER encoded certificate
* @param[in] der_size size of the certificate blob
* @param[out] desc Multiline string showing the certificate content
* which is used by libsss_certmap
Expand Down
5 changes: 3 additions & 2 deletions src/responder/pam/pamsrv_p11.c
Expand Up @@ -1049,9 +1049,10 @@ static char *get_cert_prompt(TALLOC_CTX *mem_ctx,
goto done;
}

ret = sss_certmap_get_search_filter(ctx, der, der_size, &filter, &domains);
ret = sss_certmap_expand_mapping_rule(ctx, der, der_size,
&filter, &domains);
if (ret != 0) {
DEBUG(SSSDBG_OP_FAILURE, "sss_certmap_get_search_filter failed.\n");
DEBUG(SSSDBG_OP_FAILURE, "sss_certmap_expand_mapping_rule failed.\n");
goto done;
}

Expand Down
98 changes: 97 additions & 1 deletion src/tests/cmocka/test_certmap.c
Expand Up @@ -1431,6 +1431,15 @@ static void test_sss_certmap_get_search_filter(void **state)
&filter, &domains);
assert_int_equal(ret, 0);
assert_non_null(filter);
assert_string_equal(filter, "rule100=<I>CN=Certificate\\20Authority,O=IPA.DEVEL"
"<S>CN=ipa-devel.ipa.devel,O=IPA.DEVEL");
assert_null(domains);

ret = sss_certmap_expand_mapping_rule(ctx, discard_const(test_cert_der),
sizeof(test_cert_der),
&filter, &domains);
assert_int_equal(ret, 0);
assert_non_null(filter);
assert_string_equal(filter, "rule100=<I>CN=Certificate Authority,O=IPA.DEVEL"
"<S>CN=ipa-devel.ipa.devel,O=IPA.DEVEL");
assert_null(domains);
Expand All @@ -1445,6 +1454,17 @@ static void test_sss_certmap_get_search_filter(void **state)
&filter, &domains);
assert_int_equal(ret, 0);
assert_non_null(filter);
assert_string_equal(filter, "rule99=<I>CN=Certificate\\20Authority,O=IPA.DEVEL"
"<S>CN=ipa-devel.ipa.devel,O=IPA.DEVEL");
assert_non_null(domains);
assert_string_equal(domains[0], "test.dom");
assert_null(domains[1]);

ret = sss_certmap_expand_mapping_rule(ctx, discard_const(test_cert_der),
sizeof(test_cert_der),
&filter, &domains);
assert_int_equal(ret, 0);
assert_non_null(filter);
assert_string_equal(filter, "rule99=<I>CN=Certificate Authority,O=IPA.DEVEL"
"<S>CN=ipa-devel.ipa.devel,O=IPA.DEVEL");
assert_non_null(domains);
Expand All @@ -1466,6 +1486,16 @@ static void test_sss_certmap_get_search_filter(void **state)
assert_string_equal(domains[0], "test.dom");
assert_null(domains[1]);

ret = sss_certmap_expand_mapping_rule(ctx, discard_const(test_cert_der),
sizeof(test_cert_der),
&filter, &domains);
assert_int_equal(ret, 0);
assert_non_null(filter);
assert_string_equal(filter, "rule98=userCertificate;binary=" TEST_CERT_BIN);
assert_non_null(domains);
assert_string_equal(domains[0], "test.dom");
assert_null(domains[1]);

ret = sss_certmap_add_rule(ctx, 97,
"KRB5:<ISSUER>CN=Certificate Authority,O=IPA.DEVEL",
"LDAP:rule97=<I>{issuer_dn!nss_x500}<S>{subject_dn}",
Expand All @@ -1476,6 +1506,17 @@ static void test_sss_certmap_get_search_filter(void **state)
&filter, &domains);
assert_int_equal(ret, 0);
assert_non_null(filter);
assert_string_equal(filter, "rule97=<I>O=IPA.DEVEL,CN=Certificate\\20Authority"
"<S>CN=ipa-devel.ipa.devel,O=IPA.DEVEL");
assert_non_null(domains);
assert_string_equal(domains[0], "test.dom");
assert_null(domains[1]);

ret = sss_certmap_expand_mapping_rule(ctx, discard_const(test_cert_der),
sizeof(test_cert_der),
&filter, &domains);
assert_int_equal(ret, 0);
assert_non_null(filter);
assert_string_equal(filter, "rule97=<I>O=IPA.DEVEL,CN=Certificate Authority"
"<S>CN=ipa-devel.ipa.devel,O=IPA.DEVEL");
assert_non_null(domains);
Expand All @@ -1492,6 +1533,17 @@ static void test_sss_certmap_get_search_filter(void **state)
&filter, &domains);
assert_int_equal(ret, 0);
assert_non_null(filter);
assert_string_equal(filter, "rule96=<I>O=IPA.DEVEL,CN=Certificate\\20Authority"
"<S>O=IPA.DEVEL,CN=ipa-devel.ipa.devel");
assert_non_null(domains);
assert_string_equal(domains[0], "test.dom");
assert_null(domains[1]);

ret = sss_certmap_expand_mapping_rule(ctx, discard_const(test_cert_der),
sizeof(test_cert_der),
&filter, &domains);
assert_int_equal(ret, 0);
assert_non_null(filter);
assert_string_equal(filter, "rule96=<I>O=IPA.DEVEL,CN=Certificate Authority"
"<S>O=IPA.DEVEL,CN=ipa-devel.ipa.devel");
assert_non_null(domains);
Expand All @@ -1510,6 +1562,14 @@ static void test_sss_certmap_get_search_filter(void **state)
assert_string_equal(filter, "(userCertificate;binary=" TEST_CERT_BIN ")");
assert_null(domains);

ret = sss_certmap_expand_mapping_rule(ctx, discard_const(test_cert_der),
sizeof(test_cert_der),
&filter, &domains);
assert_int_equal(ret, 0);
assert_non_null(filter);
assert_string_equal(filter, "(userCertificate;binary=" TEST_CERT_BIN ")");
assert_null(domains);

ret = sss_certmap_add_rule(ctx, 94,
"KRB5:<ISSUER>CN=Certificate Authority,O=IPA.DEVEL",
"LDAP:rule94=<I>{issuer_dn!ad_x500}<S>{subject_dn!ad_x500}",
Expand All @@ -1520,12 +1580,22 @@ static void test_sss_certmap_get_search_filter(void **state)
&filter, &domains);
assert_int_equal(ret, 0);
assert_non_null(filter);
assert_string_equal(filter, "rule94=<I>O=IPA.DEVEL,CN=Certificate Authority"
assert_string_equal(filter, "rule94=<I>O=IPA.DEVEL,CN=Certificate\\20Authority"
"<S>O=IPA.DEVEL,CN=ipa-devel.ipa.devel");
assert_non_null(domains);
assert_string_equal(domains[0], "test.dom");
assert_null(domains[1]);

ret = sss_certmap_expand_mapping_rule(ctx, discard_const(test_cert_der),
sizeof(test_cert_der),
&filter, &domains);
assert_int_equal(ret, 0);
assert_non_null(filter);
assert_string_equal(filter, "rule94=<I>O=IPA.DEVEL,CN=Certificate Authority"
"<S>O=IPA.DEVEL,CN=ipa-devel.ipa.devel");
assert_non_null(domains);
assert_string_equal(domains[0], "test.dom");
assert_null(domains[1]);

ret = sss_certmap_add_rule(ctx, 89, NULL,
"(rule89={subject_nt_principal})",
Expand All @@ -1539,6 +1609,14 @@ static void test_sss_certmap_get_search_filter(void **state)
assert_string_equal(filter, "(rule89=tu1@ad.devel)");
assert_null(domains);

ret = sss_certmap_expand_mapping_rule(ctx, discard_const(test_cert2_der),
sizeof(test_cert2_der),
&filter, &domains);
assert_int_equal(ret, 0);
assert_non_null(filter);
assert_string_equal(filter, "(rule89=tu1@ad.devel)");
assert_null(domains);

ret = sss_certmap_add_rule(ctx, 88, NULL,
"(rule88={subject_nt_principal.short_name})",
NULL);
Expand All @@ -1560,6 +1638,15 @@ static void test_sss_certmap_get_search_filter(void **state)
&filter, &domains);
assert_int_equal(ret, 0);
assert_non_null(filter);
assert_string_equal(filter, "rule87=<I>DC=devel,DC=ad,CN=ad-AD-SERVER-CA"
"<S>DC=devel,DC=ad,CN=Users,CN=t\\20u,E=test.user@email.domain");
assert_null(domains);

ret = sss_certmap_expand_mapping_rule(ctx, discard_const(test_cert2_der),
sizeof(test_cert2_der),
&filter, &domains);
assert_int_equal(ret, 0);
assert_non_null(filter);
assert_string_equal(filter, "rule87=<I>DC=devel,DC=ad,CN=ad-AD-SERVER-CA"
"<S>DC=devel,DC=ad,CN=Users,CN=t u,E=test.user@email.domain");
assert_null(domains);
Expand All @@ -1573,6 +1660,15 @@ static void test_sss_certmap_get_search_filter(void **state)
&filter, &domains);
assert_int_equal(ret, 0);
assert_non_null(filter);
assert_string_equal(filter, "rule86=<I>DC=devel,DC=ad,CN=ad-AD-SERVER-CA"
"<S>DC=devel,DC=ad,CN=Users,CN=t\\20u,E=test.user@email.domain");
assert_null(domains);

ret = sss_certmap_expand_mapping_rule(ctx, discard_const(test_cert2_der),
sizeof(test_cert2_der),
&filter, &domains);
assert_int_equal(ret, 0);
assert_non_null(filter);
assert_string_equal(filter, "rule86=<I>DC=devel,DC=ad,CN=ad-AD-SERVER-CA"
"<S>DC=devel,DC=ad,CN=Users,CN=t u,E=test.user@email.domain");
assert_null(domains);
Expand Down

0 comments on commit a2b9a84

Please sign in to comment.