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

Support for federated mode #89

Open
huang195 opened this issue Aug 8, 2023 · 5 comments
Open

Support for federated mode #89

huang195 opened this issue Aug 8, 2023 · 5 comments

Comments

@huang195
Copy link
Contributor

huang195 commented Aug 8, 2023

In federated mode, peer CA bundles are stored as separate files from the local CA bundle, svid key, and svid cert. Briefly looking at the code, it seems to be hard coding only those 3 files and not taking into account of any additional bundle files in federated mode. Can someone verify if this is the case?

@huang195
Copy link
Contributor Author

huang195 commented Jun 24, 2024

Hi @edurra thanks for working on this. I'm trying to use the new flag that you have introduced, but I am not succeeding. Can you please shed some light?

I have 2 spire clusters, and they are being federated. When I run spire-agent api fetch -socketPath /spiffe-workload-api/spire-agent.sock -write /tmp, I see 4 files are getting created in the /tmp directory, as expected:

1000710000@test-6d8c74c7d9-zcxjh:/usr/local/bin$ ls -al /tmp/
total 36
drwxrwxrwt. 1 root       root 4096 Jun 24 16:24 .
dr-xr-xr-x. 1 root       root 4096 Jun 24 16:22 ..
-rw-r--r--. 1 1000710000 root 1456 Jun 24 16:24 bundle.0.pem
-rw-r--r--. 1 1000710000 root 1375 Jun 24 16:24 federated_bundle.0.0.pem
-rw-------. 1 1000710000 root  241 Jun 24 16:24 svid.0.key
-rw-r--r--. 1 1000710000 root 1245 Jun 24 16:24 svid.0.pem

My spiffe-helper config file looks like the following:

    cert_dir = "/peer-certs"
    svid_file_name = "cert.pem"
    svid_key_file_name = "key.pem"
    svid_bundle_file_name = "ca.pem"
    include_federated_domains = true'

Only 3 files are created (which is expected):

/etc/ssl/certs/clink $ ls -al
total 24
drwxrwsrwx    2 root     10007100      4096 Jun 24 16:11 .
drwxr-xr-x    1 root     root          4096 Jun 24 16:11 ..
-rw-r--r--    1 10007100 10007100      1456 Jun 24 16:11 ca.pem
-rw-r--r--    1 10007100 10007100      1245 Jun 24 16:11 cert.pem
-rw-------    1 10007100 10007100       241 Jun 24 16:11 key.pem

and the ca.pem file just contains the local CA bundle, but no federated bundle. I would have expected that both the local CA and the federated CA would be both put into this ca.pem file.

I'm using the spiffe-helper container image from ghcr.io/spiffe/spiffe-helper:nightly.

@edurra
Copy link
Contributor

edurra commented Jun 25, 2024

Hi,

I see that the code was restructured some weeks ago and it might have affected this flag. I think issue comes from this function, which is not initializing IncludeFederatedDomains, meaning that it will be false regardless of its value in the configuration file.

I compiled SPIFFE Helper using this configuration and it worked:

	sidecarConfig := &sidecar.Config{
		AddIntermediatesToBundle: config.AddIntermediatesToBundle,
		AgentAddress:             config.AgentAddress,
		Cmd:                      config.Cmd,
		CmdArgs:                  config.CmdArgs,
		CertDir:                  config.CertDir,
		ExitWhenReady:            config.ExitWhenReady,
		IncludeFederatedDomains:  config.IncludeFederatedDomains,
		JWTBundleFilename:        config.JWTBundleFilename,
		Log:                      log,
		RenewSignal:              config.RenewSignal,
		SvidFileName:             config.SvidFileName,
		SvidKeyFileName:          config.SvidKeyFileName,
		SvidBundleFileName:       config.SvidBundleFileName,
	}

I currently don't have much time to work on a new PR (hopefully the code I posted can fix that, but I am not sure if more changes would be required). Hopefully somebody can pick it up.

@huang195
Copy link
Contributor Author

Hey @edurra, thanks for looking into it! Will see what has happened after your code was merged.

@faisal-memon
Copy link
Collaborator

Filed a pr to fix this bug: #167

@faisal-memon
Copy link
Collaborator

This is now merged into main and will be part of the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants