readium / SDKLauncher-iOS

A small iOS application to serve as a launcher/testbed for the Readium SDK.
BSD 3-Clause "New" or "Revised" License
71 stars 47 forks source link

add security exception for ios9 non-https requests #64

Closed bluefirepatrick closed 8 years ago

bluefirepatrick commented 8 years ago

changing pull request to target develop branch rather than master

msintov commented 8 years ago

Another way to fix this is to modify the plist to whitelist the local server domain only, as follows.

    <key>NSExceptionDomains</key>
    <dict>
        <key>localhost</key>
        <dict>
            <key>NSExceptionMinimumTLSVersion</key>
            <string>TLSv1.2</string>
            <key>NSExceptionRequiresForwardSecrecy</key>
            <true/>
            <key>NSExceptionAllowsInsecureHTTPLoads</key>
            <true/>
            <key>NSIncludesSubdomains</key>
            <false/>
            <key>NSThirdPartyExceptionMinimumTLSVersion</key>
            <string>TLSv1.2</string>
            <key>NSThirdPartyExceptionRequiresForwardSecrecy</key>
            <true/>
            <key>NSThirdPartyExceptionAllowsInsecureHTTPLoads</key>
            <false/>
        </dict>
    </dict>

This suggestion comes from Apple's iOS 9 App Transport Security TechNote: https://developer.apple.com/library/prerelease/ios/technotes/App-Transport-Security-Technote/index.html

danielweck commented 8 years ago

@msintov I believe 127.0.0.1 is needed instead of locahost so that the app works when flight mode is on.

bluefirepatrick commented 8 years ago

Thank you @msintov for the note. Have you tested this fix with media like audio, video and media overlays? in both connected and 'airplane mode'? iOS has some interesting quirks with regard to serving media files to QuickTime.

msintov commented 8 years ago

Thank you so much for your responses.

On an iPad Mini with 9.0.2 installed, I tested SDKLauncher-iOS with the "localhost" change to EPubViewController.m and the plist changes described above. I set the device to be in Airplane Mode with Wi-Fi off. I loaded epub30-test-0120-20131022.epub and briefly tested the following pages:

Media Overlays Playback. I pressed the play button. It played through the whole page, automatically advancing through the items on the page and playing audio.

Short Audio Clips. Played through the whole page with audio.

Embedded Media - Video: The Embedded HTML5 video player with the helicopter toy clip played.

I did not test iOS 8 or iOS 7. Perhaps the "localhost" change should be put inside a version check such as the following:

if ([[[UIDevice currentDevice] systemVersion] compare:@"9.0" options:NSNumericSearch] != NSOrderedAscending)
{
    // use "localhost" in the rootURL
}
msintov commented 8 years ago

I also added a pull request for my change, in case we want to go this direction. Thanks for letting me comment on your pull request!

danielweck commented 8 years ago

Okay, so there are two competing PRs: https://github.com/readium/SDKLauncher-iOS/pull/65 We need to pick one :)

@msintov proposal is to configure a security domain exception specifically for localhost, which requires testing for iOS v9+ as we normally use 127.0.0.1 (apparently, Media Overlays works fine even in airplane mode)

@bluefirepatrick proposal basically removes security restrictions.

Although I like @msintov more "focused" approach, my gut feeling is that we should continue to use 127.0.0.1 (instead of localhost) as this IP address may be string-matched in other places, Readium forks, etc. and I am concerned about the potential introduction of regression bugs. @bluefirepatrick 's PR is pretty much risk-free, and preserves consistency across iOS versions.

Thoughts?

bluefirepatrick commented 8 years ago

I'll check on the plist xml and try and wrap my head around the two options too.

danielweck commented 8 years ago

Done: https://github.com/readium/SDKLauncher-iOS/commit/fe80e8172a346e53979e84428739acbe44f94020