Merge pull request #636 from merlin-northern/men_7523__mender_artifac…

…t_signature_72_case_asn1_encoded_signatures_do_not_work_

fix: hardware-security returned signatures support for ECDSA256.

.gitignore

@@ -2,7 +2,6 @@
 *.o
 *.a
 *.so
-*.mender
 
 # Folders
 _obj

.gitlab-ci.yml

@@ -107,6 +107,7 @@ test:smoketests:mac:
     - ./mender-artifact-darwin write rootfs-image -t test -o test-rfs.mender -n test -f test.txt
     - ./mender-artifact-darwin read test-rfs.mender
     - ./mender-artifact-darwin validate test-rfs.mender
+    - ./mender-artifact-darwin validate --key tests/data/ec.pem tests/data/a0-signed-nitro.mender
     - ./tests/test_compressions/test_supported_compressions.sh
     - make build
   tags:
@@ -135,6 +136,7 @@ test:smoketests:linux:
     - ./mender-artifact-linux write rootfs-image -t test -o test-rfs.mender -n test -f test.txt
     - ./mender-artifact-linux read test-rfs.mender
     - ./mender-artifact-linux validate test-rfs.mender
+    - ./mender-artifact-linux validate --key tests/data/ec.pem tests/data/a0-signed-nitro.mender
     - test $(./mender-artifact-linux read --no-progress test-rfs.mender | yq eval -o json | jq -r '."Mender Artifact".Name') == "test"
     - ./tests/test_compressions/test_supported_compressions.sh
     # QA-507: lock mender-artifact to OpenSSL 1.1

artifact/signer.go

