<< Back to previous view

[NREPL-85] Add initial authentication support and simple default Created: 04/May/17  Updated: 24/May/17

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Rob Browning Assignee: Chas Emerick
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-server-support-authentication-and-add-simple-default.patch     Text File 0002-Support-NREPL_AUTH-in-message-experimental.patch    
Patch: Code

 Description   

The first of these two patches adds authentication to the server, and
the second trivially adjusts the message function to use it (as an
example).

I could easily imagine the current approach might not be acceptable,
but I'd be happy to try to make adjustments.

If this turns out to be interesting, then presumably tools like
leiningen and cider might want to provide more interesting strategies
via their own :auth-wrapper.

I haven't added automated tests for the authentication yet, but you can
run all of the existing tests like this:

NREPL_AUTH=none mvn package

and if we pursue the changes, I can see about adding authorization to
relevant parts of the test suite, if that's helpful.

If you'd like to try it out:

(umask 077 && dd bs=1 count=256 if=/dev/random | base64 > nrepl-token)
NREPL_AUTH=file:nrepl-token lein repl :headless

and then from another process in the same working directory:

(repl/message {:op :eval :code "(+ 1 1)" :auth (slurp "nrepl-token")})

(I also have a patch that adds NREPL_AUTH support to cider.)



 Comments   
Comment by Chas Emerick [ 04/May/17 8:52 AM ]

When I was first creating nREPL, I heard many serious suggestions to have built-in security apparatuses (including authentication and evaluation soapboxes, among other things). All of them aimed to use the middleware mechanism (as it looks like you do here), which of course is a very open mechanism for extending nREPL servers in use in leiningen and other contexts.

The essential question is: why shouldn't these patches form the basis for a separate nREPL library? In that context, you'll be able to experiment and accommodate feedback much faster than if the same initial bits were landed here.

Comment by Rob Browning [ 24/May/17 12:33 AM ]

I'm not sure what the library solution would look like, but fundamentally I think nrepl and the related ecosystem should be secure by default. If the JVM supported filesystem sockets, then a straightforward solution for platforms providing them would be a user-owned rw------- socket, but unfortunately that's not an option.

Regarding the current arrangement, I don't know of any precedent for providing passwordless shell access to user accounts as a default, and if if a tool like ssh or telnet were to do it, I'd be equally alarmed.

As it stands now, I think this command will probably destroy all the home directories of users running nrepl via lein or cider, on a machine where you can access localhost:

# DANGER: DO NOT RUN THIS
  for port in {1024..65535}; do
    echo 'd2:op4:eval4:code??:(future (clojure.java.shell/sh "rm" "-rf" (System/getProperty "user.home"))) \
      | timeout 1s nc 127.0.0.1 "$port"    
  done

And if that's not sufficiently unnerving, all that's required to make it remotely exploitable is an appropriate bug in some other program running on the system, like a mail server, irc server, or perhaps a browser: http://seclists.org/oss-sec/2016/q4/100

In that case, Guile was affected by a similar issue, and while I think that particular exploit might not be able to target nrepl right now (as it can only send an http request, and the headers will probably choke nrepl before it reaches the dangerous body), it's not reasonable to assume that there will never be some other bug[1] in the browser or elsewhere that allows sending arbitrary bytes to a localhost port.

Coupled with the tendency for people to use sudo with a non-zero timeout, it might also provide a full system compromise to a patient, determined attacker.

Oh, and for what it's worth, I'm not at all attached to the particular solution suggested, but I do think this is a problem that should be solved.

[1] Or feature - I suspect people wouldn't normally assume that providing the ability to send bytes to a port on localhost might carry such a risk.





[NREPL-77] Expose installed middlewares via some mechanism Created: 01/May/15  Updated: 15/May/15

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Laurent Petit Assignee: Chas Emerick
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Currently nREPL clients can identify available ops by calling the `describe` op.

But there's no way to identify which `middlewares` are available.

This is a problem for instance for `cider-nrepl` `pprint` middleware which does not define any op, but rather just enhances the return value for the `eval` op.

It would be great if nREPL could be enhanced so that it provides a way to discover which middlewares are installed.



 Comments   
Comment by Laurent Petit [ 07/May/15 4:23 AM ]

Hi Chas, do you have ideas on how to expose middlewares?
Should it be through a new op, mimicking what `describe` does, but for installed middlewares?
Should it leverage the existing `describe` middleware but with an additional (backwards-compatible) key (e.g. key = "type" value = "op" or "middleware") ?

Comment by Chas Emerick [ 10/May/15 11:48 AM ]

I've thought about this a bit. I would like to avoid exposing transient implementation details (e.g. the namespace/var name of particular middlewares) to clients like this. Doing so just makes it easy for complications to occur later (e.g. what happens when that name changes due to a project reorg, or when there's N pprint middlewares, rather than just one?).

I think it's reasonable to expect middlewares to participate in the reporting of available operations in the describe response. Even things like a pprint middleware should offer an operation or two to toggle whether or not it's active. I'd be interested in any counterexamples to this general expectation…?

Comment by Laurent Petit [ 15/May/15 4:32 PM ]

I agree that middlewares should be able to participate, and that it should be declarative.
Short of having special semantics for reporting middlewares instead of ops, what I did for `cider-nrepl` is to declare a dummy op named `pprint-middleware`, even though currently there's no pprint dedicated op.

See https://github.com/clojure-emacs/cider-nrepl/blob/11152eeb55509b2dd5eb7225526e68773cb78d78/src/cider/nrepl/middleware/pprint.clj#L59





[NREPL-69] Interrupt of load-file generates java.lang.ThreadDeath exception Created: 02/Nov/14  Updated: 02/Nov/14

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.6
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Vitalie Spinu Assignee: Chas Emerick
Resolution: Unresolved Votes: 1
Labels: errormsgs, interop, stacktrace
Environment:

Clojure 1.6.0, (both CIDER and lein's REPL-y 0.3.5). No problem with Clojure 1.5.1.



 Description   

At lein prompt (load-file "src/test/core.clj") where core.clj contains (Thread/sleep 10000) then C-c to interrupt.

{{
CompilerException java.lang.ThreadDeath, compiling/home/vitoshka/tmp/cidertest/src/blabla/core.clj:1:17)
clojure.lang.Compiler.load (Compiler.java:7142)
clojure.lang.Compiler.loadFile (Compiler.java:7086)
clojure.lang.RT$3.invoke (RT.java:318)
user/eval3418 (form-init8959987907704530559.clj:1)
clojure.lang.Compiler.eval (Compiler.java:6703)
clojure.lang.Compiler.eval (Compiler.java:6666)
clojure.core/eval (core.clj:2927)
clojure.main/repl/read-eval-print-6625/fn-6628 (main.clj:239)
clojure.main/repl/read-eval-print--6625 (main.clj:239)
clojure.main/repl/fn--6634 (main.clj:257)
clojure.main/repl (main.clj:257)
clojure.tools.nrepl.middleware.interruptible-eval/evaluate/fn--594 (interruptible_eval.clj:67)
Caused by:
ThreadDeath
java.lang.Thread.stop (Thread.java:836)
clojure.tools.nrepl.middleware.interruptible-eval/interruptible-eval/fn--636 (interruptible_eval.clj:204)
clojure.tools.nrepl.middleware/wrap-conj-descriptor/fn--408 (middleware.clj:17)
clojure.tools.nrepl.middleware.pr-values/pr-values/fn--573 (pr_values.clj:17)
clojure.tools.nrepl.middleware/wrap-conj-descriptor/fn--408 (middleware.clj:17)
clojure.tools.nrepl.middleware.load-file/wrap-load-file/fn--751 (load_file.clj:77)
clojure.tools.nrepl.middleware/wrap-conj-descriptor/fn--408 (middleware.clj:17)
clojure.tools.nrepl.middleware.session/add-stdin/fn--710 (session.clj:235)
clojure.tools.nrepl.middleware/wrap-conj-descriptor/fn--408 (middleware.clj:17)
cider.nrepl.middleware.test/wrap-test/fn--3003 (test.clj:199)
clojure.tools.nrepl.middleware/wrap-conj-descriptor/fn--408 (middleware.clj:17)
cider.nrepl.middleware.stacktrace/wrap-stacktrace/fn--2909 (stacktrace.clj:146)
}}

In CIDER, eval the buffer with C-c C-k which uses load-file op and then C-c C-b to interrupt. The relevant CIDER issue is here.






[NREPL-33] Consider making session and eval functionality more accessible Created: 08/Oct/12  Updated: 18/Nov/12

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0-beta9
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Hugo Duncan Assignee: Chas Emerick
Resolution: Unresolved Votes: 0
Labels: None


 Description   

ritz re-uses some of nrepl's private functions to avoid duplication. The uses are listed below.

