rust-keylime icon indicating copy to clipboard operation
rust-keylime copied to clipboard
verified publisher icon keylime

Rust agent does not support `dmidecode` or `hostname` for `uuid` option.

Open ansasaki opened this issue 3 years ago • 14 comments

The rust agent does not support the dmidecode nor the hostname keywords for the uuid option.

ansasaki avatar Sep 21 '22 12:09 ansasaki

Related: #280

ansasaki avatar Feb 08 '23 13:02 ansasaki

For uuid, the dmidecode -s system-uuid is available only for privileged users at /sys/devices/virtual/dmi/id/product_uuid. There is a system-generated machine-id which can be read as a normal user /etc/machine-id. Any thoughts on which one we want to use?

arkivm avatar Sep 25 '23 19:09 arkivm

I don't even think that we need that option anymore, because you can override the uuid via an env variable, if you need special generation of uuids.

@ansasaki what do you think?

THS-on avatar Sep 26 '23 05:09 THS-on

I don't even think that we need that option anymore, because you can override the uuid via an env variable, if you need special generation of uuids.

As a workaround this is true. For deployments you would need to add a systemd override to the unit file to export a new env var to do that automatically.

If we decide to go to this path, at least we should document how to do that.

aplanas avatar Sep 26 '23 08:09 aplanas

Does this hold for hostname option as well, as hostname as UUID can be overridden via an env var

arkivm avatar Sep 27 '23 22:09 arkivm

If we decide to go to this path, at least we should document how to do that.

@aplanas yes we should add that at least to the config file.

Does this hold for hostname option as well, as hostname as UUID can be overridden via an env var

@arkivm if it is easy to implement, we can implement the hostname option.

THS-on avatar Sep 28 '23 07:09 THS-on

Does this hold for hostname option as well, as hostname as UUID can be overridden via an env var

There is this inconsistency that maybe we can try to resolve. Reading this makes me feel bad:

uuid = "cloud1.company.com"

Hostnames are useful: are unique and easy to remember when writing the tenant command to enrolling it, but they are clearly no UUIDs.

I am not sure how this should be resolved, but maybe we can introduce a new field named "ID", that can contain other strings that represent the agent, that can be used if the UUID field is not present, and force that the UUID is a valid UUID and not some random string.

aplanas avatar Sep 28 '23 07:09 aplanas

@aplanas sounds like plan to introduce an id option :+1:

THS-on avatar Sep 29 '23 15:09 THS-on

Any news in this matter?

derpedda avatar Jul 04 '24 15:07 derpedda

Any news in this matter?

Not really, the priority on this is very low as nobody provided an use case where overriding the UUID using environment variables is not sufficient.

@derpedda Could you please describe your use case for this missing feature?

ansasaki avatar Jul 04 '24 16:07 ansasaki

Were going to use WereWulf to deploy keylime on an HPCcluster, overriding using environment variables would be possible of course) I tried overriding with the FQDN hostname, but the agent complains (in a very understandable way) that it is not a UUID and still decides to go for a generated one. As far as I understand, the correct behaviour as of keylime-agent/src/config.rs Did I miss something or should I file another issue?

derpedda avatar Jul 04 '24 16:07 derpedda

I think there is no need for another issue as this problem was reported via #280

I believe the best way to fix this is to implement what @aplanas suggested:

  • Add a new configuration option ID separate from UUID which supports hostnames (and maybe dmidecode in future)
  • In case UUID is set as hostname, set ID as the hostname and generate an UUID
  • Add the support to use ID instead of UUID in the tenant. This way, hostnames could be used in the tenant command.

There is an extra complication of managing ID duplicates. And even bigger complication if it is expected that the ID are supported on the API in place of the UUID. I still need to figure out how the ID resolution would be implemented in the tenant, meaning how to obtain the correct UUID given the ID.

What do you think? Would this be acceptable?

I do not promise this will be implemented soon as the solution is complicated and affects many parts of the code.

ansasaki avatar Jul 05 '24 08:07 ansasaki

Adding the extra field ID following @aplanas solution would be the most clean way to implement it indeed. I also highly support the position that the UUID field should in any case stay a UUID field. Of course this is not trivial as it impacts many places, as you said.

What comes to my mind with generated UUIDs and another identifying ID - how would duplicates with same ID but different generated UUID (same client or not) be handled? This can cause plenty of issues as well.

derpedda avatar Jul 05 '24 08:07 derpedda

What comes to my mind with generated UUIDs and another identifying ID - how would duplicates with same ID but different generated UUID (same client or not) be handled? This can cause plenty of issues as well.

I see some possible solutions with various levels of complexity:

  • Do not allow duplicates (effectively making ID unique as well)
  • Allow duplicates
    • Show matches to the tenant when the ID is not unique (effectively making it necessary to use UUID when there are duplicates)
    • Same as above, but using some heuristic to help the user (e.g. remember the tenant who added the UUID and showing this visually to the user)
  • Create namespaces
    • Allow ID duplicates globally, but not inside the namespace
      • Do not allow duplicate namespaces

ansasaki avatar Jul 05 '24 09:07 ansasaki