[superceded] Define a standard SSSOM hashing function. - #529
Conversation
This commit adds a new section to the normative part of the website, intended for specifying stuff that are not part of the SSSOM data model nor of any of the SSSOM serialisation formats, but that should still be formally specified so that they can be implemented in the expected way by the various implementations. The existing page about "chaining rules" is moved to that new section, as it does not really belong to the "data model". A new page is added to the new section to define a standard SSSOM hashing function. closes #436
| yield the following hash (in hexadecimal): | ||
| `e3bc1b4b586c6e86d0caf369d49c161163e255c4a779821f448a8e4fbd616522`. | ||
|
|
||
| Finally, encoding the binary SHA2-256 hash in ZBase32 would yield the |
There was a problem hiding this comment.
is the brevity of the hash worth the complexity of encoding/unencoding in yet another step?
There was a problem hiding this comment.
The output of the SHA2-256 hash function is a binary blob that cannot be used directly, so we need a post-hash encoding step anyway.
We could use hexadecimal encoding instead of ZBase-32 encoding, but that’s still “yet another step” that must be performed after the hashing.
As for which encoding to use, I raised that question here with some of the available options. Nobody weighed in on that issue.
There was a problem hiding this comment.
I would prefer using hexadecimal digest of the SHA2-256 hash function over zbase-32 since this is more common, and it doesn't appear that there is either built-in support for zbase-32 nor an obviously well-maintained/authoritative implementation of it in Python
There was a problem hiding this comment.
rationale behind worrying about not having an authoritative implementation - this is becoming even more of a security risk if we have to include packages like this in our infrastructure given the current state of open source
There was a problem hiding this comment.
I still personally favour zbase-32, because it gives hashes that are more human-readable than all other possible encodings.
I am not convinced about the availability of “an obviously well-maintained/authoritative implementation of it in Python” being an issue. On what grounds do you say that the several available implementations are not “obviously well-maintained“? If it’s because they have not been updated in years, I don’t believe this is a valid criterion for a function like zbase-32. It’s perfectly normal for such a function not to require any maintenance after it has been implemented correctly.
I wrote a C implementation of it 9 years ago, I am still using it in some C projects to this day and I never had to retouch it since the first write.
Worst-case scenario, in case a Python developers really does not want to have to depend on an external library, implementing zbase-32 is trivial. Here’s for example my aforementioned C version:
static char zb32[] = "ybndrfg8ejkmcpqxot1uwisza345h769";
/**
* Encodes a buffer in Z-Base32 encoding.
* This function encodes data from the provided buffer and writes the
* encoded data to the specified buffer.
*
* @param[in] src The input buffer.
* @param[in] slen The size of the @a src buffer.
* @param[out] dst The output buffer.
* @param[in] dlen The size of the @dst buffer.
*
* @return The number of bytes written to the output buffer.
*/
size_t
zbase32_encode(const char *src, size_t slen, char *dst, size_t dlen)
{
size_t i, j, k;
for ( i = j = 0; i < slen && j + 7 < dlen ; i += 5 ) {
int64_t val;
for ( val = k = 0; k < 5; k++ )
if ( i + k < slen )
val |= (int64_t)((unsigned char)src[i+k]) << ((4 - k) * 8);
dst[j++] = zb32[(val & 0xF800000000) >> 35];
dst[j++] = zb32[(val & 0x7C0000000) >> 30];
if ( i + 1 < slen ) {
dst[j++] = zb32[(val & 0x3E000000) >> 25];
dst[j++] = zb32[(val & 0x1F00000) >> 20];
}
if ( i + 2 < slen )
dst[j++] = zb32[(val & 0xF8000) >> 15];
if ( i + 3 < slen ) {
dst[j++] = zb32[(val & 0x7C00) >> 10];
dst[j++] = zb32[(val & 0x3E0) >> 5];
}
if ( i + 4 < slen )
dst[j++] = zb32[val & 0x1F];
}
return j;
}Porting it to Python is left as an exercise to the reader. ;)
There was a problem hiding this comment.
we should consider choosing a canonical CURIE prefix and URI prefix for this hash, perhaps sssom.record and https://w3id.org/sssom/record/, such that it can be used in places where CURIEs/URIs are required.
we should probably also add an example that has a mapping annotating the record_id with this
I believe this is beyond the scope of the spec. The spec defines the function. How people use it is none of the spec’s business. In particular, I personally believe the hash should not be used to fill the |
Provide examples in SSSOM/TSV rather than JSON format. Add an example of a mapping record making use of extension slots. Show for each example the canonical S-expression in addition to the final hash value.
Okay, fair. We are including non-spec documentation along with this kind of thing, so let's table this for future discussion after this PR.
Could you expand on this? I thought that one of the main ideas with We don't have to make a recommendation to do it either way, but maybe saying in the non-normative documentation: here's what's possible, when using it is appropriate, and when/why it's not a good idea |
Yes. Emphasis on the however you want. The spec providea a field to store a record identifier. How you mint an identifier is out of scope for the spec, which only mandates that the identifier must be “curifiable” (since
First, I don’t believe there should be a “standard way” of minting identifiers for SSSOM mapping records. People should be able to use whatever method they want to mint identifiers. The spec should stay out of that. After all and as far as I know, none of the specs routinely used in the semantic world (e.g. RDF, OWL) try to enforce a specific way of identifying entities – all they require is that identifiers are some kind of strings and that they must obviously be unique; they don’t care what the strings represent, if they even represent something, or how they are generated. Why should SSSOM be different? Some SSSOM users might want to use randomly generated UUIDs (because why not?): Some might want to use serially allocated numbers (my personal preference, for what it’s worth): Some others might want to use randomly generated words (weird, but again why not?): Some others yet might want to use non-opaque strings that reflect somehow the content of the record (IMO a very bad idea, but I know some people do like the idea of non-opaque identifiers): And yes, some might want to use a hash value calculated on the content of the record (hereafter called a “content-derived identifier”): Again, I believe the spec should stay well clear of all that. The documentation might suggest some possibilities but the spec should absolutely not mandate or even recommend anything. “You want to give your records some unique IDs? Sure, here’s a slot ( Then there’s the specific issue with “content-derived identifiers”, and why do I think they are a particularly spectacularly bad idea. I’ve already given my thoughts in some details at several occasions on this repo (notably when we were discussing the introduction of the (A) Content-derived identifiers make it impossible to write a mapping set entirely by hand. Regardless of the hashing method used (even if we somehow used a method that is simpler than the one proposed in this PR), the hash cannot be reallistically be computed in someone’s head. You have to use a tool to produce it. This would break an important promise of SSSOM, which is that one can always manually craft a SSSOM set with no specialised tooling at all – just a plain old spreadsheet software. (B) Content-derived identifiers are at risk of becoming “out-of-sync” with the records they supposedly identify. If you modify the record in any way but forget to re-run the ID-generating procedure (procedure that you have to run because, as stated in A, you cannot generate/update the identifier yourself), then you’ll end up with IDs that are no longer really derived from the content of the record. (It could be that are fine with that outcome, but if so, that simply means that you didn’t even need content-derived identifiers to begin with, so why bother with them in the first place.) (C) Content-derived identifiers deprive the set’s creators of the freedom to decide the difference between “updating an existing record” and “creating a new record”, because in fact there is no such thing as “updating a record” when using a content-derived identifier – any change to a record would cause the identifier to change, meaning that you are in effect always creating a new independent record. There may be cases where this is desirable, but I’ll wager those cases should be very few. (D) As a direct consequence of C, content-derived identifiers are not stable, because again, any change to the record (even a semantically meaningless change like fixing a typo) would cause the identifier to change. I fail to imagine any scenario where that could be a good thing. What’s the point of treating mapping records as uniquely identifiable semantic entities (similar to a RDF resource or a OWL class) if I cannot reliably refer to them (like in a database or something)? (E) The instability of content-derived identifiers is even worse in the specific case of SSSOM, because the content of a SSSOM record could change because of something that is out of the control of the record’s creator. Consider the following record: and now let’s imagine that Uberon curators decide to rename UBERON:6000015 from “insect thorax” to “thorax sensu Insecta”, because they decided that they prefer this way of of mentioning the species. The next time the mapping set is updated to be sure it is using the correct labels, the record thus becomes: In this scenario, the meaning of the record has not changed at all. UBERON:6000015 still represents the same concept as before, so this record still represents the very same mapping between the very same entities. And yet, because the label of UBERON:6000015 has changed, if records were identified using content-derived identifiers we would have to consider the second record as a different entity, identified with a different identifier, than the first. Did you already add some links to the record in your application database? Too bad, now you’ll have to update all references to that record so that they use the new identifier. Again, I fail to imagine a scenario when that can be desirable. Overall, content-derived identifiers can only be viable if some very specific conditions are met:
If you use SSSOM that way, then maybe content-derived identifiers can be fine. Otherwise, I believe they are an incredibly bad idea that people should not even consider until/unless they can explain very precisely why they think they actually need such identifiers. If they cannot, they should just stick to meaningless, opaque identifiers that are not tied to the content of the record – like most semantic identifiers. |
Add a table explaining how to turn the value of an extension slot into a string, based on the declared type of the extension.
|
It would be great to incorporate #529 (comment) into non-normative section of the documentation. Could you also please lint the markdown using prettier After that and reconsidering using hex digests of sha hash, I think this is ready |
|
Also FYI I have a working implementation of this in sssom-pydantic cthoyt/sssom-pydantic#75 (though extensions are not supported neither by the package as a whole, nor in the s-expression implementation) |
Will do :) though that may have to wait until tomorrow. |
Run the following command:
npx --yes prettier --check --prose-wrap always --tab-width 4 \
--write src/docs/spec-support-hashing.md
Note the `--tab-width 4`! Without it, that piece of crap that is npx
prettier would completely mangle the nested lists.
This mostly turns my comment (#529 (comment)) into a guidance page. Not convinced this is really useful, but that's what has been asked in review.
cthoyt
left a comment
cthoyt
left a comment
There was a problem hiding this comment.
We're at an impasse about hashes - I wouldn't be as happy but would accept the zbase-32 proposal if @matentzn (or other community members) find Damien's arguments better than mine.
I am mostly happy with the record-identifiers.md. I think parts of it can be toned down a bit, but I don't want to hold up this PR on it. Let's make iterative improvements on that in follow-up PRs as needed.
Let's try and resolve the hash part then call this finished.
|
So @matentzn I guess you have the deciding vote! :D Should the hash of a mapping record look like
I argue that a Z-Base32 hash is better for users, as it is smaller and easier to read and compare (in particular, less risks of mistaking a character for another, which in hexadecimal could easily happen e.g. between The fact that Z-Base32 could be slightly more complicated for developers (because they have to either find a library that provides the Z-Base32 encoding function, or implement that function themselves) should be irrelevant, or at least, less relevant than the benefit for users. That being said, having a standard hash function is more important than the encoding used, so I’ll be in the same position as @cthoyt : if @matentzn or others find that hexadecimal encoding is better overall, I won’t be as happy but I can live with it. :) |
|
I wonder if maybe we should replace the use of SHA2-256 by a simpler, non-cryptographic condensation function like FNV (RFC 9923). After all we do not need the cryptographic properties (resistance to collisions and pre-image attacks) of SHA2-256 here. All we need is to be reasonably confident that two different mapping records (that have not been crafted specifically in order to produce a collision) will produce two different hashes. According to the birthday paradox, with a FNV64 hash, we would need to hash 2^32 records (that’s more than 4 billion records) before we have more than 50% chance of two different records inadvertently producing the same hash. In the context of SSSOM that sounds enough to me. And a FNV64 hash ① would be much faster to compute and ② would fit in 8 bytes, so even a simple hexadecimal encoding would yield a very convenient 16-characters value (of the form |
|
It seems that Python’s standard library does not provide a FNV64 function, but implementing it is even more trivial than ZBase32: FNV64_PRIME = 1099511628211
FNV64_OFFSET = 14695981039346656037
def fnv64(data: bytes) -> bytes:
h = FNV64_OFFSET
for byte in data:
h = h ^ byte
h = (h * FNV64_PRIME) % 2**64
return h.to_bytes(8, "little") |
|
I agree that cryptographic security isn't needed here. Why did you go for FNV64 instead of MD5? Again, I am thinking that picking well-known, highly available algorithms is the safer way (I had never heard of FNV before) Also nit, almost all hex digests in the web world I've come in contact with are lowercased. Maybe this is different for other flavors of programmer |
Categorically against the use of MD5 here. First because it would send confusing signals, given that MD5 is a cryptographic function that happens to be broken for most purposes: on one hand it would seem to suggest that we do want a cryptographic hash function (if we do not want that then why not pick a faster non-cryptographic hash function?), on the other hand it would seem to suggest that we don’t care that much about the cryptographic properties (if we do care about the cryptographic properties then why not pick a non-broken cryptographic hash function). Second because choosing MD5 might not necessarily give implementers an easier job. MD5 is likely to be either absent or soon-to-be-removed from many crypto libraries, precisely because the function is broken (when cryptographic properties are required), obsolete, and should no longer be used. Third because using MD5 is an invitation to receive many complaints and bug reports (even more so in the era of AI-generated bug reports) along the lines of “your project is using a dangerous, obsolete function”. I don’t want to have to justify over and over again our choice of MD5 (“no, in our case we do not need cryptographic properties, so we are fine with MD5, thank you”). Either we use a cryptographic hash function, and in which case we should use a strong, non-broken one (SHA2-256 being the obvious choice: the SHA-3 competition a few years ago showed that the SHA2 family is much stronger than previously thought, and it is probably the most widely supported cryptographic hash function out there), or if we decide we don’t want need a cryptographic hash function, we make that decision explicit by deliberately choosing a non-cryptographic hash function – not by choosing a weak, broken cryptographic one. As for which non-cryptographic hash function to use:
FNV-1a is just one of the most well-known non-cryptographic hash function. From Estébanez et al., 2014:
While this paper concludes by recommending that developers looking for a non-cryptographic hash function adopt
Lastly:
I have no strong opinion on that and would not mind specifying that the hexadecimal encoding should use lower case characters. But funnily enough, on my side most hex digests I’ve come across are uppercased. :D |
|
alright, I'd support FNV as a hash 👍 |
|
And as for encoding the hash? Lowercase or uppercase hexadecimal? I have a slight preference for uppercase because that’s what the definition for Base16 encoding (“Base16 encoding” being merely a fancy name for “hexadecimal”) in RFC 4648 says (which incidentally means that it’s also what Python’s But I really don’t mind, so if you prefer lowercase that’d work for me. |
|
let's just follow the RFC then. so I think we're in agreement for all points now. Would you please decide which PR you want to move forward on, then we can call Nico for final approval |
|
Since we both agree on FNV64 + hexadecimal encoding, let’s go ahead with #534, unless someone wants to argue that we should in fact use a cryptographic hash function (and therefore stick with SHA2-256). |
cthoyt
changed the title
|
Superseded by #534. |
2 participants
Resolves [#436]
docs/have been added/updated if necessarymake testhas been run locallytests have been added/updated (if applicable)This PR adds a new section to the normative part of the website, intended for specifying stuff that are not part of the SSSOM data model nor of any of the SSSOM serialisation formats, but that should still be formally specified so that they can be implemented in the expected way by the various implementations.
The existing page about "chaining rules" is moved to that new section, as it does not really belong to the "data model".
Finally, the core of the PR is that a new page is added to the new section to define a standard SSSOM hashing function.
closes #436