Hi Gayan, The code reference you pointed at [1] is used in a prepared SQL statement [2]. The risk of SQL injection attack is minimal in that case, AFAIU.
[1] https://github.com/wso2/product-private-paas/blob/v4.1.0-rc1/extensions/das/artifacts/metering-dashboard/jaggery-files/member-info.jag#L49 [2] https://github.com/wso2/product-private-paas/blob/v4.1.0-rc1/extensions/das/artifacts/metering-dashboard/jaggery-files/member-info.jag#L53 Thanks. On Tue, Oct 20, 2015 at 11:33 PM, Akila Ravihansa Perera <raviha...@wso2.com > wrote: > 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 > -- 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