SDWebImage / SDWebImageSwiftUI

SwiftUI Image loading and Animation framework powered by SDWebImage
https://sdwebimage.github.io/SDWebImageSwiftUI
MIT License
2.16k stars 223 forks source link

TLS Client Certificates encountered error 1:89 #153

Open sdavidliu opened 3 years ago

sdavidliu commented 3 years ago

New Issue Checklist

Issue Info

Info Value
Platform Name ios
Platform Version 14.3
SDWebImage Version 5.10.2
SDWebImageSwiftUI Version 1.5.0
Integration Method Swift Package
Xcode Version Xcode 12.3
Repro rate all the time (100%)
Repro with our demo prj
Demo project link

Issue Description and Steps

I have a SwiftUI app using WebImage and I'm trying to load an image from one of our urls. The image required an Authorization header, so I set the header on app startup: SDWebImageDownloader.shared.setValue("token", forHTTPHeaderField: "Authorization") Added a couple breakpoints in SDWebImageDownloader and verified the Authorization header is added in the request. However, I am getting this error message every time:

Connection 1: TLS Client Certificates encountered error 1:89
Connection 1: encountered error(1:89)

These are some of my findings:

LeoMatallana commented 3 years ago

I'm having the same problem when loading images using Mutual TLS (mTLS)

dreampiggy commented 3 years ago

Sounds really strange.

SDWebImage use URLSession by default (which will be the same as your SwiftUI's native URLRequest, actually). Does something config effect this behavior ?

Any demo here to debug ?

LeoMatallana commented 3 years ago

@dreampiggy In my case, I kind of found a workaround. mTLS was asking for this authenticationMethod: NSURLAuthenticationMethodClientCertificate

I just modified the URLSession method to allow this type of auth method:

  if ([challenge.protectionSpace.authenticationMethod isEqualToString:NSURLAuthenticationMethodServerTrust]) {
        if (!(self.options & SDWebImageDownloaderAllowInvalidSSLCertificates)) {
            disposition = NSURLSessionAuthChallengePerformDefaultHandling;
        } else {
            credential = [NSURLCredential credentialForTrust:challenge.protectionSpace.serverTrust];
            disposition = NSURLSessionAuthChallengeUseCredential;
        }
    } 

     else if ([challenge.protectionSpace.authenticationMethod isEqualToString:NSURLAuthenticationMethodClientCertificate]) {
        if (!(self.options & SDWebImageDownloaderAllowInvalidSSLCertificates)) {
            disposition = NSURLSessionAuthChallengePerformDefaultHandling;
        } else {
            credential = [NSURLCredential credentialForTrust:challenge.protectionSpace.serverTrust];
            disposition = NSURLSessionAuthChallengeUseCredential;
        }
    }

    else {
        if (challenge.previousFailureCount == 0) {
            if (self.credential) {
                credential = self.credential;
                disposition = NSURLSessionAuthChallengeUseCredential;
            } else {
                disposition = NSURLSessionAuthChallengeCancelAuthenticationChallenge;
            }
        } else {
            disposition = NSURLSessionAuthChallengeCancelAuthenticationChallenge;
        }
    }

I'm not really sure if that's the way it should behave but is working for me right now.

I'm using this library through react native FastImage library, but I can't leave a demo to debug because the URLs that are being affected by the problem have to be accessed with a vpn. But the problem was encountered when mTLS was enabled.

I made a clean react native project to see if any of my project settings were affecting the requests, but I got the same outcome.

dreampiggy commented 3 years ago

Seems OK for your modification. If you want, I can merge that into SDWebImage's master branch and release new versions.

Actually, that delegate method can be customized by yourself using subclassing SDWebImageDownloaderOperation and override the method. Then using SDWebImageDownloaderConfig.operationClass to setup. But if this is common use case, merge into master is better.

LeoMatallana commented 3 years ago

Sorry for the really long reply. I'm not really sure if this is a common case or if this is really the way to do it. I've been using a fork with the change in the meantime.

mdelete commented 3 years ago

The reason for this issue is that HTTPS servers can check for TLS clients certs which are optional (e.g. nginx: ssl_verify_client optional;) but SDWebImageDownloaderOperation cancels the operation if there is no client cert. Correct behaviour would be to present a client cert if you have a matching one or try to proceed as if nothing happened. If the client cert was indeed optional the server will connect normally, if it was mandatory the server will close the connection.

I think changing SDWebImageDownloaderOperation.m:568 from: disposition = NSURLSessionAuthChallengeCancelAuthenticationChallenge; to: disposition = NSURLSessionAuthChallengePerformDefaultHandling; would fix this.

dreampiggy commented 3 years ago

@mdelete Sounds reasonable. Can you submit a PR ? Or I create one for fixing this ?