Recent change to VPD -s command makes it impossible to edit Product_S/N

[Grant Coles]

A change to VPD -s that went live on build 59 seems to have made it impossible to change Product_S/N. This is an issue, as when we do a motherboard replacement, we can no longer change Product_S/N to be what it was on the original device, which ruins our Auto-Enrollment. I know why this is. Commit aee401570c2584749f3003498810e7542aec8a3c added a char check system to make sure vpd keys were made up of only alphanumerics, underscores, or periods. Relevant code below:

/*

* Check if given key name is compliant to recommended format.

*/

static vpd_err_t checkKeyName(const uint8_t *name) {

unsigned char c;

while ((c = *name++)) {

if (!(isalnum(c) || c == '_' || c == '.')) {

fprintf(stderr, "[ERROR] VPD key name does not allow char [%c].\n", c);

return VPD_ERR_PARAM;

}

}

return VPD_OK;

 }

I am wondering if removed the ability to edit Product_S/N is intentional, or if this is something that was overlooked and may be address later. Also, if would be appreciated if someone had a work around, for now.

Thanks!

[Grant Coles]

Here is a link to the commit

[Mike Frysinger]

[ +hungte ]

https://chromium-review.googlesource.com/454577

workaround is trivial -- just grab an old vpd binary

-mike

--
--
Chromium OS discuss mailing list: chromium-os-discuss@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-os-discuss?hl=en

[Grant Coles]

That would work, but we have approx. 30-40 techs all working on doing these motherboard replacements, and having them all use an old binary would be a tremendous feat.

On Monday, August 21, 2017 at 11:05:22 AM UTC-5, Mike Frysinger wrote:

[ +hungte ]

https://chromium-review.googlesource.com/454577

workaround is trivial -- just grab an old vpd binary

-mike

On Mon, Aug 21, 2017 at 11:59 AM, Grant Coles <gran...@gmail.com> wrote:

A change to VPD -s that went live on build 59 seems to have made it impossible to change Product_S/N. This is an issue, as when we do a motherboard replacement, we can no longer change Product_S/N to be what it was on the original device, which ruins our Auto-Enrollment. I know why this is. Commit aee401570c2584749f3003498810e7542aec8a3c added a char check system to make sure vpd keys were made up of only alphanumerics, underscores, or periods. Relevant code below:

/*

* Check if given key name is compliant to recommended format.

*/

static vpd_err_t checkKeyName(const uint8_t *name) {

unsigned char c;

while ((c = *name++)) {

if (!(isalnum(c) || c == '_' || c == '.')) {

fprintf(stderr, "[ERROR] VPD key name does not allow char [%c].\n", c);

return VPD_ERR_PARAM;

}

}

return VPD_OK;

 }

I am wondering if removed the ability to edit Product_S/N is intentional, or if this is something that was overlooked and may be address later. Also, if would be appreciated if someone had a work around, for now.

Thanks!

--
--
Chromium OS discuss mailing list: chromium-...@chromium.org

[Hung-Te Lin]

All newer devices should not be using "Product_S/N", instead the right choice is "serial_number".

For old devices, since Product_S/N should live in RO, that should be changed only by partner's RMA center, which uses locked factory branches (with old version of VPD) that won't have this restriction.

So I'm not see any reason why ToT 'vpd' should support writing values with 'Proudct S/N'.

Thiemo/mnissler, please correct me if I got something wrong about why Auto Enrollment would need to change Product_S/N here.

Regards,

2017-08-22 0:38 GMT+08:00 Grant Coles <gran...@gmail.com>:

Oh, I see you've added the dev who pushed the commit. Thanks! I'm pretty knew to this system.

On Monday, August 21, 2017 at 11:35:50 AM UTC-5, Grant Coles wrote:

Using an old binary will more than likely be what we end up doing, however. I am just holding out hope that this will be address in the near future. Would you happen to know where I could submit this as a bug, so that my questions about why this was done could possibly be answered by the devs?

Thanks for the help!

[Thiemo Nagel]

+igorcov who's handling enrollment issues these days

 

Thiemo/mnissler, please correct me if I got something wrong about why Auto Enrollment would need to change Product_S/N here.

Setting Product_S/N to the original serial number after the motherboard has been exchanged has the effect to keep the device in Forced Re-Enrollment (formerly called "auto-enrollment"). However the preferred approach to achieve that goal is to boot the repaired device and use the enrollment flow to add it to the management domain. Grant, would it be an option for you to take that route?

[Grant Coles]

While I know I would be more than able to do this, we fix devices for a lot of schools and business, and I have no control over how they Enroll their systems. And as frustrating as it is, they have often sent devices back for re-repair because their devices has the serial_number in the VPD table as the Product_S/N. Product_S/N often drops the last letter of the serial number for some devices we repair, such as the Samsung XE303C12. When this happens, their device will no longer be enrolled, and instead of just Re-Enrolling the device, they will send it back for re-repair.

