-
Notifications
You must be signed in to change notification settings - Fork 36
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
Update provision.sh to start the server with auth #557
Update provision.sh to start the server with auth #557
Conversation
f03b768
to
46636b5
Compare
@Callisto13 I think this is it, but is there any place else where these things have to be passed in? |
The Pr type doesn't like me. :( |
52d5851
to
0d06dbe
Compare
Codecov ReportBase: 56.61% // Head: 56.53% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #557 +/- ##
==========================================
- Coverage 56.61% 56.53% -0.08%
==========================================
Files 57 57
Lines 2768 2768
==========================================
- Hits 1567 1565 -2
- Misses 1060 1061 +1
- Partials 141 142 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
can you update the help menus? and screenshot/paste in some successful calls? |
c97e3fb
to
5475683
Compare
ab40e0b
to
30236e5
Compare
I updated the help and the pass and also added it to the flintlock command, not just all. :) I will work on adding screenshots tomorrow. |
👍 sounds good eventually we will remove most (if not all) flint flags and use the config. we can then expand it to pass in any arbitrary flintlock config can you also add in the pr description something about what a valid file looks like? i can use it when i write the security section of the docs. |
actually... on that thought, why could they not simply pass in a flintlock config file which we edit? rather than file A which we extract and write to file B? |
That is also possible indeed. And would reduce the burden on the script. That's a good thought! |
The only thing that might be a problem is if they define something that we already defined? Such as |
@Callisto13 I made an attempt to just use the thing and paste it after the things we would set up. For deduplication, see my above comment. :) |
In an ideal situation they should be able to define what they like and we fill in the necessary things if they don't... but implementing that is tricky... at least in bash 😄 |
Example output: config.txt:
run script, output:
This is just a dummy run with the script's core functionality providing this deduplication and override by the user. Will include actual working once I test this thing with an actual cert. :) |
cc548f8
to
e25261f
Compare
Co-authored-by: Claudia <claudiaberesford@gmail.com>
e25261f
to
bcaa4e4
Compare
94d9314
to
2ad35ec
Compare
Config file:
Config file for flintlockd:
|
Interesting bit that hammertime did not return an auth failed, but rather something weird:
Log from flintlockd:
It is running. 🤷 |
Re-installed without TLS to show that without specifying a config it works normally:
|
Okay, I explored everything I could with this. I even enabled validation, generate the CA cert, did everything and got that flintlock is up and running:
With config:
I checked hammertime it is just forwarding the response. The interesting part is that flintlock doesn't log ANYTHING. It's as if hammertime can't even see it. Maybe there is something wrong with the port it will run on? config looks ok to me:
|
I am happy with what you have pasted here. The fault lies with hammertime probably.
I think i recall flintlock not logging failed auth attempts before when i was implementing this. I believe it happens at a different level (ie at the protocol level, not somewhere in flintlock itself) and we are not exposing any logs there (idek if we could, but might be nice to know who is trying to connect? something for the future) hopefully i will get to review this soon ty 🎉 |
Oh yeah, that makes sense. Cool! :) Thanks. :) |
Co-authored-by: Claudia <claudiaberesford@gmail.com>
what happens if they provide a config file with the yaml |
Good question. I didn't realise that the settings were yaml hahaha. I think it would blow up but I'll test it. |
We could probably just have a sed to remove anything like that before processing
… On 21 Oct 2022, at 16:11, Gergely Brautigam ***@***.***> wrote:
Good question. I didn't realise that the settings were yaml hahaha. I think it would blow up but I'll test it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
|
@Callisto13 Heh, it produces this: ---
insecure: false
grpc-endpoint: 0.0.0.0:9090
tls-cert: /root/service.pem
tls-client-ca: /root/ca-cert.pem
---:
tls-client-validate: true
verbosity: 9
bridge-name: lmbr0
tls-key: /root/service.key
containerd-socket: /run/containerd-dev/containerd.sock Incidentally, flintlock actually didn't care and started up anyways hahaha.
That said, I'll put in a check to skip anything that doesn't contain a |
The new fix generated the right config file: ---
insecure: false
grpc-endpoint: 0.0.0.0:9090
tls-cert: /root/service.pem
tls-client-ca: /root/ca-cert.pem
tls-client-validate: true
verbosity: 9
bridge-name: lmbr0
tls-key: /root/service.key
containerd-socket: /run/containerd-dev/containerd.sock flintlock config file: cat flintlock.config
---
tls-cert: /root/service.pem
tls-key: /root/service.key
tls-client-validate: true
tls-client-ca: /root/ca-cert.pem |
@Callisto13 Shall I squash the commits or will you with Github? :) |
i can when i merge 👍 |
Co-authored-by: Claudia <claudiaberesford@gmail.com>
Thanks! :) Also, this was quite a cool journey! :) |
Label the PR with the kind of change this for:
kind/feature
What this PR does / why we need it:
Update provision.sh to start the server with auth.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #539
Special notes for your reviewer:
Checklist: