Skip to content
This repository has been archived by the owner on Feb 25, 2019. It is now read-only.

Inconsistent signature length on ECDSA #68

Open
dav1app opened this issue Mar 9, 2018 · 5 comments
Open

Inconsistent signature length on ECDSA #68

dav1app opened this issue Mar 9, 2018 · 5 comments

Comments

@dav1app
Copy link

dav1app commented Mar 9, 2018

Hey,

I am building an hybrid application that shares the same code on both server and client sides. Same JS files, same modules. After some hours trying to figure out why I couldn't use WebCrypto -> Export ECDSA -> Send to the server -> Check the signature, I discovered that the signature has an inconsistent byte length and aways start with the very same bytes. This is the buf2hex read of it:

image

I found this pretty weird and almost sure that this should not happen. I run the same code on the browser to check if this behavior is normal. Here the results:

image

As I said before, this is the same code running. You can notice the difference. The code is the following. The "octano" module is just the layer to make the code runs on the browser and server. It keeps the same parameters as the original functions.

function test(){
  var publicKeyExported = {
    "crv": "P-256",
    "ext": true,
    "key_ops": [
      "verify"
    ],
    "kty": "EC",
    "x": "oVlNnOyWWwcIfhd73uMLrldbAy2YMRtuTjIY1Xz-I1o",
    "y": "RLMChUc4EOuWcjJcFr2knwxVsIiaTtLPsNRMFLK0ku4"
  }

  var privateKeyExported = {
    "crv": "P-256",
    "d": "VNLMIqrU9iBgPZIkxVrtIsB4wL6-lRE0e0SRNm0LeVY",
    "ext": true,
    "key_ops": [
      "sign"
    ],
    "kty": "EC",
    "x": "oVlNnOyWWwcIfhd73uMLrldbAy2YMRtuTjIY1Xz-I1o",
    "y": "RLMChUc4EOuWcjJcFr2knwxVsIiaTtLPsNRMFLK0ku4"
  }

  var algoDefine = {
    "name": "ECDSA",
    "namedCurve": "P-256"
  }

  var signAlgo = {name: "ECDSA", hash: {name: "SHA-256"}}

  var dataToSign = 'test'


  return Promise.all([
    octano.util.importKey("jwk", privateKeyExported, algoDefine , true , ['sign']),
    octano.util.importKey("jwk", publicKeyExported, algoDefine , true , ['verify'])
  ])
  .then(x => {
    return octano.util.signData(signAlgo, x[0], octano.util.textEncoder(dataToSign))
    .then(x=>{return octano.util.buf2hex(x)})
  })
  .catch(e => {console.log(e)})
}


test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})
test().then(x => { console.log(x)}).catch(e => {console.log(e)})

INB4: Not the font, I use monospace.
IINB4: Not a problem on the buf2hex, I've tested it in another projects.

@dav1app dav1app changed the title Inconsistent signature lenght on ECDSA Inconsistent signature lentgh on ECDSA Mar 9, 2018
@dav1app dav1app changed the title Inconsistent signature lentgh on ECDSA Inconsistent signature length on ECDSA Mar 9, 2018
@thelunararmy
Copy link
Contributor

This appears to be the same problem as outlined in #22 , we are looking into it.

@thelunararmy
Copy link
Contributor

Main problem we identified was node's crypto library uses openssl to sign data, which results DER encoded openssl signatures which intrinsically contains metadata. What we will need to do is deconstruct the DER signature, extract the signature value, pad it properly, and return it.

@amark
Copy link

amark commented Aug 28, 2018

@thelunararmy @EternalDeiwos @davimello28 also related to #74 that could probably be merged into this thread (or #22 ).

Any timeline on when this can be fixed? (In all honesty, of course, I know we're all busy)

@amark
Copy link

amark commented Aug 28, 2018

OSSL might have a nifty function for this already, here ConvertWebCryptoSignatureToDerSignature at https://github.com/PeculiarVentures/node-webcrypto-ossl/blob/master/src/ec/ec_dsa.cpp#L175 ? Is this theoretically what is needed? Or not related?

@rmhrisk
Copy link

rmhrisk commented Feb 24, 2019

As a FYI, we have created a purse TS Webcrypto polyfill also - https://github.com/PeculiarVentures/webcrypto this removes the C++ build-time dependencies. It is not full featured enough to replace our current node-webcrypto-ossl for all use cases due to dependency limitations with this approach but it may be useful for those who are encountering the problem discussed here in this bug.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants