Migrate from satori/go.uuid to gofrs/uuid

[leitzler]

Bug Report

The version azure-sdk-for-go currently use do have a critical flaw when creating UUID:s. This package doesn't create UUID:s, but since it is a flawed version that is brought in, there is a risk of future bugs or bugs in projects that use azure-sdk-for-go.

Gofrs is a community-driven effort to provide maintainers for valuable projects. They forked satori/go.uuid after it appeared to be no longer maintained while exhibiting critical flaws. The fork will most likely be given more maintainer love than the original one.

I did create a pull request #3139 that was closed due to the usage of go.uuid in the code generator.

[tariq1890]

@jhendrixMSFT Is there anything we can do about this? I'd really prefer using gofrs/uuid. Injecting satori/go.uuid as a type is really problematic as it creates a hard dependency on that library. The library lacks the required maintenance and security fixes.

[jhendrixMSFT]

I would too, the issue is it's a pretty big breaking change. We need to coordinate with our large partners and "get the message out" that we'll be making this change.

[tariq1890]

Now that I really think about this. Is it possible to just use the string type? This way we are not at the mercy of any library. The only possible con here is that there would need to be user-side validation and pattern enforcement, which isn't too bad IMO.

Thoughts @jhendrixMSFT ?

[jhendrixMSFT]

It would be a significant breaking change, more than switching to gofrs/uuid. Also depending on who you ask one could also make the argument it should be a byte array since technically that's what a UUID is (but then parsing, helpers to convert to string, etc now you're back to using a UUID type).

[tariq1890]

I understand, but I feel []byte or string is the best middle ground. While the breaking change is inevitable, I am certain that there are enough consumers who are heads deep in dependency lock-ins and they will not be able to forego their dependency on satori/uuid. Changing it to gofrs would force them to directly/indirectly on an additional uuid dependency which is just fodder for an even more messy situation.

[jhendrixMSFT]

Is it though? gofrs/uuid is a fork of satori/go.uuid and is API compatible, so in theory it should simply be a matter of changing imports.

[jhendrixMSFT]

Closing as we won't be making this change in the track 1 SDK. Note that for track 2 we've decided to leave UUIDs in string format, allowing consumers to use their preferred helper package.

[Porges]

@jhendrixMSFT at the moment we have a project that is correctly being flagged (internally at MS) due to dependency on the satori package as it has unremediated security problems (reference WS-2018-0594). Could azure-sdk-for-go at least switch to the gofrs package so it is not using a broken library?

[jhendrixMSFT]

After some internal discussion we've decided to make this fix. I scanned the affected packages and it's only ~20% so we feel that the breaking change is low enough impact to be worth it.

[jhendrixMSFT]

@ArcturusZhang could you please update the code generator to replace satori/go.uuid with gofrs/uuid?

[bganapa]

@jhendrixMSFT @ArcturusZhang Could you please confirm this issue is fixed in V1 SDK and also point out the released version where the fix is?

[ArcturusZhang]

@jhendrixMSFT @ArcturusZhang Could you please confirm this issue is fixed in V1 SDK and also point out the released version where the fix is?

The migration was done in v53.0.0: https://github.com/Azure/azure-sdk-for-go/releases/tag/v53.0.0