Re: [PATCH] Revert "firmware: dmi_scan: Use lowercase letters for UUID"

[Peter Korsgaard <peter@korsgaard.com>]

>>>>> "Jean" == Jean Delvare <jdelvare@suse.com> writes:

Hi,

 >> I get that - but it changes the content of sysfs entries, breaking real
 >> systems - E.G. a user space ABI regression.

 > I know it's very convenient to play the "user-space ABI regression"
 > card whenever you want a change reverted.

Sorry, I am really not trying to be difficult here, but that is exactly
what it is. A kernel upgrade broke working systems :/


 > However the interface itself did not change. The sysfs file name did
 > not change, the nature of its contents did not change. The only thing
 > which changed is the exact contents details of the file. In my
 > opinion that does not qualify as an "ABI regression".

The binary content of the sysfs file changed, and the change caused
working software to no longer work.

I get that my software is is not very commonly used. If this change
would have broken ps or top or any other well known utility looking in
/proc or /sys I guess we wouldn't have this discussion at all, but it is
still a regression.


 >> It is a cosmetic code change in the sense that no known software was
 >> broken with the upper case characters.

 > It bothered someone enough to create a ticket asking me to fix it in
 > dmidecode:
 > https://savannah.nongnu.org/bugs/index.php?53569

Yes, I am aware of that.


 > And it wouldn't make sense that the kernel uses a different format from
 > dmidecode.

I would personally be quite wary of such change for both dmidecode and
the kernel because of the risk of breaking existing utilities. I see
that Petter Reinholdtsen has the same concerns:

https://lists.nongnu.org/archive/html/dmidecode-devel/2018-04/msg00001.html

But ok, not my choice to make. I also get that dmidecode doesn't have
the same no-regression policy for its output as the kernel has.



 >> > If "cryptsetup luksOpen" does not lowercase digits before computing its
 >> > key passphrase, then it's not RFC 4122-compliant and should be fixed.
 >> 
 >> cryptsetup naturally doesn't know anything about RFC 4122. It just reads
 >> a disk encryption keyphrase which happen to include the content of
 >> id/product_uuid because of my scripts.

 > OK, so basically your script infringes RFC 4122, and instead of fixing
 > that, you ask me to stop respecting that RFC on my side.

To be clear, the RFC states:

The formal definition of the UUID string representation is
provided by the following ABNF:

hexDigit =
            "0" / "1" / "2" / "3" / "4" / "5" / "6" / "7" / "8" / "9" /
            "a" / "b" / "c" / "d" / "e" / "f" /
            "A" / "B" / "C" / "D" / "E" / "F"

(E.G. lower case AND upper case)

And:

The hexadecimal values "a" through "f" are output as lower case
characters and are case insensitive on input.

So in other words, no conforming implementation should have any issues
handling an upper case UUID string, and the change to lower case was for
cosmetic reasons rather than to fix a parsing issue with conforming
software.


 > Out of curiosity, what's the purpose of your encryption strategy? That
 > someone who would open your computer and steal only the disk (as
 > opposed to stealing the whole computer) would be unable to read the
 > disk's contents? Do thieves really do that?


The systems are locked down, so they cannot (easily) be tricked into
revealing their secrets at runtime or boot non-trusted software. The
disk encryption protects against somebody moving the disk to another
machine and reading the secrets.

I agree that a better solution may be to store the per-device key
in E.G. a TPM instead, but that was not available for all use cases.



 >> > Nak. This is too late. Changing it again would just add confusion.
 >> 
 >> Please reconsider. 4.17 is from June, and 4.19 has only recently become
 >> LTS.

 > I can't see how this is related with kernel 4.19 becoming LTS.

Only in the sense that that LTS kernels see wider use than non-LTS
kernels (E.G. I discovered this when moving from 4.14.x to 4.19.x).


 > Look, you can imagine that I was perfectly aware of what I was doing
 > when I made that change, and that I pondered the decision carefully at
 > that time. And my decision was that the change should be made. As far
 > as I'm concerned, this ship has sailed already, sorry.

Sorry, what is the perceived risk of reverting this change? Just the
minor inconsistency between the dmidecode and sysfs output? As stated
above, the RFC requires conforming parsers to handle upper case as well.

-- 
Bye, Peter Korsgaard