Hi Imesh,

Please see inline.

On Tue, Oct 20, 2015 at 11:08 PM, Imesh Gunaratne <im...@wso2.com> wrote:

> On Tue, Oct 20, 2015 at 10:25 PM, Akila Ravihansa Perera <
> raviha...@wso2.com> wrote:
>>
>>
>> I think the proper way to secure the Jaggery services is by using SSO.
>>
>
> I tend to disagree on this statement. SSO is used when authenticating a
> human to a series of software systems. An API should not use SSO for
> authentication rather it should use session based authentication either by
> creating session tokens or API Keys, refer this [3].
>

SSO can be used to authenticate the user who is accessing the
monitoring/metering dashboard, thereby authenticating him to the API as
well. We don't need to maintain a session token/cookie for this which is an
overhead for this scenario, IMO.


>
>
>> According to the thread on wso2dev@ with subject "SingleSignOn support
>> in DAS Analytics Dashboard" this is not yet supported in DAS. The approach
>> taken in analytics.jsg as you mentioned require a separate login screen as
>> in [1]. IMHO, this is not a suitable method to secure a Jaggery based API.
>>
>> No, have a look at the analytics.jag authentication logic. It first
> accepts an Authorization header and creates a session token. Authorization
> header can accept basic auth, see [4]. Afterwards corresponding calls are
> authenticated using authToken/JSESSIONID.
>

If we are using basic auth, the user needs to submit the credentials via a
form, which is again an overhead. Any implementation of
authToken/JSESSIONID would require the user to submit credential at some
layer. What I'm suggesting is to authenticate the user via SSO to the DAS
dashboard, which would authenticate him to the API as well. I'm -1 on
developing a separate login screen for this.


>
>
>> Regarding table names in SQL queries; this is not the best approach to
>> design the API but these table names are escaped from request parameters
>> [2] which would minimize the risk of a SQL injection attack. This is
>> definitely a potential security issue as well as an API design issue we
>> need to fix. But I think fixing this will need a major refactoring to the
>> Jaggery files. wdyt?
>>
>
> No, we can simply fix this by creating an API per table/entity.
>

This would not be manageable as we add entities in future I think. We can
handle this internally by not using the request parameter variable to
create the query. But rather choose the correct sql query conditionally
based on the request parameter. This would basically result in rewriting
all Jaggery files.

Thanks.



>
>> [1]
>> https://github.com/wso2/carbon-dashboards/blob/master/apps/portal/theme/templates/login.jag
>> [2]
>> https://github.com/wso2/product-private-paas/blob/v4.1.0-rc1/extensions/das/artifacts/metering-dashboard/jaggery-files/member-info.jag#L26
>>
>> [3] https://www.owasp.org/index.php/REST_Security_Cheat_Sheet
> [4]
> https://github.com/wso2/carbon-analytics/blob/d7d4f7c31981eb6aff8921fefba7c030eb11a80a/components/analytics-io/org.wso2.carbon.analytics.jsservice/src/main/java/org/wso2/carbon/analytics/jsservice/Utils.java#L352
>
> Thanks
>
> On Tue, Oct 20, 2015 at 10:25 PM, Akila Ravihansa Perera <
> raviha...@wso2.com> wrote:
>
>> Hi Imesh,
>>
>> I think the proper way to secure the Jaggery services is by using SSO.
>> According to the thread on wso2dev@ with subject "SingleSignOn support
>> in DAS Analytics Dashboard" this is not yet supported in DAS. The approach
>> taken in analytics.jsg as you mentioned require a separate login screen as
>> in [1]. IMHO, this is not a suitable method to secure a Jaggery based API.
>>
>> Regarding table names in SQL queries; this is not the best approach to
>> design the API but these table names are escaped from request parameters
>> [2] which would minimize the risk of a SQL injection attack. This is
>> definitely a potential security issue as well as an API design issue we
>> need to fix. But I think fixing this will need a major refactoring to the
>> Jaggery files. wdyt?
>>
>> [1]
>> https://github.com/wso2/carbon-dashboards/blob/master/apps/portal/theme/templates/login.jag
>> [2]
>> https://github.com/wso2/product-private-paas/blob/v4.1.0-rc1/extensions/das/artifacts/metering-dashboard/jaggery-files/member-info.jag#L26
>>
>> Thanks.
>>
>> On Tue, Oct 20, 2015 at 8:38 PM, Imesh Gunaratne <im...@wso2.com> wrote:
>>
>>> Hi,
>>>
>>> It looks like there are two security issues in the APIs exposed by the
>>> DAS metering and monitoring dashboards [1], [2]:
>>>
>>>    - APIs have no authentication mechanism
>>>    - Table name is concatenated in the SQL queries
>>>
>>> We may need to add an authentication check similar to analytics.jag [3]:
>>>
>>> var authParam = request.getHeader(AUTHORIZATION_HEADER);
>>>     if (authParam != null) {
>>>         credentials = JSUtils.authenticate(authParam);
>>>         authenticationAdminStub = new
>>> AuthenticationAdminStub(authenticationWSUrl);
>>>         authenticationAdminStub.login(credentials[0], credentials[1],
>>> LOCALHOST);
>>>         var serviceContext =
>>> authenticationAdminStub._getServiceClient().getLastOperationContext()
>>>                 .getServiceContext();
>>>         var sessionCookie =
>>> serviceContext.getProperty(HTTPConstants.COOKIE_STRING);
>>>         options.setProperty(HTTPConstants.COOKIE_STRING, sessionCookie);
>>>     } else {
>>>         var token = session.get(AUTH_TOKEN);
>>>         if (token != null) {
>>>             options.setProperty(HTTPConstants.COOKIE_STRING, token);
>>>         } else {
>>>             log.error("user is not authenticated!");
>>>             response.status = HTTP_USER_NOT_AUTHENTICATED;
>>>             print('{ "status": "Failed", "message": "User is not
>>> authenticated." }');
>>>             return;
>>>         }
>>>     }
>>>
>>> In addition we may need to avoid concatenating table names in SQL
>>> queries.
>>>
>>> [1]
>>> https://github.com/wso2/product-private-paas/tree/v4.1.0-rc1/extensions/das/artifacts/metering-dashboard/jaggery-files
>>> [2]
>>> https://github.com/wso2/product-private-paas/tree/v4.1.0-rc1/extensions/das/artifacts/monitoring-dashboard/jaggery-files
>>> [3]
>>> https://github.com/wso2/carbon-dashboards/blob/master/apps/portal/controllers/apis/analytics.jag#L88
>>>
>>> I think we may need to cancel this vote and do RC2 by fixing these
>>> problems.
>>>
>>> Thanks
>>>
>>> On Tue, Oct 20, 2015 at 5:02 PM, Akila Ravihansa Perera <
>>> raviha...@wso2.com> wrote:
>>>
>>>> Hi,
>>>>
>>>> This is the first release candidate of WSO2 Private PaaS 4.1.0.
>>>>
>>>> This release fixes the following issues:
>>>> https://wso2.org/jira/issues/?filter=12464
>>>>
>>>> Please download, test and vote. The vote will be open for 72 hours or
>>>> as needed.
>>>>
>>>> *​Source and binary distribution files:*
>>>> https://svn.wso2.org/repos/wso2/scratch/PPAAS/wso2ppaas-4.1.0-rc1
>>>>
>>>> *Maven staging repository:*
>>>> http://maven.wso2.org/nexus/content/repositories/orgwso2ppaas-027/
>>>>
>>>> *The tag to be voted upon:*
>>>> https://github.com/wso2/product-private-paas/tree/v4.1.0-rc1
>>>>
>>>>
>>>> [ ] Broken - do not release (explain why)
>>>> [ ] Stable - go ahead and release
>>>>
>>>>
>>>> Thanks,
>>>> The WSO2 Private PaaS Team
>>>>
>>>> _______________________________________________
>>>> Dev mailing list
>>>> Dev@wso2.org
>>>> http://wso2.org/cgi-bin/mailman/listinfo/dev
>>>>
>>>>
>>>
>>>
>>> --
>>> *Imesh Gunaratne*
>>> Senior Technical Lead
>>> WSO2 Inc: http://wso2.com
>>> T: +94 11 214 5345 M: +94 77 374 2057
>>> W: http://imesh.gunaratne.org
>>> Lean . Enterprise . Middleware
>>>
>>>
>>
>>
>> --
>> Akila Ravihansa Perera
>> WSO2 Inc.;  http://wso2.com/
>>
>> Blog: http://ravihansa3000.blogspot.com
>>
>
>
>
> --
> *Imesh Gunaratne*
> Senior Technical Lead
> WSO2 Inc: http://wso2.com
> T: +94 11 214 5345 M: +94 77 374 2057
> W: http://imesh.gunaratne.org
> Lean . Enterprise . Middleware
>
>


-- 
Akila Ravihansa Perera
WSO2 Inc.;  http://wso2.com/

Blog: http://ravihansa3000.blogspot.com
_______________________________________________
Dev mailing list
Dev@wso2.org
http://wso2.org/cgi-bin/mailman/listinfo/dev

Reply via email to