microsoftconnect / ms-intune-app-sdk-ios

Intune App SDK for iOS enables data protection and mobile app management features in iOS mobile apps with Microsoft Intune
91 stars 27 forks source link

App Security findings in IntuneMAM SDK #38

Closed Nookaraju closed 3 years ago

Nookaraju commented 5 years ago

Describe the bug: When our app(linked with IntuneMAM SDK) is scanned by a security tool for app security findings, there are some issues that were from Intune MAM SDK.

To Reproduce List of issues: *1. MD5 usage Issue details:

The following code locations used the deprecated the CC_MD functions to generate a message digest:

[CUTCategories md5] calls _CC_MD5()
[CMARCryptoUtils md5Hash:] calls _CC_MD5()

2. Switch from NSCoding to NSSecureCoding

NSCoding protocol used in binary <>.app/Frameworks/IntuneMAM.framework/IntuneMAM

Description: NSSecureCoding should be used.

(FIXED FROM ADAL 4.0) 3. Switch from Deprecated UIWebView API to WKWebView --

-[ADAuthenticationViewController loadView:] calls -[_OBJC_CLASS_$_UIWebView initWithFrame:]

(ADAL Lib) 4. Null Initialization Vector Used --

The following code locations used the CommonCrypto APIs to perform AES encryption with a NULL Initialization Vector:

-[ADBrokerKeyHelper decryptBrokerResponse:key:size:error:] calls _CCCrypt(_, _, _, _, _, NULL, _)

5. Broken Cryptography API (SHA1) used

The following code locations used CC_SHA1 functions to generate a message digest:

-[ADPkeyAuthHelper computeThumbprint:isSha2:] calls _CC_SHA1() .      _(**ADAL Lib**)_
-[MSIDPkeyAuthHelper computeThumbprint:isSha2:] calls _CC_SHA1()
-[MSIDExtensions msidSHA1] calls _CC_SHA1()

Expected behavior: IntuneMAM framework should not have app security issues

Intune App SDK for iOS (please complete the following information):

Additional context: Add any other context about the problem here.

Nookaraju commented 5 years ago

Any update on this?

Nookaraju commented 5 years ago

@Kyle-Reis These security issues are holding our app release, can you please take look and give us update?

Kyle-Reis commented 5 years ago

Hey @Nookaraju, we’re investigating items 1 and 2 on the list. The remaining items all appear to be coming from ADAL, so you should file an issue here.

The MD5 hashes we generate are meant to obfuscate things like user’s UPN and URL endpoints in our logs, and aren’t used for encrypting actual corporate data. We’ll look into whether there is another lightweight obfuscation method available.

We only use the NSCoding protocol to serialize/deserialize to the iOS keychain, which is protected by the OS and only accessible to apps signed by the same publisher. We will look into moving to NSSecureCoding for an added layer of protection.

I don’t think you should consider either of the items above blockers to release, but if you feel differently please let us know :)

Nookaraju commented 5 years ago

@Kyle-Reis Thanks for your reply. I have created separate issue for ADAL(link)

Just wanted to know, when we can expect the fixes for first 2 issues from Intune MAM SDK.

Thank you.

Nookaraju commented 5 years ago

Added new item 6. App Uses Deprecated API to Authenticate Through a Web Service

Kyle-Reis commented 5 years ago

Hey @Nookaraju, the changes to address items 1 and 2 are now being reviewed. Item 6 looks like it is also in ADAL, and not in Intune.

Nookaraju commented 5 years ago

@Kyle-Reis Thanks for your quick reply. I have moved item 6 to ADAL lib.

Nookaraju commented 5 years ago

@Kyle-Reis Item 3 is very important now as Apple will stop accepting the app submission if we use UIWebView.

Check this out, got it from Apple. 11055304-a7ac-4c40-886f-5b1b0b2085c4

Kyle-Reis commented 5 years ago

Hey @Nookaraju, item 3 is an ADAL 2 issue. However, the latest version of our SDK supports ADAL 4, so I would try upgrading Intune to the latest and upgrading to ADAL 4 as I believe they've moved from UIWebView to WKWebView in that release. Note: the current 11.1.2 release of our Swift framework does have a known issue with building for simulator (see #47). We've already addressed that issue and will make the fix available soon, but you should be able to build and test on physical devices without issue.

Nookaraju commented 5 years ago

@Kyle-Reis Upgrading to ADAL 4 fixed the UIWebView issue.

But Security Tool found one more issue:

The following code locations within the App use Security.framework functions to generate cryptographic key pairs: -[CMAREncryptionProvider generateKeysWithPublicKeyTag:PrivateKeyTag:AccessGroup:] calls _SecKeyGeneratePair()

Please refer to Apple's official documentation at https://developer.apple.com/documentation/security/certificate_key_and_trust_services/keys/storing_keys_in_the_secure_enclave for more details.

Kyle-Reis commented 5 years ago

Hello @Nookaraju, Items 1 and 2 have been addressed in our latest release(11.2.0). Note: the tool will still pick up on MD5 function calls as we needed to preserve backwards compat. Once adoption of newer SDKs has reached critical mass, we will drop support for MD5.

As for the Secure Enclave, we did some investigation into using it some time ago, and found that it would not suit our needs, as encrypted data needs to be shared between applications of different publishers, which the Secure Enclave does not currently support.

abasore commented 3 years ago

Closing this issue due to inactivity. If issue persists in the newest SDK version, feel free to open a new issue.