Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add disclaimer and new section about known implementations #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

brycx
Copy link
Contributor

@brycx brycx commented Jun 25, 2021

This introduces several changes as part of #37.

  • We put a disclaimer at the beginning of the list of known implementations. This way, it's clear that this list does is not to be interpreted as an official endorsement.

  • Another separate list is added in the beginning. It's purpose is, that libraries that have been tested previously to be correct, can be added here. We still don't guarantee this list is updated, but can still provide a better starting point compared to some of the libraries listed later, that may not be compliant at all.

I'd like your take on this additional section @tuupola. The intention is not to provide an endorsement list, just one that contains libraries that have been tested successfully at least once. I think the idea with a checkmark, indicating compliance, as implemented by PASETO suggested in #37 is not the right approach. We don't expect people working on this specification to set aside time to audit a new library, each time a new author wants to add theirs. (The list in this PR are just the ones I'm familiar with)

With PASETO, I've myself added a library to their list, where I myself indicated that it passed test vectors. I have no idea if this was verified by a third-party at any time. If not, the end result seem equal to simply a author adding their new library - the same level of confidence can be derived from that.

The new list can be updated on an ad-hoc basis. If someone wanting to use a library verifies it and reports here that it can be moved up the "known to comply at some point" library, we can do so. This was an alternative to a "Recommended libraries" section, but if this that seems better to you @tuupola, we can make that instead.

The last point I want to address, is that: As I've written about here, some libraries listed suffer security problems in addition to non-compliance. Do we wish to remove these entirely?

@grempe
Copy link

grempe commented Jun 25, 2021

The last point I want to address, is that: As I've written about here, some libraries listed suffer security problems in addition to non-compliance. Do we wish to remove these entirely?

IMHO FWIW, you should not link to them if they have known security issues. I would also suggest creating a bug on any project with those (or other non-conformant issues discovered) so that others who may be exploring use of that lib can become aware of a potential issue while evaluating use of the library.

@AmanAgnihotri
Copy link

@brycx, I went over your blog post: Rolling your own crypto gone wrong: A look at a .NET Branca implementation, which was referenced in the blog post linked above. It was an informative read.

Recently, I myself made a .NET implementation of Branca and made it available as a NuGet package. I also found various issues with the branca-dotnet library and listed them out in issue #19.

You can find more information about it here:

#19 (comment)

@tuupola
Copy link
Owner

tuupola commented Aug 22, 2021

Sorry it took a while. I am back from summer holidays.

I do agree now that we have proper test vectors should require listed implementations to actually implement them. As a transition period two different listings: one listing with implementations passing the test vectors and another one with implementations without test vectors. Then nudge implementations to add unit tests and after a while remove ones which do not have unit tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants