Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fleet provisioning demo to get certificate and private key via CreateKeysAndCertificate #1875

Merged
merged 11 commits into from
Aug 23, 2023

Conversation

giuspen
Copy link
Contributor

@giuspen giuspen commented Aug 18, 2023

I simply created a new fleet provisioning demo that instead of using the CreateCertificatefromCsr API, uses the CreateKeysAndCertificate API, tested successfully, any feedback welcome

@giuspen giuspen requested a review from a team as a code owner August 18, 2023 07:40
@giuspen giuspen changed the title fleet provisioning demoto get certificate and private key via CreateKeysAndCertificate fleet provisioning demo to get certificate and private key via CreateKeysAndCertificate Aug 18, 2023
@archigup
Copy link
Member

Thanks for contributing this demo! Looks good to me; apologies for delay in reviewing.

@n9wxu n9wxu merged commit 0c209c0 into aws:main Aug 23, 2023
10 checks passed
@paulbartell
Copy link
Member

paulbartell commented Aug 24, 2023

@giuspen Thanks for the contribution. @n9wxu merged your PR in, but I have some concerns.

In general, use of the CreateKeysAndCertificate API is discouraged due to key material being transmitted over the wire. While communication to AWS IoT Core is typically encrypted, it's very much preferred to use the CreateCertificatefromCsr API call unless it is not possible to do. Primarily this would be due to code size restrictions or lack of a secure entropy source.

In general, it is best from a security standpoint to avoid transmitting private keys at all unless absolutely necessary even if the transmission medium is encrypted.

@giuspen
Copy link
Contributor Author

giuspen commented Aug 24, 2023

@paulbartell thanks for your comment, I have been trying to use the CreateCertificatefromCsr API demo in order to then use that downloaded certificate for the mutual auth demo without success. It is not clear after I have the certificate which private key is associated. I would expect either not being able to run the demo until I point to my already existing private key or that at the end of the demo I have the privat key and certificate pair. Could you improve that demo or help me understand how to do that?

@paulbartell
Copy link
Member

It is not clear after I have the certificate which private key is associated.

@giuspen: With the CreateCertificatefromCsr API, the associated private key is the private key which signed the Certificate Signing Request included in the request to CreateCertificatefromCsr. Often this means that the existing key is re-used with the new certificate.

Does that answer you question?

@giuspen
Copy link
Contributor Author

giuspen commented Aug 29, 2023

@paulbartell in the CreateCertificatefromCsr demo (as in the new demo CreateKeysAndCertificate) I have to create manually one claim private key and one claim certificate and pass the paths.
I have tried to use the same claim private key together with the received new certificate to run the mqtt mutual auth demo but without success. Am I doing something wrong and the claim private key is the right one or is the private key generated inside of the CreateCertificatefromCsr demo? If the second is true, could you help me to optionally save the generated private key to disk similarly to what I do in the other demo? (See DOWNLOADED_CERT_WRITE_PATH and DOWNLOADED_PRIVATE_KEY_WRITE_PATH )

@giuspen giuspen deleted the GP_fleet_provisioning_keys_cert_demo branch September 1, 2023 14:30
@giuspen
Copy link
Contributor Author

giuspen commented Sep 1, 2023

@paulbartell I'm working on https://github.com/giuspen/aws-iot-device-sdk-embedded-C/tree/GP_fleet_provisioning_with_csr_demo to add the changes to optionally write the generated private key to disk similarly to the keys and certificate demo, a little help to get it into the right format to be written would be great.

@giuspen
Copy link
Contributor Author

giuspen commented Sep 1, 2023

With the following I have written the EC PRIVATE KEY https://github.com/giuspen/aws-iot-device-sdk-embedded-C/blob/100c34120fe1e45683cf10eb7db7c27281910304/demos/fleet_provisioning/fleet_provisioning_with_csr/pkcs11_operations.c#L1157 but while the demo succeeds, then the mutual auth demo will fail using this private key and certificate (the same works instead with keys cert demo)

[INFO] [FLEET_PROVISIONING_DEMO] [fleet_provisioning_with_csr_demo.c:791] Demo completed successfully.
[INFO] [FLEET_PROVISIONING_DEMO] [fleet_provisioning_with_csr_demo.c:803] Written /opt/certs/testconn/device.pem.crt successfully.
[INFO] [FLEET_PROVISIONING_DEMO] [fleet_provisioning_with_csr_demo.c:831] Written /opt/certs/testconn/private.pem.key successfully.

