ApplicationInstance.CheckApplicationInstanceCertificate side effect

[jordan-gullen-nextdc]

Hi team,

I believe the implementation of ApplicationInstance.CheckApplicationInstanceCertificate should be changed.

The function has a side effect of deleting your configured certificate and creating a new one if the configured certificate was found to be invalid. This was a problem for me as the deletion and replacement consider the check to return true leaving the user completely unaware of this side effect. The other issue is the certificate that it creates mimics the subject name of the original certificate and is self-signed which makes it hard to identify this swap happened and still results in a connection problem as the certificate is not trusted.

Please update this function to only check the certificate with no side effects.

Snippet from ApplicationInstance.cs for the function in question.

        /// <summary>
        /// Checks for a valid application instance certificate.
        /// </summary>
        /// <param name="silent">if set to <c>true</c> no dialogs will be displayed.</param>
        /// <param name="minimumKeySize">Minimum size of the key.</param>
        /// <param name="lifeTimeInMonths">The lifetime in months.</param>
        public async Task<bool> CheckApplicationInstanceCertificate(
            bool silent,
            ushort minimumKeySize,
            ushort lifeTimeInMonths)
        {
            Utils.Trace(Utils.TraceMasks.Information, "Checking application instance certificate.");

            ApplicationConfiguration configuration = null;

            if (m_applicationConfiguration == null)
            {
                await LoadApplicationConfiguration(silent);
            }

            configuration = m_applicationConfiguration;
            bool certificateValid = false;

            // find the existing certificate.
            CertificateIdentifier id = configuration.SecurityConfiguration.ApplicationCertificate;

            if (id == null)
            {
                throw ServiceResultException.Create(StatusCodes.BadConfigurationError, "Configuration file does not specify a certificate.");
            }

            X509Certificate2 certificate = await id.Find(true);

            // check that it is ok.
            if (certificate != null)
            {
                certificateValid = await CheckApplicationInstanceCertificate(configuration, certificate, silent, minimumKeySize);
            }
            else
            {
                // check for missing private key.
                certificate = await id.Find(false);

                if (certificate != null)
                {
                    throw ServiceResultException.Create(StatusCodes.BadConfigurationError, "Cannot access certificate private key. Subject={0}", certificate.Subject);
                }

                // check for missing thumbprint.
                if (!String.IsNullOrEmpty(id.Thumbprint))
                {
                    if (!String.IsNullOrEmpty(id.SubjectName))
                    {
                        CertificateIdentifier id2 = new CertificateIdentifier();
                        id2.StoreType = id.StoreType;
                        id2.StorePath = id.StorePath;
                        id2.SubjectName = id.SubjectName;

                        certificate = await id2.Find(true);
                    }

                    if (certificate != null)
                    {
                        string message = Utils.Format(
                            "Thumbprint was explicitly specified in the configuration." +
                            "\r\nAnother certificate with the same subject name was found." +
                            "\r\nUse it instead?\r\n" +
                            "\r\nRequested: {0}" +
                            "\r\nFound: {1}",
                            id.SubjectName,
                            certificate.Subject);

                        throw ServiceResultException.Create(StatusCodes.BadConfigurationError, message);
                    }
                    else
                    {
                        string message = Utils.Format("Thumbprint was explicitly specified in the configuration. Cannot generate a new certificate.");
                        throw ServiceResultException.Create(StatusCodes.BadConfigurationError, message);
                    }
                }
            }

            if ((certificate == null) || !certificateValid)
            {
                certificate = await CreateApplicationInstanceCertificate(configuration,
                    minimumKeySize, lifeTimeInMonths);

                if (certificate == null)
                {
                    string message = Utils.Format(
                        "There is no cert with subject {0} in the configuration." +
                        "\r\n Please generate a cert for your application,",
                        "\r\n then copy the new cert to this location:" +
                        "\r\n{1}",
                        id.SubjectName,
                        id.StorePath);
                    throw ServiceResultException.Create(StatusCodes.BadConfigurationError, message);
                }
            }
            else
            {
                if (configuration.SecurityConfiguration.AddAppCertToTrustedStore)
                {
                    // ensure it is trusted.
                    await AddToTrustedStore(configuration, certificate);
                }
            }

            return true;
        }

[mregen]

Hi @jordan-gullen-nextdc, agreed, this is an issue especially when there was a signed certificate.

[mregen]

Imho we should never auto update an existing cert which just expired. Also never auto update a CA issued cert. User can delete the existing cert if a new is required or use tools to update it, since typically it requires also to establish trust on the connecting client/servers.

[mrsuciu]

We need to check if the certificate error is suppressible and let it continue if so and skip generating a new one.
It might be that the mechanism which deletes the "invalid" certificate kicks in and triggers this behavior.