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

Security issue reported by our security team #3304

Closed
kalyanAsadi opened this issue Dec 15, 2020 · 10 comments
Closed

Security issue reported by our security team #3304

kalyanAsadi opened this issue Dec 15, 2020 · 10 comments
Assignees
Labels
iot Issues related to the IoT SDK

Comments

@kalyanAsadi
Copy link

kalyanAsadi commented Dec 15, 2020

Use of weak hashing algorithm

  • We observed that weak hashing algorithm like MD5 and SHA1 are being used in the application. these hashing algorithms are considered weak due to collision issues.
    Usage banned / deprecated functions
  • We observed that banned/deprecated (API) like _strlen,_strncpy, _memcpy are being used in the AWS SDK.

AWSCognitoIdentityUserpool, AWSCognitoIdentityASF, AWSEXTRuntimeExtensions are using banned/deprecated functions like strlen, strncpy, memcpy.
AWSS3TransferManager uses MD5 algorithm

Environment(please complete the following information):**

  • AWS SDK Version 2.19.1
  • Dependency Manager - Cocoapods
  • Swift Version : 5.0
  • Xcode Version: 12.0.1

Is there any way to update these deprecated functions in API to updated ones.
Please let me know if anything needed from my side

Thanks & Regards
A.N.Kalyan
Sr. iOS Developer

@ruiguoamz
Copy link
Contributor

Hi, @kalyanAsadi

Thanks for reporting it.

  • We did a 2.20.0 AWS iOS SDK release last week to remove the deprecated MD5 methods: see release note. For SHA1, I am going to take a look.

  • For banned/deprecated (API) like _strlen,_strncpy, _memcpy, I am also going to take a look.

@ruiguoamz ruiguoamz added the pending-triage Issue is pending triage label Dec 15, 2020
@ruiguoamz
Copy link
Contributor

@kalyanAsadi

  • I do see apis like CC_MD5 and CC_SHA1_Init still being used, do you mean these two apis?
  • I can see that strncpy is marked as deprecated by Apple, but not for strlen and memcpy Can you provide more info or context on this one?

@ruiguoamz ruiguoamz added the pending-community-response Issue is pending response from the issue requestor label Dec 16, 2020
@kalyanAsadi
Copy link
Author

kalyanAsadi commented Dec 18, 2020 via email

@kalyanAsadi
Copy link
Author

One more thing to mention.
I tried to update AWS SDK using Cocoapod like this
pod ‘AWSCore’
pod ‘AWSAutoScaling’
pod ‘AWSCloudWatch’
pod ‘AWSDynamoDB’
pod ‘AWSEC2’
pod ‘AWSElasticLoadBalancing’
pod ‘AWSKinesis’
pod ‘AWSLambda’
pod ‘AWSMachineLearning’
pod ‘AWSMobileAnalytics’
pod ‘AWSS3’
pod ‘AWSSES’
pod ‘AWSSimpleDB’
pod ‘AWSSNS’
pod ‘AWSSQS’
pod ‘AWSCognito’

But its not getting updating to latest 2.20.0 instead its giving 2.19.1
Please check the attached screenshot.

Screenshot 2020-12-18 at 1 07 01 PM

@palpatim
Copy link
Member

@kalyanAsadi Regarding your Pod issue: You have specified your project requires the deprecated AWSMobileAnalytics and AWSCognito, which were removed in 2.20.0. CocoaPods is resolving your dependency graph to satisfy the requirements of the two removed Pods.

Please remove the AWSMobileAnalytics and AWSCognito pods, and run pod update to update to the latest versions.

@kalyanAsadi
Copy link
Author

Thanks for the answer.
Could you please take a look at _strlen,_strncpy, _memcpy

@palpatim
Copy link
Member

palpatim commented Dec 23, 2020

Thank you for reporting your concerns. We have addressed each of them below.

Weak hashing algorithms

  • MD5

  • SHA1:

    • The SDK uses SHA1 in the file AWSSignature.m. The HMACSign public method accepts a CCHmacAlgorithm parameter and supports HMAC signing using SHA1. We will maintain support for SHA1 until legacy applications complete migrations to SigV4 signing. Internally, the SDK only uses SHA256.

      This usage does not represent a security issue.

    • The SDK uses SHA1 in the file AWSSRWebSocket.m. That file is part of the web socket connection library, “SocketRocket”, which validates the Sec-WebSocket-Key header by calculating a SHA1 hash of a well-known GUID, as specified by the WebSocket RFC (https://tools.ietf.org/html/rfc6455 ).

      This usage does not represent a security issue.

    • The SDK uses SHA1 in the file AWSIoTCSR.m. The generateCSRForCertificate public method uses SHA1 to create the digest for Certificate Signing Requests. We have confirmed that, even if the client uses SHA1 to sign the Certificate Signing Request, the IoT service generates a certificate signed with an appropriate strong hashing algorithm.

      This usage does not represent a security issue. However, SHA256 provides a stronger algorithm for signing the request, so we are currently replacing this method’s use of SHA1 with SHA256 to sign Certificate Signing Requests. We will update this issue when we have posted the pull request.

C API usage
You reported uses of three different C APIs that you noted as “banned/deprecated”: strlen, strncpy, and memcpy. We have investigated each of these usages and found that they are being used appropriately and are protected with appropriate validity checks in all cases.

These usages do not represent a security issue.

@palpatim
Copy link
Member

We've posted #3345 to update the IoT digest algorithm to SHA256

@palpatim palpatim added iot Issues related to the IoT SDK pending-release Code has been merged but pending release and removed pending-triage Issue is pending triage pending-community-response Issue is pending response from the issue requestor labels Dec 23, 2020
@palpatim
Copy link
Member

We have released this patch on https://github.com/aws-amplify/aws-sdk-ios/releases/tag/2.21.1. Please let us know if you have any further questions.

@palpatim palpatim added closing-soon-if-no-response and removed pending-release Code has been merged but pending release labels Dec 24, 2020
@stale
Copy link

stale bot commented Jan 3, 2021

This issue has been automatically closed because of inactivity. Please open a new issue if are still encountering problems.

@stale stale bot closed this as completed Jan 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iot Issues related to the IoT SDK
Projects
None yet
Development

No branches or pull requests

4 participants