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.

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

Server Info API #626

Closed
abitmore opened this issue Feb 2, 2018 · 49 comments · Fixed by #2617
Closed

Server Info API #626

abitmore opened this issue Feb 2, 2018 · 49 comments · Fixed by #2617
Labels
1b User Story The User Story details a requirement. It may reference a parent Epic. It may reference child Task(s) 2d Developing Status indicating currently designing and developing a solution 3b Feature Classification indicating the addition of novel functionality to the design 6 API Impact flag identifying the application programing interface (API) 6 Security Impact flag identifying system/user security api feature

Comments

@abitmore
Copy link
Member

abitmore commented Feb 2, 2018

Do we have API's so that clients can know the API servers' version and configured parameters? For example, the history_api::get_market_history_buckets API provides some info to clients which is useful.

Inspired by this discussion.

Update: @sschiessl-bcp requested some info: #626 (comment)

## CORE TEAM TASK LIST - [ ] Design / Develop Solution - [x] Assigned: @manikey123 - [x] Note: All effort will be tracked on #782
@abitmore abitmore added the api label Feb 2, 2018
@abitmore abitmore added this to the Future Non-Consensus-Changing Release milestone Feb 2, 2018
@HarukaMa
Copy link
Contributor

HarukaMa commented Feb 3, 2018

If such API could provide a estimated load of the node it would be very helpful, but dunno if it's viable to estimate it...

@jmjatlanta
Copy link
Contributor

I believe this belongs in the login_api. The call will return a collection of settings.

Note: Care must be taken as to not expose information that could become an attack vector.

If no one else is currently working on this, I'll attempt to tackle it. I'll concentrate on the server version, and then information can be added from there.

Any questions/comments/advice, let me know.

@pmconrad
Copy link
Contributor

Please keep the exposed information minimal, for reasons of security. Server load is nobody's business, for example.

@abitmore
Copy link
Member Author

The rationale of creating this issue, is that some API nodes didn't upgrade when a new feature is released, or didn't enable some plugins. It's good for clients to easily know the API server's capacity when connected, otherwise, for better user experience, the clients have to guess/detect with some methods. Nevertheless some info may be sensitive.

@jmjatlanta
Copy link
Contributor

jmjatlanta commented Feb 26, 2018

Here are my thoughts and questions:

Almost any information retrieved from the server can be considered a security risk. I try to always err on the side of security. But we must find the balance between giving clients the information they need to do their job, and strong security. Therefore, in the following list, I ask those with a better knowledge of the risks to assign each one of these a value of low, medium, or high risk.

  1. API version (GRAPHENE_CURRENT_DB_VERSION)
  2. Which plugins are active
  3. Plugin parameters

I suggest that low risk items be included in the information returned from the API. Medium risk items should be available if a command line parameter is passed allowing the information (granularity will need to be discussed), and the implementation of high risk items should be evaluated individually, and not as part of this issue.

I'm fairly certain that if (2) and (3) are considered anything but low risk, the items in (3) will need to be evaluated on a case-by-case basis.

I feel I am at a disadvantage here, as I do not have use cases for how the API will be used. So I ask those with an interest for this API addition to chime in with why they need it.

@pmconrad
Copy link
Contributor

I'm fairly certain that if (2) and (3) are considered anything but low risk, the items in (3) will need to be evaluated on a case-by-case basis.

This.
I consider 2 medium-high risk and 3 high risk. You do not want someone to know that you're running a witness node for example, and you definitely don't want them to know your block signing key.

There are very few plugins that might be of legitimate interest to a client, and even for those some parameters might be considered sensitive.

Note that GRAPHENE_CURRENT_DB_VERSION is not directly connected to the API. We don't have an explicit API version at this time (versioned API would be cool to have though).

@litepresence
Copy link
Contributor

litepresence commented Apr 5, 2018

  • API version (GRAPHENE_CURRENT_DB_VERSION)
  • Which plugins are active
  • Plugin parameters

what if all of that information was packaged into single hash value?

I feel I am at a disadvantage here, as I do not have use cases for how the API will be used. So I ask those with an interest for this API addition to chime in with why they need it.

something simple that I notice on various versions is if a price is supposed to be 2100 satoshi on the market history, some nodes will display for example:
0.00002100000000365749651
while others will show
0.00002099999999932415674
then every once in a while I'll get a node that shows something pretty close... but obviously wrong like:
0.00002069875198725416548
which I suspect is a version issue; a correction made to how closed orders in this case are calculated; that has been resolved in later versions

