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

SSL support #620

Closed
rvvincelli opened this issue Mar 25, 2020 · 9 comments
Closed

SSL support #620

rvvincelli opened this issue Mar 25, 2020 · 9 comments

Comments

@rvvincelli
Copy link

rvvincelli commented Mar 25, 2020

Background

Hi team, thanks for quickly addressing my other issue today.

First of all: should this issue be a duplicate, my apologies - just link me to the first one; also because SSL support sounds important - perhaps I'm missing something myself; it's been a long coding day - sorry in advance.

As we make progress, the ArangoDB instance we use works on HTTPS because it is a production one.

When initializing the db with:
java -jar admin-0.4.2.jar db-init http+ssl://root:test@<IP>/spline
we get:
java.io.IOException: Reached the end of the stream.
This version is the latest I see.

We know that this could be caused by the protocol mismatch, but the command works against an unsecure Arango, and we have the suspect that this is just a failed SSL handshake actually. With the official arangosh client, making an unsecure connection to a secure server will fail indeed.

By looking at the code it looks like the builder has no SSL at all, see here, indeed there is only one schema in the admin client, arangodb. In the Java driver they have an example.

Feature

The requested feature is to support SSL, possibly with a different scheme, like arangodbs or arangodb+ssl.

Example [Optional]

See the builder example for SSL above from the driver codebase.

Proposed Solution [Optional]

It does not seem like a lot of work so I cannot promise anything but we might submit a patch ourselves - that would take time though.

Thanks again!

cc @patrickdehoon

@wajda
Copy link
Contributor

wajda commented Mar 26, 2020

When initializing the db with:
java -jar admin-0.4.2.jar db-init http+ssl://root:test@/spline
we get:
java.io.IOException: Reached the end of the stream.

That's strange, you should get java.net.MalformedURLException instead. At the moment Spline only support ArangoDB connection string starting with arangodb://

Regarding the rest I agree. Will see if we can squeeze it into 0.5 release

@wajda wajda added enhancement and removed feature labels Mar 26, 2020
@wajda wajda added this to the 0.5.0 milestone Mar 26, 2020
@wajda
Copy link
Contributor

wajda commented Mar 26, 2020

I would propose to change the connection URL schema. Instead of arangodb we could use the protocol that will be actually used for the connection, e.g:

# unsecured
vst://
http+json://
http+vpack://

# secured
vst+ssl://
https+json://
https+vpack://

For backward compatibility, we can also leave arangodb://, that would mean a default protocol (currently vst).

@rvvincelli
Copy link
Author

Hi @wajda , thanks for your answer.

I do get that exception if the URL is malformed indeed, so if the scheme is not arangodb.

The feature is implemented, using unsecure SSL though and forcing the user to give permission to do so. Tests are not updated yet but I will open a PR anyway to get started. A second schema has been added thus, arangodbs.

Thank you,

@wajda
Copy link
Contributor

wajda commented Mar 26, 2020

Also, using MongoDB connection string format as an example, we could add ?connectionOptions part, to pass additional config parameters to the ArangoDB driver.

@rvvincelli
Copy link
Author

I've opened a PR @wajda , see here; not mergeable yet but I managed to instantiate the Spline database successfully with the admin tool. Thanks,

@wajda
Copy link
Contributor

wajda commented Mar 26, 2020

@rvvincelli, perfect, thanks you. I'll take a look at it. But while you are on it, could you please change the schema from arangodbs to vst+ssl according to my previous post? We could do add support for the rest stuff that I mentioned later.

@rvvincelli
Copy link
Author

Thanks for checking it out @wajda ! I am not sure I can really commit to that because it feels like a big change to me given that my knowledge on the code base is super limited. What I can do is to update the PR adding unit tests, wdyt? Thanks,

@wajda
Copy link
Contributor

wajda commented Mar 26, 2020

Ok, please do. We'll take it from there.

@wajda
Copy link
Contributor

wajda commented Apr 8, 2020

The protocol selection topic has moved to #631
arangodbs schema looks good to me for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

3 participants