[CM-22] Authority Key Identifier support #6

sandeepmistry · 2018-07-24T20:26:07Z

As requested in #4. @mattiabertorello this needs some testing.

I believe after this is merged, all existing provisioned devices will be invalid.

…ic cert

sandeepmistry · 2018-07-24T20:26:52Z

src/ArduinoCloud.cpp

+  const byte authorityKeyIdentifier[20] = {
+    0xb2, 0xed, 0xef, 0xed, 0x3b, 0xbf, 0xc7, 0x71, 0x75, 0x24, 0x33, 0xd1, 0xae, 0x8b, 0x54, 0xed, 0x97, 0x14, 0x7a, 0x1d
+  };
+


@mattiabertorello I've hardcoded the value in the lib. for now.

Will this value be constant?

The value of the authorityKeyIdentifier must be saved in the crypto chip and initialized during the provisioning sketch, so it can change because you can sign the csr with different ca.
For example we will have a CA for dev and another CA for prod

But to support the actual flow during the provisioning sketch if the authorityKeyIdentifier field is empty, no error should be throw.

Thanks

A tip
If you put the issue key in the pull request you will link the PR with the issue.

I've made changes in
b264428 to persist this value in the crypto chip and prompt for in during provisioning sketch.

…visioning sketch

…nto one slot

AlbyIanna · 2018-08-22T15:22:36Z

examples/utility/Provisioning/Provisioning.ino

  int inLength = in.length();
+  in.toUpperCase();


authorityKeyIdentifier wasn't being capitalized. To prevent that from happening in the future, I added this line and remove the others about serialNumber and signature.

AlbyIanna · 2018-08-22T15:24:27Z

examples/utility/Provisioning/Provisioning.ino

