Skip to content

fix(flightsql): protect configured authorization metadata - #1153

Open
fallintoplace wants to merge 2 commits into
apache:mainfrom
fallintoplace:fix/flightsql-auth-params
Open

fix(flightsql): protect configured authorization metadata#1153
fallintoplace wants to merge 2 commits into
apache:mainfrom
fallintoplace:fix/flightsql-auth-params

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

What

The Flight SQL credential metadata was populated with configured authentication and then overwritten by arbitrary connection parameters. A parameter named authorization could replace a configured token or basic credential. Parameters are now copied first and configured authentication is applied last.

Test

  • go test ./arrow/flight/flightsql/driver -run TestRequestMetadataKeepsConfiguredAuthorization -count=1

@fallintoplace
fallintoplace marked this pull request as ready for review August 8, 2026 15:52

@zeroshade zeroshade left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found one blocking issue: case-variant connection parameters can still override configured Flight SQL authorization after gRPC normalizes metadata keys.

Blocking — authorization collision remains case-insensitive (arrow/flight/flightsql/driver/utils.go:40)

This protects only the exact lowercase key. gRPC v1.82.1 lowercases all PerRPCCredentials keys before transmission, so parameter key Authorization collides with configured authorization. Map iteration then determines which value wins.

I reproduced this on head 7e49a2faa101 with an in-memory gRPC client and server: the server received Bearer attacker instead of the configured token on request 5.

Please normalize parameter keys before applying configured authentication, or remove/reject authorization case-insensitively, and add a mixed-case transport-level regression. See the inline comment on arrow/flight/flightsql/driver/utils.go:40.


This review was drafted by an AI-assisted tool and confirmed by an Apache Arrow Go maintainer. After you've addressed the points above and pushed an update, an Apache Arrow Go maintainer — a real person — will take the next look at the PR. The findings cite the project's review criteria; if you think one of them is mis-applied, please reply on the PR and a maintainer will weigh in.

More on how Apache Arrow Go handles maintainer review: CONTRIBUTING.md.

md := make(map[string]string, len(g.params)+1)

// Authentication parameters
for k, v := range g.params {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gRPC lowercases every key returned by PerRPCCredentials before sending it, so a parameter named Authorization remains distinct in this map but later collides with the configured lowercase authorization. Since both maps are iterated, either value can win. An exact-head in-memory gRPC probe reproduced this: the server received Bearer attacker on request 5. Please normalize parameter keys before applying configured auth, or exclude authorization case-insensitively, and add a mixed-case transport-level regression.

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

Successfully merging this pull request may close these issues.

2 participants