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

Read certificates as PEM strings or from files #283

Merged
merged 3 commits into from
Dec 12, 2023

Conversation

mtmk
Copy link
Collaborator

@mtmk mtmk commented Dec 12, 2023

No description provided.

@mtmk mtmk requested a review from caleblloyd December 12, 2023 11:02
@mtmk mtmk linked an issue Dec 12, 2023 that may be closed by this pull request
@caleblloyd
Copy link
Contributor

caleblloyd commented Dec 12, 2023

What do you think about offering a couple of callbacks instead? Then some static methods to help configure the callbacks for common cases? I can see many more requests in this area on the horizon - PFX certificates, private keys with passwords, etc

public sealed record NatsTlsOpts {
    public Func<ValueTask<X509Certificate2>>? LoadClientCert { get; init; }

    public Func<ValueTask<X509Certificate2Collection>>? LoadCaCerts { get; init; }

    public static Func<ValueTask<X509Certificate2>> LoadClientCertFromPemString(string certPem, string keyPem)
    {
        var clientCert = X509Certificate2.CreateFromPem(certPem, keyPem);
        return () => ValueTask.FromResult(clientCert);
    }

    public static Func<ValueTask<X509Certificate2Collection>> LoadCaCertsFromPemString(string caPem)
    {
        var caCerts = new X509Certificate2Collection();
        caCerts.ImportFromPem(caPem);
        return () => ValueTask.FromResult(caCerts);
    }
}

Note that this is not a full replacement for LocalCertificateSelectionCallback and RemoteCertificateValidationCallback - it's just a way to load other X509 Certificates and use them with our default LocalCertificateSelectionCallback and RemoteCertificateValidationCallback implementations. I can also see being asked to provide overrides for LocalCertificateSelectionCallback and RemoteCertificateValidationCallback in the future, but of course we can address that later.

@mtmk
Copy link
Collaborator Author

mtmk commented Dec 12, 2023

@caleblloyd why don't you just hack it into the branch? Not sure exactly what you mean.

@caleblloyd
Copy link
Contributor

Sure, will open a PR to this one

caleblloyd and others added 2 commits December 12, 2023 19:09
Signed-off-by: Caleb Lloyd <caleb@synadia.com>
Signed-off-by: Caleb Lloyd <caleb@synadia.com>
Copy link
Contributor

@caleblloyd caleblloyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mtmk mtmk merged commit da25199 into main Dec 12, 2023
9 checks passed
@mtmk mtmk deleted the 281-allow-base64-encoded-certificates-instead-of-paths branch December 12, 2023 20:34
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.

Allow base64 encoded certificates instead of paths
2 participants