Implement cookie authentication for eclair json-rpc api #2040
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2040 +/- ##
==========================================
- Coverage 87.54% 87.54% -0.01%
==========================================
Files 161 162 +1
Lines 12599 12607 +8
Branches 526 524 -2
==========================================
+ Hits 11030 11037 +7
- Misses 1569 1570 +1
|
|
I have tested that this pr work on Windows and Linux. It would be grate if someone could try on Mac OS since I can't. |
| eclair.api.port | API HTTP port | 8080 | ||
| eclair.api.password | API password (BASIC) | "" (must be set if the API is enabled) | ||
| eclair.api.password | API password (BASIC) | "" (change to something else if you want to enable password authentication) | ||
| eclair.api.cookie-enabled | API cookie authentication | true |
There was a problem hiding this comment.
We should be consistent with bitcoin core's naming:
| eclair.api.cookie-enabled | API cookie authentication | true | |
| eclair.api.safecookie | Enable API safe cookie authentication | true |
| @@ -13,7 +13,10 @@ eclair { | |||
| enabled = false // disabled by default for security reasons | |||
| binding-ip = "127.0.0.1" | |||
| port = 8080 | |||
| password = "" // password for basic auth, must be non empty if json-rpc api is enabled | |||
| password = "" // password for basic auth, must be non empty to enable password authentication | |||
| cookie-enabled = true | |||
There was a problem hiding this comment.
| cookie-enabled = true | |
| use-safecookie = true // NB: when using cookie authentication, password authentication will be automatically disabled |
| password = "" // password for basic auth, must be non empty if json-rpc api is enabled | ||
| password = "" // password for basic auth, must be non empty to enable password authentication | ||
| cookie-enabled = true | ||
| cookie-group-readable = false |
There was a problem hiding this comment.
Please add a comment explaining that field.
| None | ||
| } | ||
| if (apiPassword.isEmpty && apiCookiePassword.isEmpty) { | ||
| throw new RuntimeException("json-rpc api requires that either password or cookie is enabled") |
There was a problem hiding this comment.
Instead of an unnamed exception, you should replace the EmptyAPIPasswordException in Setup.scala with a APICredentialsNotConfiguredException
| override val password: Option[String] = apiPassword | ||
| override val cookiePassword: Option[String] = apiCookiePassword |
There was a problem hiding this comment.
We really shouldn't support both cookie and password auth at the same time.
The reason you would enable cookie authentication is to explicitly get rid of the user/password auth.
When cookie auth is enabled, this should automatically disable password auth.
I think we should force the user to clear its password field in reference.conf, like Bitcoin Core does, as this makes it very explicit for the user, and we should verify that they have done so.
Once that's done, I don't think we need to modify the Service class, there will still be a single password field that will be non-optional (containing either the user password or cookie password depending on which is enabled).
| if (groupReadable) { | ||
| posixView.setPermissions(PosixFilePermissions.fromString("rw-r-----")) | ||
| } else { | ||
| posixView.setPermissions(PosixFilePermissions.fromString("rw-------")) |
There was a problem hiding this comment.
Do you need to change it in that case? What are the default permissions if you don't do anything?
| val aclEntry = AclEntry.newBuilder() | ||
| .setPermissions(Set.from(AclEntryPermission.values()).asJava) | ||
| .setPrincipal(aclView.getOwner) | ||
| .setType(AclEntryType.ALLOW) | ||
| .build() | ||
| // setAcl() replaces all acl entries for the file. | ||
| // That means setting acl with an list containing only an entry that gives access to the owner, everyone else lose access to the file. | ||
| aclView.setAcl(List(aclEntry).asJava) |
There was a problem hiding this comment.
Why is that necessary if you can't make the file group-readable on windows?
What exactly are you changing here compared to the default permissions?
| // That means setting acl with an list containing only an entry that gives access to the owner, everyone else lose access to the file. | ||
| aclView.setAcl(List(aclEntry).asJava) | ||
| if (groupReadable) { | ||
| logger.warn("could not make the .cookie file group readable") |
There was a problem hiding this comment.
This shouldn't be a warning, it should be an exception.
We should note in the reference.conf that this field cannot be set to true on windows.
| logger.warn("could not change the permissions of the .cookie file") | ||
| } | ||
|
|
||
| Files.writeString(path, s"${Service.CookieUserName}:$hexPassword", StandardCharsets.UTF_8) |
There was a problem hiding this comment.
It feels like we should use ASCII instead of UTF8 here, we don't actually need UTF8, do we?
|
Closing this inactive PR. |
fixes #1437
This PR add cookie authentication for eclair API. It is enabled by default when the API is enabled and the default cookie file is
.cookiein the datadir.It is possible to have both cookie and password authentication enabled at the same time and the API can be enabled without password. I used a separate configuration option for flag because I don't like how it is done in bitcoind where cookie depends on if an rpcpassword is set.
It is also possible to to use
eclair.api.cookie-group-readableto make the generated cookie file group readable on Linux.