@@ -20,6 +20,7 @@ import (
 	"crypto/rand"
 	"crypto/rsa"
 	"crypto/x509"
+	"encoding/asn1"
 	"encoding/base64"
 	"encoding/pem"
 	"math/big"
@@ -77,6 +78,7 @@ func (r *RSA) Verify(message, sig []byte, key interface{}) error {
 // ECDSA Crypto interface implementation
 const ecdsa256curveBits = 256
 const ecdsa256keySize = 32
+const ecdsa256Asn1SizeBytes = 72
 
 type ECDSA256 struct{}
 
@@ -158,11 +160,56 @@ func MarshalECDSASignature(r, s *big.Int) ([]byte, error) {
 }
 
 func UnmarshalECDSASignature(sig []byte) (r, s *big.Int, e error) {
-	// check if the size of the key matches provided one
-	if len(sig) != 2*ecdsa256keySize {
-		return nil, nil, errors.Errorf("signer: invalid ecdsa key size: %d", len(sig))
+	// if the length of the signature is the double of key size, we know (and assume)
+	// that it was us who created it via MarshalECDSASignature.
+	// After the introduction of the hardware security support there is another possibility
+	// (see below)
+	if len(sig) == 2*ecdsa256keySize {
+		return UnmarshalECDSASignatureMenderArtifact(sig)
 	}
 
+	// in case of a key supplied via PKCS#11 URI, we have no control over what the signature is
+	// since it is designed to be actually verified via the same mechanism (PKCS#11 URI).
+	// We know here that it is ECDSA key, and judging form the size we can assume
+	// that it is ASN.1 encoded. If so, then it should be 72 bytes, the less or equal is here
+	// to support keys of size 71 bytes (strangely seen in the wild). In other words:
+	// if the signature has not been created with MarshalECDSASignature, then we assume
+	// it is to be decoded via ASN.1, with the protection on the signature length.
+	if len(sig) == ecdsa256Asn1SizeBytes ||
+		len(sig) == ecdsa256Asn1SizeBytes-1 {
+		return UnmarshalECDSASignatureASN1(sig)
+	}
+
+	return nil, nil, errors.Errorf("signer: invalid ecdsa key size: %d", len(sig))
+}
+
+func UnmarshalECDSASignatureASN1(sig []byte) (r *big.Int, s *big.Int, err error) {
+	var value asn1.RawValue
+	_, err = asn1.Unmarshal(sig, &value)
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "cannot unmarshal asn1 data")
+	}
+	bytesValue := value.Bytes
+	if len(bytesValue) > 0 {
+		var v asn1.RawValue
+		bytesValue, err = asn1.Unmarshal(bytesValue, &v)
+		if err != nil {
+			return nil, nil, errors.Wrap(err, "cannot unmarshal asn1 r value")
+		}
+		r = big.NewInt(0).SetBytes(v.Bytes)
+	}
+	if len(bytesValue) > 0 {
+		var v asn1.RawValue
+		_, err = asn1.Unmarshal(bytesValue, &v)
+		if err != nil {
+			return nil, nil, errors.Wrap(err, "cannot unmarshal asn1 s value")
+		}
+		s = big.NewInt(0).SetBytes(v.Bytes)
+	}
+	return r, s, nil
+}
+
+func UnmarshalECDSASignatureMenderArtifact(sig []byte) (r *big.Int, s *big.Int, err error) {
 	// get the signature; see corresponding `Sign` function for more details
 	// about serialization
 	r = big.NewInt(0).SetBytes(sig[:ecdsa256keySize])

artifact/signer_test.go

@@ -259,5 +259,10 @@ func TestECDSARaw(t *testing.T) {
 	// use wrong size key for verification
 	err = r.Verify([]byte("my message"), []byte("signature"), crypt.Key)
 	assert.Error(t, err)
+	assert.Contains(t, errors.Cause(err).Error(), "signer: invalid ecdsa key size:")
+
+	// use wrong size key for verification
+	err = r.Verify([]byte("my message"), []byte("signature longer than the 72 bytes for 256 means that we definitely are having something we do not recognize in any way, which should happen here, ever since MEN-7523."), crypt.Key)
+	assert.Error(t, err)
 	assert.Contains(t, errors.Cause(err).Error(), "invalid ecdsa key size")
 }

tests/data/README.md

@@ -0,0 +1,53 @@
+# tests data files
+
+## ECDSA256 Nitro HSM case
+
+### generate the keys in the HSM
+
+```sh
+pkcs11-tool --module /usr/lib/arm-linux-gnueabihf/opensc-pkcs11.so -l --pin xx --keypairgen --key-type EC:prime256v1 --id xx
+```
+
+save the public key:
+
+```sh
+pkcs11-tool  --read-object --type pubkey --id xx -o pub.key
+```
+
+save in PEM format:
+
+```sh
+openssl pkey -pubin -in pub.key -outform PEM -out ec.pem
+```
+
+### generate artifact and sign
+
+configure OpenSSL, add to the config:
+
+```
+[openssl_init]
+engines=engine_section
+
+[engine_section]
+pkcs11 = pkcs11_section
+
+[pkcs11_section]
+engine_id = pkcs11
+MODULE_PATH = /usr/lib/x86_64-linux-gnu/pkcs11/opensc-pkcs11.so
+init = 0
+```
+
+get the PKCS#11 URI:
+
+```sh
+pkcs11-tool --login --pin xx --list-objects
+p11tool --login --set-pin xx --list-all-privkeys 'URL you got from above'
+```
+
+sign:
+
+```sh
+mender-artifact sign --key-pkcs11 "${key}" /tmp/a0.mender -o /tmp/a0-signed-nitro.mender
+# where key is the PKCS#11 URI to your private key
+```
+

tests/data/ec.pem

@@ -0,0 +1,4 @@
+-----BEGIN PUBLIC KEY-----
+MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEDTjFpi9+tJ8KnTPQff34Gpj4E4uc
+T4ML17IzkcWZhtY1O44fqMHmejS0umai8xJlD0Z/THhTiuRO8PC3vpkU9g==
+-----END PUBLIC KEY-----