tavis-software / Tavis.UriTemplates

.Net implementation of the URI Template Spec https://tools.ietf.org/html/rfc6570
Apache License 2.0
171 stars 39 forks source link

Consider adding digital signatures to DLL and Nuget Package and/or moving to DNF or similar #105

Open CZEMacLeod opened 1 year ago

CZEMacLeod commented 1 year ago

As part of migrating from Microsoft.Azure.ActiveDirectory.GraphClient to Microsoft.Graph due to the deprecation of the old package and endpoints, my project ended up depending on Microsoft.Kiota.Abstractions which in turn depends on this 'third-party' package.

I understand that the author and owner of this package now works with Microsoft and on the Kiota project, and that this package has good coverage and tests, however it does represent a potential issue in software bom and supply chain management. This is a key concern of many at the moment, and, especially as this is now used (indirectly) for access to office 365 and azure, I feel it important to bring this up.

The licence, and the fact that it appears to be 'third-party', do not unduly concern me, however, there are 2 issues I see directly at the moment. Neither the built DLL, nor the NuGet package have digital signatures applied, and therefore it is an easy target for supply chain interception. Also NET 8 SDK has started turning on more validation and verification of packages, and I only see this increasing.

While I understand that it would not be good to maintain 2 copies of the code, by embedding copies of the code directly into the Microsoft.Kiota.Abstractions package, perhaps some alternative could be investigated. Either trying to get the dotnet team to onboard a version (similar to how Json.Net got 'replaced' for internal projects with System.Text.Json) or perhaps by moving this project to the DotNet Foundation where you could take advantage of their build infrastructure to get certificates etc.

See the original issue and discussion here - https://github.com/microsoft/kiota-abstractions-dotnet/issues/98

darrelmiller commented 1 year ago

You raise valid points. I will discuss this with the team.

baywet commented 1 year ago

To recap the options:

Anything else I'm missing?

CZEMacLeod commented 1 year ago

Did anything come of this? I know it is probably low priority, but for traceability reasons it would be nice to see this more formally adopted, and/or full digital signatures on the package and dll.

baywet commented 1 year ago

We've made some progress on this front, working with our friend at Red Hat (including @andreaTP), we realized that RFC6570 implementations were all over the place in every language, often relied on single maintainers (hit by a bus strategy), some are inactive, some are bringing undesired additional dependencies, etc... None of that is great.

Andrea started the work to stand up a new organization with implementations that move at the same pace across languages here. This organization is co-owned by Red Hat and Microsoft, builds on the learnings we all made in that space, and kiota client will transition over time to those new implementations. there is already a PR up for dotnet

As for this library, we'll probably need to compare what additional value it provides on top of those barebone implementations, and figure out a path for alignment and/or deprecation.

CZEMacLeod commented 1 year ago

@baywet I like the look of this initiative. Couple of things I find slightly odd (perhaps it is part of the ideology of keeping all language implementations the same):

  1. The package name and assembly name don't match (which isn't necessary, but I believe is recommended practise) StdUriTemplate.dll != Std.UriTemplate
  2. The namespace is all lowercase instead of pascal case and again does not match the package name. stduritemplate != StdUriTemplate or Std.UriTemplate
  3. The package and AssemblyDescription have yet another casing/spacing Std Uritemplate implementation for .NET.

Also, and these maybe nit-picky but some are basically my original issues:

  1. The NuGet package does not have a digital signature.
  2. The dll does not have a digital signature. (It is strong named, but that is not the same as having an Authenticode signature.)
  3. The package does not have any tags on it, nor does it include the RFC 6570 L4 in the description.
  4. The package is missing source link, not deterministic, missing compiler flags and other standard information.
  5. Std.UriTemplate.Test I think is published by mistake, and should probably be unlisted.
  6. The package is published by @andreaTP alone and the package does not indicate that it is officially supported by Microsoft or Red Hat - in that sense it looks as 'third-party' as the original Tavis.UriTemplates which (apart from digital signatures) did meet all the normal standards for packages and dotnet.

Further to item 9, the org information for std-uritemplate does not tell you anything about who the organisation is, or who is backing it, and that information isn't in the main repo either.

I appreciate that this is alpha level (by the 0.x.x package version) and is still being put together, but I'd prefer this not to be a step backwards, and all the learning from this implementation and release not be lost.

I'll maybe raise an issue or two over on the new repo regarding these, but I don't want to be negative about it when it is just getting off the ground.

andreaTP commented 1 year ago

Hi @CZEMacLeod and thanks a lot for the super-useful feedback! Some of the short-comings are simple consequences to my inexperience in the dotnet ecosystem.

Regarding 1 / 2 / 3 / 6 / 7 -> those are all great suggestions, it would be great if you could submit a PR to fix those issues 🙏

On the "digital signature" subject -> since this is a recurrent cost (and this seems to be the only affordable one) and RedHat is collaborating on Kiota but not particularly invested in .NET. I leave the subject on @baywet plate 🙏

On 8 -> done, unlisted, thanks for noticing! On 9 -> to my understanding this is not a "regression" compared to the current situation

baywet commented 1 year ago

Thanks for the great feedback. @CZEMacLeod would you mind opening issues on the target repo for the different points so we can have discussions about those and bring them to resolution please? (and also avoid impacting this repo)