URL: https://github.com/SSSD/sssd/pull/814 Author: sumit-bose Title: #814: certmap: allow missing KU in OpenSSL version Action: opened
PR body: """ Make sure a missing key-usage (KU) is not treated as an error and is handled equally in the NSS and OpenSSL implementation
Related to https://bugzilla.redhat.com/show_bug.cgi?id=1660899
Additionally this pull-request contains patches which adds two new certificate related commands to the sssctl utility. 'sssctl cert-show' displays the content of the certificate in a format used by SSSD's certificate mapping and matching rules. 'sssctl cert-map' show the users the certificate is mapped to. Both commands expect the base64 encoded certificate on the command line. """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/814/head:pr814 git checkout pr814
URL: https://github.com/SSSD/sssd/pull/814 Author: sumit-bose Title: #814: certmap: allow missing KU in OpenSSL version Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/814/head:pr814 git checkout pr814
URL: https://github.com/SSSD/sssd/pull/814 Author: sumit-bose Title: #814: certmap: allow missing KU in OpenSSL version Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/814/head:pr814 git checkout pr814
URL: https://github.com/SSSD/sssd/pull/814 Author: sumit-bose Title: #814: certmap: allow missing KU in OpenSSL version Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/814/head:pr814 git checkout pr814
URL: https://github.com/SSSD/sssd/pull/814 Author: sumit-bose Title: #814: certmap: allow missing KU in OpenSSL version Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/814/head:pr814 git checkout pr814
URL: https://github.com/SSSD/sssd/pull/814 Title: #814: certmap: allow missing KU in OpenSSL version
sumit-bose commented: """ After a couple of updates the patches now parse the CI here and the internal one as well. """
See the full comment at https://github.com/SSSD/sssd/pull/814#issuecomment-492244812
URL: https://github.com/SSSD/sssd/pull/814 Title: #814: certmap: allow missing KU in OpenSSL version
jhrozek commented: """ There are some coverity warnings: `Error: COMPILER_WARNING: sssd-2.1.1/src/lib/certmap/sss_certmap.c: scope_hint: In function 'sss_cert_dump_content' sssd-2.1.1/src/lib/certmap/sss_certmap.c:998:65: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t' {aka 'long unsigned int'} [-Wformat=] # out = talloc_asprintf_append(out, "Extended Key Usage #%d: %s%s%s%s\n", # ~^ # 996| for (o = 0; c->extended_key_usage_oids[o] != NULL; o++) { # 997| eku_str = sss_eku_oid2name(c->extended_key_usage_oids[o]); # 998|-> out = talloc_asprintf_append(out, "Extended Key Usage #%d: %s%s%s%s\n", # 999| o, c->extended_key_usage_oids[o], # 1000| eku_str == NULL ? "" : " (",
Error: OVERRUN (CWE-119): sssd-2.1.1/src/lib/certmap/sss_certmap.c:1007: cond_at_most: Checking "s->san_opt < SAN_END" implies that "s->san_opt" may be up to 12 on the true branch. sssd-2.1.1/src/lib/certmap/sss_certmap.c:1013: overrun-local: Overrunning array "san_parsed_template" of 12 24-byte elements at element index 12 (byte offset 311) using index "s->san_opt" (which evaluates to 12). # 1011| if (out == NULL) return ENOMEM; # 1012| # 1013|-> if (san_parsed_template[s->san_opt].name != NULL) { # 1014| ret = expand_san(ctx, &san_parsed_template[s->san_opt], c->san_list, # 1015| &expanded);
Error: OVERRUN (CWE-119): sssd-2.1.1/src/lib/certmap/sss_certmap.c:1007: cond_at_most: Checking "s->san_opt < SAN_END" implies that "s->san_opt" may be up to 12 on the true branch. sssd-2.1.1/src/lib/certmap/sss_certmap.c:1014: overrun-local: Overrunning array of 288 bytes at byte offset 288 by dereferencing pointer "&san_parsed_template[s->san_opt]". # 1012| # 1013| if (san_parsed_template[s->san_opt].name != NULL) { # 1014|-> ret = expand_san(ctx, &san_parsed_template[s->san_opt], c->san_list, # 1015| &expanded); # 1016| if (ret != EOK) {
Error: OVERRUN (CWE-119): sssd-2.1.1/src/lib/certmap/sss_certmap.c:1007: cond_at_most: Checking "s->san_opt < SAN_END" implies that "s->san_opt" may be up to 12 on the true branch. sssd-2.1.1/src/lib/certmap/sss_certmap.c:1019: overrun-local: Overrunning array "san_parsed_template" of 12 24-byte elements at element index 12 (byte offset 311) using index "s->san_opt" (which evaluates to 12). # 1017| return ret; # 1018| } # 1019|-> out = talloc_asprintf_append(out, " %s=%s\n\n", # 1020| san_parsed_template[s->san_opt].name, # 1021| expanded);``
``` """
See the full comment at https://github.com/SSSD/sssd/pull/814#issuecomment-494385966
URL: https://github.com/SSSD/sssd/pull/814 Title: #814: certmap: allow missing KU in OpenSSL version
jhrozek commented: """ There are some coverity warnings:
``` Error: COMPILER_WARNING: sssd-2.1.1/src/lib/certmap/sss_certmap.c: scope_hint: In function 'sss_cert_dump_content' sssd-2.1.1/src/lib/certmap/sss_certmap.c:998:65: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t' {aka 'long unsigned int'} [-Wformat=] # out = talloc_asprintf_append(out, "Extended Key Usage #%d: %s%s%s%s\n", # ~^ # 996| for (o = 0; c->extended_key_usage_oids[o] != NULL; o++) { # 997| eku_str = sss_eku_oid2name(c->extended_key_usage_oids[o]); # 998|-> out = talloc_asprintf_append(out, "Extended Key Usage #%d: %s%s%s%s\n", # 999| o, c->extended_key_usage_oids[o], # 1000| eku_str == NULL ? "" : " (",
Error: OVERRUN (CWE-119): sssd-2.1.1/src/lib/certmap/sss_certmap.c:1007: cond_at_most: Checking "s->san_opt < SAN_END" implies that "s->san_opt" may be up to 12 on the true branch. sssd-2.1.1/src/lib/certmap/sss_certmap.c:1013: overrun-local: Overrunning array "san_parsed_template" of 12 24-byte elements at element index 12 (byte offset 311) using index "s->san_opt" (which evaluates to 12). # 1011| if (out == NULL) return ENOMEM; # 1012| # 1013|-> if (san_parsed_template[s->san_opt].name != NULL) { # 1014| ret = expand_san(ctx, &san_parsed_template[s->san_opt], c->san_list, # 1015| &expanded);
Error: OVERRUN (CWE-119): sssd-2.1.1/src/lib/certmap/sss_certmap.c:1007: cond_at_most: Checking "s->san_opt < SAN_END" implies that "s->san_opt" may be up to 12 on the true branch. sssd-2.1.1/src/lib/certmap/sss_certmap.c:1014: overrun-local: Overrunning array of 288 bytes at byte offset 288 by dereferencing pointer "&san_parsed_template[s->san_opt]". # 1012| # 1013| if (san_parsed_template[s->san_opt].name != NULL) { # 1014|-> ret = expand_san(ctx, &san_parsed_template[s->san_opt], c->san_list, # 1015| &expanded); # 1016| if (ret != EOK) {
Error: OVERRUN (CWE-119): sssd-2.1.1/src/lib/certmap/sss_certmap.c:1007: cond_at_most: Checking "s->san_opt < SAN_END" implies that "s->san_opt" may be up to 12 on the true branch. sssd-2.1.1/src/lib/certmap/sss_certmap.c:1019: overrun-local: Overrunning array "san_parsed_template" of 12 24-byte elements at element index 12 (byte offset 311) using index "s->san_opt" (which evaluates to 12). # 1017| return ret; # 1018| } # 1019|-> out = talloc_asprintf_append(out, " %s=%s\n\n", # 1020| san_parsed_template[s->san_opt].name, # 1021| expanded);``
``` """
See the full comment at https://github.com/SSSD/sssd/pull/814#issuecomment-494385966
URL: https://github.com/SSSD/sssd/pull/814 Author: sumit-bose Title: #814: certmap: allow missing KU in OpenSSL version Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/814/head:pr814 git checkout pr814
URL: https://github.com/SSSD/sssd/pull/814 Title: #814: certmap: allow missing KU in OpenSSL version
sumit-bose commented: """ Hi,
thanks for checking, the new version should fix the reported issues.
bye, Sumit """
See the full comment at https://github.com/SSSD/sssd/pull/814#issuecomment-494774556
URL: https://github.com/SSSD/sssd/pull/814 Author: sumit-bose Title: #814: certmap: allow missing KU in OpenSSL version Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/814/head:pr814 git checkout pr814
URL: https://github.com/SSSD/sssd/pull/814 Title: #814: certmap: allow missing KU in OpenSSL version
sumit-bose commented: """ Hi,
the lastest version should fix both comments.
bye, Sumit """
See the full comment at https://github.com/SSSD/sssd/pull/814#issuecomment-495509912
URL: https://github.com/SSSD/sssd/pull/814 Title: #814: certmap: allow missing KU in OpenSSL version
jhrozek commented: """ I'm sorry it took me almost a week to test the new patches. They seem to work fine: ``` [root@client ~]# sssctl cert-show $(base64 -w0 /home/jhrozek/certuser.der) ldb: unable to dlopen /usr/lib64/ldb/modules/ldb/memberof.la : /usr/lib64/ldb/modules/ldb/memberof.la: invalid ELF header sss cert content (format might change): Issuer: CN=federation test CA,OU=federation test,O=federation Subject: CN=certuser,OU=SSSD test,O=SSSD Key Usage: 176(0x00b0) (digitalSignature,keyEncipherment,dataEncipherment) Extended Key Usage #0: 1.3.6.1.5.5.7.3.1 (serverAuth) Extended Key Usage #1: 1.3.6.1.5.5.7.3.2 (clientAuth)
[root@client ~]# sssctl cert-map $(base64 -w0 /home/jhrozek/certuser.der) ldb: unable to dlopen /usr/lib64/ldb/modules/ldb/memberof.la : /usr/lib64/ldb/modules/ldb/memberof.la: invalid ELF header certuser ``` """
See the full comment at https://github.com/SSSD/sssd/pull/814#issuecomment-496689261
URL: https://github.com/SSSD/sssd/pull/814 Title: #814: certmap: allow missing KU in OpenSSL version
jhrozek commented: """ btw cert without KU: ``` sssctl cert-show $(base64 -w0 /home/jhrozek/certuser2.der) ldb: unable to dlopen /usr/lib64/ldb/modules/ldb/memberof.la : /usr/lib64/ldb/modules/ldb/memberof.la: invalid ELF header sss cert content (format might change): Issuer: CN=federation test CA,OU=federation test,O=federation Subject: CN=certuser2,OU=SSSD test,O=SSSD Key Usage: 4294967295(0xffffffff) (digitalSignature,nonRepudiation,keyEncipherment,dataEncipherment,keyAgreement,keyCertSign,cRLSign,encipherOnly,decipherOnly) Extended Key Usage #0: 1.3.6.1.5.5.7.3.1 (serverAuth) Extended Key Usage #1: 1.3.6.1.5.5.7.3.2 (clientAuth) ``` And with the first patch reverted: ``` sssctl cert-show $(base64 -w0 /home/jhrozek/certuser2.der) ldb: unable to dlopen /usr/lib64/ldb/modules/ldb/memberof.la : /usr/lib64/ldb/modules/ldb/memberof.la: invalid ELF header (Tue May 28 23:21:13:583505 2019) [sssd] [sssctl_cert_show] (0x0010): Failed to parsed certificate ```
--> ACK """
See the full comment at https://github.com/SSSD/sssd/pull/814#issuecomment-496694708
URL: https://github.com/SSSD/sssd/pull/814 Title: #814: certmap: allow missing KU in OpenSSL version
Label: +Accepted
URL: https://github.com/SSSD/sssd/pull/814 Title: #814: certmap: allow missing KU in OpenSSL version
jhrozek commented: """ * master: * f91d54e2d56f5babca6f6b3ca6e1a158fa889b45 * b0525a69c1dd979dcfabf5b24fe6b023a7d919fb * e122f495b98123db2f065b2c557d7b8d2f776a10 * 1c40208aa1e0f9a17cc4f336c99bcaa6977592d3 * e1734ba828470d00370c44c95da56822fdcc104d * aef8e49b7ee2e7743d6981070d61bc89b7c8fcfb """
See the full comment at https://github.com/SSSD/sssd/pull/814#issuecomment-496696129
URL: https://github.com/SSSD/sssd/pull/814 Author: sumit-bose Title: #814: certmap: allow missing KU in OpenSSL version Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/814/head:pr814 git checkout pr814
URL: https://github.com/SSSD/sssd/pull/814 Title: #814: certmap: allow missing KU in OpenSSL version
Label: +Pushed
URL: https://github.com/SSSD/sssd/pull/814 Title: #814: certmap: allow missing KU in OpenSSL version
jhrozek commented: """ btw please let me know if you want the patches backported to sssd-1-16 as well (or just push them yourself :-)) """
See the full comment at https://github.com/SSSD/sssd/pull/814#issuecomment-496696347
sssd-devel@lists.fedorahosted.org
-
jhrozek -
sumit-bose