Skip to content

Commit 7f32595

Browse files
cwdobbinscwdobbins
authored
GEOMESA-3581 Trino - Base64-encode authorization credentials (#3585)
1 parent 119c154 commit 7f32595

9 files changed

Lines changed: 116 additions & 118 deletions

File tree

‎geomesa-trino/geomesa-trino-datastore/src/main/java/org/locationtech/geomesa/trino/datastore/AuthTokens.java‎

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,17 @@
1111
import java.util.Collection;
1212

1313
/**
14-
* Validation for authorization tokens against the characters that carry structural
15-
* meaning somewhere in the auth transport chain:
14+
* Validates authorization tokens against their join/split delimiter (,):
1615
*
1716
* <ul>
18-
* <li>{@code |} — joins tokens in the JDBC {@code auths} extra credential;</li>
19-
* <li>{@code ,} — joins tokens in the {@code is_visible} SQL literal (and splits them
20-
* again inside the UDF), and delimits auth-mapping file lists;</li>
21-
* <li>{@code ;} / {@code :} — delimit the Trino JDBC {@code extraCredentials}
22-
* {@code name:value;name:value} wire encoding;</li>
23-
* <li>whitespace and non-printable/non-ASCII — rejected by the Trino JDBC driver's
24-
* extraCredentials validation.</li>
17+
* <li>{@code ,} — joins tokens in the {@code is_visible} SQL literal, splits the decoded
18+
* {@code auths} credential payload, and delimits auth-mapping file lists. A comma inside
19+
* a token (invalid) would be split into sub-tokens, granting each fragment as an
20+
* authorization that was never issued.</li>
2521
* </ul>
2622
*
27-
* <p>A token containing any of these cannot round-trip losslessly between the datastore's
28-
* client-side conjunct and the plugin's server-side row filter: it would be silently split
29-
* into sub-tokens, GRANTING each fragment as an authorization that was never issued. So
30-
* producers reject such tokens loudly (see {@link #validate}) and resolvers drop them
31-
* (fail-closed) rather than propagate them.
23+
* <p>Producers reject a comma-bearing token loudly (see {@link #validate}) and resolvers drop
24+
* it (fail-closed) rather than propagate it.
3225
*
3326
* <p>Mirrors the plugin's {@code org.locationtech.geomesa.trino.security.AuthTokens}
3427
* byte-for-byte — the two modules are isolated (neither is on the other's classpath), so
@@ -38,18 +31,11 @@ final class AuthTokens {
3831

3932
private AuthTokens() {}
4033

41-
/** True iff the token is non-empty printable ASCII with no delimiter characters. */
34+
/** True iff the token is non-empty and contains no comma — the sole delimiter
35+
* once the auths payload is base64url-encoded for transport and the {@code is_visible}
36+
* literal is quote-escaped. */
4237
static boolean isValid(String token) {
43-
if (token == null || token.isEmpty()) {
44-
return false;
45-
}
46-
for (int i = 0; i < token.length(); i++) {
47-
char c = token.charAt(i);
48-
if (c <= ' ' || c > '~' || c == ',' || c == '|' || c == ';' || c == ':') {
49-
return false;
50-
}
51-
}
52-
return true;
38+
return token != null && !token.isEmpty() && token.indexOf(',') < 0;
5339
}
5440

5541
/** Throws on the first invalid token. Call before joining tokens for transport —
@@ -61,9 +47,8 @@ static void validate(Collection<String> auths) {
6147
for (String token : auths) {
6248
if (!isValid(token)) {
6349
throw new IllegalArgumentException(
64-
"Invalid authorization token '" + token + "': tokens must be non-empty printable"
65-
+ " ASCII without ',', '|', ';', ':', or whitespace (structural delimiters in the"
66-
+ " auth transport)");
50+
"Invalid authorization token '" + token + "': tokens must be non-empty and"
51+
+ " must not contain ','");
6752
}
6853
}
6954
}

‎geomesa-trino/geomesa-trino-datastore/src/main/java/org/locationtech/geomesa/trino/datastore/TrinoDataStore.java‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.locationtech.geomesa.security.AuthorizationsProvider;
1616

1717
import java.io.IOException;
18+
import java.nio.charset.StandardCharsets;
1819
import java.sql.*;
1920
import java.util.*;
2021
import java.util.concurrent.TimeUnit;
@@ -122,18 +123,26 @@ public void dispose() {
122123
*
123124
* <p>The Trino JDBC {@code extraCredentials} property is {@code name:value}
124125
* pairs delimited by SEMICOLONS (NOT commas), and each value must be printable
125-
* ASCII with no spaces. So auth tokens are joined with PIPES and the secret is
126-
* a second pair: {@code auths:basic|privileged;secret:<value>}. Throws
127-
* {@code IllegalArgumentException} on a token containing a transport delimiter —
128-
* the server-side resolver would re-split it into auths that were never issued
129-
* (see {@link AuthTokens}). */
126+
* ASCII with no spaces. The auth tokens are comma-joined and then base64url
127+
* encoded (unpadded) into a single value — the base64url alphabet can never
128+
* collide with the wire delimiters, so the transport is injection-proof by
129+
* construction: {@code auths:<base64url(tok1,tok2)>;secret:<value>}. The
130+
* server-side {@code ExtraCredentialAuthorizationResolver} decodes the same
131+
* format.
132+
*
133+
* <p>Tokens are still validated against {@link AuthTokens} before encoding:
134+
* the SQL row-filter layer joins tokens with commas into the
135+
* {@code is_visible} literal, so the no-delimiter token contract remains
136+
* load-bearing there even though the wire itself no longer requires it. */
130137
static Properties connectionProperties(String user, List<String> auths, String secret) {
131138
AuthTokens.validate(auths);
132139
Properties props = new Properties();
133140
props.setProperty("user", user);
134141
StringBuilder creds = new StringBuilder();
135142
if (auths != null && !auths.isEmpty()) {
136-
creds.append(AUTHS_CREDENTIAL).append(':').append(String.join("|", auths));
143+
String encoded = Base64.getUrlEncoder().withoutPadding()
144+
.encodeToString(String.join(",", auths).getBytes(StandardCharsets.UTF_8));
145+
creds.append(AUTHS_CREDENTIAL).append(':').append(encoded);
137146
}
138147
if (secret != null && !secret.isEmpty()) {
139148
if (creds.length() > 0) creds.append(';'); // semicolon delimits pairs

‎geomesa-trino/geomesa-trino-datastore/src/test/java/org/locationtech/geomesa/trino/datastore/TrinoDataStoreConnectionTest.java‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,25 +32,22 @@ void emptyAuthsSendNoCredential() {
3232
}
3333

3434
@Test
35-
void authsBecomePipeDelimitedExtraCredential() {
36-
// Pipe-delimited, NOT space/comma: the Trino JDBC extraCredentials value
37-
// forbids spaces and uses comma/colon as structural separators. The
38-
// resolver splits the value on pipes, commas, or whitespace.
35+
void authsBecomeBase64UrlExtraCredential() {
3936
Properties props = TrinoDataStore.connectionProperties("svc", List.of("basic", "privileged"), null);
40-
assertThat(props.getProperty("extraCredentials")).isEqualTo("auths:basic|privileged");
37+
assertThat(props.getProperty("extraCredentials")).isEqualTo("auths:YmFzaWMscHJpdmlsZWdlZA");
4138
}
4239

4340
@Test
4441
void singleAuthEncodes() {
4542
Properties props = TrinoDataStore.connectionProperties("svc", List.of("basic"), null);
46-
assertThat(props.getProperty("extraCredentials")).isEqualTo("auths:basic");
43+
assertThat(props.getProperty("extraCredentials")).isEqualTo("auths:YmFzaWM");
4744
}
4845

4946
@Test
5047
void secretAddedAsSecondPairDelimitedBySemicolon() {
5148
// Trino JDBC extraCredentials delimits name:value pairs with SEMICOLONS.
5249
Properties props = TrinoDataStore.connectionProperties("svc", List.of("basic", "privileged"), "tok3n");
53-
assertThat(props.getProperty("extraCredentials")).isEqualTo("auths:basic|privileged;secret:tok3n");
50+
assertThat(props.getProperty("extraCredentials")).isEqualTo("auths:YmFzaWMscHJpdmlsZWdlZA;secret:tok3n");
5451
}
5552

5653
@Test
@@ -68,11 +65,8 @@ void secretSentEvenWithoutAuths() {
6865
}
6966

7067
@Test
71-
void authTokenContainingTransportDelimiterIsRejected() {
72-
// '|' joins tokens in the credential value, ';'/':' delimit its pairs, and the
73-
// server-side resolver splits on pipes/commas/whitespace — a token containing
74-
// any of them would be silently re-split into auths that were never issued.
75-
for (String bad : List.of("FOO,BAR", "FOO|BAR", "FOO;BAR", "FOO:BAR", "FOO BAR", "")) {
68+
void authTokenContainingCommaIsRejected() {
69+
for (String bad : List.of("FOO,BAR", "")) {
7670
assertThatThrownBy(() ->
7771
TrinoDataStore.connectionProperties("svc", List.of("basic", bad), null))
7872
.isInstanceOf(IllegalArgumentException.class)

‎geomesa-trino/geomesa-trino-datastore/src/test/java/org/locationtech/geomesa/trino/datastore/TrinoFeatureSourceVisibilityTest.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,21 @@ void combineBothWrapsFilterAndAnds() {
4343
}
4444

4545
@Test
46-
void authTokenContainingTransportDelimiterIsRejected() {
47-
for (String bad : List.of("FOO,BAR", "FOO|BAR", "FOO;BAR", "FOO:BAR", "FOO BAR", "")) {
46+
void authTokenContainingCommaIsRejected() {
47+
for (String bad : List.of("FOO,BAR", "")) {
4848
assertThatThrownBy(() ->
4949
TrinoFeatureSource.visibilityConjunct("__vis__", List.of("basic", bad)))
5050
.isInstanceOf(IllegalArgumentException.class)
5151
.hasMessageContaining("authorization token");
5252
}
5353
}
5454

55+
@Test
56+
void nonCommaSpecialCharsAreAllowed() {
57+
assertThat(TrinoFeatureSource.visibilityConjunct("__vis__", List.of("FOO;BAR", "FOO:BAR", "FOO BAR")))
58+
.isEqualTo("is_visible(\"__vis__\", 'FOO;BAR,FOO:BAR,FOO BAR')");
59+
}
60+
5561
@Test
5662
void combineHandlesAbsentParts() {
5763
assertThat(TrinoFeatureSource.combineWhere(null, null)).isEmpty();

‎geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/security/AuthTokens.java‎

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,17 @@
1111
import java.util.Collection;
1212

1313
/**
14-
* Validation for authorization tokens against the characters that carry structural
15-
* meaning somewhere in the auth transport chain:
14+
* Validates authorization tokens against their join/split delimiter (,):
1615
*
1716
* <ul>
18-
* <li>{@code |} — joins tokens in the JDBC {@code auths} extra credential;</li>
19-
* <li>{@code ,} — joins tokens in the {@code is_visible} SQL literal (and splits them
20-
* again inside the UDF), and delimits auth-mapping file lists;</li>
21-
* <li>{@code ;} / {@code :} — delimit the Trino JDBC {@code extraCredentials}
22-
* {@code name:value;name:value} wire encoding;</li>
23-
* <li>whitespace and non-printable/non-ASCII — rejected by the Trino JDBC driver's
24-
* extraCredentials validation.</li>
17+
* <li>{@code ,} — joins tokens in the {@code is_visible} SQL literal, splits the decoded
18+
* {@code auths} credential payload, and * delimits auth-mapping file lists. A comma inside
19+
* a token would be split into sub-tokens, GRANTING each fragment as an authorization that
20+
* was never issued.</li>
2521
* </ul>
2622
*
27-
* <p>A token containing any of these cannot round-trip losslessly between the datastore's
28-
* client-side conjunct and the plugin's server-side row filter: it would be silently split
29-
* into sub-tokens, GRANTING each fragment as an authorization that was never issued. So
30-
* producers reject such tokens loudly (see {@link #validate}) and resolvers drop them
31-
* (fail-closed) rather than propagate them.
23+
* <p>Producers reject a comma-bearing token loudly (see {@link #validate}) and resolvers drop
24+
* it (fail-closed) rather than propagate it.
3225
*
3326
* <p>Mirrors the datastore's {@code org.locationtech.geomesa.trino.datastore.AuthTokens}
3427
* byte-for-byte — the two modules are isolated (neither is on the other's classpath), so
@@ -38,18 +31,11 @@ final class AuthTokens {
3831

3932
private AuthTokens() {}
4033

41-
/** True iff the token is non-empty printable ASCII with no delimiter characters. */
34+
/** True iff the token is non-empty and contains no comma — the sole structural delimiter
35+
* once the auths payload is base64url-encoded for transport and the {@code is_visible}
36+
* literal is quote-escaped. */
4237
static boolean isValid(String token) {
43-
if (token == null || token.isEmpty()) {
44-
return false;
45-
}
46-
for (int i = 0; i < token.length(); i++) {
47-
char c = token.charAt(i);
48-
if (c <= ' ' || c > '~' || c == ',' || c == '|' || c == ';' || c == ':') {
49-
return false;
50-
}
51-
}
52-
return true;
38+
return token != null && !token.isEmpty() && token.indexOf(',') < 0;
5339
}
5440

5541
/** Throws on the first invalid token. Call before joining tokens for transport —
@@ -61,9 +47,8 @@ static void validate(Collection<String> auths) {
6147
for (String token : auths) {
6248
if (!isValid(token)) {
6349
throw new IllegalArgumentException(
64-
"Invalid authorization token '" + token + "': tokens must be non-empty printable"
65-
+ " ASCII without ',', '|', ';', ':', or whitespace (structural delimiters in the"
66-
+ " auth transport)");
50+
"Invalid authorization token '" + token + "': tokens must be non-empty and"
51+
+ " must not contain ','");
6752
}
6853
}
6954
}

‎geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/security/ExtraCredentialAuthorizationResolver.java‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import java.nio.charset.StandardCharsets;
1414
import java.security.MessageDigest;
15+
import java.util.Base64;
1516
import java.util.LinkedHashSet;
1617
import java.util.Map;
1718
import java.util.Set;
@@ -20,12 +21,14 @@
2021

2122
/**
2223
* {@link AuthorizationResolver} that reads a session's authorization tokens
23-
* straight from a Trino <em>extra credential</em> carrying a comma-delimited
24-
* list (e.g. {@code auths=basic,privileged}). Intended for deployments fronted by a
25-
* trusted service mesh that authenticates the caller and injects the
26-
* authoritative token set: the mesh rewrites its identity header (e.g.
27-
* {@code x-auths}) into Trino's {@code X-Trino-Extra-Credential} client
28-
* header, which Trino exposes here via {@link ConnectorIdentity#getExtraCredentials()}.
24+
* from a Trino <em>extra credential</em> carrying a base64url-encoded
25+
* token list. The decoded payload is a comma or whitespace delimited list.
26+
* Base64url keeps the wire value free of the extraCredentials pair delimiters
27+
* by construction. Intended for deployments fronted by a trusted service mesh
28+
* that authenticates the caller and injects the authoritative token set:
29+
* the mesh base64url-encodes its identity header (e.g. {@code x-auths}) into Trino's
30+
* {@code X-Trino-Extra-Credential} client header, which Trino exposes here
31+
* via {@link ConnectorIdentity#getExtraCredentials()}.
2932
*
3033
* <p>Because the tokens flow through unchanged there is no mapping file to
3134
* maintain and nothing to drift from the platform's clearances. Selected with:
@@ -102,12 +105,16 @@ private boolean secretMatches(ConnectorIdentity identity) {
102105
return MessageDigest.isEqual(expectedSecret, presented.getBytes(StandardCharsets.UTF_8));
103106
}
104107

105-
private static void addTokens(Set<String> out, String csv) {
106-
if (csv == null) return;
107-
// Split on pipes, commas, or whitespace. The JDBC datastore joins tokens
108-
// with pipes (extraCredentials values forbid spaces); a mesh header may use
109-
// a comma or space list. Accept all three.
110-
for (String token : csv.split("[\\s,|]+")) {
108+
private static void addTokens(Set<String> out, String encoded) {
109+
if (encoded == null) return;
110+
String decoded;
111+
try {
112+
decoded = new String(Base64.getUrlDecoder().decode(encoded.trim()), StandardCharsets.UTF_8);
113+
} catch (IllegalArgumentException e) {
114+
LOG.warn("Authorization extra credential is not valid base64url; resolving to no auths");
115+
return;
116+
}
117+
for (String token : decoded.split(",")) {
111118
String t = token.trim();
112119
if (t.isEmpty()) continue;
113120
if (!AuthTokens.isValid(t)) {

0 commit comments

Comments
 (0)