Would it be possible to make these functions public? More subjectively, it might also be worth considering factoring out the session functionality into it's own namespace (including msg and possibly queue-eval), so the functionality is not split across the session middleware and the interruptible-eval middleware.

The debug nrepl server:
https://github.com/pallet/ritz/blob/develop/nrepl/src/ritz/nrepl.clj#L189

This uses clojure.tools.nrepl.middleware.session/create-session and clojure.tools.nrepl.middleware.session/session-out

ritz provides an eval op that tracks source forms:
https://github.com/pallet/ritz/blob/develop/nrepl-middleware/src/ritz/nrepl/middleware/tracking_eval.clj

This uses clojure.tools.nrepl.middleware.interruptible-eval/queue-eval and clojure.tools.nrepl.middleware.interruptible-eval/configure-executor






[NREPL-29] Provide a mechanism for overriding an operation Created: 09/Sep/12  Updated: 18/Apr/14

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0-beta9, 0.2.0-beta10
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Hugo Duncan Assignee: Chas Emerick
Resolution: Unresolved Votes: 0
Labels: None


 Description   

When specifying middleware, it would be much easier for the user to be able to override a default middleware without having to specify a handler.

For example, if there is a default middleware providing the "complete" operation, the user should be able to just specify their preferred completion middleware, without having to specify all middleware as a handler.

One way to do this might be to check metadata for the provided operations of the specified metadata, and ensure that either the default middleware for that operation is removed, or that the specified middleware takes precendence (which may be simpler when a middleware provides multiple operations).



 Comments   
Comment by Chas Emerick [ 14/Sep/12 7:44 AM ]

Agreed.

Just making sure that the order in which additional middlewares are provided is taken as a default stack order will suffice for most use cases. Transforming middlewares (either in full or in part) would need another metadata slot, :replace perhaps, though it seems like that would be much more difficult to get right.





[NREPL-24] :session key is overloaded Created: 10/Aug/12  Updated: 03/Oct/12

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Hugo Duncan Assignee: Chas Emerick
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The session middleware takes the :session key from the message, and uses it as an id to lookup session data, which it then places on the same key.

Using a separate key for the session id and the session data would be less confusing, and would allow easier checking of the availability of the session data in the message map.



 Comments   
Comment by Chas Emerick [ 10/Aug/12 11:26 PM ]

The session id will need to continue to come in in the :session slot, as I don't want to break clients (and, there's very, very few middlewares out there yet).

Suggestions for names for the actual session atom? :the-session? :-/





[NREPL-15] Allow clients to specify an ID for newly-retained sessions Created: 29/Mar/12  Updated: 12/Oct/12

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Unresolved Votes: 0
Labels: None


 Description   

...particularly important where less sophisticated clients and polling-oriented transports are involved (e.g. HTTP).






[NREPL-4] Provide sane multiplexing of output in the face of multithreaded, asynchronous operation Created: 30/Sep/11  Updated: 12/Nov/14

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Basically, (send-off some-agent println) & co. should get what's printed to out in that agent's thread back to the nREPL client — not silently let it dump out to System/out.

Portal ostensibly does this well. Examine their approach, see if it is compatible with nREPL's objectives.

Ill-formed brain dump:

  • multiplex new out's to System/out
    • (still won't solve clojure.test/test-out content will disappearing into the ether when it's loaded when out is bound to an nREPL out; maybe we should ensure out is bound to System/out while code is being loaded?)
  • optionally multiplex System/out and System/err
  • optionally join multiplexed S/out and S/err, receive :stdout, :stderr msgs


 Comments   
Comment by Federico Ragona [ 12/Nov/14 1:59 PM ]

Seems to be fixed, I was unable to reproduce the issue using

REPL-y 0.3.5, nREPL 0.2.6
Clojure 1.6.0

Should this issue be closed?





[NREPL-3] Adopt default port Created: 30/Sep/11  Updated: 13/Jun/14

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Unresolved Votes: 2
Labels: None


 Description   

7888 is "free", at least in IANA.

Most users want to put an nREPL port on their app, and that should always be on a typical port. Auto-selection of a port is only desirable in tooling scenarios, and tooling authors can pass an argument to start-server.

This will change the behaviour of (start-server).



 Comments   
Comment by Bozhidar Batsov [ 13/Jun/14 9:55 AM ]

Sounds like a good idea.





Generated at Wed Sep 20 11:40:28 CDT 2017 using JIRA 4.4#649-r158309.