SigV4_GenerateHTTPAuthorization #27

yourslab · 2021-08-02T07:07:54Z

Contains all the functionality needed for SigV4_GenerateHTTPAuthorization. Although unit tests are not included as part of this PR to make reviews easier, they have already been started separately in another PR #29

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

source/include/sigv4_internal.h

muneebahmed10 · 2021-08-03T22:17:16Z

source/include/sigv4_internal.h

+typedef struct HmacContext
+{
+    const SigV4CryptoInterface_t * pCryptoInterface;
+    char key[ SIGV4_HASH_MAX_BLOCK_LENGTH ];


Why make this a char when the crypto interface takes a uint8_t *?

Oh I thought I had replaced the crypto interface to also be char *

source/sigv4.c

muneebahmed10

Since it includes changes from #28, that should go in first and this can be rebased off of it

source/sigv4.c

Update auto-generated .md files Add test and tool directories (aws#1) Adding test and tools directories, with CMock submodule. CI Actions (aws#2) Add header files + default configurations (aws#3) - Add files sigv4.h and sigv4_config_defaults.h - Add public-facing API elements detailed in design doc Format Date Header for ISO8601 Compliance (aws#4) Add optional utility function to format date header returned from AWS IoT (ex. in temp tokens) for compliance with the ISO8601 format required for authentication Add unit tests for SigV4_AwsIotDateToIso8601() (aws#8) Setup proof infrastructure for CBMC (aws#7) * Add Litani and templates for CBMC * Add sample proof * Implement CBMC proof for SigV4_AwsIotDateToIso8601 * Unwind all loops such that no unwinding errors occur Change submodule to use https rather than ssh for aws-templates-for-cbmc-proofs (aws#12) * CBMC fix test (do not merge) * Change AWS templates to https instead of ssh * Revert README Update README.md and LICENSE files (aws#14) Update README.md and LICENSE files before changing repo's visibility status (to public). Add remaining doxygen + link verification checks (aws#15) Add doxygen + link verifier checks (the library-specific doxygen content will be added in a separate PR for further review). [SigV4] CBMC proof for Sigv4_awsIotdatetoISO8601 API (aws#19) * Sigv4_AWSIOtDateToISO8601 CBMC PROOF * Unit test coverage changes Add release workflow (+ revert to previous license) (aws#18) change permissions of run_cbmc_proofs.py (aws#21) [Sigv4] Doxygen content updates (aws#22) * doxygen doc update * lexicon update Update proof tools (+disable submodule cloning by default) (aws#20) This commit advances Litani to release 1.10.0, and the starter kit to the tip-of-tree. This brings the following improvements: - Profiling - Litani measures the memory usage of the CBMC safety checking and coverage checking jobs - The dashboard includes box-and-whisker diagrams for memory use per proof - The dashboard includes a graph of how many parallel jobs are running over the whole run, making it easy to choose a CI machine with enough parallelism - It is now possible to designate particular proofs as "EXPENSIVE"; Litani runs expensive proofs serially, ensuring that they do not over-consume resources like RAM. - UI improvements - Each pipeline page includes a table of contents - Each pipeline page includes a dependency graph of the pipeline - Each job on the pipeline page has a hyperlink to that job - The terminal output is now less noisy SigV4_GenerateHTTPAuthorization() API Functionality (aws#16) * Squash of outdated aws#13 commits * Hold for checks * Add definitions for sorting structures * Include parsing functions * Fix old commit error * Missing asserts * (temporarily allow warnings) * Spell check + include partial context * More updates to lexicon+doxygen * Add asserts for private func. * Move access after asserts * Clarify pointer increment * Update postfix syntax for correct operator precedence * Feedback changes only * + remove accidental duplicate Implement credential scope Implement generate credential query Validation of parameter count Solution a bit overcomplicated Squash bugs and canonical query parameters should also be sorted by value Finish canonicalize query Fix canonical URI encoding Add hash helper function Add hmac implementation Add newline chars for canonical request Finish writing of canonical request Hex-encoded hash of canonical request matches Write string to sign Fix bug Refactor writeStringToSign for complexity Allow HMAC keys to be passed through separate function calls Add code for generating signing key Fix hmac bug Generate the final signature correctly Fix bug Fix newline not being written Merge Shivangi's code Stylistic changes Link OpenSSL to the test Add unit tests attaining branch coverage of 71% Integrate Shivangi's latest changes Output authBufLen when complete Update logic when headers are precanonicalized. Add additional parameter checks for block/digest len Add documentation Fix test case Get complexity <= 8 for private functions Reduce complexity Remove use of %zu Revert changes to test as it was added to another PR Uncrustify and add doxygen strings. Add docs Resolve doxygen errors and lexicon.txt Document all private functions Fix remaining doxygen errors Update lexicon.txt Remove duplicate declaration Remove assertions on pQuery being NULL Add log messages for insufficient memory errors Uncrustify

source/sigv4.c

gshvang · 2021-08-09T23:42:10Z

source/sigv4.c

+    if( returnStatus == SigV4Success )
+    {
+        encodedLen = canonicalContext.bufRemaining;
+        returnStatus = completeHashAndHexEncode( pParams->pHttpParameters->pPayload,


Hashing the payload again may not be required as Application is already hashing the payload and passing it as value of x-amz-content-sha56 header. We can directly use that value. I have discussed it with @aggarw13 already.

I have created a separate branch in my fork for logic updates of the optimization. I will create a separate PR for it.

@gshvang @aggarw13 Should we add another option instead that allows the customer to choose whether to hash the payload or not?

gshvang · 2021-08-09T23:42:53Z

source/sigv4.c

+                                               char * pAuthBuf,
+                                               size_t * authBufLen,
+                                               char ** pSignature,
+                                               size_t * signatureLen )


Assertion of input parameters is missing.

This is an API function which has separate function for input validation.

source/sigv4.c

gshvang · 2021-08-10T00:42:19Z

source/include/sigv4.h

+
+    /**
+     * @brief The maximum number of query parameters was exceeded while parsing
+     * the query string input parameter.


It will be helpful to customer if we mention which config to update for changing the value for SigV4MaxQueryPairCountExceeded

source/sigv4.c

muneebahmed10 reviewed Aug 3, 2021

View reviewed changes

muneebahmed10 reviewed Aug 4, 2021

View reviewed changes

source/sigv4.c Show resolved Hide resolved

source/sigv4.c Outdated Show resolved Hide resolved

muneebahmed10 previously approved these changes Aug 5, 2021

View reviewed changes

yourslab dismissed muneebahmed10’s stale review via 8285943 August 5, 2021 09:38

gshvang reviewed Aug 5, 2021

View reviewed changes

source/sigv4.c Show resolved Hide resolved

yourslab force-pushed the generate-auth-header branch 4 times, most recently from 1c2a0ea to 72971e2 Compare August 6, 2021 15:01

amazon-auto and others added 7 commits August 9, 2021 18:30

Merge doxygen

e1d370f

Create SigV4ConstString_t type

5140fa6

Fix checks

7cb9491

Hygiene improvements in URI encoding logic

43258d6

Hygiene improvements in Authorization Header prefix value logic

bcd8e72

Minor improvements

962a2f7

yourslab force-pushed the generate-auth-header branch from 8bbce6c to 962a2f7 Compare August 9, 2021 22:41

yourslab added 2 commits August 9, 2021 18:46

Resolve compiler warning

ab889d5

Fix checks

0a81e65

aggarw13 force-pushed the generate-auth-header branch from 7ef4dc2 to 0a81e65 Compare August 9, 2021 23:23

gshvang reviewed Aug 9, 2021

View reviewed changes

aggarw13 force-pushed the generate-auth-header branch from 1a78455 to e7174b5 Compare August 10, 2021 00:30

Address review comments

613363d

aggarw13 force-pushed the generate-auth-header branch from e7174b5 to 613363d Compare August 10, 2021 00:33

gshvang reviewed Aug 10, 2021

View reviewed changes

More comment changes

2380cab

gshvang previously approved these changes Aug 10, 2021

View reviewed changes

Address stylistic comments from Muneeb

2f00c31

aggarw13 dismissed gshvang’s stale review via 2f00c31 August 10, 2021 01:20

aggarw13 force-pushed the generate-auth-header branch from f9ae9b9 to 2f00c31 Compare August 10, 2021 01:20

gshvang approved these changes Aug 10, 2021

View reviewed changes

n9wxu approved these changes Aug 10, 2021

View reviewed changes

aggarw13 merged commit a71ec2a into aws:main Aug 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SigV4_GenerateHTTPAuthorization #27

SigV4_GenerateHTTPAuthorization #27

yourslab commented Aug 2, 2021 •

edited

Loading

muneebahmed10 Aug 3, 2021

yourslab Aug 4, 2021

muneebahmed10 left a comment

gshvang Aug 9, 2021

aggarw13 Aug 9, 2021

yourslab Aug 10, 2021

gshvang Aug 9, 2021

aggarw13 Aug 10, 2021

gshvang Aug 10, 2021

SigV4_GenerateHTTPAuthorization #27

SigV4_GenerateHTTPAuthorization #27

Conversation

yourslab commented Aug 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muneebahmed10 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yourslab commented Aug 2, 2021 •

edited

Loading