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

[FEATURE REQUEST] Add support for Client SSL certificate #870

Closed
AkechiShiro opened this issue Apr 12, 2023 · 15 comments · Fixed by #892
Closed

[FEATURE REQUEST] Add support for Client SSL certificate #870

AkechiShiro opened this issue Apr 12, 2023 · 15 comments · Fixed by #892
Labels
enhancement New feature or request pinned

Comments

@AkechiShiro
Copy link

Is your feature request related to a problem? Please describe.
Feroxbuster should be able to take as an option a custom TLS client certificate, there might be instances where this is needed to connect to a server.
Similar to ffuf's feature request : ffuf/ffuf#477

Describe the solution you'd like
A new flag --cert in Client settings, allowing to pass a certificate in PEM/DER format.

Describe alternatives you've considered
I've tried to see if it was possible to add/trust the certificate system wide but it is really not simple, see https://superuser.com/questions/1690574/is-it-possible-to-add-system-wide-client-certificates-on-linux-openssl

@AkechiShiro AkechiShiro added the enhancement New feature or request label Apr 12, 2023
@epi052
Copy link
Owner

epi052 commented Apr 12, 2023

This is a really interesting feature, thanks for submitting it!

I don't have the time right now to implement this, but will pin it for now. If someone else feels like taking it up, I'd happily accept a PR in the interim.

@epi052 epi052 added the pinned label Apr 12, 2023
@AkechiShiro
Copy link
Author

Thanks for accepting this feature request, would you be able to describe the modification needed or some of the steps needed to take in order to implement this feature ? I might want to try and take a look but I'm not sure if I'll be able to implement the feature.

@epi052
Copy link
Owner

epi052 commented Apr 13, 2023

absolutely! Some things that come to mind

  • add a new command line option (i have a checklist for this)
    • when new cli arg is present, parse given file and store it in the Configuration struct for when the client is built
  • modify the client::initialize definition to accept an Option<String> or whatever makes sense for the cert
  • modify the calls to client::initialize to pass in an optional client certificate based on the config
    • should be able to search the code base for client::initialize. i counted 5 real calls, and 2 calls in unit tests
  • modify how the client is built to use a client cert if one is specified
  • write some kind of test that proves we're doing what we thing we're doing
    • it doesn't look like httpmock supports certificate stuff, may need to poke around and see what can be done here
  • add some documentation at feroxbuster-docs

@epi052
Copy link
Owner

epi052 commented Apr 13, 2023

If you decide to give it a shot, let me know here and we'll assign the feature to you. I'll be happy to answer questions as you go.

@aancw
Copy link
Contributor

aancw commented Apr 28, 2023

After reading the reqwest documentation, you can implement the feature using add_root_certificate to connect to server using self sign certificate
https://docs.rs/reqwest/latest/reqwest/struct.ClientBuilder.html#method.add_root_certificate

But, it will take a long way to go. Because as @epi052 mentioned, we need to customize feroxbuster and modify the correct function when --cert is supplied.

@aancw
Copy link
Contributor

aancw commented Apr 29, 2023

I'm still trying to figure out the cert implementation for feroxbuster and working in progress. For the local webserver, I use go-httpbin because it support many additional feature like gzip, self sign cert and more.

https://github.com/mccutchen/go-httpbin

@epi052
Copy link
Owner

epi052 commented Apr 29, 2023

@aancw my understanding is that the request is to support passing a client certificate.

This would be useful when the server expects mutual authentication

only adding a root certificate on the client side would not fulfill requirements for when a client must authenticate itself to a server (again, i believe this is what the request as for)

@AkechiShiro was the feature request was for the client side of a mutual auth connection or am i confused?

@lavafroth
Copy link
Contributor

lavafroth commented Apr 30, 2023

Hey everyone. I drafted a PR to add the --certificate flag to pass a file to be added as a root certificate. I'd appreciate it if any of you took a look at the changes and made sure it looks good.

@AkechiShiro
Copy link
Author

AkechiShiro commented May 1, 2023

@epi052 you are correct, my feature request was about client certificate authentification and not about using a custom CA certificate.

Thanks nonetheless for the work done by @lavafroth

@aancw
Copy link
Contributor

aancw commented May 1, 2023

Well done @lavafroth for fast implementation of the feature 👍🏻

@epi052
Copy link
Owner

epi052 commented May 6, 2023

@all-contributors add @AkechiShiro for ideas

@allcontributors
Copy link
Contributor

@epi052

I've put up a pull request to add @AkechiShiro! 🎉

@epi052
Copy link
Owner

epi052 commented May 6, 2023

@all-contributors add @lavafroth for code

@allcontributors
Copy link
Contributor

@epi052

@lavafroth already contributed before to code

@epi052
Copy link
Owner

epi052 commented May 6, 2023

good bot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pinned
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants