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.

Sign up for GitHub

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

Jump to bottom

Fix policy parser to accept multiple IP address per statement [JIRA: RCS-222] #1178

Merged
merged 1 commit into from
Jul 31, 2015
Merged

Fix policy parser to accept multiple IP address per statement [JIRA: RCS-222] #1178

merged 1 commit into from
Jul 31, 2015

Conversation

kuenishi
Copy link
Contributor

@kuenishi kuenishi commented Jul 3, 2015

Following style of IP address description in IPAddress Condition have
not been working as parser implicitly assumed single IP address per
'aws:SourceIp' condition.

"IpAddress": {
    "aws:SourceIp": ["127.0.0.1/32", "192.168.100.0/24"]
}

@kuenishi kuenishi added the Bug label Jul 3, 2015
@shino
Copy link
Contributor

shino commented Jul 9, 2015

Did some manual tests, good ones and not good.

With the purpose of scenario of allowing all GET access to certain bucket
from any users with specific IP ranges, set bucket policy by [1].

Good ones: After PUTting an object by the bucket owner, both anonymous
user and another CS user can GET the object 🎊

On the other hand, an error happens when PUT is called by CS user
who is not the bucket owner. The expected results is 403 Access Denied.

command:

% s3cmd -c .s3cfg.15018.bob put rebar.config s3://ip-filter/a

error log

15:09:27.399 [error] Webmachine error at path "/buckets/ip-filter/location" : {error,{badmatch,{context,{1436,422167,395828},false,{rcs_user_v2,"bob","bob","bob@example.com","BZJLA0GJPPHEDJJPKQPD","3BlCQiOQqFPjn-6TgQEm83TaIELBWyaLtecwEA==","14a1569d8323b671401fd66bfaa0e65e015fbe29ea7fc4735ef68d602485e075",[],enabled},{riakc_obj,<<"moss.users">>,<<"BZJLA0GJPPHEDJJPKQPD">>,<<107,206,97,96,96,96,204,96,202,5,82,60,71,131,54,114,183,94,157,161,192,192,146,115,41,131,41,145,49,143,149,33,178,83,232,60,95,22,0>>,[{{dict,3,16,16,8,80,48,{[],[],[],[],[],[],[],[],[],[],[],[],...},...},...}],...},...}},...} in riak_cs_wm_utils:handle_policy_eval_result/5 line 734

debug log (extraced) of s3cmd

DEBUG: Sending request method_string='GET', uri='http://ip-filter.s3.amazonaws.com/?location', headers={'Authorization': 'AWS BZJLA0GJPPHEDJJPKQPD:bMJ/S2LpXFe4qj8KTRoNlsMDlY4=', 'x-amz-date': 'Thu, 09 Jul 2015 06:15:16 +0000'}, body=(0 bytes)
DEBUG: Response: {'status': 500, 'headers': {'date': 'Thu, 09 Jul 2015 06:15:16 GMT', 'content-length': '170', 'content-type': 'text/html', 'server': 'Riak CS'}, 'reason': 'Internal Server Error', 'data': '<html><head><title>500 Internal Server Error</title></head><body><h1>Internal Server Error</h1>The server encountered an error while processing this request</body></html>'}
DEBUG: ConnMan.put(): closing proxy connection (keep-alive not yet supported)
DEBUG: S3Error: 500 (Internal Server Error)
DEBUG: HttpHeader: date: Thu, 09 Jul 2015 06:15:16 GMT
DEBUG: HttpHeader: content-length: 170
DEBUG: HttpHeader: content-type: text/html
DEBUG: HttpHeader: server: Riak CS
DEBUG: Unicodising 'Malformed error XML returned from remote server.' using UTF-8
DEBUG: DeUnicodising u'Malformed error XML returned from remote server.' using UTF-8
ERROR: Error parsing xml: Malformed error XML returned from remote server..  ErrorXML: <html><head><title>500 Internal Server Error</title></head><body><h1>Internal Server Error</h1>The server encountered an error while processing this request</body></html>
WARNING: Retrying failed request: /?location
WARNING: 500 (Internal Server Error):

[1] https://gist.github.com/shino/fce9b4311afe6f54abad

@Basho-JIRA Basho-JIRA changed the title Fix policy parser to accept multiple IP address per statement Fix policy parser to accept multiple IP address per statement [JIRA: RCS-222] Jul 10, 2015
@kuenishi kuenishi added this to the 2.1.0 milestone Jul 10, 2015
@kuenishi kuenishi force-pushed the bugfix/multiple-ip-in-policy branch from fd6e2a7 to 9045aca Compare July 30, 2015 12:51
@kuenishi
Copy link
Contributor Author

Single-line-without-any-test fix added.

@kuenishi kuenishi force-pushed the bugfix/multiple-ip-in-policy branch from 48151cb to 1d805af Compare July 30, 2015 13:41
@shino
Copy link
Contributor

shino commented Jul 31, 2015

The above 500 error when putting object by other users were introduced
by the commit [1]. It changes User in function arguments to User0
and assigns canonical ID to User binding. To call handle_policy_call_result(),
it should use User0 instead of User.

Then, the 500 error is not introduced by this PR, I recommend to remove
"fix" commit [3] and defer the actual fix. Sorry for pointing out the bug
out side of this PR's scope.

[1] 61180ba?w=1
[2] 61180ba?w=1#diff-caff706fcdff3c29cf3b9beaea88d1faR484
[3] 9045aca

@kuenishi kuenishi force-pushed the bugfix/multiple-ip-in-policy branch from 1d805af to 25ce996 Compare July 31, 2015 02:20
@kuenishi
Copy link
Contributor Author

Ah, sorry, I already included the fix, squashed, and force-pushed.

@kuenishi
Fix policy parser to accept multiple IP address per statement
0ec4bee 
Following style of IP address description in IPAddress Condition have
not been working as parser implicitly assumed single IP address per
'aws:SourceIp' condition.

```
"IpAddress": {
    "aws:SourceIp": ["127.0.0.1/32", "192.168.100.0/24"]
}
```
@kuenishi kuenishi force-pushed the bugfix/multiple-ip-in-policy branch from 25ce996 to 0ec4bee Compare July 31, 2015 02:25
borshop added a commit that referenced this pull request Jul 31, 2015
@borshop
Merge pull request #1178 from basho/bugfix/multiple-ip-in-policy
25ee8a6 
Fix policy parser to accept multiple IP address per statement  [JIRA: RCS-222]

Reviewed-by: shino
borshop added a commit that referenced this pull request Jul 31, 2015
@borshop
Merge pull request #1178 from basho/bugfix/multiple-ip-in-policy
f1e5b14 
Fix policy parser to accept multiple IP address per statement  [JIRA: RCS-222]

Reviewed-by: shino
@kuenishi
Copy link
Contributor Author

@borshop merge

@borshop borshop merged commit 0ec4bee into develop Jul 31, 2015
@kuenishi kuenishi deleted the bugfix/multiple-ip-in-policy branch July 31, 2015 08:48
@kuenishi
Copy link
Contributor Author

For release note:

Multiple IP address description under single condition statement of a bucket policy was not being properly parsed as lists.

@shino shino mentioned this pull request Aug 3, 2015
Closed
@kuenishi kuenishi mentioned this pull request Aug 3, 2015
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@kuenishi kuenishi

Labels
Bug JIRA: Closed
Projects
None yet
Milestone
2.1.0
Development

Successfully merging this pull request may close these issues.
4 participants
@kuenishi @shino @borshop @Basho-JIRA