guac icon indicating copy to clipboard operation
guac copied to clipboard

[feature] Proposal: Specify HTTP Headers for CLI commands to enable Auth proxies

Open jeffmendoza opened this issue 1 year ago • 3 comments

Many GUAC cli commands (guacone collect files, guacone query vuln) use an HTTP client to connect to the GUAC GraphQL server. The GUAC server does not provide any sort of AuthN/AuthZ support.

In the short term, I propose not adding any Auth support to GUAC and suggesting the GUAC services be run behind any kind of proxy that will do AuthN/AuthZ (AuthZ would only be allow/don't allow without any introspection to the queries). To enable this, the client commands will allow specifying raw http headers. This could support any type of Auth, such as basic / api key / or an Oauth flow ran outside the tool.

Every cli command that opens an HTTP connection will use the option "header-file". This will be expected to point to a CSV file on disk that looks like this:

name, value
Authorization, Basic abc123

A helper function will be added under pkg/cli that will read the file and parse using https://pkg.go.dev/encoding/csv ReadAll(). When a client is created it will use a custom transport that implements the RoundTripper interface. This transport will simply wrap http.DefaultTransport and add headers to the request using r.Header.Add().

Describe alternatives you've considered

Instead of a file, the cli could expect an array of strings and parse them. This would be cumbersome to the user to make sure it is correct for each call. It is easier to setup a file. Also a cli options show up on ps on a system, while a file can use filesystem permissions.

The file could be yaml/json instead of csv. However, I don't see the need for the flexability of those formats. We don't need nested objects, just simple key-value pairs.

Instead of accepting raw headers, the cli could either accept a basic auth password or build in an oauth flow with a provider like GitHub. At this point, choosing a number of providers is not clear. Which ones to choose, each one needs to be implemented, and some will not be supported. This may be a step later, but just allowing raw headers can be a solution for now that doesn't make us commit to certain providers.

jeffmendoza avatar Apr 04 '24 15:04 jeffmendoza

Sounds good to me!

pxp928 avatar Apr 05 '24 15:04 pxp928

would it be better to use something like rfc822 encoding and mimic the cURL -H @file parameter ?

On the negative side, I imagine that'd require a third dependency such as this. On the upside, I can see somebody copying a cURL file that "works". I also suspect that testing proxy configuration using cURL first may make people's life easier

SantiagoTorres avatar Apr 05 '24 18:04 SantiagoTorres

Agree on using a rfc822 file. I tested out this code and it works well:

package main

import (
	"log"
	"net/http"
	"os"

	"github.com/ProtonMail/gluon/rfc822"
)

func main() {
	// Read in header file to byte[]
	bts, err := os.ReadFile("headers.txt")
	if err != nil {
		log.Fatal(err)
	}

	// Create a header object from github.com/ProtonMail/gluon/rfc822
	// library. Store this object
	rfcHeaders, err := rfc822.NewHeader(bts)
	if err != nil {
		log.Fatal(err)
	}

	httpHeaders := http.Header{}

	// In the custom transport middleware, add headers to any request
	// (r.Header.Add)
	rfcHeaders.Entries(func(k, v string) {
		httpHeaders.Add(k, v)
	})

	// This log shows that this is working properly.
	if err := httpHeaders.Write(os.Stdout); err != nil {
		log.Fatal(err)
	}
}

jeffmendoza avatar Apr 08 '24 22:04 jeffmendoza

Looks like I missed some things, like https://github.com/guacsec/guac/blob/d95860c03e8956a89573ca47156737dddb4701ab/pkg/ingestor/ingestor.go#L156-L157 . I am going to revisit my fix. I will probably make the flag a top-level flag for each command that supports it (as opposed to putting it at the sub-command level).

nchelluri avatar Apr 18 '24 18:04 nchelluri