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

cli: allow user to be passed into to debug zip command #111120

Open
rafiss opened this issue Sep 22, 2023 · 2 comments
Open

cli: allow user to be passed into to debug zip command #111120

rafiss opened this issue Sep 22, 2023 · 2 comments
Labels
A-cli-admin CLI commands that pertain to controlling and configuring nodes C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@rafiss
Copy link
Collaborator

rafiss commented Sep 22, 2023

Addendum

The RPC on the server-side requires that the certificate used to connect is either for root or node. So the CLI change should not be made unless that changes. See:

cockroach/pkg/rpc/auth.go

Lines 315 to 332 in 0a4cbd7

// We are using TLS, but the peer is not using a client tenant cert.
// In that case, we only allow RPCs if the principal is 'node' or
// 'root' and the tenant scope in the cert matches this server
// (either the cert has scope "global" or its scope tenant ID
// matches our own).
//
// TODO(benesch): the vast majority of RPCs should be limited to
// just NodeUser. This is not a security concern, as RootUser has
// access to read and write all data, merely good hygiene. For
// example, there is no reason to permit the root user to send raw
// Raft RPCs.
certUserScope, err := security.GetCertificateUserScope(clientCert)
if err != nil {
return nil, err
}
if err := checkRootOrNodeInScope(certUserScope, a.tenant.tenantID); err != nil {
return nil, err
}

In order for this to be done, something like #51454 should be completed first. That issue involves re-implementing every endpoint used by debug zip in a different way, which is either a cross team project, or something that would be under the Server team.


cockroach debug zip only works for the root user right now. If you try to use a different user, it shows an error like:

> ./cockroach debug zip --url 'postgresql://tester@localhost:36257/defaultdb?sslcert=certs%2Fclient.tester.crt&sslkey=certs%2Fclient.tester.key&sslmode=verify-full&sslrootcert=certs%2Fca.crt' d.zip

warning: --url specifies user/password, but command "zip" does not accept user/password details - details ignored

ERROR: invalid argument "postgresql://tester@localhost:36257/defaultdb?sslcert=certs%2Fclient.tester.crt&sslkey=certs%2Fclient.tester.key&sslmode=verify-full&sslrootcert=certs%2Fca.crt" for "--url" flag: invalid file name for "sslcert": expected "client.root.crt", got "client.tester.crt"

The command should not ignore the provided user. It should also allow the --user flag to work.

Jira issue: CRDB-31775

Epic CC-25593

@rafiss rafiss added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-cli-admin CLI commands that pertain to controlling and configuring nodes labels Sep 22, 2023
@knz
Copy link
Contributor

knz commented Sep 22, 2023

xref #51454

@adbrenn
Copy link

adbrenn commented Dec 8, 2023

Hi @rafiss - sorry for the naive question, but which DB would be responsible for making the necessary changes to address this issue? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli-admin CLI commands that pertain to controlling and configuring nodes C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Development

No branches or pull requests

3 participants