Clarify usage of ID field in CRL

[RonanMacF]

Certificate Revocation Lists has the following definition.

// A certificate revocation list (CRL)
message CertificateRevocationList {
  // Type of the CRL.
  CertificateType type = 1;
  // CRL encoding type.
  CertificateEncoding encoding = 2;

  // Actual CRL.
  // The exact encoding depends upon the type of CRL.
  // for X509, this should be a PEM encoded CRL.
  bytes certificate_revocation_list = 3;

  // ID of this CRL, which is the CRL file hash.
  string id = 4;
}

It is not clear what the purpose of the ID is here. The comment indicates it is a file hash, is it referring to sha256sum, x509 hash etc.

What is the expected usage of this ID. Is this supposed to be the name of the CRL in the filesystem? If so, I feel like it might be better to leave this as an implementation detail up to the server. Or if this really is wanted, then should we have a similar field in Certificate, which we currently don't have?

Or perhaps the hash is used for some form of validation where we, the server, ensure the OpenSSL hash is the same as the hash provided in the spec, this also seems suboptimal as hashing algorithms could be subject to change.

It might be reasonable to remove this field altogether and let the server decide how it wants to name the CRL on the box.

[marcushines]

It is just a consistent hash of the file - it doesn't imply anything about how the vendor stores the bytes

[brianneville]

Is it important then for the server to know what hashing algorithm was used here?

Without knowing what algorithm was used, the only thing that the server could do with this id value would be to log it.
Knowing what algorithm was used would be required for the server to perform validation (if that is the envisioned use for this field)

[RonanMacF]

What value does this have for the client? Why would a client provide this hash if the server does not need to care about it. It seems redundant to have it and we can just remove it to simplify the message.

[tomek-US]

Yes, this field is definitely controversial ;-)

The idea behind adding it was to avoid calling the openSSH's re-hashing on the switch every time the CRL is rotated and this is the only reason for it to be present.

I am open to removing it and letting the implementation 'do its thing'.

[RonanMacF]

Either way the server will have to, or should, perform it's own hasing on it to ensure it's valid. gRPC-go implementation of CRL used the x509 hash so irrespective of what the client gives we will have to generate that ourselves to ensure gRPC core can pick it up and if the hash provided is not what gRPC core is expecting we will ignore it. Removing this field would get my vote