-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Service Accounts - Authentication with file tokens #70543
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
Changes from 21 commits
938edc6
104db52
6253645
0107f30
289b640
449051a
d5b4ecb
05997e6
c325995
f933778
d0b1135
3f40476
821d150
56671cc
3dee4d6
c2c25d6
893f3f0
586a0fc
bfbb848
a615dd7
367b5e4
d3791d0
737caba
f9b8768
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,171 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
package org.elasticsearch.xpack.core.security.authc.service; | ||
|
||
import org.apache.logging.log4j.LogManager; | ||
import org.apache.logging.log4j.Logger; | ||
import org.apache.logging.log4j.message.ParameterizedMessage; | ||
import org.elasticsearch.common.CharArrays; | ||
import org.elasticsearch.common.Strings; | ||
import org.elasticsearch.common.UUIDs; | ||
import org.elasticsearch.common.hash.MessageDigests; | ||
import org.elasticsearch.common.settings.SecureString; | ||
import org.elasticsearch.xpack.core.security.authc.AuthenticationToken; | ||
import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken; | ||
import org.elasticsearch.xpack.core.security.authc.service.ServiceAccount.ServiceAccountId; | ||
|
||
import java.io.ByteArrayInputStream; | ||
import java.io.ByteArrayOutputStream; | ||
import java.io.Closeable; | ||
import java.io.IOException; | ||
import java.io.InputStream; | ||
import java.nio.charset.StandardCharsets; | ||
import java.util.Arrays; | ||
import java.util.Base64; | ||
import java.util.Objects; | ||
import java.util.regex.Pattern; | ||
|
||
/** | ||
* A decoded credential that may be used to authenticate a {@link ServiceAccount}. | ||
* It consists of: | ||
* <ol> | ||
* <li>A {@link #getAccountId() service account id}</li> | ||
* <li>The {@link #getTokenName() name of the token} to be used</li> | ||
* <li>The {@link #getSecret() secret credential} for that token</li> | ||
* </ol> | ||
*/ | ||
public class ServiceAccountToken implements AuthenticationToken, Closeable { | ||
|
||
public static final String INVALID_TOKEN_NAME_MESSAGE = "service account token name must have at least 1 character " + | ||
"and at most 256 characters that are alphanumeric (A-Z, a-z, 0-9) or hyphen (-) or underscore (_). " + | ||
"It must not begin with an underscore (_)."; | ||
|
||
private static final Pattern VALID_TOKEN_NAME = Pattern.compile("^[a-zA-Z0-9-][a-zA-Z0-9_-]{0,255}$"); | ||
|
||
public static final byte MAGIC_BYTE = '\0'; | ||
public static final byte TOKEN_TYPE = '\1'; | ||
public static final byte RESERVED_BYTE = '\0'; | ||
public static final byte FORMAT_VERSION = '\1'; | ||
public static final byte[] PREFIX = new byte[] { MAGIC_BYTE, TOKEN_TYPE, RESERVED_BYTE, FORMAT_VERSION }; | ||
|
||
private static final Logger logger = LogManager.getLogger(ServiceAccountToken.class); | ||
|
||
private final ServiceAccountId accountId; | ||
private final String tokenName; | ||
private final SecureString secret; | ||
|
||
public ServiceAccountToken(ServiceAccountId accountId, String tokenName, SecureString secret) { | ||
this.accountId = accountId; | ||
this.tokenName = tokenName; | ||
this.secret = secret; | ||
ywangd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
public ServiceAccountToken(String qualifiedName, SecureString secret) { | ||
final String[] split = Strings.delimitedListToStringArray(qualifiedName, "/"); | ||
if (split == null || split.length != 3) { | ||
throw new IllegalArgumentException( | ||
"The qualified name of a service token should take format of 'namespace/service_name/token_name'," + | ||
" got [" + qualifiedName + "]"); | ||
} | ||
this.accountId = new ServiceAccountId(split[0], split[1]); | ||
this.tokenName = split[2]; | ||
this.secret = secret; | ||
ywangd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
public ServiceAccountId getAccountId() { | ||
return accountId; | ||
} | ||
|
||
public String getTokenName() { | ||
return tokenName; | ||
} | ||
|
||
public SecureString getSecret() { | ||
return secret; | ||
} | ||
|
||
public String getQualifiedName() { | ||
return getAccountId().asPrincipal() + "/" + tokenName; | ||
} | ||
|
||
public SecureString asBearerString() throws IOException { | ||
try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { | ||
out.writeBytes(PREFIX); | ||
out.write(getQualifiedName().getBytes(StandardCharsets.UTF_8)); | ||
out.write(':'); | ||
out.write(secret.toString().getBytes(StandardCharsets.UTF_8)); | ||
final String base64 = Base64.getEncoder().withoutPadding().encodeToString(out.toByteArray()); | ||
return new SecureString(base64.toCharArray()); | ||
} | ||
} | ||
|
||
public static ServiceAccountToken fromBearerString(SecureString bearerString) throws IOException { | ||
final byte[] bytes = CharArrays.toUtf8Bytes(bearerString.getChars()); | ||
logger.trace("parsing token bytes {}", MessageDigests.toHexString(bytes)); | ||
try (InputStream in = Base64.getDecoder().wrap(new ByteArrayInputStream(bytes))) { | ||
final byte[] prefixBytes = in.readNBytes(4); | ||
if (prefixBytes.length != 4 || false == Arrays.equals(prefixBytes, PREFIX)) { | ||
logger.trace(() -> new ParameterizedMessage( | ||
"service account token expects the 4 leading bytes to be {}, got {}.", | ||
Arrays.toString(PREFIX), Arrays.toString(prefixBytes))); | ||
return null; | ||
} | ||
final char[] content = CharArrays.utf8BytesToChars(in.readAllBytes()); | ||
final int i = UsernamePasswordToken.indexOfColon(content); | ||
if (i < 0) { | ||
logger.trace("failed to extract qualified service token name and secret, missing ':'"); | ||
return null; | ||
} | ||
return new ServiceAccountToken(new String(Arrays.copyOfRange(content, 0, i)), | ||
new SecureString(Arrays.copyOfRange(content, i + 1, content.length))); | ||
} | ||
} | ||
|
||
@Override | ||
public void close() { | ||
secret.close(); | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (this == o) | ||
return true; | ||
if (o == null || getClass() != o.getClass()) | ||
return false; | ||
ServiceAccountToken that = (ServiceAccountToken) o; | ||
return accountId.equals(that.accountId) && tokenName.equals(that.tokenName) && secret.equals(that.secret); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(accountId, tokenName, secret); | ||
} | ||
|
||
public static ServiceAccountToken newToken(ServiceAccountId accountId, String tokenName) { | ||
return new ServiceAccountToken(accountId, tokenName, UUIDs.randomBase64UUIDSecureString()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we validate the tokenName here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is validated by the caller. But your comment prompted me to do some more thinkings. I now moved the validation logic inside the constructor so it is always triggered. As a side effect, this means when we try parse a service account token from bearer string, an invalid token name will cause the parse to fail and continue down to authentication chain. Before this change, the behaviour was authentication would fail (because there would be no match for the qualified token name) and authentication chain would stop. I think this behaviour change is OK and actually more consistent. Because we also have validation logic inside |
||
} | ||
|
||
@Override | ||
public String principal() { | ||
return accountId.asPrincipal(); | ||
} | ||
|
||
@Override | ||
public Object credentials() { | ||
return secret; | ||
} | ||
|
||
@Override | ||
public void clearCredentials() { | ||
close(); | ||
} | ||
|
||
public static boolean isValidTokenName(String name) { | ||
return name != null && VALID_TOKEN_NAME.matcher(name).matches(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
package org.elasticsearch.xpack.core.security.authc.service; | ||
|
||
import org.elasticsearch.test.ESTestCase; | ||
|
||
import java.util.Arrays; | ||
import java.util.Set; | ||
import java.util.stream.Collectors; | ||
import java.util.stream.IntStream; | ||
|
||
import static org.hamcrest.Matchers.is; | ||
|
||
public class ServiceAccountTokenTests extends ESTestCase { | ||
|
||
private static final Set<Character> VALID_TOKEN_NAME_CHARS = Set.of( | ||
'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', | ||
'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', | ||
'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', | ||
'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', | ||
'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', | ||
'-', '_' | ||
); | ||
|
||
private static final Set<Character> INVALID_TOKEN_NAME_CHARS = Set.of( | ||
'!', '"', '#', '$', '%', '&', '\'', '(', ')', '*', '+', ',', '.', '/', ':', ';', '<', '=', '>', '?', '@', '[', | ||
'\\', ']', '^', '`', '{', '|', '}', '~', ' ', '\t', '\n', '\r'); | ||
|
||
public void testIsValidTokenName() { | ||
final String tokenName1 = randomTokenName(); | ||
assertThat(ServiceAccountToken.isValidTokenName(tokenName1), is(true)); | ||
|
||
final String tokenName2 = "_" + randomTokenName().substring(1); | ||
assertThat(ServiceAccountToken.isValidTokenName(tokenName2), is(false)); | ||
|
||
assertThat(ServiceAccountToken.isValidTokenName(null), is(false)); | ||
|
||
final String tokenName3 = randomInvalidTokenName(); | ||
assertThat(ServiceAccountToken.isValidTokenName(tokenName3), is(false)); | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we not also need tests for building/parsing the bearer token? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have those in |
||
public static String randomTokenName() { | ||
final Character[] chars = randomArray( | ||
1, | ||
256, | ||
Character[]::new, | ||
() -> randomFrom(VALID_TOKEN_NAME_CHARS)); | ||
final String name = Arrays.stream(chars).map(String::valueOf).collect(Collectors.joining()); | ||
return name.startsWith("_") ? "-" + name.substring(1) : name; | ||
} | ||
|
||
public static String randomInvalidTokenName() { | ||
if (randomBoolean()) { | ||
final String tokenName = randomTokenName(); | ||
final char[] chars = tokenName.toCharArray(); | ||
IntStream.rangeClosed(1, randomIntBetween(1, chars.length)) | ||
.forEach(i -> chars[randomIntBetween(0, chars.length - 1)] = randomFrom(INVALID_TOKEN_NAME_CHARS)); | ||
return new String(chars); | ||
} else { | ||
return randomFrom("", " ", randomAlphaOfLength(257), null); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
apply plugin: 'elasticsearch.java-rest-test' | ||
|
||
dependencies { | ||
javaRestTestImplementation project(':x-pack:plugin:core') | ||
javaRestTestImplementation project(':client:rest-high-level') | ||
javaRestTestImplementation project(':x-pack:plugin:security') | ||
// let the javaRestTest see the classpath of main | ||
javaRestTestImplementation project.sourceSets.main.runtimeClasspath | ||
} | ||
|
||
testClusters.javaRestTest { | ||
testDistribution = 'DEFAULT' | ||
numberOfNodes = 2 | ||
|
||
extraConfigFile 'node.key', file('src/javaRestTest/resources/ssl/node.key') | ||
extraConfigFile 'node.crt', file('src/javaRestTest/resources/ssl/node.crt') | ||
extraConfigFile 'ca.crt', file('src/javaRestTest/resources/ssl/ca.crt') | ||
extraConfigFile 'service_tokens', file('src/javaRestTest/resources/service_tokens') | ||
|
||
setting 'xpack.ml.enabled', 'false' | ||
setting 'xpack.license.self_generated.type', 'trial' | ||
|
||
setting 'xpack.security.enabled', 'true' | ||
setting 'xpack.security.authc.token.enabled', 'true' | ||
setting 'xpack.security.authc.api_key.enabled', 'true' | ||
|
||
setting 'xpack.security.http.ssl.enabled', 'true' | ||
setting 'xpack.security.http.ssl.certificate', 'node.crt' | ||
setting 'xpack.security.http.ssl.key', 'node.key' | ||
setting 'xpack.security.http.ssl.certificate_authorities', 'ca.crt' | ||
|
||
setting 'xpack.security.transport.ssl.enabled', 'true' | ||
setting 'xpack.security.transport.ssl.certificate', 'node.crt' | ||
setting 'xpack.security.transport.ssl.key', 'node.key' | ||
setting 'xpack.security.transport.ssl.certificate_authorities', 'ca.crt' | ||
setting 'xpack.security.transport.ssl.verification_mode', 'certificate' | ||
|
||
keystore 'bootstrap.password', 'x-pack-test-password' | ||
keystore 'xpack.security.transport.ssl.secure_key_passphrase', 'node-password' | ||
keystore 'xpack.security.http.ssl.secure_key_passphrase', 'node-password' | ||
|
||
user username: "test_admin", password: 'x-pack-test-password', role: "superuser" | ||
user username: "elastic/fleet", password: 'x-pack-test-password', role: "superuser" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of interest, why is this in core? I can't see any reason why it needs to be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it does not need to be. It was a mistake. I noticed
UsernamePasswordToken
is in thecore
package and assumed that was the place to go. But now I realised all other token types are in fact in thesecurity
package. I moved this class andServiceAccount
back tosecurity
.