What does the Forced Re-Enrollment system use to verify enrollment or to re-enroll? Product_S/N or serial_number. Searching through old bug reports, I found where a change was made so that enrollment would check the VPD table for "sn", "Product_S/N", and then "serial_number" in that order (since serial_number would sometimes have a name conflict). Is that still the case? And if so, would placing the "Product_S/N" value into the table under the "sn" key solve this issue of the device losing enrollment?

Thanks so much, you've all been a tremendous help!

[Grant Coles]

Here is where I am seeing how the devices decides on which serial number value in the vpd table to use. It is likely that this is no longer in use, but if it is not, then I have no idea how the serial number is selected. https://codereview.chromium.org/7493063/diff/1/chrome/browser/policy/device_policy_identity_strategy.cc

[Igor Covganet]

This is the order of the keys taken as candidates for serial number:

https://cs.corp.google.com/chromeos_internal/src/platform2/login_manager/server_backed_state_key_generator.cc?q=ComputeK+file:%5Esrc/platform2/login_manager/+package:%5Echromeos_internal$&l=46

If you put in VPD serial_number to have the same value as Product_S/N while the first is missing, the FRE should trigger. Please let us know if that's not the case.

[Mike Frysinger]

that's an internal link that doesn't work for the public.  here's the public link:

https://chromium.googlesource.com/chromiumos/platform2/+/master/login_manager/server_backed_state_key_generator.cc#46

-mike

--
--
Chromium OS discuss mailing list: chromium-os-discuss@chromium.org

[Grant Coles]

I don't actually have a way to access that page, sorry! But, if I understand correctly, if I put the Product_S/N into "serial_number", when Product_S/N is not in the hash table, it should trigger FRE?

[Igor Covganet]

On Tue, Aug 22, 2017 at 5:04 PM, Grant Coles <gran...@gmail.com> wrote:

I don't actually have a way to access that page, sorry! But, if I understand correctly, if I put the Product_S/N into "serial_number", when Product_S/N is not in the hash table, it should trigger FRE?

Right. It's important for the value to be present in the cache too, so you will need to run two commands actually:

vpd -i RW_VPD -s serial_number=..

dump_vpd_log --force

On Tuesday, August 22, 2017 at 10:00:56 AM UTC-5, Igor Covganet wrote:

This is the order of the keys taken as candidates for serial number:

https://cs.corp.google.com/chromeos_internal/src/platform2/login_manager/server_backed_state_key_generator.cc?q=ComputeK+file:%5Esrc/platform2/login_manager/+package:%5Echromeos_internal$&l=46

If you put in VPD serial_number to have the same value as Product_S/N while the first is missing, the FRE should trigger. Please let us know if that's not the case.

Google Germany GmbH

Erika-Mann-Str. 33

80636 München

Registergericht und -nummer: Hamburg, HRB 86891

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle

Diese E-Mail ist vertraulich. Falls sie diese fälschlicherweise erhalten haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter, löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen, dass die E-Mail an die falsche Person gesendet wurde.

    

This e-mail is confidential. If you received this communication by mistake, please don't forward it to anyone else, please erase all copies and attachments, and please let me know that it has gone to the wrong person.

Der Inhalt dieser E-Mail spiegelt den derzeitigen Stand der Verhandlungen wider und dient als Basis für weitere Gespräche. Er soll keine rechtlich verbindliche Verpflichtung begründen. Eine solche Verpflichtung wird allein durch einen zwischen allen beteiligten Parteien abgeschlossenen, schriftlichen Vertrag geschaffen.

The above terms reflect a potential business arrangement, are provided solely as a basis for further discussion, and are not intended to be and do not constitute a legally binding obligation. No legally binding obligations will be created, implied, or inferred until an agreement in final form is executed in writing by all parties involved.

[Grant Coles]

I did manage to find that file in the repository, though! Seems as though the order is "Product_S/N", "serial_number", "Product_SN", then finally "sn".

[Thiemo Nagel]

we fix devices for a lot of schools and business, and I have no control over how they Enroll their systems.

I see. That makes a lot of sense.

What does the Forced Re-Enrollment system use to verify enrollment or to re-enroll?

This is the code which generates the keys: https://chromium.googlesource.com/chromiumos/platform2/+/master/login_manager/server_backed_state_key_generator.cc#162

For practical purposes you should be safe if you copy over all of the following (if existing):

* stable_device_secret_DO_NOT_SHARE

* Product_S/N

* serial_number

To address the problem of vpd syntax: In an offline conversation, Mattias has suggested to make an exception for "Product_S/N" which seems like a fair approach to me. Hung-Te, what do you think?

On Tue, Aug 22, 2017 at 5:03 PM, Mike Frysinger <vap...@chromium.org> wrote:

that's an internal link that doesn't work for the public.  here's the public link:

https://chromium.googlesource.com/chromiumos/platform2/+/master/login_manager/server_backed_state_key_generator.cc#46

-mike

[Grant Coles]

Adding "Product_S/N" as an except would be a tremendous help for us!

