-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Configure security for the initial node cli #74868
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 all commits
85d2bd2
d9c67d4
db94ef8
9a67ac5
574bb3b
43c046d
972d478
30e8909
f7577c2
0fa3654
30f6acf
b49052d
489f91e
5509f0d
5b82c15
0f38a6b
0512405
12778b2
8c1037e
527a8f2
20ecd99
27d13e5
db45629
9fd2225
a4de429
754bc10
a3d1eb2
e93cddb
73db017
dfd6ed0
4ceb860
74490ce
71cc266
eb4714d
c4d5067
aab36d6
1ddf48a
c13b061
e2f591e
426539b
2598dd3
9c29107
0413fc6
22bcc9e
53faea9
ddccb4a
c9f5b20
a4a6674
3a490c6
16edee5
353eb3e
f17a921
d36d80c
f67354b
a9b4989
3e42bff
a25f022
00557ac
440aeab
ef69a79
c3be9a7
9abd975
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 |
|---|---|---|
|
|
@@ -18,7 +18,6 @@ | |
| import org.apache.lucene.util.StringHelper; | ||
| import org.elasticsearch.ElasticsearchException; | ||
| import org.elasticsearch.Version; | ||
| import org.elasticsearch.cli.KeyStoreAwareCommand; | ||
| import org.elasticsearch.cli.Terminal; | ||
| import org.elasticsearch.cli.UserException; | ||
| import org.elasticsearch.common.PidFile; | ||
|
|
@@ -238,37 +237,11 @@ static SecureSettings loadSecureSettings(Environment initialEnv) throws Bootstra | |
| } | ||
|
|
||
| static SecureSettings loadSecureSettings(Environment initialEnv, InputStream stdin) throws BootstrapException { | ||
| final KeyStoreWrapper keystore; | ||
| try { | ||
| keystore = KeyStoreWrapper.load(initialEnv.configFile()); | ||
| } catch (IOException e) { | ||
| throw new BootstrapException(e); | ||
| } | ||
|
|
||
| SecureString password; | ||
| try { | ||
| if (keystore != null && keystore.hasPassword()) { | ||
| password = readPassphrase(stdin, KeyStoreAwareCommand.MAX_PASSPHRASE_LENGTH); | ||
| } else { | ||
| password = new SecureString(new char[0]); | ||
| } | ||
| } catch (IOException e) { | ||
| throw new BootstrapException(e); | ||
| } | ||
|
|
||
| try (password) { | ||
| if (keystore == null) { | ||
| final KeyStoreWrapper keyStoreWrapper = KeyStoreWrapper.create(); | ||
| keyStoreWrapper.save(initialEnv.configFile(), new char[0]); | ||
| return keyStoreWrapper; | ||
| } else { | ||
| keystore.decrypt(password.getChars()); | ||
| KeyStoreWrapper.upgrade(keystore, initialEnv.configFile(), password.getChars()); | ||
| } | ||
| return KeyStoreWrapper.bootstrap(initialEnv.configFile(), () -> readPassphrase(stdin, KeyStoreWrapper.MAX_PASSPHRASE_LENGTH)); | ||
|
Contributor
Author
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. Because the startup script writes to the node's keystore, it can also bootstrap the keystore. |
||
| } catch (Exception e) { | ||
| throw new BootstrapException(e); | ||
| } | ||
| return keystore; | ||
| } | ||
|
|
||
| // visible for tests | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ | |
| import org.apache.lucene.util.SetOnce; | ||
| import org.elasticsearch.cli.ExitCodes; | ||
| import org.elasticsearch.cli.UserException; | ||
| import org.elasticsearch.common.CheckedSupplier; | ||
| import org.elasticsearch.common.Randomness; | ||
| import org.elasticsearch.common.hash.MessageDigests; | ||
|
|
||
|
|
@@ -45,6 +46,7 @@ | |
| import java.nio.charset.StandardCharsets; | ||
| import java.nio.file.AccessDeniedException; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.LinkOption; | ||
| import java.nio.file.Path; | ||
| import java.nio.file.StandardCopyOption; | ||
| import java.nio.file.attribute.PosixFileAttributeView; | ||
|
|
@@ -72,6 +74,9 @@ | |
| */ | ||
| public class KeyStoreWrapper implements SecureSettings { | ||
|
|
||
| /** Arbitrarily chosen maximum passphrase length */ | ||
| public static final int MAX_PASSPHRASE_LENGTH = 128; | ||
|
|
||
| /** An identifier for the type of data that may be stored in a keystore entry. */ | ||
| private enum EntryType { | ||
| STRING, | ||
|
|
@@ -101,7 +106,7 @@ private static class Entry { | |
| "~!@#$%^&*-_=+?").toCharArray(); | ||
|
|
||
| /** The name of the keystore file to read and write. */ | ||
| private static final String KEYSTORE_FILENAME = "elasticsearch.keystore"; | ||
| public static final String KEYSTORE_FILENAME = "elasticsearch.keystore"; | ||
|
|
||
| /** The version of the metadata written before the keystore data. */ | ||
| static final int FORMAT_VERSION = 4; | ||
|
|
@@ -194,6 +199,29 @@ public static void addBootstrapSeed(KeyStoreWrapper wrapper) { | |
| Arrays.fill(characters, (char)0); | ||
| } | ||
|
|
||
| public static KeyStoreWrapper bootstrap(Path configDir, CheckedSupplier<SecureString, Exception> passwordSupplier) throws Exception { | ||
|
Contributor
Author
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. Moved so it can be used in the pre-node-startup code. |
||
| KeyStoreWrapper keystore = KeyStoreWrapper.load(configDir); | ||
|
|
||
| SecureString password; | ||
| if (keystore != null && keystore.hasPassword()) { | ||
| password = passwordSupplier.get(); | ||
| } else { | ||
| password = new SecureString(new char[0]); | ||
| } | ||
|
|
||
| try (password) { | ||
| if (keystore == null) { | ||
| final KeyStoreWrapper keyStoreWrapper = KeyStoreWrapper.create(); | ||
| keyStoreWrapper.save(configDir, new char[0]); | ||
| return keyStoreWrapper; | ||
| } else { | ||
| keystore.decrypt(password.getChars()); | ||
| KeyStoreWrapper.upgrade(keystore, configDir, password.getChars()); | ||
| } | ||
| } | ||
| return keystore; | ||
| } | ||
|
|
||
| /** | ||
| * Loads information about the Elasticsearch keystore from the provided config directory. | ||
| * | ||
|
|
@@ -477,11 +505,16 @@ private void decryptLegacyEntries() throws GeneralSecurityException, IOException | |
|
|
||
| /** Write the keystore to the given config directory. */ | ||
| public synchronized void save(Path configDir, char[] password) throws Exception { | ||
| save(configDir, password, true); | ||
| } | ||
|
|
||
| public synchronized void save(Path configDir, char[] password, boolean preservePermissions) throws Exception { | ||
| ensureOpen(); | ||
|
|
||
| Directory directory = new NIOFSDirectory(configDir); | ||
| // write to tmp file first, then overwrite | ||
| String tmpFile = KEYSTORE_FILENAME + ".tmp"; | ||
albertzaharovits marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Path keystoreTempFile = configDir.resolve(tmpFile); | ||
| try (IndexOutput output = directory.createOutput(tmpFile, IOContext.DEFAULT)) { | ||
| CodecUtil.writeHeader(output, KEYSTORE_FILENAME, FORMAT_VERSION); | ||
| output.writeByte(password.length == 0 ? (byte)0 : (byte)1); | ||
|
|
@@ -515,17 +548,55 @@ public synchronized void save(Path configDir, char[] password) throws Exception | |
| final String message = String.format( | ||
| Locale.ROOT, | ||
| "unable to create temporary keystore at [%s], write permissions required for [%s] or run [elasticsearch-keystore upgrade]", | ||
| configDir.resolve(tmpFile), | ||
| keystoreTempFile, | ||
| configDir); | ||
| throw new UserException(ExitCodes.CONFIG, message, e); | ||
| } catch (final Exception e) { | ||
| try { | ||
| Files.deleteIfExists(keystoreTempFile); | ||
| } catch (Exception ex) { | ||
| e.addSuppressed(e); | ||
| } | ||
| throw e; | ||
| } | ||
|
|
||
| Path keystoreFile = keystorePath(configDir); | ||
| Files.move(configDir.resolve(tmpFile), keystoreFile, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); | ||
| PosixFileAttributeView attrs = Files.getFileAttributeView(keystoreFile, PosixFileAttributeView.class); | ||
| if (attrs != null) { | ||
| // don't rely on umask: ensure the keystore has minimal permissions | ||
| attrs.setPermissions(PosixFilePermissions.fromString("rw-rw----")); | ||
| if (preservePermissions) { | ||
| try { | ||
| // check that replace doesn't change the owner | ||
| if (Files.exists(keystoreFile, LinkOption.NOFOLLOW_LINKS) && | ||
| false == Files.getOwner(keystoreTempFile, LinkOption.NOFOLLOW_LINKS).equals(Files.getOwner(keystoreFile, | ||
| LinkOption.NOFOLLOW_LINKS))) { | ||
| String message = String.format( | ||
| Locale.ROOT, | ||
| "will not overwrite keystore at [%s], because this incurs changing the file owner", | ||
| keystoreFile); | ||
| throw new UserException(ExitCodes.CONFIG, message); | ||
| } | ||
| PosixFileAttributeView attrs = Files.getFileAttributeView(keystoreTempFile, PosixFileAttributeView.class); | ||
| if (attrs != null) { | ||
| // don't rely on umask: ensure the keystore has minimal permissions | ||
| attrs.setPermissions(PosixFilePermissions.fromString("rw-rw----")); | ||
| } | ||
| } catch (Exception e) { | ||
| try { | ||
| Files.deleteIfExists(keystoreTempFile); | ||
| } catch (Exception ex) { | ||
| e.addSuppressed(ex); | ||
| } | ||
| throw e; | ||
| } | ||
| } | ||
|
|
||
| try { | ||
| Files.move(keystoreTempFile, keystoreFile, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); | ||
| } catch (Exception e) { | ||
| try { | ||
| Files.deleteIfExists(keystoreTempFile); | ||
| } catch (Exception ex) { | ||
| e.addSuppressed(ex); | ||
| } | ||
| throw e; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -584,7 +655,7 @@ public static void validateSettingName(String setting) { | |
| /** | ||
| * Set a string setting. | ||
| */ | ||
| synchronized void setString(String setting, char[] value) { | ||
| public synchronized void setString(String setting, char[] value) { | ||
| ensureOpen(); | ||
| validateSettingName(setting); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,6 +31,7 @@ | |
| import org.bouncycastle.operator.jcajce.JcaContentSignerBuilder; | ||
| import org.bouncycastle.pkcs.PKCS10CertificationRequest; | ||
| import org.bouncycastle.pkcs.jcajce.JcaPKCS10CertificationRequestBuilder; | ||
| import org.elasticsearch.common.Randomness; | ||
| import org.elasticsearch.common.Strings; | ||
| import org.elasticsearch.common.network.NetworkAddress; | ||
| import org.elasticsearch.common.network.NetworkUtils; | ||
|
|
@@ -256,7 +257,7 @@ static PKCS10CertificationRequest generateCSR(KeyPair keyPair, X500Principal pri | |
| * Gets a random serial for a certificate that is generated from a {@link SecureRandom} | ||
| */ | ||
| public static BigInteger getSerial() { | ||
| SecureRandom random = new SecureRandom(); | ||
| SecureRandom random = Randomness.createSecure(); | ||
|
Contributor
Author
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. Maybe it is useful in test repro to have the same keys and certs (but the validity date changes for certs). |
||
| BigInteger serial = new BigInteger(SERIAL_BIT_LENGTH, random); | ||
| assert serial.compareTo(BigInteger.valueOf(0L)) >= 0; | ||
| return serial; | ||
|
|
@@ -268,7 +269,7 @@ public static BigInteger getSerial() { | |
| public static KeyPair generateKeyPair(int keysize) throws NoSuchAlgorithmException { | ||
| // generate a private key | ||
| KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance("RSA"); | ||
| keyPairGenerator.initialize(keysize); | ||
| keyPairGenerator.initialize(keysize, Randomness.createSecure()); | ||
| return keyPairGenerator.generateKeyPair(); | ||
| } | ||
|
|
||
|
|
@@ -314,4 +315,13 @@ public static GeneralName createCommonName(String cn) { | |
| final ASN1Encodable[] sequence = {new ASN1ObjectIdentifier(CN_OID), new DERTaggedObject(true, 0, new DERUTF8String(cn))}; | ||
| return new GeneralName(GeneralName.otherName, new DERSequence(sequence)); | ||
| } | ||
|
|
||
| /** | ||
| * See RFC 2247 Using Domains in LDAP/X.500 Distinguished Names | ||
| * @param domain active directory domain name | ||
| * @return LDAP DN, distinguished name, of the root of the domain | ||
| */ | ||
| public static String buildDnFromDomain(String domain) { | ||
| return "DC=" + domain.replace(".", ",DC="); | ||
| } | ||
| } | ||
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.
The suggested workaround is not valid anymore and the causing bug has been closed.