Make SSSD in 20.04 using OpenSSL and p11-kit (instead of NSS) for p11_child

Bug #1905790 reported by Marco Trevisan (Treviño)
16
This bug affects 1 person
Affects Status Importance Assigned to Milestone
sssd (Ubuntu)
Fix Released
High
Unassigned
Focal
Fix Released
High
Marco Trevisan (Treviño)

Bug Description

[ Impact ]

SSSD supports in 20.04 two security backends: NSS and OpenSSL
(speaking in past tense as upstream dropped NSS support completely).

Those two backends are used for various generic crypto features (so they are interchangeable), but also for the management of the PKCS#11 modules for smart cards.

In this case, the main problem is that by using NSS it also relies on the presence of a "system NSS" database [1] that is something present in Fedora and RHEL, but not in ubuntu or generic Linux distributions.

In order to make SSSD to find a smart card module, we would then need to create a such database that mentions a p11kit proxy that will eventually load the p11-kit module and then add the card CA certificate to the same DB (see more details in [2]).
And even in such case... It will not work at login phase.

This is making support for Smart-card based authentication in 20.04 quite complicated, and hard to implement in professional environments (see bug #1865226).

As per this, recompiling SSSD's p11_child to use OpenSSL (as it already happens starting from 20.10) would be enough to make the this tool (the one in charge for smartcard authentications and certificate matching) to be able to get the smartcard devices from p11-kit allowed modules and to check their certificate using CA certificates in the ubuntu system ca certificate files (or other configured file).

One more mayor reason to do this, is also that if we fix 20.04 now to use the "proper" method, people who will configure smartcard access there via SSSD (not easily possible right now) won't be affected by future migrations.

[ Proposed Implementations ]

1) Use p11-kit and openssl for p11_child, by changing the build/test system (preferred)
   https://salsa.debian.org/3v1n0-guest/sssd/-/commits/p11-kit-p11_child

2) Build both versions and package things accordingly (hackish)
   https://salsa.debian.org/3v1n0-guest/sssd/-/commits/p11-kit-p11_child-v1

3) Recompile SSSD completely to use libcrypto as backend

The option 3) has been finally choosen, but we also require migration scripts on upgrade.

[ Test case ]

With a smartcard reader available (and with a card in its slot) as reported by:
 $ p11-kit list-modules

launch:
 $ sudo /usr/libexec/sssd/p11_child --pre -d 10 --debug-fd=2 \
   --nssdb=/etc/ssl/certs/ca-certificates.crt

The tool should find your card:

(2020-11-26 21:34:22:020395): [p11_child[100729]] [do_card] (0x4000): Module List:
(2020-11-26 21:34:22:020481): [p11_child[100729]] [do_card] (0x4000): common name: [p11-kit-trust].
(2020-11-26 21:34:22:020497): [p11_child[100729]] [do_card] (0x4000): dll name: [/usr/lib/x86_64-linux-gnu/pkcs11/p11-kit-trust.so].
(2020-11-26 21:34:22:020569): [p11_child[100729]] [do_card] (0x4000): Description [/etc/ssl/certs/ca-certificates.crt PKCS#11 Kit ] Manufacturer [PKCS#11 Kit ] flags [1] removable [false] token present [true].
(2020-11-26 21:34:22:020611): [p11_child[100729]] [do_card] (0x4000): common name: [opensc-pkcs11].
(2020-11-26 21:34:22:020646): [p11_child[100729]] [do_card] (0x4000): dll name: [/usr/lib/x86_64-linux-gnu/pkcs11/opensc-pkcs11.so].
(2020-11-26 21:34:22:025443): [p11_child[100729]] [do_card] (0x4000): Description [VMware Virtual USB CCID 00 00 VMware ] Manufacturer [VMware ] flags [7] removable [true] token present [true].
(2020-11-26 21:34:22:025725): [p11_child[100729]] [do_card] (0x4000): Found [MARCO TREVISAN (PIN CNS0)] in slot [VMware Virtual USB CCID 00 00][0] of module [1][/usr/lib/x86_64-linux-gnu/pkcs11/opensc-pkcs11.so].

Then:
 1) If you previously configured SSSD match rules and/or CA certificates:
    - You should still get your certificate public key printed as output
    - Configured login with smartcard should continue working

 2) If SSSD was not configured to do smartcard authentication:
    - p11_child may fail if the card certificate was not previously added to
      the trusted DB, but this is outside of this test case.
    - What it matters is that the card is found.

[ Regression potential ]

While the change may involve quite different code paths when it comes to security features, I think we trust OpenSSL enough to be an acceptable crypto backend for PKCS#11 operations. Behavior should not change, also assuming that upstream dropped NSS support completely in latest release [3], keeping the same functionalities.

As per a further review of this by xnox [4], we can safely assume that SSSD does not use libcrypto for operations where its behavior should differ from NSS. As it's needed only for certificates handling.

