Crashing when using altered keys.

[garygriswold]

Working with the sample code, I tried changing a valid signing key and a valid verification key by 1 character. The change in the signing key crashed the client. The change in the verification key crashed the server. The following are the errors.

On client, when signing key was altered.

Error: error:0D07209B:asn1 encoding routines:ASN1_get_object:too long
    at Error (native)
    at Sign.sign (crypto.js:327:26)
    at Object.signRequest (/Users/garygriswold/node_modules/http-signature/lib/signer.js:166:26)
    at Object.<anonymous> (/Users/garygriswold/BibleApp/Server/www/DigitalSignatureClient.js:22:15)
    at Module._compile (module.js:460:26)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Function.Module.runMain (module.js:501:10)
    at startup (node.js:129:16)

On server, when validation key was altered.

Error: PEM_read_bio_PUBKEY failed
    at Error (native)
    at Verify.verify (crypto.js:356:23)
    at Object.verifySignature (/Users/garygriswold/node_modules/http-signature/lib/verify.js:31:19)
    at Server.<anonymous> (/Users/garygriswold/BibleApp/Server/www/DigitalSignatureServer.js:14:22)
    at Server.emit (events.js:110:17)
    at HTTPParser.parserOnIncoming [as onIncoming] (_http_server.js:491:12)
    at HTTPParser.parserOnHeadersComplete (_http_common.js:111:23)
    at Socket.socketOnData (_http_server.js:343:22)
    at Socket.emit (events.js:107:17)
    at readableAddChunk (_stream_readable.js:163:16)

[arekinath]

What would you expect to happen instead? Particularly on the client, throwing an Error is the only sensible thing I can see for it to do in the face of a clearly invalid key being provided?

[garygriswold]

Alex,

If you are not willing to change it, I can always write my own wrapper that returns false in the verification method whether the input is null, invalid, or simply the wrong key. From my experience, I can’t imagine a code review coming to any other recommendation. For a user of an application, which uses this library, a client program crash needs to be avoided because it leaves a non-technical the end user with no idea of the source of the problem. While a response that they are unauthorized will at least help the end user understand who to contact to correct the problem.

If you are willing to give me one more response, I would very much like to know if your opinions on library error handling are entirely your own, or do they also reflect programming policy inside Joyent, or does Joyent have no policy on such matters?

Sincerely yours,

Gary Griswold

On Sep 10, 2015, at 1:58 PM, Alex Wilson notifications@github.com wrote:

What would you expect to happen instead? Particularly on the client, throwing an Error is the only sensible thing I can see for it to do in the face of a clearly invalid key being provided?

— Reply to this email directly or view it on GitHub https://github.com/joyent/node-http-signature/issues/46#issuecomment-139327627.

[arekinath]

If you want to know about the general Joyent views on error handling in node.js, please see https://www.joyent.com/developers/node/design/errors -- in this particular case, we have a synchronous API and this is an operational error (the function was given invalid data). The standard way to return this to the consumer of the API is by throwing an Error (see the example of what JSON.parse does).

Your application can use a try { } catch { } block to handle this case, it does not need to crash all the way back to the end user.