-
-
Notifications
You must be signed in to change notification settings - Fork 6
Implement payload checksum generation and validation #174
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
Changes from all commits
7a811a2
9bba7e5
63704da
d7383be
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ component singleton { | |
|
|
||
| // Inject module settings | ||
| property name="moduleSettings" inject="coldbox:modulesettings:cbwire"; | ||
|
|
||
| // Inject module service | ||
| property name="moduleService" inject="coldbox:moduleService"; | ||
|
|
||
|
|
@@ -43,8 +43,8 @@ component singleton { | |
| ._withKey( arguments.key ); | ||
|
|
||
| // If the component is lazy loaded, we need to generate an x-intersect snapshot of the component | ||
| return arguments.lazy ? | ||
| local.instance._generateXIntersectLazyLoadSnapshot( params=arguments.params ) : | ||
| return arguments.lazy ? | ||
| local.instance._generateXIntersectLazyLoadSnapshot( params=arguments.params ) : | ||
| local.instance._render(); | ||
| } | ||
|
|
||
|
|
@@ -53,7 +53,7 @@ component singleton { | |
| * | ||
| * @incomingRequest The JSON struct payload of the incoming request. | ||
| * @event The event object. | ||
| * | ||
| * | ||
| * @return A struct representing the response with updated component details or an error message. | ||
| */ | ||
| function handleRequest( incomingRequest, event ) { | ||
|
|
@@ -73,6 +73,7 @@ component singleton { | |
| } | ||
| // Perform additional deserialization of the component snapshots | ||
| local.payload.components = local.payload.components.map( function( _comp ) { | ||
| _validateChecksum( arguments._comp.snapshot ); | ||
| arguments._comp.snapshot = deserializeJSON( arguments._comp.snapshot ); | ||
| return arguments._comp; | ||
| } ); | ||
|
|
@@ -118,13 +119,45 @@ component singleton { | |
| return local.componentsResult; | ||
| } | ||
|
|
||
| /** | ||
| * Calculates a checksum for the component's payload, inserts the checksum into the payload, | ||
| * and returns the updated payload as a JSON string. | ||
| * | ||
| * @payload struct | the payload to calculate the checksum for | ||
| * | ||
| * @return string | ||
| */ | ||
| function _caclulateChecksum( snapshot ) { | ||
| var secret = moduleSettings.keyExists("secret") ? moduleSettings.secret : hash( moduleSettings.moduleRootPath ); | ||
| var serializedSnapshot = serializeJson( arguments.snapshot ); | ||
| var checksum = hmac( serializedSnapshot, secret, "HMACSHA256"); | ||
| return replace( serializedSnapshot, '"checksum":""', '"checksum":"#checksum#"', "all" ) | ||
| } | ||
|
|
||
| /** | ||
| * Validates checksum for the component's data from snapshot. | ||
| * | ||
| * @payload string | the JSON string of the component snapshot as posted by livewire | ||
| * | ||
| * @return void | ||
| */ | ||
| function _validateChecksum( snapshot ) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Change method to accept snapshot as a struct so we can avoid regex parsing
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Livewire passes the payload as a JSON string. The crux of the issue is that we can't use deserialize the payload, remove the checksum key, then serialize the struct and expect to get the same JSON string that was originally used to generate the checksum like livewire does. PHP and JS both keep struct/object ordering, but CFML doesn't and deserializeJSON() doesn't have an option to produce an ordered struct. I can avoid the regex completely by deserializing the payload, getting the checksum value, using replace on the original JSON string payload to clear out the value of the checksum, then verifying the checksum. Note: I changed the payload to always include and empty string for the checksum before generating the checksum. I hope I am clearly explaining the issue. I will try to get this all cleaned up in the next little bit so you can take a look at the changes. |
||
| if( !isJson( snapshot ) ) throw( type="CBWIRECorruptPayloadException", message="Payload is not valid JSON." ); | ||
| var deserializedSnapshot = deserializeJSON( arguments.snapshot ); | ||
| if( !deserializedSnapshot.keyExists("checksum") ) throw( type="CBWIRECorruptPayloadException", message="Checksum Not Found." ); | ||
| var secret = moduleSettings.keyExists("secret") ? moduleSettings.secret : hash( moduleSettings.moduleRootPath ); | ||
| if( deserializedSnapshot.checksum != hmac( replace( snapshot, deserializedSnapshot.checksum, "", "one" ), secret, "HMACSHA256") ){ | ||
| throw( type="CBWIRECorruptPayloadException", message="Checksum Mismatch." ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Uploads all files from the request to the specified destination | ||
| * after verifying the signed URL. | ||
| * | ||
| * | ||
| * @incomingRequest The JSON struct payload of the incoming request. | ||
| * @event The event object. | ||
| * | ||
| * | ||
| * @return A struct representing the response with updated component details or an error message. | ||
| */ | ||
| function handleFileUpload( incomingRequest, event ) { | ||
|
|
@@ -158,10 +191,10 @@ component singleton { | |
|
|
||
| /** | ||
| * Handles the preview of a file by reading the file metadata and sending it back to the client. | ||
| * | ||
| * | ||
| * @incomingRequest The JSON struct payload of the incoming request. | ||
| * @event The event object. | ||
| * | ||
| * | ||
| * @return file contents | ||
| */ | ||
| function handleFilePreview( incomingRequest, event ){ | ||
|
|
@@ -191,18 +224,18 @@ component singleton { | |
| * @componentName The name of the component to instantiate, possibly including a namespace. | ||
| * @params Optional parameters to pass to the component constructor. | ||
| * @key Optional key to use when retrieving the component from WireBox. | ||
| * | ||
| * | ||
| * @return The instantiated component object. | ||
| * @throws ApplicationException If the component cannot be found or instantiated. | ||
| */ | ||
| function createInstance( name ) { | ||
| // Determine if the component name traverses a valid namespace or directory structure | ||
| local.fullComponentPath = arguments.name; | ||
|
|
||
| if ( !local.fullComponentPath contains "wires." ) { | ||
| local.fullComponentPath = "wires." & local.fullComponentPath; | ||
| } | ||
|
|
||
| if ( find( "@", local.fullComponentPath ) ) { | ||
| // This is a module reference, find in our module | ||
| var params = listToArray( local.fullComponentPath, "@" ); | ||
|
|
@@ -243,9 +276,9 @@ component singleton { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| /** | ||
| * Returns the path to the modules folder. | ||
| * | ||
| * | ||
| * @module string | The name of the module. | ||
| * | ||
| * @return string | ||
|
|
@@ -286,8 +319,8 @@ component singleton { | |
| * Returns the path to the wires folder within a module path. | ||
| * | ||
| * @module string | The name of the module. | ||
| * | ||
| * @return string | ||
| * | ||
| * @return string | ||
| */ | ||
| function getModuleWiresPath( module ) { | ||
| local.moduleRegistry = moduleService.getModuleRegistry(); | ||
|
|
@@ -296,7 +329,7 @@ component singleton { | |
|
|
||
| /** | ||
| * Returns the ColdBox RequestContext object. | ||
| * | ||
| * | ||
| * @return The ColdBox RequestContext object. | ||
| */ | ||
| function getEvent(){ | ||
|
|
@@ -305,7 +338,7 @@ component singleton { | |
|
|
||
| /** | ||
| * Returns any request assets defined by components during the request. | ||
| * | ||
| * | ||
| * @return struct | ||
| */ | ||
| function getRequestAssets() { | ||
|
|
@@ -316,7 +349,7 @@ component singleton { | |
|
|
||
| /** | ||
| * Returns the ColdBox ConfigSettings object. | ||
| * | ||
| * | ||
| * @return struct | ||
| */ | ||
| function getConfigSettings(){ | ||
|
|
@@ -325,15 +358,15 @@ component singleton { | |
|
|
||
| /** | ||
| * Returns an array of preprocessor instances. | ||
| * | ||
| * | ||
| * @return An array of preprocessor instances. | ||
| */ | ||
| function getPreprocessors(){ | ||
| // Check if we've already scanned the folder | ||
| if( structKeyExists( variables, "preprocessors" ) ){ | ||
| return variables.preprocessors; | ||
| } | ||
| // List of preprocesssors here. Had to hard code instead of using | ||
| // List of preprocesssors here. Had to hard code instead of using | ||
| // directoryList because of filesystem differences in various OSes | ||
| local.files = [ | ||
| "TemplatePreprocessor.cfc", | ||
|
|
@@ -351,18 +384,18 @@ component singleton { | |
|
|
||
| /** | ||
| * Returns CSS styling needed by Livewire. | ||
| * | ||
| * | ||
| * @return string | ||
| */ | ||
| function getStyles( cache=true ) { | ||
| if (structKeyExists(variables, "styles") && arguments.cache ) { | ||
| return variables.styles; | ||
| } | ||
|
|
||
| savecontent variable="local.html" { | ||
| include "styles.cfm"; | ||
| } | ||
|
|
||
| variables.styles = local.html; | ||
| return variables.styles; | ||
| } | ||
|
|
@@ -372,10 +405,10 @@ component singleton { | |
| * We don't cache the results like we do with | ||
| * styles because we need to generate a unique | ||
| * CSRF token for each request. | ||
| * | ||
| * | ||
| * @return string | ||
| */ | ||
| function getScripts() { | ||
| function getScripts() { | ||
| savecontent variable="local.html" { | ||
| include "scripts.cfm"; | ||
| } | ||
|
|
@@ -384,7 +417,7 @@ component singleton { | |
|
|
||
| /** | ||
| * Returns HTML to persist the state of anything inside the call. | ||
| * | ||
| * | ||
| * @return string | ||
| */ | ||
| function persist( name ) { | ||
|
|
@@ -393,7 +426,7 @@ component singleton { | |
|
|
||
| /** | ||
| * Ends the persistence of the state of anything inside the call. | ||
| * | ||
| * | ||
| * @return string | ||
| */ | ||
| function endPersist() { | ||
|
|
@@ -402,10 +435,10 @@ component singleton { | |
|
|
||
| /** | ||
| * Generates a secure signature for the upload URL. | ||
| * | ||
| * | ||
| * @baseURL string | The base URL for the upload request. | ||
| * @expires string | The expiration time for the request. | ||
| * | ||
| * | ||
| * @return string | ||
| */ | ||
| function generateSignature(baseUrl, expires) { | ||
|
|
@@ -419,7 +452,7 @@ component singleton { | |
|
|
||
| /** | ||
| * Generates a CSRF token for the current request. | ||
| * | ||
| * | ||
| * @return string | ||
| */ | ||
| function generateCSRFToken() { | ||
|
|
@@ -429,7 +462,7 @@ component singleton { | |
|
|
||
| /** | ||
| * Returns the base URL for incoming requests. | ||
| * | ||
| * | ||
| * @return string | ||
| */ | ||
| function getBaseURL() { | ||
|
|
@@ -457,7 +490,7 @@ component singleton { | |
|
|
||
| /** | ||
| * Verifies signed upload URL. | ||
| * | ||
| * | ||
| * @return boolean | ||
| */ | ||
| function verifySignedUploadURL( expires, signature ) { | ||
|
|
@@ -523,11 +556,11 @@ component singleton { | |
|
|
||
| /** | ||
| * Returns the URI endpoint for updating CBWIRE components. | ||
| * | ||
| * | ||
| * @return string | ||
| */ | ||
| function getUpdateEndpoint() { | ||
| var settings = variables.moduleSettings; | ||
| var settings = variables.moduleSettings; | ||
| return settings.keyExists( "updateEndpoint") && settings.updateEndpoint.len() ? settings.updateEndpoint : "/cbwire/update"; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to change this up a bit. You are passing in the snapshot as a string into
_validateChecksumbut we are already deserializing it below this line into an object. I would move this to after the deserialization call so that we don't have to use regex methods ( which are slow ) for parsing the checksum.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I was too caught up with the issue of unordered structs in CF as opposed to JS and PHP, thus causing potentially different JSON strings after serializeJSON/deserializeJSON that I didn't even think about deserializing to get checksum value, doing a replace on the original payload string to remove the checksum and then verifying... I will get that fixed up shortly.