asset API: loosen restrictions on qualifiers

[mostynb]

The spec previously stated that servers must reject Fetch requests if there were unsupported qualifiers specified. This was too strict- consider for example a scenario where the client specifies both SHA256 and SHA512 qualifiers, and the server only supports one of these. Supporting only one of these should be sufficient, and the server should be free to accept such a request.

[malt3]

I agree with a part of your request: the client should be allowed to specify multiple equivalent hashes (using different algorithms), and the server should be allowed to pick any one of those.
Right now however, the client is not even allowed to send more than one hash (at least there is no way to encode this correctly):

• Every qualifier name can only exist once per request
• The only known qualifier used for expected hashes of content is checksum.sri

I think there are three less invasive changes that would all make your use-case possible:

• In the Qualifier Lexicon, specify new qualifier names for every digest function (like checksum.sri.sha256) and specifically mention that everything under checksum.sri is interchangeable (the server can choose which checksum to use).
• Allow checksum.sri to be a list of comma-separated qualifiers with the same meaning (interchangeable hashes, you are allowed to pick one)
• Allow qualifier names to be used more than once (making them interchangeable)

In addition to any of these, it would be nice if the server could signal which qualifiers have been used (so the client can update its internal knowledge accordingly - only for checksums that were validated).
The server should also avoid to cache fetched content for checksums it hasn't actually validated. Otherwise the server could accidentally lie about what it validated to the client.

[mostynb]

I agree with a part of your request: the client should be allowed to specify multiple equivalent hashes (using different algorithms), and the server should be allowed to pick any one of those. Right now however, the client is not even allowed to send more than one hash (at least there is no way to encode this correctly):

There is no central list of valid qualifiers, only some suggested ones. Clients could specify hashes with different qualifiers instead of multiple checksum.sri qualifiers.

To generalise my example, if one qualifier is "strong" (like a cryptographic hash) then all other qualifiers may be redundant and safely ignored (assuming the set of qualifiers is not inconsistent).

[malt3]

There is no central list of valid qualifiers, only some suggested ones. Clients could specify hashes with different qualifiers instead of multiple checksum.sri qualifiers.

I agree. That is my first suggestion (documenting the additional qualifiers in the lexicon is an optional step - you could start using them right away).

To generalise my example, if one qualifier is "strong" (like a cryptographic hash) then all other qualifiers may be redundant and safely ignored (assuming the set of qualifiers is not inconsistent).

That's not what you suggest in the change of the spec. Your change allows the server to ignore any strong qualifiers as it chooses, which leaves clients to guess what validation (if any) has been performed.
It's also not trivial to define what a strong qualifier is. The client may say that md5 is a weak qualifier (for obvious reasons), but the server could classify it as "strong". See also this comment.

I want to avoid ambiguity between client and server as much as possible. Another way to achieve this is to add a new field in the request where the client indicates which qualifiers are equivalent to the client (so the server can choose one of them).

[peterebden]

Allow checksum.sri to be a list of comma-separated qualifiers with the same meaning (interchangeable hashes, you are allowed to pick one)

Is this not already the case, but with whitespace separation instead of commas? The SRI spec linked from the qualifier lexicon allows for multiple qualifiers in one entry.
Given that, I like this much better than the alternatives; I don't think the documents here should describe a set of accepted hash functions if it's at all possible for implementations to define those. We could specify that anything prefixed in checksum.sri. is also an SRI hash, but that feels clunky (and unnecessary).

If we're going to use SRI, I prefer the idea of sticking with the existing spec as far as possible. However, I worry that it's originally designed as a client-side validation and we're using it here server-side. I think that works sufficiently with the original MUST wording, but if it's a MAY the client doesn't actually know the validation has been performed.

To me, the original issue is sufficiently well solved by the client sending multiple specifiers within the one checksum.sri qualifier, and the server either validating one of those that it understands or failing if it can't. We've used it this way, that's how I assumed it was supposed to work from the choice of SRI in the first place.

[malt3]

Is this not already the case, but with whitespace separation instead of commas? The SRI spec linked from the qualifier lexicon allows for multiple qualifiers in one entry.