here's current example:

unix - EST time - 16 digit price - volume

market is BTS:OPEN.BTC

screenshot from 2018-04-05 13 50 04
screenshot from 2018-04-05 13 49 28

@sschiessl-bcp
Copy link

Server info would be super helpful, even if it's just the running release information. Allowed apis would also very cool (asset_api etc), plugin settings to some extent (e.g. ES, extent of history).

But of course security first, fully agree there. Minimal expectation would be release information.

Thanks!

@pmconrad
Copy link
Contributor

pmconrad commented Apr 6, 2018

what if all of that information was packaged into single hash value

Bad idea, IMO. Better explicitly deliver interesting information that is not a security risk.

@abitmore
Copy link
Member Author

Quoted from bitshares/bitshares-ui#1521 (comment):

The amount of history is not available AFAIK.

@ryanRfox ryanRfox added 1b User Story The User Story details a requirement. It may reference a parent Epic. It may reference child Task(s) 2a Discussion Needed Prompt for team to discuss at next stand up. 3b Feature Classification indicating the addition of novel functionality to the design 6 API Impact flag identifying the application programing interface (API) 6 Security Impact flag identifying system/user security labels Jun 14, 2018
@ryanRfox
Copy link
Contributor

@pmconrad feels this is a security risk:
#668 (comment)

Please comment further about what should developed regarding OP within the Issue here (rather than the PR).

@sschiessl-bcp
Copy link

sschiessl-bcp commented Jun 15, 2018

From the UI's perspective (not just reference wallet) it will be beneficial to know configuration.

I would suggest having an API that exposes

  • history size
  • available plugins, this should include ES (since this opens up lots of advanced features for UI)
  • server build / release
  • optional string to be defined by node operator for self-identification

Besides the last, the information can be queried manually as well (count returned history size, try access plugin and get exception, try calling non-existing calls that are specific to a server relase). Can you elaborate on the possible security risks @pmconrad ?

@jmjatlanta
Copy link
Contributor

If a flaw is found for certain versions / plugins or a combination of each, an attacker can query the network for which peers have that combination, making her job easier.

But I believe there are legitimate reasons to provide this information. So I believe providing this ability, along with the ability to turn it off (or requiring them to turn it on), gives us flexibility with some security.

I look forward to additional comments.

@sschiessl-bcp
Copy link

If a flaw is found for certain versions / plugins or a combination of each, an attacker can query the network for which peers have that combination, making her job easier.

An attacker that is capable enough to enter a node will also have no problem preparing a script that queries the API nodes for certain calls that allows establishing the version as well.

@xloem
Copy link
Contributor

xloem commented Apr 22, 2019

just to mention #1726, it could be nice to provide the url of the rpc server as well.

I suppose for security it would be best if private addresses were censored, or maybe a way around security concerns would be to make configurable what information is included.

@jmjatlanta
Copy link
Contributor

Here is the latest iteration of requirements. I ask that others chime in where I am wrong.

  1. The feature should be optional, and "on" by default. Calling the method is not an error when the feature is turned off. It simply will return nothing.

  2. When on, it will return the API version and the protocol version. The way these versions are calculated are yet to be formalized.

  3. Plugins will be queried for their information, and each plugin will return redacted data about configuration. Returning nothing is valid.

Previous PRs for this issue will need to be reworked or thrown away. I believe that if we can all agree on the above 3 points, and agree on some kind of versioning scheme for API and protocol, we can implement this fairly quickly.

@xloem
Copy link
Contributor

xloem commented Apr 29, 2019

If acceptable, please include the RPC server url.

@pmconrad
Copy link
Contributor

2 should include a user-configurable custom string as requested by @sschiessl-bcp . Possibly other top-level settings as well? Can't see anything relevant at first glance.

Clarifications on 3:

  • only plugins that are enabled and accessible by the logged-in user will be queried
  • plugins that return nothing will not be listed in the output (as opposed to listed with empty info)

@abitmore
Copy link
Member Author

Done via #2617.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1b User Story The User Story details a requirement. It may reference a parent Epic. It may reference child Task(s) 2d Developing Status indicating currently designing and developing a solution 3b Feature Classification indicating the addition of novel functionality to the design 6 API Impact flag identifying the application programing interface (API) 6 Security Impact flag identifying system/user security api feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants