Message ID | 20190115101522.21042-6-peter@korsgaard.com |
---|---|
State | Superseded |
Headers | show |
Series | [v2,1/7] tpm2-tss: do not enforce -fstack-protector-all | expand |
Peter, All, On 2019-01-15 11:15 +0100, Peter Korsgaard spake thusly: > tpm2-tools is commonly used with the resource manager, tpm2-abrmd - But it > CAN be used without, E.G. by setting the TPM2TOOLS_TCTI_NAME environment > variable to communicate directly with the kernel driver: > > export TPM2TOOLS_TCTI_NAME=device > > For some use cases (E.G. initramfs) it makes sense to use tpm2-tools > without abrmd, so downgrade the dependency from select to imply, so abrmd is > enabled by default but can be explicitly disabled. > > Signed-off-by: Peter Korsgaard <peter@korsgaard.com> > --- > package/tpm2-tools/Config.in | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/package/tpm2-tools/Config.in b/package/tpm2-tools/Config.in > index cc87e2a1bf..f4622b4ec9 100644 > --- a/package/tpm2-tools/Config.in > +++ b/package/tpm2-tools/Config.in > @@ -8,7 +8,7 @@ config BR2_PACKAGE_TPM2_TOOLS > select BR2_PACKAGE_LIBCURL > select BR2_PACKAGE_LIBGLIB2 > select BR2_PACKAGE_OPENSSL > - select BR2_PACKAGE_TPM2_ABRMD # run-time > + imply BR2_PACKAGE_TPM2_ABRMD # run-time Sorry, but I reiterate my position: I don't like the use of 'imply'. Either the thing is mandatory, in which case we select it or depend on it, or the thing is optional, in which case we elt the user enable it. Use of imply does not sound nice to me, because it is not authoritative. I'm afraid we get reports of users complaining that "sometimes the stuff is enabled when I do X, while sometmes it is not enabled when I do the same X.' The coutner argument has been that we were now trying to make sensible choices for the user, so that things "work out of the box". My position is that it is an illusion, because making things "just work" is more often than not more involving than just enabling a package. For example, when dealing with TPM and such: keys and certs provisionning and checking the chain of trust and such is only scratching the surface. People that want to deal with this topic better know what they *are* doing, as it is a sensible topic. Those people will have to understand what they need if they do not already know. Regards, Yann E. MORIN. > select BR2_PACKAGE_TPM2_TSS > help > TPM (Trusted Platform Module) 2.0 CLI tools based on system > -- > 2.11.0 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: > Peter, All, > On 2019-01-15 11:15 +0100, Peter Korsgaard spake thusly: >> tpm2-tools is commonly used with the resource manager, tpm2-abrmd - But it >> CAN be used without, E.G. by setting the TPM2TOOLS_TCTI_NAME environment >> variable to communicate directly with the kernel driver: >> >> export TPM2TOOLS_TCTI_NAME=device >> >> For some use cases (E.G. initramfs) it makes sense to use tpm2-tools >> without abrmd, so downgrade the dependency from select to imply, so abrmd is >> enabled by default but can be explicitly disabled. >> >> Signed-off-by: Peter Korsgaard <peter@korsgaard.com> >> --- >> package/tpm2-tools/Config.in | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/package/tpm2-tools/Config.in b/package/tpm2-tools/Config.in >> index cc87e2a1bf..f4622b4ec9 100644 >> --- a/package/tpm2-tools/Config.in >> +++ b/package/tpm2-tools/Config.in >> @@ -8,7 +8,7 @@ config BR2_PACKAGE_TPM2_TOOLS >> select BR2_PACKAGE_LIBCURL >> select BR2_PACKAGE_LIBGLIB2 >> select BR2_PACKAGE_OPENSSL >> - select BR2_PACKAGE_TPM2_ABRMD # run-time >> + imply BR2_PACKAGE_TPM2_ABRMD # run-time > Sorry, but I reiterate my position: I don't like the use of 'imply'. > Either the thing is mandatory, in which case we select it or depend on > it, or the thing is optional, in which case we elt the user enable it. I understand you don't like it, but what is the alternative? Just mention the optional-but-likely-to-be-needed dependency in the help text? That is IMHO worse than imply. For this specific case, tpm2-tools fails with a somewhat confusing error message if tpm2-abrmd isn't available unless a specific command line option / environment variable is used: # tpm2_pcrlist ** (process:8628): WARNING **: 11:38:39.606: Failed to create connection with service: GDBus.Error:org.freedesktop.DBus.Error.ServiceUnknown: The name com.intel.tss2.Tabrmd was not provided by any .service files ERROR: Failed to initialize TABRMD TCTI context: 0xa0008 The solution is to set the TCTI name to device, either through the TPM2TOOLS_TCTI_NAME environment variable or the --tcti option. This is imho exactly the kind of use cases imply has been made for. > Use of imply does not sound nice to me, because it is not authoritative. > I'm afraid we get reports of users complaining that "sometimes the stuff > is enabled when I do X, while sometmes it is not enabled when I do the > same X.' Is that any different than changing toolchain options or toggling BR2_PACKAGE_BUSYBOX_SHOW_OTHERS? > The coutner argument has been that we were now trying to make sensible > choices for the user, so that things "work out of the box". My position > is that it is an illusion, because making things "just work" is more > often than not more involving than just enabling a package. I agree that we probably cannot do this perfectly, but a solution for E.G. 80% of the use cases is still an improvement, as long as the remaining 20% can still change things. > For example, when dealing with TPM and such: keys and certs provisionning > and checking the chain of trust and such is only scratching the surface. > People that want to deal with this topic better know what they *are* doing, > as it is a sensible topic. Those people will have to understand what they > need if they do not already know. Sure, but we can atleast ensure that the tpm2-tools utilites do not fail out of the box because of a missing obscure dependency and that fairly common use cases are possible.
Peter, All, On 2019-01-16 12:43 +0100, Peter Korsgaard spake thusly: > >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: > > Peter, All, > > On 2019-01-15 11:15 +0100, Peter Korsgaard spake thusly: > >> tpm2-tools is commonly used with the resource manager, tpm2-abrmd - But it > >> CAN be used without, E.G. by setting the TPM2TOOLS_TCTI_NAME environment > >> variable to communicate directly with the kernel driver: > >> > >> export TPM2TOOLS_TCTI_NAME=device > >> > >> For some use cases (E.G. initramfs) it makes sense to use tpm2-tools > >> without abrmd, so downgrade the dependency from select to imply, so abrmd is > >> enabled by default but can be explicitly disabled. > >> > >> Signed-off-by: Peter Korsgaard <peter@korsgaard.com> > >> --- > >> package/tpm2-tools/Config.in | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/package/tpm2-tools/Config.in b/package/tpm2-tools/Config.in > >> index cc87e2a1bf..f4622b4ec9 100644 > >> --- a/package/tpm2-tools/Config.in > >> +++ b/package/tpm2-tools/Config.in > >> @@ -8,7 +8,7 @@ config BR2_PACKAGE_TPM2_TOOLS > >> select BR2_PACKAGE_LIBCURL > >> select BR2_PACKAGE_LIBGLIB2 > >> select BR2_PACKAGE_OPENSSL > >> - select BR2_PACKAGE_TPM2_ABRMD # run-time > >> + imply BR2_PACKAGE_TPM2_ABRMD # run-time > > > Sorry, but I reiterate my position: I don't like the use of 'imply'. > > > Either the thing is mandatory, in which case we select it or depend on > > it, or the thing is optional, in which case we elt the user enable it. > > I understand you don't like it, but what is the alternative? Just > mention the optional-but-likely-to-be-needed dependency in the help > text? That is IMHO worse than imply. > For this specific case, tpm2-tools fails with a somewhat confusing error > message if tpm2-abrmd isn't available unless a specific command line > option / environment variable is used: > > # tpm2_pcrlist > > ** (process:8628): WARNING **: 11:38:39.606: Failed to create connection with service: GDBus.Error:org.freedesktop.DBus.Error.ServiceUnknown: The name com.intel.tss2.Tabrmd was not provided by any .service files > ERROR: Failed to initialize TABRMD TCTI context: 0xa0008 > > The solution is to set the TCTI name to device, either through the > TPM2TOOLS_TCTI_NAME environment variable or the --tcti option. Well, this is very clearly explained in the man page for tpm2_pcrlist. ;-) And it turns out that the use of tpm2-abrmd is in fact totally unneeded, even to get proper concurrent access, when using the in-kernel manager exposed as /dev/tmprmN (so says the man page). So, in this case, a userland daemon is not even needed to begin with. But since any person not completely insane would still want it, then it should stay a select. The very minor minority who really, like really-really, do not want tpm2-abrmd can still remove it with a post-build script. > This is imho exactly the kind of use cases imply has been made for. I still think its semantics are much more fuzzy than select or depends-on, and it can cause confusion. Also, I'm still on the side that I prefer the user to know what they are doing rather than us baby-sitting them. > > Use of imply does not sound nice to me, because it is not authoritative. > > I'm afraid we get reports of users complaining that "sometimes the stuff > > is enabled when I do X, while sometmes it is not enabled when I do the > > same X.' > > Is that any different than changing toolchain options or toggling > BR2_PACKAGE_BUSYBOX_SHOW_OTHERS? Sorry, I don't follow you. When you toggle those options, they always do the same thing. But with imply, if you do something like: make distclean make menuconfig --> enable tpm2-tools Then tpm2-abrmd is enabled. But if you now go with: make distclean make defconfig make menuconfig --> enable tpm2-tools Then tpm2-abrmd is not enabled, because it was already disabled in the .config. So, this is more confusing than anything else to me, because "sometimes it works, sometimes it does not". Whereas, the toolchain options or the busybox-show-others will always do the same thing. > > The coutner argument has been that we were now trying to make sensible > > choices for the user, so that things "work out of the box". My position > > is that it is an illusion, because making things "just work" is more > > often than not more involving than just enabling a package. > > I agree that we probably cannot do this perfectly, but a solution for > E.G. 80% of the use cases is still an improvement, as long as the > remaining 20% can still change things. > > > For example, when dealing with TPM and such: keys and certs provisionning > > and checking the chain of trust and such is only scratching the surface. > > People that want to deal with this topic better know what they *are* doing, > > as it is a sensible topic. Those people will have to understand what they > > need if they do not already know. > > Sure, but we can atleast ensure that the tpm2-tools utilites do not fail > out of the box because of a missing obscure dependency and that fairly > common use cases are possible. If you go that route, then you must ensure that the kernel has TPM support configured in. That is, for the kernel we build; we're leaving out in the cold those who build their kernel out of Buildroot... Yes, tangential... So, as usual, that's my opinion. Regards, Yann E. MORIN.
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: Hi, >> ** (process:8628): WARNING **: 11:38:39.606: Failed to create >> connection with service: >> GDBus.Error:org.freedesktop.DBus.Error.ServiceUnknown: The name >> com.intel.tss2.Tabrmd was not provided by any .service files >> ERROR: Failed to initialize TABRMD TCTI context: 0xa0008 >> >> The solution is to set the TCTI name to device, either through the >> TPM2TOOLS_TCTI_NAME environment variable or the --tcti option. > Well, this is very clearly explained in the man page for tpm2_pcrlist. ;-) Correct, but it is still not obvious. > And it turns out that the use of tpm2-abrmd is in fact totally unneeded, > even to get proper concurrent access, when using the in-kernel manager > exposed as /dev/tmprmN (so says the man page). So, in this case, a > userland daemon is not even needed to begin with. Correct, but the in-kernel manager is relatively new (4.12). There is various tradeoffs between the in-kernel and user space managers. There was a presentation about it at Plumbers 2017: https://blog.linuxplumbersconf.org/2017/ocw//system/presentations/4818/original/TPM2-kernel-evnet-app_tricca-sakkinen.pdf > But since any person not completely insane would still want it, then it > should stay a select. The very minor minority who really, like > really-really, do not want tpm2-abrmd can still remove it with a > post-build script. Sorry, why do you think I am insane for not wanting tpm2-abrmd? Cleaning up with a post-build script is pretty horrible, especially as tpm2-abrmd pulls in dbus. >> This is imho exactly the kind of use cases imply has been made for. > I still think its semantics are much more fuzzy than select or depends-on, > and it can cause confusion. Yes, that is the entire point, E.G. a "weak" select. It behaves the same as 'default y if foo'. >> Is that any different than changing toolchain options or toggling >> BR2_PACKAGE_BUSYBOX_SHOW_OTHERS? > Sorry, I don't follow you. When you toggle those options, they always do > the same thing. > But with imply, if you do something like: > make distclean > make menuconfig > --> enable tpm2-tools > Then tpm2-abrmd is enabled. > But if you now go with: > make distclean > make defconfig > make menuconfig > --> enable tpm2-tools > Then tpm2-abrmd is not enabled, because it was already disabled in the > .config. > So, this is more confusing than anything else to me, because "sometimes > it works, sometimes it does not". > Whereas, the toolchain options or the busybox-show-others will always do > the same thing. The point is that they influence other "unrelated" options. Another example is the places where we have multiple backends where we do 'select foo if !bar'. Here as well behaviour when you then go an enable bar depends on if you have already exited menuconfig or not (E.G. foo will stay enabled or not). >> Sure, but we can atleast ensure that the tpm2-tools utilites do not fail >> out of the box because of a missing obscure dependency and that fairly >> common use cases are possible. > If you go that route, then you must ensure that the kernel has TPM > support configured in. That is, for the kernel we build; we're leaving > out in the cold those who build their kernel out of Buildroot... And that we indeed do when it is possible / the needed kernel options aren't obvious. In this case the kernel has several different tpm drivers, so we cannot really do it.
On 17/01/2019 16:58, Yann E. MORIN wrote: > But with imply, if you do something like: > > make distclean > make menuconfig > --> enable tpm2-tools > > Then tpm2-abrmd is enabled. > > But if you now go with: > > make distclean > make defconfig > make menuconfig > --> enable tpm2-tools > > Then tpm2-abrmd is not enabled, because it was already disabled in the > .config. This, for me, is the crux of the matter. I agree with Yann that this is confusing. Especially because 'make some-defconfig; make menuconfig' is the usual workflow. So the value of this imply is almost nothing in practice. So let me take this occasion to review the cases of imply that we already have (obviously they're not yet written with the imply keyword). BR2_ARC_ATOMIC_EXT BR2_TARGET_ROOTFS_JFFS2_NOCLEANMARKER BR2_PACKAGE_LUA_32BITS BR2_PACKAGE_OPUS_FIXED_POINT BR2_TOOLCHAIN_EXTERNAL_HAS_SSP BR2_TOOLCHAIN_EXTERNAL_INET_RPC These are not confusing IMO because they only become visible after selecting some other option that is not enabled by default. BR2_PACKAGE_IFUPDOWN_SCRIPTS This one is somewhat less confusing because in the 'make defconfig; make menuconfig' scenario, the option will already be enabled. However, if you later on switch to a custom skeleton, the ifupdown-scripts will stay enabled. Still, that is very similar to the situation for packages that got select'ed: once you remove the option that caused that package to be enabled, it will stay enabled even after you disable the option that triggered it. In conclusion, we currently already have some confusion caused by 'make defconfig; make menuconfig' situations, but currently they only go in one direction: something that was enabled will stay enabled even if you don't need it any more. Using 'imply' in the way proposed by Peter would introduce a different kind of confusion: options that don't get enabled though they should be. I would say, the design of defaults in Kconfig is simply wrong. To make it work well, there should be tracking of whether a value was set automatically or by the user. But let's not go there :-) So, that doesn't mean that imply should be banned entirely. It could still be useful in some cases, like the ifupdown scripts. For the situations like the one in this patch, I would say that we could relax a little the 'avoid extra per-package configuration options'. In fact, extra configuration options in Config.in don't cost that much. They don't really make the menus larger because they're only visible when the package is selected. And the .mk handling is the same as for an automatic optional dependency. That said, in this specific case of tpm2-tools, I have the feeling that an additional option is not appropriate. Since the two packages are right next to each other, that is almost the same as having the suboption. So I would go for the help text instead. Regards, Arnout
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes: Hi, > For the situations like the one in this patch, I would say that we could relax > a little the 'avoid extra per-package configuration options'. In fact, extra > configuration options in Config.in don't cost that much. They don't really make > the menus larger because they're only visible when the package is selected. And > the .mk handling is the same as for an automatic optional dependency. That said, > in this specific case of tpm2-tools, I have the feeling that an additional > option is not appropriate. Since the two packages are right next to each other, > that is almost the same as having the suboption. So I would go for the help text > instead. Ok, I'll send an updated patch that drops the select and adds a note in the help text instead.
Peter, All, On 2019-01-17 20:01 +0100, Peter Korsgaard spake thusly: > >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: [--SNIP--] > > But since any person not completely insane would still want it, then it > Sorry, why do you think I am insane for not wanting tpm2-abrmd? I do apologise if you took offense, my words were careless, and I should have known better to not write that. Regards, Yann E. MORIN.
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: > Peter, All, > On 2019-01-17 20:01 +0100, Peter Korsgaard spake thusly: >> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: > [--SNIP--] >> > But since any person not completely insane would still want it, then it >> Sorry, why do you think I am insane for not wanting tpm2-abrmd? > I do apologise if you took offense, my words were careless, and I should > have known better to not write that. No problem!
diff --git a/package/tpm2-tools/Config.in b/package/tpm2-tools/Config.in index cc87e2a1bf..f4622b4ec9 100644 --- a/package/tpm2-tools/Config.in +++ b/package/tpm2-tools/Config.in @@ -8,7 +8,7 @@ config BR2_PACKAGE_TPM2_TOOLS select BR2_PACKAGE_LIBCURL select BR2_PACKAGE_LIBGLIB2 select BR2_PACKAGE_OPENSSL - select BR2_PACKAGE_TPM2_ABRMD # run-time + imply BR2_PACKAGE_TPM2_ABRMD # run-time select BR2_PACKAGE_TPM2_TSS help TPM (Trusted Platform Module) 2.0 CLI tools based on system
tpm2-tools is commonly used with the resource manager, tpm2-abrmd - But it CAN be used without, E.G. by setting the TPM2TOOLS_TCTI_NAME environment variable to communicate directly with the kernel driver: export TPM2TOOLS_TCTI_NAME=device For some use cases (E.G. initramfs) it makes sense to use tpm2-tools without abrmd, so downgrade the dependency from select to imply, so abrmd is enabled by default but can be explicitly disabled. Signed-off-by: Peter Korsgaard <peter@korsgaard.com> --- package/tpm2-tools/Config.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)