Today I learned. In that case, it's probably a good idea to add a short note in the lexicon hinting at the fact that more than one checksum may be sent (separated by whitespace).

EDIT: Done in #330

[sluongng]

See also #301 (comment). Edit: Ah I think @malt3 linked this above.

[werkt]

Looking at the protocol, we also have the digest_function field in requests, with the following note:

  // The digest function the server must use to compute the digest.                                                                                                                                                           
  //                                                                                                                                                                                                                          
  // If unset, the server SHOULD default to SHA256.                                                                                                                                                                           
  build.bazel.remote.execution.v2.DigestFunction.Value digest_function = 6;

Since the sri only refers to values specified by the client, it sounds like this means that the server must present/compute this digest separately for the purpose of the Response population. Can we specifically associate this language for this field with the response digest field?

[werkt]

* Allow `checksum.sri` to be a list of comma-separated qualifiers with the same meaning (interchangeable hashes, you are allowed to pick one)

https://www.w3.org/TR/SRI/#agility refers to parsing metadata handling a list of multiple sris via split a string on spaces - why would we go with commas here in disagreement with the standard?
It also specifies that the user-agent in the case of multiple integrities provided, is to pick the one with the strongest hash, not just any. The enumeration of 'strongest' is left to the user-agent, but I bet it wouldn't want you to pick the sha256 over the sha384, and it says that known-weak ones would be discarded, and not contribute to any integrity checks.

* Allow qualifier names to be used more than once (making them interchangeable)

Why would this make them interchangeable? Now you have an ordering/list problem...

[sluongng]

Looking at the protocol, we also have the digest_function field in requests, with the following note:

  // The digest function the server must use to compute the digest.                                                                                                                                                           
  //                                                                                                                                                                                                                          
  // If unset, the server SHOULD default to SHA256.                                                                                                                                                                           
  build.bazel.remote.execution.v2.DigestFunction.Value digest_function = 6;

Since the sri only refers to values specified by the client, it sounds like this means that the server must present/compute this digest separately for the purpose of the Response population. Can we specifically associate this language for this field with the response digest field?

It would be extremely painful if we do that.

In Bazel ecosystem, there are cases such as rules_js where the sri integrity might come from an upstream package manager ecosystem which uses SHA284 or SHA512, while the build tool (Bazel) was using SHA256/BLAKE3.

Forcing the sri to be the same as the response digest function would be disruptive for this ecosystem and requires build rules authors and/or users to maintain a manual mapping of integrity themselves.

It also specifies that the user-agent in the case of multiple integrities provided, is to pick the one with the strongest hash, not just any.

Reading 3.3.3 -> 3.3.5 carefully, I think you are right.

1. Multiple sri needs to be space separated.
2. Among multiple sri, the server picks the strongest algo to verify
3. Each algo can come with multiple expected values (!!!)
4. As long as one of the expected values matches the download result, the server should accept the result.

So it sounds like you can have a payload like this:

  "qualifiers": [
    {
      "name": "checksum.sri",
      "value": "sha1-abcadf sha256-123dafa sha256-sadfasd sha512-mmdmfdmf sha512-mmmrrrr"
    }
  ]

and the server should do the following:

1. Ignore sha1 due to weak hashing function (see 3.2)
2. Implement a strongest algo picker to pick between sha256 and sha512. (it's possible that the server does not support sha512) (see 3.3.4)
3. Iterate through multiple values of the picked algo and verify download result. (3.3.5)
   • If it matches one of the values, checksum.sri qualifier is successful.
   • If none match, checksum.sri qualifier failed.

[malt3]

It also specifies that the user-agent in the case of multiple integrities provided, is to pick the one with the strongest hash, not just any.

This is what the specification says. However, I still think that for a client using the Fetch service, this is practically not any different from allowing the server to just check any of the provided checksums.

1. Which algorithms are considered strong (or strong enough) changes over time
2. Server and client can disagree on the order of prioritization
3. A server can choose which subset of all hash functions it is willing to work with

So if I am building a client that is agnostic of the server (doesn't know about implementation details or configuration), the only guarantee I have is that the server verifies at least one of the provided checksums in checksum.sri (or it rejects my request if none of the provided checksums are supported).

[sluongng]

Yeah, from the Client perspective, you should only pick the checksum.sri that you are ok with the server verifying. If there are sri that should be ignored, don't add them to the request's qualifier haha.

The only thing the client should pay attention to is to avoid using sha1 or md5.

[werkt]

@sluongng

It would be extremely painful if we do that.

Painful or not, some correspondence with the Digest (and DigestFunction) in the response should be indicated.

The notes in the response do allude to this, where it says that:

  // This field SHOULD be set to the digest function that was used by the server                                                                                                                                              
  // to compute [FetchBlobResponse.blob_digest].                                                                                                                                                                              
  // Clients could use this to determine whether the server honors                                                                                                                                                            
  // [FetchBlobRequest.digest_function] that was set in the request.                                                                                                                                                          
  //                                                                                                                                                                                                                          
  // If unset, clients SHOULD default to use SHA256 regardless of the requested                                                                                                                                               
  // [FetchBlobRequest.digest_function].

This implies it's completely up to the server's discretion what they want to do with it, but consider the practical case:

• User requests sha256, but only presents sha384 sris.
• Server responds with sha384, because it only computed the digest in the integrity.
• Client use case is ambiguous at this point.
  • To retrieve the content (bazel's only use case right now that I'm aware of), you have to use the sha384.
  • It cannot verify it except with the sha384, and to get it keyed to sha256, you'd have to upload into the CAS from the client (after downloading and computing it).
  • Other use cases may vary, and I've not heard them (server-side resolution for missing blobs?).
  • Would be happy to consider reasons why not digesting with the function the client requests is a good idea, but someone has to point to a more comprehensive use of these APIs with a benefit.

So it sounds like you can have a payload like this:

...

and the server should do the following:

...

I agree with all of these states/procedures, per the spec. The only effective checksums are those with the strongest hash function, and any of them matching will succeed the download.

@malt3

This is what the specification says. However, I still think that for a client using the Fetch service, this is practically not any > different from allowing the server to just check any of the provided checksums.

Which algorithms are considered strong (or strong enough) changes over time
Server and client can disagree on the order of prioritization
A server can choose which subset of all hash functions it is willing to work with

All of these things being true, the realization is that the Client in this case is not the user-agent, it is the (spec exemplified) html author adding tags. It is foregoing that user-agent role by requesting via the fetch api. The html author is no more in control of the browser user-agent than the client is here. The server's guidelines should be the spec, and it is outside of the specification to dictate how the server should implement these: there is no agnosticism permitted here, hence the digest flexibility requirement.

[EdSchouten]

Looking at the protocol, we also have the digest_function field in requests, with the following note:

  // The digest function the server must use to compute the digest.                                                                                                                                                           
  //                                                                                                                                                                                                                          
  // If unset, the server SHOULD default to SHA256.                                                                                                                                                                           
  build.bazel.remote.execution.v2.DigestFunction.Value digest_function = 6;

Since the sri only refers to values specified by the client, it sounds like this means that the server must present/compute this digest separately for the purpose of the Response population. Can we specifically associate this language for this field with the response digest field?

It would be extremely painful if we do that.

In Bazel ecosystem, there are cases such as rules_js where the sri integrity might come from an upstream package manager ecosystem which uses SHA284 or SHA512, while the build tool (Bazel) was using SHA256/BLAKE3.

Forcing the sri to be the same as the response digest function would be disruptive for this ecosystem and requires build rules authors and/or users to maintain a manual mapping of integrity themselves.

Not only that. Even though the Remote Asset API is currently only used as a way to speed up downloads of remote assets, it could also be used to completely avoid the downloading of assets on the client, having the client place these files in input roots of actions "by reference".

Because REv2 doesn't allow input roots containing files with different digest functions, it would be severely restrictive if the Remote Asset API required that the digest function of these requests matches the one in checksum.sri.

[werkt]

@sluongng @EdSchouten

Since the sri only refers to values specified by the client, it sounds like this means that the server must present/compute this digest separately for the purpose of the Response population. Can we specifically associate this language for this field with the response digest field?

Forcing the sri to be the same as the response digest function would be disruptive for this ecosystem and requires build rules authors and/or users to maintain a manual mapping of integrity themselves.

Clarifying here - there's no association between the sris' hash-algos, and the requested digested function. Explicitly, they're being isolated in my request for additional language - they have nothing to do with one another. The requested digest function should only correspond to the digest function (and computed digest) in the response. Since there are other algos in the mix, I suggested that we add language to tie the two together (Request.DigestFunction -> Response.{Digest,DigestFunction}) distinct from any qualifiers, if a server chooses to implement them.

[malt3]

Since there are other algos in the mix, I suggested that we add language to tie the two together (Request.DigestFunction -> Response.{Digest,DigestFunction}) distinct from any qualifiers, if a server chooses to implement them.

I'm in favor of adding language for this. It appears to match everyone's expectations. This behavior is also implemented by all servers I could find so far.

In addition, it would be nice to have some other field in the response to signal additional validations performed by the server (like a field specifying which part of checksum.sri was verified by the server), but I'm not convinced that this is easy to do, without being too focused on checksum.sri. Unless someone else has good ideas on how to convey this extra information I think we can focus on the important changes:

• Make it more obvious that checksum.sri can contain more than one checksum, separated by whitespace (aka [remote asset api] Clarify encoding of checksum.sri #330)
• Clarify that checksum.sri and other qualifiers are not connected to the digest. The server should either use the requested digest_function, the default (SHA256) if the request doesn't specify one, or reject the request.

[werkt]

In addition, it would be nice to have some other field in the response to signal additional validations performed by the server (like a field specifying which part of checksum.sri was verified by the server)

Response already contains a Qualifier field for population with the utilized checksum.sri.

[malt3]

Response already contains a Qualifier field for population with the utilized checksum.sri.

That's correct, but right now it's hard to reason about it in the case of checksum.sri:
If the server validated one out of three checksums that are part of that qualifier in the request, does the server respond with the full string (three checksums), or only with the checksum it has validated?
As a client, can I rely on the response in any way?

[werkt]

That's correct, but right now it's hard to reason about it in the case of checksum.sri: If the server validated one out of three checksums that are part of that qualifier in the request, does the server respond with the full string (three checksums), or only with the checksum it has validated? As a client, can I rely on the response in any way?

The language is supportive, but not prescriptive, for such filtering. I don't think any interactions with remote are wholly reliable just by [comment] text in this specification, and there's not enough wide coverage to say that a server is or is not compliant. Telling servers explicitly what they must provide hasn't been the theme of expansions here, it tends to be what the clients react with.

[malt3]

I think it would be permissible by the current spec to introduce a new qualifier that is only used to convey validated checksums (checksum.sri.validated).
Since it is new, we could document the intended use as part of the response.
Maybe it's a good idea to discuss this in another issue or discussion, to get back to the original point.

[bergsieker]

Trying to summarize the discussion here, with a few of my own opinions:

• The digest_function is independent of any qualifiers set in the request. This is by design--the qualifiers are intended for (among other things) validating the the external resources that are pulled in, while the digest_function is explicitly the used to specify the desired output format of the stored resources. For example, it's entirely permissible to specify that the external resources should be validated using SHA512, but that you want them to be stored in the CAS using a SHA-256 digest.
• It is expected that request.digest_function and response.digest_function should match, except in cases where one of more of the client or server does not support these fields. (In such cases, an unset field should be treated as SHA256.) Per @werkt the current language here is ambiguous, but I can't think of a case where it would be useful for the server to set a different value from what the client requested. I'm supportive of clarifying the language here.
• The SRI specification already provides for indicating multiple validation methods, and choosing between them. As noted, this should be part of the suggested lexicon, not the API itself.
• It seems reasonable to define a response Qualifier to indicate which verifications were performed. Similarly, this should be part of the lexicon, not the API itself.

All of the above should be farmed off to separate PRs.

Per the original request: should the server be allowed to ignore unsupported Qualifiers? IMO it should NOT. While the cited example of multiple verification methods is reasonable, allowing the server to ignore unsupported verification methods would be specifying which Qualifiers it is OK to ignore (or to ignore if there's another, equivalent, qualifier already present). I prefer a model where the server MUST respect all qualifiers, but the qualifier itself can be defined flexibly (as is the case with checksum.sri).