The only binary that is really affected in its behavior is p11_child (as per p11-kit usage instead of NSS for getting pkcs#11 modules).

So this change will break only those setup (if there are any, given that smartcard access is currently not supported by ubuntu) that have been manually configured using an unsupported system NSS db.

While we're providing a post-install script that migrates the possibly configured NSS CA certificates, there could be still possible regressions:

1) certificates not to be handled (referenced) in the same way, for example in the SSSD
   certmap: the mapping between users and their certificate could change, not making an
   user being able to access to the system anymore, not being correctly be correctly
   associated to a certificate.

   -> This can be fixed by adapting the [certmap/*/*] options in sssd.conf

2) custom p11-kit modules configured as allowed in the NSS database and not recognized by
   p11-kit, won't be accepted anymore, so again login won't work as p11_child won't find a
   module.

   -> Modules can be added creating .module files in /usr/share/p11-kit/modules/

So 1) can be the mayor concern here, even though I assume the few custom installations that there might be around can be adapted to this, in case this proves to be an important regression we can go back to use NSS as backend for libsss_certs, but still using p11-kit + openssl for p11_child.

Instead 2) can be a lower problem to handle, in case of a regression we can think of listing all the modules added to the NSS database, and if any, generate a module file for it, but I'd prefer to avoid this unless needed as we should trust them.

Said this, given the fact that there are probably not known implementations using this system for authentication in Ubuntu, I'm confident that we can accept those two regressions as they are, but being prepared to handle them (as described) if they end up in being real concerns.

[1] https://github.com/SSSD/sssd/blob/sssd-2_3_1/src/responder/pam/pamsrv.c#L53
[2] https://hackmd.io/@3v1n0/ubuntu-smartcard-login#NSS-Database-to-be-deprecated-post-2004
[3] https://github.com/SSSD/sssd/issues/1041
[4] https://bugs.launchpad.net/ubuntu/+source/sssd/+bug/1905790/comments/10

Related branches

Changed in sssd (Ubuntu Focal):
importance: Undecided → High
description: updated
description: updated
tags: added: server-next
Revision history for this message
Robie Basak (racb) wrote :

> While the change may involve quite different code paths when it comes to security features, I think we trust OpenSSL enough to be an acceptable crypto backend. And behavior should not change.

Are you sure about this? TLS has a wide variety of protocol options and the supported vs. "available" cryptosystem matrix is complex. Won't these all change if the underlying implementation changes?

Changed in sssd (Ubuntu Focal):
assignee: nobody → Sergio Durigan Junior (sergiodj)
description: updated
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> Are you sure about this? TLS has a wide variety of protocol options and the supported vs.
> "available" cryptosystem matrix is complex. Won't these all change if the underlying
> implementation changes?

Well, I focused mostly in the PKCS#11 changes, but for all its internal crypto operations SSSD had for some long time now [1] started supporting OpenSSL, replaced as default [2] and finally dropped [3] NSS at all and the two crypto backends have been used as feature-parity alternatives.

Probably not enough to compare, but from what I see in these matrices [4], there's basically nothing that NSS supports and OpenSSL doesn't (while it's true the other way around).

Not to mention that we already switched to an OpenSSL-based version of SSSD in 21.10, and even if its user base can't be compared to 20.04, so far I didn't read about related issues [5].

That said, if the SRU team would feel more confident in only having the p11_child to be built with OpenSSL, it should be technically possible, of course not as easy (and probably safer and more future-proof) as switching completely.

[1] https://github.com/SSSD/sssd/issues/4521
[2] https://github.com/SSSD/sssd/pull/1042
[3] https://github.com/SSSD/sssd/issues/1041
[4] https://en.wikipedia.org/wiki/Comparison_of_TLS_implementations
[5] https://github.com/SSSD/sssd/issues?q=is%3Aissue+openssl+

Revision history for this message
Robie Basak (racb) wrote : Re: [Bug 1905790] Re: Recompile SSSD in 20.04 using OpenSSL (instead of NSS) support

On Tue, Dec 01, 2020 at 03:33:45AM -0000, Marco Trevisan (Treviño) wrote:
> Probably not enough to compare, but from what I see in these matrices
> [4], there's basically nothing that NSS supports and OpenSSL doesn't
> (while it's true the other way around).

OK, but what about build configuration and default enabled cryptosuites
and suchlike? For example we've "locked down" OpenSSL's default
configuration to no longer support some older cryptosuites. Will
swapping NSS for OpenSSL cause user configurations to narrow the set of
cryptosuites that are enabled?

What if, for example, someone has an LDAP server that only supports
older TLS, and switching to OpenSSL causes their sssd LDAP TLS client to
require newer TLS because of our stronger defaults? What I describe
would result in a regression for that user until they reconfigure
things. Is this a realistic possibility?

> Not to mention that we already switched to an OpenSSL-based version of
> SSSD in 21.10, and even if its user base can't be compared to 20.04, so
> far I didn't read about related issues [5].

I think you're thinking of functional regressions here (ie. introducing
actual bugs), whereas I'm more bothered about regressing edge case user
configurations (eg. introducing a change that requires users to change
their local configurations to avoid a behavioural regression).

tags: added: rls-ff-incoming
description: updated
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote : Re: Recompile SSSD in 20.04 using OpenSSL (instead of NSS) support

> What if, for example, someone has an LDAP server that only supports
> older TLS, and switching to OpenSSL causes their sssd LDAP TLS client to
> require newer TLS because of our stronger defaults? What I describe
> would result in a regression for that user until they reconfigure
> things. Is this a realistic possibility?

First, are we sure that such scenario would currently work in current NSS?

I can't say whether that's a realistic scenario, we would need metrics, but I also think that if you're forcing a more secure behavior it's not to me a regression, it's making people aware that they're misbehaving.

As we do SRU a browser version that no longer accepts a deprecated crypto mechanisms, potentially causing an user regression, I don't see a problem in doing it other tools.

It may require an admin action? Yes, but that's acceptable IMHO when the system in use is known to be not secure.
And IMHO we're responsible for that too, not just accept people to use unsafe methods by default.

> I think you're thinking of functional regressions here (ie. introducing
> actual bugs), whereas I'm more bothered about regressing edge case user
> configurations (eg. introducing a change that requires users to change
> their local configurations to avoid a behavioural regression).

I'm thinking at those too (and especially in my scenario), but given there's right now no known actual and reported regression (not just in Ubuntu, but everywhere in the web I've searched for), so while there might be indeed edge cases until I don't have proofs of them I still thinking that the proposed change can only cause an improvement.

--

BTW, unrelated to this, but this request mostly is triggered by bug #1865226, and to support it reliably we need to use open-ssl based p11_child.

Revision history for this message
Robie Basak (racb) wrote : Re: [Bug 1905790] Re: Recompile SSSD in 20.04 using OpenSSL (instead of NSS) support
Download full text (4.3 KiB)

On Tue, Dec 01, 2020 at 03:22:33PM -0000, Marco Trevisan (Treviño) wrote:
> > What if, for example, someone has an LDAP server that only supports
> > older TLS, and switching to OpenSSL causes their sssd LDAP TLS client to
> > require newer TLS because of our stronger defaults? What I describe
> > would result in a regression for that user until they reconfigure
> > things. Is this a realistic possibility?
>
> First, are we sure that such scenario would currently work in current
> NSS?

I don't know. You tell me! I expect this to be considered and
investigated in advance of an SRU.

> I can't say whether that's a realistic scenario, we would need metrics,
> but I also think that if you're forcing a more secure behavior it's not
> to me a regression, it's making people aware that they're misbehaving.
>
> As we do SRU a browser version that no longer accepts a deprecated
> crypto mechanisms, potentially causing an user regression, I don't see a
> problem in doing it other tools.

I agree that it may be reasonable in principle to bump up default
cryptosystem requirements during the lifetime of an LTS on security
grounds. However that decision should be made deliberately and carefully
as part of a security-driven review into the trade-offs between security
enhancement and user regression.

In the case of browsers, this review is done by upstreams and
distributions generally have no choice in the matter. In the case of
such a change being driven by Ubuntu, I'd expect the review to be driven
*by the security concern itself*, probably have input from the security
team, and for the proposed change to have a specific security-enhancing
goal.

Swapping out NSS for OpenSSL without analyzing these types changes, and
therefore possibly *accidentally* adjusting this type of configuration,
does not meet my expectations detailed above and in my opinion is
unacceptable.

You seem to be claiming that my example would enhance security, albeit
in a breaking way, and is thus acceptable. Without analyzing the
details, how do you know my example is the reality though? How do you
know it isn't regressing security in a breaking way?

> It may require an admin action? Yes, but that's acceptable IMHO when the system in use is known to be not secure.
> And IMHO we're responsible for that too, not just accept people to use unsafe methods by default.
>
> > I think you're thinking of functional regressions here (ie. introducing
> > actual bugs), whereas I'm more bothered about regressing edge case user
> > configurations (eg. introducing a change that requires users to change
> > their local configurations to avoid a behavioural regression).
>
> I'm thinking at those too (and especially in my scenario), but given
> there's right now no known actual and reported regression (not just in
> Ubuntu, but everywhere in the web I've searched for), so while there
> might be indeed edge cases until I don't have proofs of them I still
> thinking that the proposed change can only cause an improvement.

I disagree with your approach here. To land an SRU we are expected to
consider what regressions *might* occur, even if we don't specifically
have evidence about them. Lack of known ...

Read more...

summary: - Recompile SSSD in 20.04 using OpenSSL (instead of NSS) support
+ Recompile SSSD in 20.04 using OpenSSL (instead of NSS) support for
+ p11_child
description: updated
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote : Re: Recompile SSSD in 20.04 using OpenSSL (instead of NSS) support for p11_child

Soo... Given we prefer to stay conservative and not change SSSD crypto backend fully (to be clear, I would have preferred it to follow upstream, not to provide a solution that will change in next LTS no matter what, and avoid having "frankensteins", but wasn't a strong requirement for me) I've been exploring ways to get only the component we care (p11_child) to use p11-kit and openssl.

As per this, I've prepared two possible approaches in two patches (I'd just squash those or pick one in case).

The simplest approach [1] was to just compile the NSS version and then only the p11_child using OpenSSL and then manually install to the package... Ensuring that we always pass to it a PEM CA cert file. Works, but will not allow us to test it using upstream tests.

So, I've added a further patch that acts mostly on upstream code and removes the usage of libnss ONLY from p11_child and its related operations (smartcard and ssh auth), you can see it better in this patch-queue branch (check the default one to see the debian/* changes):
 - https://salsa.debian.org/3v1n0-guest/sssd/-/commits/patch-queue/p11-kit-p11_child

This works properly and it's tested, you can try the packages at:
 - https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/4361.1

Theoretically, it would be even possible to keep support for an NSS p11_child (i.e. provide two binaries, and select which one to use depending on the db defined in configuration file), but as said in the bug description I don't think that such change would actually matter for anyone, as we don't provide a system NSS database.

Robie, this would be better SRU approach?

[1] https://salsa.debian.org/3v1n0-guest/sssd/-/commits/p11-kit-p11_child-v1

summary: - Recompile SSSD in 20.04 using OpenSSL (instead of NSS) support for
+ Make SSSD in 20.04 using OpenSSL and p11-kit (instead of NSS) for
p11_child
description: updated
description: updated
tags: added: patch
Revision history for this message
Robie Basak (racb) wrote : Re: [Bug 1905790] Re: Recompile SSSD in 20.04 using OpenSSL (instead of NSS) support for p11_child

On Wed, Dec 02, 2020 at 03:29:43AM -0000, Marco Trevisan (Treviño) wrote:
> Soo... Given we prefer to stay conservative and not change SSSD crypto

I didn't say that!

> backend fully (to be clear, I would have preferred it to follow
> upstream, not to provide a solution that will change in next LTS no
> matter what, and avoid having "frankensteins", but wasn't a strong
> requirement for me) I've been exploring ways to get only the component
> we care (p11_child) to use p11-kit and openssl.

This is certainly a valuable angle to look at - thanks!

> Robie, this would be better SRU approach?

I think you misunderstand me. I'm not saying that your upload *has* to
be narrow. I've not formed an opinion that yet. What I'm saying is that
whatever size of scope you choose, there must be a regression analysis
that covers that scope.

If you take a widely scope, then I expect a regression analysis to cover
what I feel are the obvious possible implications of that change. By
nature of it being wider, the regression analysis can be expected to be
more work, of course. Because a wider scope generally correlates with
increased regression risk, I'd also expect a justification of why the
narrow scope is less desirable. But the analysis is still necessary and
must not be skipped.

If you take a narrow scope, then that's correlated with lower regression
risk, and because a regression analysis would be narrower in scope to
match, it might well be less work.

I appreciate that sometimes it's harder or riskier to narrow the scope,
so I'm still open to widening the scope - *if* there is an appropriate
justification *and* full regression analysis of that wider scope
provided.

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

This does raise a question as to why we don't provide a system nssdb. I think we should. I wonder if libnss or libnss3-tools could ship ca-certificates hook to provide a system nssdb certificate store.

If we are changing backends, and certs were provided for the nss backend, imho we should automatically convert them and keep them active for the openssl backend. However unlikely it is that somebody made nss-based p11_child work.

In nss, we do set minimum TLS version TLSv1.2 but we do not enforce 112 bits of security like we do with OpenSSL. Specifically that is 2k RSA minimum key size, nor prohibit SHA1/MD5 cert hashes, and any cipher suites that use RC4. These changes in minimum requirements do not affect p11_child, but would affect sssd itself when talking over ldaps. I would be worried that an LDAP server has a lowish sized key in their cert, and suddenly an upgrade of sssd, once caches expire, prevent logins.

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

If we want to change the main sssd backend from nss to openssl, imho it would be prudent enough to use http://manpages.ubuntu.com/manpages/hirsute/en/man3/SSL_set_security_level.3ssl.html APIs to set_security_level to 1.

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Actually, I don't see sssd at all using TLS connections, does it? It seems that to perform ldaps connections, it uses libldap from openldap which in turn uses GnuTLS. And any and all TLS LDAPS options are simply passed through to the libldap.

Inspecting all sssd binary packages I can see that only p11_child is the only one using libssl and that does not do TLS.

libsss-certmap0 uses libcrypto.so.1.1 only for certificate parsing but not for TLS.

Thus changing nss => openssl backend should be immaterial to what sssd uses from them.

The only concern from me is to migrate custom certs that p11_child trusts, if there are any configured, and migration is needed between the backends. I don't know how to configure p11_child but I do have smartcard reader and multiple smartcards so happy to test things =)

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

>> Soo... Given we prefer to stay conservative and not change SSSD crypto
>
> I didn't say that!

I know, I'm not saying that you took a decision on that but I was
speaking in plural form as I recognize what you say in the sense that
indeed there may be cases which we don't think of that we could break.

>> backend fully (to be clear, I would have preferred it to follow
>> upstream, not to provide a solution that will change in next LTS no
>> matter what, and avoid having "frankensteins", but wasn't a strong
>> requirement for me) I've been exploring ways to get only the component
>> we care (p11_child) to use p11-kit and openssl.
>
> This is certainly a valuable angle to look at - thanks!
>
>> Robie, this would be better SRU approach?
>
> I think you misunderstand me. I'm not saying that your upload *has* to
> be narrow. I've not formed an opinion that yet. What I'm saying is that
> whatever size of scope you choose, there must be a regression analysis
> that covers that scope.

I understood this, reason why I thought that, given we have the chance
to make it a narrower scope, then I tried to get that done.

> But the analysis is still necessary and must not be skipped.

Sure, not trying to do that, I'm just saying that I can't over all the
cases myself.

> I appreciate that sometimes it's harder or riskier to narrow the scope,
> so I'm still open to widening the scope - *if* there is an appropriate
> justification *and* full regression analysis of that wider scope
> provided.

Problem is that I'm quite sure we can't cover all the cases in a such
complicated piece of software that may be configured in so many ways.
Thus the reason I thought narrowing the scope was a better idea.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote : Re: [Bug 1905790] Re: Make SSSD in 20.04 using OpenSSL and p11-kit (instead of NSS) for p11_child

> This does raise a question as to why we don't provide a system nssdb. I
> think we should. I wonder if libnss or libnss3-tools could ship ca-
> certificates hook to provide a system nssdb certificate store.

I don't think it makes much sense at this point as most of the tools
that were depending on NSS are leaving it anyways (curl, sssd...) and
even Fedora is trying to get rid the usage of libnss completely and only
support one crypto backend.

So, I was thinking of doing that and it could be a possibility, but
wouldn't still be a futurable solution as we'd regress in next LTS, and
so we'd end up providing a solution for this LTS (for something that we
didn't support so far) that is going to be broken in the next version.
And I don't think it's a professional thing to ask our users to setup
something and reconfigure it at next mayor update when we can start with
the right foot now.

> If we are changing backends, and certs were provided for the nss
> backend, imho we should automatically convert them and keep them active
> for the openssl backend. However unlikely it is that somebody made nss-
> based p11_child work.

Yeah, as I said isn't hard to do... The only problem I see is that the
postinst script for NSS should depend on libnss3-tools (if we don't
write us something in C that is shipped with SSSD) in order to read the
certs and export them to the OpenSSL chain.

As you said, it's quite unlikely, but could happen.

> Actually, I don't see sssd at all using TLS connections, does it? It
> seems that to perform ldaps connections, it uses libldap from openldap
> which in turn uses GnuTLS. And any and all TLS LDAPS options are simply
> passed through to the libldap.

I had this feeling too, both looking at the code and at the various logs
I found around, where I noticed that connection was handled differently,
but not being the maximum expert here, I preferred not to talk. So happy
you say so.

> Inspecting all sssd binary packages I can see that only p11_child is the
> only one using libssl and that does not do TLS.

Yeah, exactly... It does only certs management basically.

> Thus changing nss => openssl backend should be immaterial to what sssd
> uses from them.

Ok, good to hear.

> I don't know how to configure p11_child but I do have
> smartcard reader and multiple smartcards so happy to test things =)

I wrote a bit of hints in this document, should help:
 https://hackmd.io/@3v1n0/ubuntu-smartcard-login

description: updated
Revision history for this message
Timo Aaltonen (tjaalton) wrote :

re: system nssdb; let's not go there anymore, Fedora already moved to openssl system-wide

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

+1 to Timo to not go for "system nssdb" for the cause of this case here.
Also system-wide-trust would be bug 1647285 and is quite a different scope.

description: updated
description: updated
Changed in sssd (Ubuntu Focal):
assignee: Sergio Durigan Junior (sergiodj) → Marco Trevisan (Treviño) (3v1n0)
Revision history for this message
Robie Basak (racb) wrote :

> + certs = CERT_CreateSubjectCertList (NULL, handle, &cert->derSubject,

Doesn't this need a return value test? AFAICT, CERT_CreateSubjectCertList might return NULL, and CERTLIST_HEAD (certs) will unconditionally look up a member? There's a second instance of this pattern in print_trusted_certificates().

However, since the postinst only calls nss-database-pem-exporter from inside import_nss_ca_certs(), the "set -e" won't have any effect there, so I think this is OK in practice. I'd normally ask for more explicit error handling (or at least comments in the postinst) but since this migration code will only exist in this SRU, I think it's OK to leave it as-is.

> + if dpkg --compare-versions "$2" lt-nl 2.2.3-3ubuntu0.2; then

Doesn't this now need bumping to 0.4? The current version in focal-updates is 2.2.3-3ubuntu0.3. Otherwise I think the upgrade path won't activate for anyone already on 2.2.3-3ubuntu0.2 or 2.2.3-3ubuntu0.3?

Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

On Thursday, February 18 2021, Robie Basak wrote:

>> + certs = CERT_CreateSubjectCertList (NULL, handle,
> &cert->derSubject,
>
> Doesn't this need a return value test? AFAICT,
> CERT_CreateSubjectCertList might return NULL, and CERTLIST_HEAD (certs)
> will unconditionally look up a member? There's a second instance of this
> pattern in print_trusted_certificates().

Agreed. I can expand the code to make it check for NULL.

> However, since the postinst only calls nss-database-pem-exporter from
> inside import_nss_ca_certs(), the "set -e" won't have any effect there,
> so I think this is OK in practice. I'd normally ask for more explicit
> error handling (or at least comments in the postinst) but since this
> migration code will only exist in this SRU, I think it's OK to leave it
> as-is.
>
>> + if dpkg --compare-versions "$2" lt-nl 2.2.3-3ubuntu0.2; then
>
> Doesn't this now need bumping to 0.4? The current version in focal-
> updates is 2.2.3-3ubuntu0.3. Otherwise I think the upgrade path won't
> activate for anyone already on 2.2.3-3ubuntu0.2 or 2.2.3-3ubuntu0.3?

Yes, this one slipped past my radar. There were two more uploads since
Marco posted his first MP, and although he did rebase it against the
latest sssd on Focal, we forgot about this check.

How should I proceed here? Should I just upload the new package with
the same version, since it wasn't accepted yet?

Thanks,

--
Sergio
GPG key ID: E92F D0B3 6B14 F1F4 D8E0 EB2F 106D A1C8 C3CB BF14

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Ok I was quite sure that CERT_LIST_HEAD was already guarding us from NULL pointers (as in many NSS places i didn't see the check) but it's not the case [1], so thanks!

[1] https://searchfox.org/mozilla-central/source/security/nss/lib/certdb/certt.h#361

Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

OK, new package (with the same version) uploaded now, which addresses the comments made by Robie. Let me know what you think. Thanks!

Revision history for this message
Robie Basak (racb) wrote : Please test proposed package

Hello Marco, or anyone else affected,

Accepted sssd into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/sssd/2.2.3-3ubuntu0.4 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in sssd (Ubuntu Focal):
status: New → Fix Committed
tags: added: verification-needed verification-needed-focal
Revision history for this message
Valters Jansons (sigv) wrote :
Download full text (8.0 KiB)

Performing verification on Focal (20.04) as described in test steps.

Local test system has a 4th generation Yubikey attached.
The Yubikey is a smartcard reader with an integrated card.
There's a certificate on card, issued from internal non-default CA.

 # # Install `p11-kit` for test case use.
 # apt install p11-kit
 # apt-cache policy p11-kit | grep Installed:
  Installed: 0.23.20-1ubuntu0.1

 # # Install `ykcs11` for Yubikey smartcard use on system.
 # # This could also be `opensc` or any other module package.
 # apt install ykcs11
 # apt-cache policy ykcs11 | grep Installed:
  Installed: 2.0.0-2
 # # Allow auto-discovery of ykcs11 PKCS#11 module:
 # echo 'module: ../libykcs11.so' > \
   /usr/share/p11-kit/modules/ykcs11.module

 # # Install SSSD from -updates.
 # apt install sssd/focal-updates
 # apt-cache policy sssd | grep Installed:
  Installed: 2.2.3-3ubuntu0.3

 # # Execute described test case.
 # p11-kit list-modules | grep -Eve '^ '
p11-kit-trust: p11-kit-trust.so
    library-description: PKCS#11 Kit Trust Module
    library-manufacturer: PKCS#11 Kit
    library-version: 0.23
    token: System Trust
ykcs11: ../libykcs11.so
    library-description: PKCS#11 PIV Library (SP-800-73)
    library-manufacturer: Yubico (www.yubico.com)
    library-version: 2.0
    token: YubiKey PIV #1234567
 # sudo /usr/libexec/sssd/p11_child --pre -d 10 --debug-fd=2 \
   --nssdb=/etc/ssl/certs/ca-certificates.crt
(Sat Feb 27 14:21:22:579260 2021) [[sssd[p11_child[3511]]]] [main] (0x0400): p11_child started.
(Sat Feb 27 14:21:22:579307 2021) [[sssd[p11_child[3511]]]] [main] (0x2000): Running in [pre-auth] mode.
(Sat Feb 27 14:21:22:579315 2021) [[sssd[p11_child[3511]]]] [main] (0x2000): Running with effective IDs: [0][0].
(Sat Feb 27 14:21:22:579322 2021) [[sssd[p11_child[3511]]]] [main] (0x2000): Running with real IDs [0][0].
(Sat Feb 27 14:21:22:581129 2021) [[sssd[p11_child[3511]]]] [do_card] (0x4000): Default Module List:
(Sat Feb 27 14:21:22:581145 2021) [[sssd[p11_child[3511]]]] [do_card] (0x4000): common name: [NSS Internal PKCS #11 Module].
(Sat Feb 27 14:21:22:581151 2021) [[sssd[p11_child[3511]]]] [do_card] (0x4000): dll name: [(null)].
(Sat Feb 27 14:21:22:581156 2021) [[sssd[p11_child[3511]]]] [do_card] (0x4000): Dead Module List:
(Sat Feb 27 14:21:22:581160 2021) [[sssd[p11_child[3511]]]] [do_card] (0x4000): DB Module List:
(Sat Feb 27 14:21:22:581165 2021) [[sssd[p11_child[3511]]]] [do_card] (0x4000): common name: [NSS Internal Module].
(Sat Feb 27 14:21:22:581170 2021) [[sssd[p11_child[3511]]]] [do_card] (0x4000): dll name: [(null)].
(Sat Feb 27 14:21:22:581175 2021) [[sssd[p11_child[3511]]]] [do_card] (0x4000): Description [NSS Internal Cryptographic Services Mozilla Foundation ] Manufacturer [Mozilla Foundation ] flags [9] removable [false] token present [true].
(Sat Feb 27 14:21:22:581182 2021) [[sssd[p11_child[3511]]]] [do_card] (0x4000): Description [NSS User Private Key and Certificate Services Mozilla Foundation ] Manufacturer [Mozilla Foundation ] flags [9] removable [false] token present [true].
(Sat Feb 27 14:21:22:5811...

Read more...

tags: added: verification-done verification-done-focal
removed: verification-needed verification-needed-focal
Revision history for this message
Valters Jansons (sigv) wrote :

LP appears to have stripped spaces from the `grep` command.
There was filtering on output to reduce verbosity.

Instead of what is seen in previous comment:
# p11-kit list-modules | grep -Eve '^ '
The actual executed verification command there is:
# p11-kit list-modules | grep -Eve '^ {5}'

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package sssd - 2.2.3-3ubuntu0.4

---------------
sssd (2.2.3-3ubuntu0.4) focal; urgency=medium

  [ Marco Trevisan ]
  * debian/control:
    - Add missing (test) dependencies as per libcrypto usage (LP: #1905790)
    - Update Maintainer to Ubuntu devs
  * debian/rules: Compile using libcrypto as crypto backend (LP: #1905790)
  * debian/nss-database-pem-exporter: Add to sssd-common and run on postinst.
    When upgrading from previous versions (that were compiled using the NSS
    crypto backend) we need to migrate the trusted CA certificates that the
    user may have added to the SSSD's NSS system database (that defaults to
    /etc/pki/nssdb).
    To do this, and not to introduce a new dependency on libnss3-tools
    (which is not shipped by default, other than making the parsing not
    working in some scenarios) I've added a small C tool that we compile and
    install as part of the sssd-common package which is able to get all the
    trusted CA certificates for a NSS database and export them in PEM
    format.
    The nss-database-pem-exporter is then used in the postinst script where
    we now:
     1. Read the SSSD settings
     2. Convert all the certificates in the configured NSS databases
     3. Store them all, appending them to the (new) default location
        (/etc/sssd/pki/sssd_auth_ca_db.pem)
     4. Disables the configured locations if pointing to NSS dbs (needed or
        we'll leave the configuration with broken values).
    At this point nss-database-pem-exporter is then the only binary in the
    package that still depends on NSS libraries. (LP: #1905790)
  * debian/patches:
    - Get libsofthsm2 from right path for each architecture, this is now used
      for real (wasn't before) to test p11k components with libcrypto and
      p11-kit, also avoids a test build failure on armhf (LP: #1905790)

  [ Valters Jansons ]
  * Avoid sending malformed SYSLOG_IDENTIFIER to journald (LP: #1908065):
    - d/rules: Set --with-syslog=journald in override_dh_auto_configure.
    - d/p/lp-1908065-01-debug_prg_name-format.patch:
      Upstream patch to clean up program names.
    - d/p/lp-1908065-02-syslog_identifier-format.patch:
      Upstream patch to include "sssd[]" identifier in program names.
    - d/p/lp-1908065-03-remove-syslog_identifier.patch:
      Upstream patch to remove custom SYSLOG_IDENTIFIER from Journald.

 -- Marco Trevisan (Treviño) <email address hidden> Thu, 11 Feb 2021 15:31:14 -0500

Changed in sssd (Ubuntu Focal):
status: Fix Committed → Fix Released
Revision history for this message
Łukasz Zemczak (sil2100) wrote : Update Released

The verification of the Stable Release Update for sssd has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :
Download full text (10.6 KiB)

Thanks Valters for your verification!

It's always better when someone that didn't commit the fix can help with it.

I've also done further verification to ensure that the migration happens as expected, so my sssd.conf was:

[sssd]
enable_files_domain = True
services = pam
certificate_verification = no_ocsp

[certmap/implicit_files/marco]
matchrule = <SUBJECT>.*TRVMRC[A-Z0-9]+/6090010669298009\.YOrY0zOk5CdMby2Z2O/HnVRA8Ao.*

[pam]
pam_cert_auth = True
pam_verbosity = 10
debug_level = 10
#pam_cert_db_path = /etc/ssl/certs/ca-certificates.crt
# pam_cert_db_path = /etc/pki/nssdb
pam_cert_db_path = /etc/pki/nssdb2
ca_db = /etc/pki/nssdb2
#ca_db = /etc/pki/nssdb

With /etc/pki/nssdb2 configured so that it was able to read my reader and containing the relative CA certificate:

$ sudo /usr/libexec/sssd/p11_child --pre -d 10 --debug-fd=2 --nssdb=/etc/pki/nssdb2
(Mon Mar 1 15:16:29:470908 2021) [[sssd[p11_child[70818]]]] [main] (0x0400): p11_child started.
(Mon Mar 1 15:16:29:470980 2021) [[sssd[p11_child[70818]]]] [main] (0x2000): Running in [pre-auth] mode.
(Mon Mar 1 15:16:29:470991 2021) [[sssd[p11_child[70818]]]] [main] (0x2000): Running with effective IDs: [0][0].
(Mon Mar 1 15:16:29:470998 2021) [[sssd[p11_child[70818]]]] [main] (0x2000): Running with real IDs [0][0].
(Mon Mar 1 15:16:31:152580 2021) [[sssd[p11_child[70818]]]] [do_card] (0x4000): Default Module List:
(Mon Mar 1 15:16:31:152668 2021) [[sssd[p11_child[70818]]]] [do_card] (0x4000): common name: [NSS Internal PKCS #11 Module].
(Mon Mar 1 15:16:31:152697 2021) [[sssd[p11_child[70818]]]] [do_card] (0x4000): dll name: [(null)].
(Mon Mar 1 15:16:31:152706 2021) [[sssd[p11_child[70818]]]] [do_card] (0x4000): common name: [PKCS#11 Kit modules proxy].
(Mon Mar 1 15:16:31:152715 2021) [[sssd[p11_child[70818]]]] [do_card] (0x4000): dll name: [/usr/lib/x86_64-linux-gnu/p11-kit-proxy.so].
(Mon Mar 1 15:16:31:152724 2021) [[sssd[p11_child[70818]]]] [do_card] (0x4000): Dead Module List:
(Mon Mar 1 15:16:31:152732 2021) [[sssd[p11_child[70818]]]] [do_card] (0x4000): DB Module List:
(Mon Mar 1 15:16:31:152750 2021) [[sssd[p11_child[70818]]]] [do_card] (0x4000): common name: [NSS Internal Module].
(Mon Mar 1 15:16:31:152759 2021) [[sssd[p11_child[70818]]]] [do_card] (0x4000): dll name: [(null)].
(Mon Mar 1 15:16:31:152769 2021) [[sssd[p11_child[70818]]]] [do_card] (0x4000): Description [NSS Internal Cryptographic Services Mozilla Foundation ] Manufacturer [Mozilla Foundation ] flags [9] removable [false] token present [true].
(Mon Mar 1 15:16:31:152818 2021) [[sssd[p11_child[70818]]]] [do_card] (0x4000): Description [NSS User Private Key and Certificate Services Mozilla Foundation ] Manufacturer [Mozilla Foundation ] flags [1] removable [false] token present [true].
(Mon Mar 1 15:16:31:153898 2021) [[sssd[p11_child[70818]]]] [do_card] (0x4000): Description [VMware Virtual USB CCID 00 00 VMware ] Manufacturer [VMware ] flags [7] removable [true] token present [true].
(Mon Mar 1 15:16:31:153949 2021) [...

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Ah, and of course the SSSD pem file is properly populated:

$ sudo openssl crl2pkcs7 -nocrl -certfile /etc/sssd/pki/sssd_auth_ca_db.pem | openssl pkcs7 -print_certs -noout | grep subject | wc -l
421

Mathew Hodson (mhodson)
no longer affects: ca-certificates (Ubuntu Focal)
no longer affects: ca-certificates (Ubuntu)
Revision history for this message
Karl Grindley (karlg100) wrote :

This change had created a denial of service configuration bug for an untold number of smart card configured (and smart card requires) systems.

p11_child requires with the OpenSSL PEM full cert chain to function. the NSSDB version does not.

So for folks that have configured the minimum in the NSSDB by only adding the issuing certificate (and not chain of certs to the root), smart card authentication will now fail by simply updating to the latest Ubuntu 20.04 packages. The nssdb to pam conversion script does not check chain of trust in the new pam file.

So when untold number of systems roll this out with require_cert_auth in the pam stack to be NIST 800-171 compliant, they will all be bricked and no way to login.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Karl,

The script (https://github.com/3v1n0/nss-database-pem-exporter) can be definitely adjusted to handle that, that meant to be simple as this requirement wasn't considered.

Any help is appreciated though.

Revision history for this message
Karl Grindley (karlg100) wrote :

I don't know of a great way to test this without pulling apart p11_child, or using it as part of a pre-flight check somehow during the package update. The problem here is you'd need a PKI cert to test that preflight.

As a failsafe, a dialog during upgrade with a preflight check of require_cert_auth in /etc/pam/common-password to throw a warning if the user continues with smart card enforcement. Force the user to ack to proceed, otherwise fail the package install.

Perhaps adding a debconf flag to allow bypassing this dialog of this by sysadmins.

Revision history for this message
Karl Grindley (karlg100) wrote :
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.