in ,

Always Review Your Dependencies, AGPL Edition, Hacker News


Before adding a dependency to one of my software projects, I do some basic vetting of the dependency. Among the things I check are:

  • How is the code licensed?
  • Who are the authors?
  • Are there any any serious unresolved issues in the issue tracker?
  • Is there a history of serious bugs in the issue tracker?
  • What kind of code review process is used for pull requests?

Finally, I do a cursory review of the code. I look for anything blatantly insecure or malicious, and try to get a feel for the quality of the code base. I look for “Brown M & Ms” – minor inattention to detail that might indicate a larger problem.

I repeat the above recursively on transitive dependencies as many times as necessary. I also repeat the cursory code review any time I upgrade a dependency.

This is quite a bit of work, but is necessary to avoid falling victim to attacks likeevent-stream. I was recently reminded of yet another reason to review dependencies, as I reviewed Duo’shighly-publicizedGo library for WebAuthn,github.com/duo-labs/webauthn.

It started off poorly when I noticed some Brown M & M’s: despite being a library, it was logging messages to stdout, and there were several code smells which indicated inexperience with Go. Sure enough, these minor issues foreshadowed a far larger problem: when I started reviewing the transitive dependency github.com/katzenpost/core/crypto/eddsa, I was greeted with anAGPLv3 license header.

This was bad news for most people wanting to use Duo’s WebAuthn library. Although Duo had licensed their library under a BSD license, when you linked your application with Duo’s library, you’d also be linking with the AGPL-licensed library, creating a “modified” work in the eyes of the (A) GPL, thus subjecting your application tosection 43 of the AGPL:

Notwithstanding any other provision of this License, if you modify the Program, your modified version must prominently offer all users interacting with it remotely through a computer network (if your version supports such interaction) an opportunity to receive the Corresponding Source of your version by providing access to the Corresponding Source from a network server at no charge, through some standard or customary means of facilitating copying of software.

In other words, if you used github.com/duo-labs/webauthn in a public-facing web app, your web app had to be open source.

The most galling thing about this dependency is that it’s redundant withgolang.org/x/crypto/ed, which is one of Go’s

quasi-standard “x” libraries. In fact, github.com/duo-labs/webauthn originally used golang.org/x/crypto/ed(***********************************************. That changed during a pull request from an external collaborator titled “Consolidate COSE things to their own area“. In the process of moving some code from one file to another, this pull request subtly changed the implementation ofOKPPublicKeyData.Verify.

Here's the oldOKPPublicKeyData.Verify, which uses golang.org/x/crypto/ed

// Verify Octet Key Pair (OKP) Public Key Signaturefunc(

k**************************** OKPPublicKeyDataVerify(data************************ (byte********************,sig[]byte************************ ((bool) ************************,error

){f

:=HasherFromCOSEAlg**************************** (COSEAlgorithmIdentifier) ​​******************************** (((k.********************** (PublicKeyData) ************************************************************ (Algorithm) ))(:=f () ****************************(. Write************ () (**************** (data ********************************returned.Verify****************** k************************

XCoord

,h************************************** (Sum) ************************************** (nil),sig () **************************** nil

Here's the newOKPPublicKeyData.Verify, which uses the AGPL-licensed github.com/katzenpost/core/crypto/eddsa:

// Verify Octet Key Pair (OKP) Public Key Signaturefunc(

k**************************** OKPPublicKeyDataVerify(data************************ (byte********************,sig[]byte************************ ((bool) ************************,error

){f

:=HasherFromCOSEAlg**************************** (COSEAlgorithmIdentifier) ​​******************************** (((k.********************** (PublicKeyData) ************************************************************ (Algorithm) ))(:=f () ****************************(. Write************ () (**************** (data ********************************varoKey

eddsa****************************** (PublicKey******************err:=oKey************************** FromBytes****************** (((k.********************** (XCoord) )iferr=******************** (nil) ************************** (){

returnfalse
,err****************returnoKey
**********************Verify

. **************** (h (h) ************************.Sum(******************** (nil) ), ******************************** (sig) ),

nil

There was zero explanation provided for this change. The pull request wasreviewed by two Duo employees, who approved and merged it.

Aside: this is why I don't like to accept pull requests that move code around. Even if the new code organization is better, it's usually not worth the time it takes to ensure the pull request isn't doing anything extra.

Ifiled an issueabout the AGPL-licensed dependency, and the developersswitched backto using golang.org/x/crypto/ed25519. Nevertheless, I've decided not to use github.com/duo-labs/webauthn. The bulk of the library and its dependencies are to support a WebAuthn misfeature called attestation, which I have less-than-zero desire to use. I just finished writing a vastly simpler, attestation-free library which is less than one tenth the size (I will open source it soon - watch this space).************ () (**************** There is another lesson here, which is that complicated "features" like attestation that serve a minority's use case shouldn't be added to Web standards.)************************ Developing this library is less costly than the liability of using an existing WebAuthn Go library.

This incident reminded me of why I like programming in Go. Go's extensive standard library, along with its quasi-standard "x" libraries, mean that the dependency graph of my projects is typically quite small. The bulk of my trust is consolidated in the Go project, and thanks to their stellar reputation and solid operating procedures, I don't feel a need to review the source code of the Go compiler and standard libraries. Even though I love Rust, I am terrified every time I look at the dependency graph of a typical Rust library: I usually see dozens of transitive dependencies written by Internet randos whom I have zero reason to trust. Vetting all those dependencies takes far too much time, which is why I'm much less productive in Rust than Go.

One final note: as a fan of verifiable data structures like Certificate Transparency, I have to love the new Go checksum database

. However, the checksum database does you no good if you don't take the time to review your dependencies. Unfortunately, I've already seen one over-enthusiastic Go user claim that the Go checksum database solves all problems with dependency management. It does not. There's no easy way around this basic fact: you have to review your dependencies.**************
************************** (Read More) ************************************

What do you think?

Leave a Reply

Your email address will not be published. Required fields are marked *

GIPHY App Key not set. Please check settings

Golden Globes 2020: 1917 and Fleabag lead British invasion with major wins – The Guardian, Theguardian.com

Golden Globes 2020: 1917 and Fleabag lead British invasion with major wins – The Guardian, Theguardian.com

XRP price rises 8%, breaks back above $0.20