Closed thomas-fossati closed 1 year ago
when we instantiate a Verifier <...> This can potentially widen the DoS surface of a go-cose application
The implementer is passing a public key and currently it is on them to make sure it is a correct one. Nothing prevents the implementer from making the appropriate checks before using their own key. Furthermore, it is a reasonable expectation to be provided with a valid argument to begin with.
It is interesting though that Go went ahead with a panic
and not the error
in such a case.
Furthermore, it is a reasonable expectation to be provided with a valid argument to begin with.
I don't know you, but for an old-school derelict like myself input validation has always been no. 1 rule of secure coding practices 😄
It is interesting though that Go went ahead with a
panic
and not theerror
in such a case.
I agree with you that was a pretty interesting choice -- which makes upfront validation without crashing the caller even more valuable.
input validation has always been no. 1 rule
But you get that key from somewhere, are you saying you do not verify it before using in the library?
[...] are you saying you do not verify it before using in the library?
at the moment we just check that the underlying type is as expected (see https://github.com/veraison/go-cose/blob/4dbb9a7434cb75b3cbab3a24076002e8b1ee38a5/verifier.go#L45). 064ff82 adds the "on curve" test.
at the moment we just check that the underlying type is as expected
^^ My question was more about the production code that gets the key from somewhere and passes it to this library for a verification.
Otherwise, I am just trying to understand if it is a good idea to add this curve check to the library, because:
IsOnCurve(x, y)
check already because otherwise there might be a panic as the library does not do that check for them.Marshal, MarshalCompressed, Add, Double, ScalarMult
that would result in a panic. None of these methods are used here it seems
Currently, when we instantiate a Verifier we do not check that the supplied public key EC point is on the supposed curve.
This is not best practice. See for example BCP225 in the JWS/JWT context:
It'd also result in a
panic
when the go-cose application is built against go1.19. See go1.19 release notes:This can potentially widen the DoS surface of a go-cose application. Users should be shielded from such situation.