Mr. Nagel, the only issue with copying over all of that information is where could we store "serial_number"? If we were to replace "serial_number" with "Product_S/N" as suggested above, "serial_number" would lose the checksum at the end. Is this checksum required?

On Tuesday, August 22, 2017 at 10:15:14 AM UTC-5, Thiemo Nagel wrote:

we fix devices for a lot of schools and business, and I have no control over how they Enroll their systems.

I see. That makes a lot of sense.

What does the Forced Re-Enrollment system use to verify enrollment or to re-enroll?

This is the code which generates the keys: https://chromium.googlesource.com/chromiumos/platform2/+/master/login_manager/server_backed_state_key_generator.cc#162

For practical purposes you should be safe if you copy over all of the following (if existing):

* stable_device_secret_DO_NOT_SHARE

* Product_S/N

* serial_number

To address the problem of vpd syntax: In an offline conversation, Mattias has suggested to make an exception for "Product_S/N" which seems like a fair approach to me. Hung-Te, what do you think?

On Tue, Aug 22, 2017 at 5:03 PM, Mike Frysinger <vap...@chromium.org> wrote:

that's an internal link that doesn't work for the public.  here's the public link:

https://chromium.googlesource.com/chromiumos/platform2/+/master/login_manager/server_backed_state_key_generator.cc#46

-mike

On Tue, Aug 22, 2017 at 11:00 AM, 'Igor Covganet' via Chromium OS discuss <chromium-...@chromium.org> wrote:

This is the order of the keys taken as candidates for serial number:

https://cs.corp.google.com/chromeos_internal/src/platform2/login_manager/server_backed_state_key_generator.cc?q=ComputeK+file:%5Esrc/platform2/login_manager/+package:%5Echromeos_internal$&l=46

If you put in VPD serial_number to have the same value as Product_S/N while the first is missing, the FRE should trigger. Please let us know if that's not the case.

--
--
Chromium OS discuss mailing list: chromium-...@chromium.org

[Thiemo Nagel]

where could we store "serial_number"? If we were to replace "serial_number" with "Product_S/N" as suggested above, "serial_number" would lose the checksum at the end. Is this checksum required?

I'm not sure I understand what you mean, but you gave me another idea: Since it's possible to delete arbitrary VPD keys (no syntax check), "Product_S/N" may be replaced by "serial_number" without any change in functionality. The following should work when executed on the new board:

vpd -i RO_VPD -d Product_S/N

vpd -i RO_VPD -s serial_number=[value of *Product_S/N* of the old board]

# In case the old board had a stable_device_secret_DO_NOT_SHARE key:

vpd -i RO_VPD -s stable_device_secret_DO_NOT_SHARE=[device secret of old board]

# In case the old board didn't have a stable_device_secret_DO_NOT_SHARE key:

vpd -i RO_VPD -d stable_device_secret_DO_NOT_SHARE

This avoids having to hard-code an exception for "Product_S/N" into the vpd binary.

[Grant Coles]

Right, this is what was suggested above, more or less. The only problem is that "serial_number" contains a checksum character at the end, whereas "Product_S/N" does not. So will not having that checksum cause issues anywhere else? Here is the relevant code, including comments, for what order FRE selects the serial number. It mentions this checksum:
 

// These are the machine serial number keys that we check in order until we

// find a non-empty serial number. The VPD spec says the serial number should be

// in the "serial_number" key for v2+ VPDs. However, legacy devices used a

// different key to report their serial number, which we fall back to if

// "serial_number" is not present.

//

// Product_S/N is still special-cased due to inconsistencies with serial

// numbers on Lumpy devices: On these devices, serial_number is identical to

// Product_S/N with an appended checksum. Unfortunately, the sticker on the

// packaging doesn't include that checksum either (the sticker on the device

// does though!). The former sticker is the source of the serial number used by

// device management service, so we prefer Product_S/N over serial number to

// match the server.

//

// TODO(mnissler): Move serial_number back to the top once the server side uses

// the correct serial number.

const char* kMachineInfoSerialNumberKeys[] = {

"Product_S/N", // Lumpy/Alex devices

"serial_number", // VPD v2+ devices

"Product_SN", // Mario

"sn", // old ZGB devices (more recent ones use serial_number)

 };

[Thiemo Nagel]

Right, this is what was suggested above, more or less.

Great, it seems I've understood you correctly after all. :)

The only problem is that "serial_number" contains a checksum character at the end, whereas "Product_S/N" does not. So will not having that checksum cause issues anywhere else?

Just to be clear: I'm suggesting to transplant the value of "Product_S/N" into "serial_number". After that, "serial_number" does not contain a checksum. This is not a problem because Chrome OS never had been aware of the checksum in the first place: Whenever a "Product_S/N" exists, it is preferred over "serial_number". Thus the checksum is never read and never used.

Does that make sense?

[Grant Coles]

Absolutely! I will make sure word gets out that this will be the solution we will use. You have been amazing!

[Thiemo Nagel]

Glad I was able to help! Please update this thread in case you run into problems.