Skip to content
This repository has been archived by the owner on Aug 23, 2020. It is now read-only.

Added defaults for remote-limt-api so sensitive calls are blocked by … #944

Merged
merged 4 commits into from Sep 16, 2018

Conversation

kwek20
Copy link
Contributor

@kwek20 kwek20 commented Aug 21, 2018

Description

Provides defaults for keeping an iri node secure when you do not provide flags.

Fixes #306

Type of change

  • Bug fix (a non-breaking change which fixes an issue)

How Has This Been Tested?

Started default IRI node with port parameter -> correct calls are blocked
Started IRI with --remote-api-limit for just blocking getNeighbors, and only that call is blocked

Checklist:

  • My code follows the style guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • New and existing unit tests pass locally with my changes

@kwek20 kwek20 requested a review from karimodm August 21, 2018 19:49
int API_PORT = 14265;
String API_HOST = "localhost";
String[] REMOTE_LIMIT_API = new String[] {"addNeighbors", "getNeighbors", "removeNeighbors"};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also add attachToTangle & interruptAttachingToTangle to the default list.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to List<String> REMOTE_LIMIT_API

In IotaUtils you can add the following method:

 public static <T> List<T> createImmutableList(T... a) {
        return Collections.unmodifiableList(Arrays.asList(a));
    }

Note that once we upgrade Java we can start using List.of() instead of doing this ugly thing.
See:
https://docs.oracle.com/javase/9/docs/api/java/util/List.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alon-e isnt attachToTangle used in a lot of tutorials? That will probably confuse people if its disabled by default

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is also a product decision I am tagging @jakubcech.
I want to remind everyone that since changing the defaults breaks backwards compatibility, perhaps it shouldn't be released together with the changes we made for configurations in #724.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

attachToTangle & interruptAttachingToTangle should be in the list. We should update our docs if it's broken by this change somewhere.

@@ -600,9 +601,10 @@ protected void setBelowMaxDepthTransactionLimit(int maxAnalyzedTransactions) {
}

public interface Defaults {
//API
//API
Copy link
Contributor

@alon-e alon-e Aug 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix indentation? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why it did that, i checked in code and it doesnt look like that :o

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might happen if you use tabs instead of spaces (https://www.youtube.com/watch?v=SsoOG6ZeyUI :trollface: )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I don't see it fixed yet... You can configure your IDE to convert tabs to spaces.

@@ -25,7 +26,7 @@
//API
protected int port = Defaults.API_PORT;
protected String apiHost = Defaults.API_HOST;
protected List<String> remoteLimitApi = new ArrayList<>();
protected List<String> remoteLimitApi = new ArrayList<>(Arrays.asList(Defaults.REMOTE_LIMIT_API));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I think wrapping with Collections.unmodifiableList() would have been more suitable than ArrayList...

int API_PORT = 14265;
String API_HOST = "localhost";
String[] REMOTE_LIMIT_API = new String[] {"addNeighbors", "getNeighbors", "removeNeighbors"};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to List<String> REMOTE_LIMIT_API

In IotaUtils you can add the following method:

 public static <T> List<T> createImmutableList(T... a) {
        return Collections.unmodifiableList(Arrays.asList(a));
    }

Note that once we upgrade Java we can start using List.of() instead of doing this ugly thing.
See:
https://docs.oracle.com/javase/9/docs/api/java/util/List.html

@iotaledger iotaledger deleted a comment Aug 27, 2018
@kwek20 kwek20 changed the title Added defaults for remote-limt-api so sensitice calls are blocked by … Added defaults for remote-limt-api so sensitive calls are blocked by … Aug 30, 2018
@@ -600,9 +601,10 @@ protected void setBelowMaxDepthTransactionLimit(int maxAnalyzedTransactions) {
}

public interface Defaults {
//API
//API
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I don't see it fixed yet... You can configure your IDE to convert tabs to spaces.

int API_PORT = 14265;
String API_HOST = "localhost";
List<String> REMOTE_LIMIT_API = IotaUtils.createImmutableList("addNeighbors", "getNeighbors", "removeNeighbors", "attachToTangle", "interruptAttachingToTangle");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you set your IDE to have a column width of 120.
I will try to add more checkstyle rules real soon. I know style comments are annoying. A computer should give them and not a human.

@GalRogozinski
Copy link
Contributor

@jakubcech and @DyrellC
Before this is released we should have an automation test.

@GalRogozinski GalRogozinski merged commit 2250b0a into iotaledger:dev Sep 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restrict --remote flag to opt-in sensitive API calls instead of opt-out
5 participants