convertersystems / opc-ua-client

Visualize and control your enterprise using OPC Unified Architecture (OPC UA) and Visual Studio.
MIT License
403 stars 119 forks source link

Bug: Expired server certificates are accepted #212

Closed szalai-balazs-david closed 3 years ago

szalai-balazs-david commented 3 years ago

If the OPC UA Server has an expired certificate, no exception is thrown and a connection is established.

(Tested on Windows 10 client and B&R PLC server, where the server certificate was expired 2 days before the connection was made. Other clients showed the expected behavior of failing to connect.)

As an easy solution, you could add an additional if check in the beginning of DirectoryStore.ValidateRemoteCertificateAsync():

        public Task<bool> ValidateRemoteCertificateAsync(X509Certificate target, ILogger? logger = null)
        {
            if (AcceptAllRemoteCertificates)
            {
                return Task.FromResult(true);
            }

            if (target == null)
            {
                throw new ArgumentNullException(nameof(target));
            }

            // New if check
            if (!target.IsValidNow)
            {
                throw new Org.BouncyCastle.Security.Certificates.CertificateExpiredException();
            }
            ...

Alternatively, instead of throwing an exception, you could just return FALSE, but then the caller doesn't know the exact reason of failing the validation.

awcullen commented 3 years ago

I updated the method as you suggested. I'll make a release soon. Thanks for reporting this issue!

szalai-balazs-david commented 3 years ago

That was quick! Thank you!