$ ./bin/mqtt_demo_mutual_auth
[INFO] [DEMO] [mqtt_demo_mutual_auth.c:677] Establishing a TLS session to a26kyletilkzqt-ats.iot.eu-central-1.amazonaws.com:8883.
[ERROR] [Transport_OpenSSL_Sockets] [openssl_posix.c:843] Failed to receive data over network: SSL_read failed: ErrorStatus=DH lib.
[ERROR] [MQTT] [core_mqtt_serializer.c:2613] A single byte was not read from the transport: transportStatus=-1.
[ERROR] [MQTT] [core_mqtt.c:2424] CONNACK recv failed with status = MQTTRecvFailed.

@giuspen
Copy link
Contributor Author

giuspen commented Sep 1, 2023

(the private key saved example)
-----BEGIN EC PRIVATE KEY-----
MHcCAQEEIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAoAoGCCqGSM49
AwEHoUQDQgAEKCjc/e7XtV9H4gaB/+zYk6Q7EBG42YMTVbnGeLt6y2xd2yibRloo
pqP7DdJnVcc34VaVjs+TWW849qdwzt45/A==
-----END EC PRIVATE KEY-----

@giuspen
Copy link
Contributor Author

giuspen commented Sep 2, 2023

There is a bug in the fleet provisioning demo with CSR.

If I use directly the private key mbedtls_pk_context as generated in C_GenerateKeyPair it works fine.

diff --git a/source/portable/mbedtls/core_pkcs11_mbedtls.c b/source/portable/mbedtls/core_pkcs11_mbedtls.c
index 31354fc..cf76d96 100644
--- a/source/portable/mbedtls/core_pkcs11_mbedtls.c
+++ b/source/portable/mbedtls/core_pkcs11_mbedtls.c
@@ -5659,6 +5659,13 @@ CK_DECLARE_FUNCTION( CK_RV, C_GenerateKeyPair )( CK_SESSION_HANDLE hSession,
                         mbedtlsLowLevelCodeOrDefault( lMbedTLSResult ) ) );
             xResult = CKR_FUNCTION_FAILED;
         }
+        else
+        {
+            #define PRIV_KEY_BUFFER_LENGTH                         2048
+            char privatekey[ PRIV_KEY_BUFFER_LENGTH ];
+            int ret_val = mbedtls_pk_write_key_pem( &xCtx, privatekey, PRIV_KEY_BUFFER_LENGTH );
+            printf("\n%s\n", privatekey);
+        }
     }
 
     if( xResult == CKR_OK )

The rebuilding of the private key mbedtls_pk_context in the demo is clearly wrong

        if( mbedtlsRet == 0 )
        {
            mbedtlsRet = extractEcPublicKey( p11Session, &xEcdsaContext, privKeyHandle );
        }

        if( mbedtlsRet == 0 )
        {
            signingContext.p11Session = p11Session;
            signingContext.p11PrivateKey = privKeyHandle;

            memcpy( &privKeyInfo, header, sizeof( mbedtls_pk_info_t ) );

            privKeyInfo.sign_func = privateKeySigningCallback;
            privKey.pk_info = &privKeyInfo;
            privKey.pk_ctx = &xEcdsaContext;

            mbedtls_x509write_csr_set_key( &req, &privKey );

            mbedtlsRet = mbedtls_x509write_csr_pem( &req, ( unsigned char * ) pCsrBuffer,
                                                    csrBufferLength, &randomCallback,
                                                    &p11Session );
        }

        if( mbedtlsRet == 0 )
        {
            mbedtlsRet = mbedtls_pk_write_key_pem( &privKey, pPrivateKey, privKeyBufferLength );

            if( mbedtlsRet == 0 )
            {
                *pOutPrivKeyLength = strlen( pPrivateKey );
            }
        }

Example of difference in the keys

diff private.pem.key __private.pem.key
2c2
< MHcCAQEEIJ6nQjn+8UpEye4YBZG/gEkudbe5W0aAGu27+sHqCCNFoAoGCCqGSM49
---
> MHcCAQEEIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAoAoGCCqGSM49

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

Successfully merging this pull request may close these issues.

4 participants