@@ -190,6 +188,6 @@ void hexStringToBytes(const String& in, byte out[], int length) {
    byte highByte = (highChar <= '9') ? (highChar - '0') : (highChar + 10 - 'A');
    byte lowByte = (lowChar <= '9') ? (lowChar - '0') : (lowChar + 10 - 'A');

-    out[outLength++] = (highByte << 4) | lowByte;
+    out[outLength++] = (highByte << 4) | (lowByte & 0xF);


this mask will prevent lowByte to overflow

AlbyIanna · 2018-08-22T16:25:33Z

@sandeepmistry There is a problem with the reconstruction of the certificate. Martino and I resolved it parly in the last commit, but still the signature in the reconstructed certificate is slightly different from the original. It seems that some bytes are being added at the beginning of the signature and some in the middle. For example, this is the certificate i tried to store on the board:

Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            4f:bc:e6:33:00:3b:89:82:68:b3:37:fb:a7:ce:f0:2e
    Signature Algorithm: ecdsa-with-SHA256
        Issuer: C=US, O=Arduino LLC US, OU=IT, CN=Arduino
        Validity
            Not Before: Aug 22 16:00:00 2018 GMT
            Not After : Aug 22 16:00:00 2049 GMT
        Subject: CN=b338d4e1-6e4d-4dc0-b417-208662b16aea
        Subject Public Key Info:
            Public Key Algorithm: id-ecPublicKey
                Public-Key: (256 bit)
                pub: 
                    04:6d:d1:d6:6d:8e:ab:b8:b4:6b:d8:a0:b3:83:95:
                    61:a1:ca:ce:a7:7d:7b:41:8e:5c:ee:b0:09:db:ac:
                    f9:b4:3b:f5:0e:17:c1:e4:6f:4c:b2:91:f0:36:a7:
                    11:6c:be:8e:4c:7e:b6:3e:9e:5a:a5:79:f3:fa:ef:
                    4a:26:4c:4a:45
                ASN1 OID: prime256v1
                NIST CURVE: P-256
        X509v3 extensions:
            X509v3 Authority Key Identifier: 
                keyid:5B:3E:2A:6B:8E:C9:B0:1A:A8:54:E6:36:9B:8C:09:F9:FC:E1:B9:80

    Signature Algorithm: ecdsa-with-SHA256
         30:46:02:21:00:97:79:a0:c7:01:f3:89:84:c1:e5:f4:c7:fa:
         c1:69:be:3a:b2:d4:e8:76:42:3a:6f:cb:15:6e:29:a1:f7:5b:
         b1:02:21:00:af:20:94:2b:d6:7f:b7:db:81:73:74:86:08:6f:
         c1:73:b9:1d:92:ed:a2:b0:dd:84:63:bc:11:68:32:ec:5f:0e

and this is the certificate reconstructed by the sketch:

Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            4f:bc:e6:33:00:3b:89:82:68:b3:37:fb:a7:ce:f0:2e
    Signature Algorithm: ecdsa-with-SHA256
        Issuer: C=US, O=Arduino LLC US, OU=IT, CN=Arduino
        Validity
            Not Before: Aug 22 16:00:00 2018 GMT
            Not After : Aug 22 16:00:00 2049 GMT
        Subject: CN=b338d4e1-6e4d-4dc0-b417-208662b16aea
        Subject Public Key Info:
            Public Key Algorithm: id-ecPublicKey
                Public-Key: (256 bit)
                pub: 
                    04:6d:d1:d6:6d:8e:ab:b8:b4:6b:d8:a0:b3:83:95:
                    61:a1:ca:ce:a7:7d:7b:41:8e:5c:ee:b0:09:db:ac:
                    f9:b4:3b:f5:0e:17:c1:e4:6f:4c:b2:91:f0:36:a7:
                    11:6c:be:8e:4c:7e:b6:3e:9e:5a:a5:79:f3:fa:ef:
                    4a:26:4c:4a:45
                ASN1 OID: prime256v1
                NIST CURVE: P-256
        X509v3 extensions:
            X509v3 Authority Key Identifier: 
                keyid:5B:3E:2A:6B:8E:C9:B0:1A:A8:54:E6:36:9B:8C:09:F9:FC:E1:B9:80

    Signature Algorithm: ecdsa-with-SHA256
         30:44:02:20:30:46:02:21:00:97:79:a0:c7:01:f3:89:84:c1:
         e5:f4:c7:fa:c1:69:be:3a:b2:d4:e8:76:42:3a:6f:cb:15:6e:
         02:20:29:a1:f7:5b:b1:02:21:00:af:20:94:2b:d6:7f:b7:db:
         81:73:74:86:08:6f:c1:73:b9:1d:92:ed:a2:b0:dd:84

If you compare them (https://text-compare.com/) you'll see the differences in the signature.

AlbyIanna · 2018-08-27T15:00:53Z

@sandeepmistry Apparently the problem lies in the format of the signature we send to the sketch, because it includes ASN1 metadata about the bitstream. For now, I'm just adding these lines to the Provisioning sketch to remove the unwanted bytes.

if (signature.length() == 128) {
  // do nothing, already in binary format
} else if (signature.length() == 144) {
  // ASN.1 format with a sequence (0x30 0x46) of 2 integers of len 33 (0x02 0x21)
  signature = signature.substring(10, 10+64) + signature.substring(80, 80+64);
} else {
  // ASN.1 format with a sequence (0x30 0x46) of 2 integers of len 32 (0x02 0x20)
  signature = signature.substring(8, 8+64) + signature.substring(76, 76+64);
}

This allows to store the certificate correctly, but i'm not sure this is the right solution. Anyway, whatever is the solution, I guess it would be better to implement it in the library itself instead of in the sketch.
(cc: @cmaglie)

sandeepmistry · 2018-08-27T15:02:01Z

@AlbyIanna can we get the API side fixed? This was working before right?

mattiabertorello · 2018-08-27T15:40:14Z

var data struct {
	X, Y *big.Int
}
_, err = asn1.Unmarshal(cert.Signature, &data)
if err != nil {
	panic(errors.Wrapf(err, "unmarshal asn1"))
}

res := app.ArduinoCompressedv1{
	Serial:    fmt.Sprintf("%032x", cert.SerialNumber),
	NotBefore: cert.NotBefore,
	NotAfter:  cert.NotAfter,
	Signature: fmt.Sprintf("%064x%064x", data.X, data.Y),
}

I found out in version v1 of iot-api the code that split the signature in two big.Int the signature but in v2 this part of code is absent

res.Compressed = &app.ArduinoCompressedv2{
	Serial:    fmt.Sprintf("%x", cert.SerialNumber),
	NotBefore: cert.NotBefore,
	NotAfter:  cert.NotAfter,
	Signature: fmt.Sprintf("%x", cert.Signature),
}

So the problem It caused by the missing lines.
Like @AlbyIanna I think the better solution is to integrate the code the remove the byte in the library or at least we can leave it in the sketch base.

This is because if a user want to run the provisioning sketch he can do it only with openssl command line tool and copy the signature from the output of the tool

sandeepmistry · 2018-08-27T15:42:28Z

This is because if a user want to run the provisioning sketch he can do it only with openssl command line tool and copy the signature from the output of the tool

I disagree still :)

If the goal is to use with Arduino Cloud, not many of our user base will use openssl cli.

mattiabertorello · 2018-08-27T15:58:04Z

That's a good point but we lost a lot of days for that only to understand what was the problem.
Also It could be better for us to testing the libraries or to create a custom environment without the api only with command line tools.
We should also improve the resilient of the provisioning sketch for example if there is a bad input the sketch should fail.
Thanks

sandeepmistry · 2018-08-27T17:05:54Z

I'm up for adding validation in the sketch.

However, the real concern is why the API's on the server side changed so much once we had a spec we agreed to and was stable.

mattiabertorello · 2018-08-28T13:04:15Z

Ok we decide that for the moment we will restore the same behavior of the V1 API but with different name. You can follow the change on this issue https://arduino.atlassian.net/browse/CM-146

AlbyIanna · 2018-08-28T16:54:01Z

Now it seems to me that everything is working fine. @sandeepmistry can we merge this?

sandeepmistry added 2 commits July 24, 2018 16:22

Add ability to add Authority Key Identifier to reconstructed SSL publ…

b67e678

…ic cert

Set Authority Key Identifier when re-constructing cert

3f4ab80

sandeepmistry assigned mattiabertorello Jul 24, 2018

sandeepmistry commented Jul 24, 2018

View reviewed changes

mattiabertorello changed the title ~~Authority Key Identifier support~~ [CM-22] Authority Key Identifier support Jul 25, 2018

sandeepmistry and others added 3 commits July 25, 2018 18:17

Use ECCX08 slot to store Authority Key Identifier, and request in pro…

b264428

…visioning sketch

Combine certificate serial number and authority key identifier data i…

8d3eaeb

…nto one slot

fix hex to byte conversion

156fc10

AlbyIanna reviewed Aug 22, 2018

View reviewed changes

sandeepmistry merged commit 6cf3249 into master Aug 28, 2018

sandeepmistry deleted the auth-key-identifier branch August 28, 2018 17:30

sandeepmistry mentioned this pull request Aug 28, 2018

Missing feature of tls authentication authorityKeyIdentifier #4

Closed

cmaglie unassigned mattiabertorello Feb 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[CM-22] Authority Key Identifier support #6

[CM-22] Authority Key Identifier support #6

Uh oh!

sandeepmistry commented Jul 24, 2018

Uh oh!

sandeepmistry Jul 24, 2018

Uh oh!

mattiabertorello Jul 25, 2018 •

edited

Loading

Uh oh!

sandeepmistry Jul 25, 2018

Uh oh!

AlbyIanna Aug 22, 2018

Uh oh!

AlbyIanna Aug 22, 2018

Uh oh!

AlbyIanna commented Aug 22, 2018

Uh oh!

AlbyIanna commented Aug 27, 2018

Uh oh!

sandeepmistry commented Aug 27, 2018

Uh oh!

mattiabertorello commented Aug 27, 2018

Uh oh!

sandeepmistry commented Aug 27, 2018

Uh oh!

mattiabertorello commented Aug 27, 2018

Uh oh!

sandeepmistry commented Aug 27, 2018

Uh oh!

mattiabertorello commented Aug 28, 2018

Uh oh!

AlbyIanna commented Aug 28, 2018

Uh oh!

Uh oh!

[CM-22] Authority Key Identifier support #6

[CM-22] Authority Key Identifier support #6

Uh oh!

Conversation

sandeepmistry commented Jul 24, 2018

Uh oh!

sandeepmistry Jul 24, 2018

Choose a reason for hiding this comment

Uh oh!

mattiabertorello Jul 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandeepmistry Jul 25, 2018

Choose a reason for hiding this comment

Uh oh!

AlbyIanna Aug 22, 2018

Choose a reason for hiding this comment

Uh oh!

AlbyIanna Aug 22, 2018

Choose a reason for hiding this comment

Uh oh!

AlbyIanna commented Aug 22, 2018

Uh oh!

AlbyIanna commented Aug 27, 2018

Uh oh!

sandeepmistry commented Aug 27, 2018

Uh oh!

mattiabertorello commented Aug 27, 2018

Uh oh!

sandeepmistry commented Aug 27, 2018

Uh oh!

mattiabertorello commented Aug 27, 2018

Uh oh!

sandeepmistry commented Aug 27, 2018

Uh oh!

mattiabertorello commented Aug 28, 2018

Uh oh!

AlbyIanna commented Aug 28, 2018

Uh oh!

Uh oh!

mattiabertorello Jul 25, 2018 •

edited

Loading