diff --git a/changelog/unreleased/SOLR-18010-security-json-atomic-replace.yml b/changelog/unreleased/SOLR-18010-security-json-atomic-replace.yml new file mode 100644 index 000000000000..e2786e61dec8 --- /dev/null +++ b/changelog/unreleased/SOLR-18010-security-json-atomic-replace.yml @@ -0,0 +1,7 @@ +title: Standalone security.json is replaced through a synced temporary file on POSIX file systems, so a process crash during an edit leaves the previous file in place. Symbolic links, hard-linked files and file systems without POSIX attributes are written in place as before. +type: fixed +authors: + - name: Nick Shanin +links: + - name: SOLR-18010 + url: https://issues.apache.org/jira/browse/SOLR-18010 diff --git a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java index a5d951968001..62f850dc18a7 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java @@ -21,9 +21,23 @@ import java.io.InputStream; import java.io.OutputStream; import java.lang.invoke.MethodHandles; +import java.nio.channels.FileChannel; +import java.nio.file.AccessDeniedException; +import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.FileAttribute; +import java.nio.file.attribute.GroupPrincipal; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFileAttributes; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.EnumSet; import java.util.List; +import java.util.Set; +import java.util.UUID; import org.apache.solr.common.SolrException; import org.apache.solr.common.util.CommandOperation; import org.apache.solr.common.util.Utils; @@ -36,6 +50,8 @@ /** Security Configuration Handler which works on standalone local files */ public class SecurityConfHandlerLocal extends SecurityConfHandler { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + private static final Set OWNER_READ_WRITE = + EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE); protected Path securityJsonPath; private final Object securityJsonLock = new Object(); @@ -96,8 +112,8 @@ protected boolean persistConf(SecurityConfig securityConfig) throws IOException SolrException.ErrorCode.SERVER_ERROR, "Failed persisting security.json to SOLR_HOME. Object was empty."); } - try (OutputStream securityJsonOs = Files.newOutputStream(securityJsonPath)) { - securityJsonOs.write(Utils.toJSON(securityConfig.getData())); + try { + replaceSecurityJson(Utils.toJSON(securityConfig.getData())); log.debug("Persisted security.json to {}", securityJsonPath); return true; } catch (Exception e) { @@ -108,6 +124,136 @@ protected boolean persistConf(SecurityConfig securityConfig) throws IOException } } + private void replaceSecurityJson(byte[] json) throws IOException { + if (!atomicReplaceSupported()) { + writeInPlace(json); + return; + } + Path tmpPath = + securityJsonPath.resolveSibling( + securityJsonPath.getFileName() + "." + UUID.randomUUID() + ".tmp"); + boolean replacing = Files.exists(securityJsonPath); + try { + Files.createFile(tmpPath, temporaryFileAttributes(replacing)); + } catch (AccessDeniedException e) { + log.warn( + "Could not create a temporary file next to {}, writing it in place", securityJsonPath, e); + writeInPlace(json); + return; + } + try { + Files.write(tmpPath, json); + syncFile(tmpPath); + if (Files.exists(securityJsonPath)) { + if (!hasSameOwner(tmpPath)) { + log.warn("{} is owned by another user, writing it in place", securityJsonPath); + writeInPlace(json); + return; + } + try { + copyGroup(tmpPath); + } catch (IOException e) { + log.warn("Could not keep the group of {}, writing it in place", securityJsonPath, e); + writeInPlace(json); + return; + } + copyPermissions(tmpPath); + } + try { + moveTemporaryFile(tmpPath); + } catch (IOException e) { + log.warn( + "Could not move a temporary file over {}, writing it in place", securityJsonPath, e); + writeInPlace(json); + return; + } + syncParentDirectory(); + } finally { + try { + Files.deleteIfExists(tmpPath); + } catch (IOException e) { + log.warn("Could not remove temporary file {}", tmpPath, e); + } + } + } + + private static FileAttribute[] temporaryFileAttributes(boolean replacing) { + return replacing + ? new FileAttribute[] {PosixFilePermissions.asFileAttribute(OWNER_READ_WRITE)} + : new FileAttribute[0]; + } + + private void writeInPlace(byte[] json) throws IOException { + try (OutputStream securityJsonOs = Files.newOutputStream(securityJsonPath)) { + securityJsonOs.write(json); + } + } + + private boolean atomicReplaceSupported() throws IOException { + return supportsAtomicReplace( + securityJsonPath.getFileSystem().supportedFileAttributeViews(), + Files.isSymbolicLink(securityJsonPath)) + && !hasHardLinks(); + } + + private boolean hasHardLinks() throws IOException { + return Files.exists(securityJsonPath) + && (int) Files.getAttribute(securityJsonPath, "unix:nlink") > 1; + } + + static boolean supportsAtomicReplace(Set fileAttributeViews, boolean symbolicLink) { + return !symbolicLink && fileAttributeViews.contains("posix"); + } + + private void syncParentDirectory() { + try { + syncDirectory(securityJsonPath.getParent()); + } catch (IOException e) { + log.warn( + "Could not sync directory {} after replacing security.json", + securityJsonPath.getParent(), + e); + } + } + + private void syncFile(Path path) throws IOException { + try (FileChannel channel = FileChannel.open(path, StandardOpenOption.WRITE)) { + channel.force(true); + } + } + + private void copyPermissions(Path tmpPath) throws IOException { + Files.setPosixFilePermissions(tmpPath, Files.getPosixFilePermissions(securityJsonPath)); + } + + private void copyGroup(Path tmpPath) throws IOException { + GroupPrincipal group = + Files.readAttributes(securityJsonPath, PosixFileAttributes.class).group(); + Files.getFileAttributeView(tmpPath, PosixFileAttributeView.class).setGroup(group); + } + + private boolean hasSameOwner(Path tmpPath) throws IOException { + return Files.getOwner(tmpPath).equals(Files.getOwner(securityJsonPath)); + } + + private void moveTemporaryFile(Path tmpPath) throws IOException { + try { + Files.move( + tmpPath, + securityJsonPath, + StandardCopyOption.ATOMIC_MOVE, + StandardCopyOption.REPLACE_EXISTING); + } catch (AtomicMoveNotSupportedException e) { + Files.move(tmpPath, securityJsonPath, StandardCopyOption.REPLACE_EXISTING); + } + } + + private void syncDirectory(Path dir) throws IOException { + try (FileChannel channel = FileChannel.open(dir, StandardOpenOption.READ)) { + channel.force(true); + } + } + @Override public String getDescription() { return "Edit or read security configuration locally in SOLR_HOME"; diff --git a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerFailurePathsTest.java b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerFailurePathsTest.java new file mode 100644 index 000000000000..c64bee61d0f7 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerFailurePathsTest.java @@ -0,0 +1,465 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.handler.admin; + +import static org.apache.solr.SolrTestCaseJ4.assumeWorkingMockito; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.nio.channels.FileChannel; +import java.nio.charset.StandardCharsets; +import java.nio.file.CopyOption; +import java.nio.file.FileSystem; +import java.nio.file.FileSystemException; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.OpenOption; +import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.FileAttribute; +import java.nio.file.attribute.FileAttributeView; +import java.nio.file.attribute.FileOwnerAttributeView; +import java.nio.file.attribute.FileTime; +import java.nio.file.attribute.GroupPrincipal; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFileAttributes; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.UserPrincipal; +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.List; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Stream; +import org.apache.lucene.tests.mockfile.FilterFileChannel; +import org.apache.lucene.tests.mockfile.FilterFileSystemProvider; +import org.apache.solr.SolrTestCase; +import org.apache.solr.client.solrj.SolrRequest; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.util.ContentStreamBase; +import org.apache.solr.core.CoreContainer; +import org.apache.solr.request.SolrQueryRequestBase; +import org.apache.solr.response.SolrQueryResponse; +import org.apache.solr.security.RuleBasedAuthorizationPlugin; + +/** + * Failure paths of the temporary-file write, forced through a file system that wraps the default + * one. The production code is not changed for the tests. + */ +public class SecurityConfHandlerFailurePathsTest extends SolrTestCase { + + private static final String EDIT = "{'set-user-role': {'alice': 'admin'}}"; + private static final String FAKE_OWNER_NAME = "other-owner"; + private static final String FAKE_GROUP_NAME = "other-group"; + private static final UserPrincipal FAKE_OWNER = () -> FAKE_OWNER_NAME; + private static final GroupPrincipal FAKE_GROUP = () -> FAKE_GROUP_NAME; + + private enum Fault { + SYNC_TEMP_FILE, + SYNC_DIRECTORY, + MOVE, + PERMISSION_COPY, + OWNER_MISMATCH, + GROUP_COPY, + GROUP_SET_FAILS + } + + private FaultyFileSystemProvider faulty; + private Path solrHome; + private Path securityJson; + + public void testFailedSyncKeepsExistingSecurityJson() throws Exception { + assumeWorkingMockito(); + setUpSolrHome("securityConfHandlerFaultsSync"); + byte[] seed = Files.readAllBytes(securityJson); + faulty.faults.add(Fault.SYNC_TEMP_FILE); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + assertThrows(SolrException.class, () -> postAuthorizationEdit(handler, EDIT)); + assertFalse("the file was moved after a failed sync", faulty.events.contains("move")); + assertArrayEquals(seed, Files.readAllBytes(securityJson)); + assertNoTemporaryFiles(solrHome); + } + + public void testTemporaryFileIsSyncedBeforeItIsMoved() throws Exception { + assumeWorkingMockito(); + setUpSolrHome("securityConfHandlerFaultsOrder"); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + postAuthorizationEdit(handler, EDIT); + assertEquals(List.of("sync", "move", "dirSync"), faulty.events); + assertTrue(Files.readString(securityJson, StandardCharsets.UTF_8).contains("alice")); + assertNoTemporaryFiles(solrHome); + } + + public void testDirectorySyncFailureAfterMoveDoesNotFailEdit() throws Exception { + assumeWorkingMockito(); + setUpSolrHome("securityConfHandlerFaultsDirSync"); + faulty.faults.add(Fault.SYNC_DIRECTORY); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + postAuthorizationEdit(handler, EDIT); + assertEquals(List.of("sync", "move", "dirSync"), faulty.events); + assertTrue(Files.readString(securityJson, StandardCharsets.UTF_8).contains("alice")); + assertNoTemporaryFiles(solrHome); + } + + public void testRefusedMoveFallsBackToInPlaceWrite() throws Exception { + assumeWorkingMockito(); + setUpSolrHome("securityConfHandlerFaultsMove"); + Object fileKey = fileKey(); + faulty.faults.add(Fault.MOVE); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + postAuthorizationEdit(handler, EDIT); + assertTrue("the move was not attempted", faulty.events.contains("move")); + assertTrue(Files.readString(securityJson, StandardCharsets.UTF_8).contains("alice")); + assertEquals("security.json was replaced, not written in place", fileKey, fileKey()); + assertNoTemporaryFiles(solrHome); + } + + public void testFailedPermissionCopyKeepsExistingSecurityJson() throws Exception { + assumeWorkingMockito(); + setUpSolrHome("securityConfHandlerFaultsPerms"); + byte[] seed = Files.readAllBytes(securityJson); + faulty.faults.add(Fault.PERMISSION_COPY); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + assertThrows(SolrException.class, () -> postAuthorizationEdit(handler, EDIT)); + assertFalse( + "the file was moved after a failed permission copy", faulty.events.contains("move")); + assertArrayEquals(seed, Files.readAllBytes(securityJson)); + assertNoTemporaryFiles(solrHome); + } + + public void testOwnerMismatchFallsBackToInPlaceWrite() throws Exception { + assumeWorkingMockito(); + setUpSolrHome("securityConfHandlerFaultsOwner"); + Object fileKey = fileKey(); + faulty.faults.add(Fault.OWNER_MISMATCH); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + postAuthorizationEdit(handler, EDIT); + assertFalse("the file was replaced for another owner", faulty.events.contains("move")); + assertTrue(Files.readString(securityJson, StandardCharsets.UTF_8).contains("alice")); + assertEquals(fileKey, fileKey()); + assertNoTemporaryFiles(solrHome); + } + + public void testGroupIsCopiedToTemporaryFile() throws Exception { + assumeWorkingMockito(); + setUpSolrHome("securityConfHandlerFaultsGroup"); + Object fileKey = fileKey(); + faulty.faults.add(Fault.GROUP_COPY); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + postAuthorizationEdit(handler, EDIT); + assertEquals(List.of(FAKE_GROUP_NAME), faulty.groupsSet); + assertTrue(faulty.events.contains("move")); + assertNotEquals("security.json was not replaced", fileKey, fileKey()); + assertNoTemporaryFiles(solrHome); + } + + public void testFailedGroupCopyFallsBackToInPlaceWrite() throws Exception { + assumeWorkingMockito(); + setUpSolrHome("securityConfHandlerFaultsGroupFail"); + Object fileKey = fileKey(); + faulty.faults.add(Fault.GROUP_COPY); + faulty.faults.add(Fault.GROUP_SET_FAILS); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + postAuthorizationEdit(handler, EDIT); + assertFalse("the file was replaced after a failed group copy", faulty.events.contains("move")); + assertTrue(Files.readString(securityJson, StandardCharsets.UTF_8).contains("alice")); + assertEquals(fileKey, fileKey()); + assertNoTemporaryFiles(solrHome); + } + + private void setUpSolrHome(String name) throws IOException { + Path base = createTempDir(name); + assumeTrue( + "POSIX file permissions are not supported on this file system", + base.getFileSystem().supportedFileAttributeViews().contains("posix")); + faulty = new FaultyFileSystemProvider(base.getFileSystem()); + solrHome = faulty.wrapPath(base); + securityJson = solrHome.resolve("security.json"); + Files.write(securityJson, seedSecurityJson().getBytes(StandardCharsets.UTF_8)); + } + + private Object fileKey() throws IOException { + return Files.readAttributes(securityJson, BasicFileAttributes.class).fileKey(); + } + + private static String seedSecurityJson() { + return "{\"authentication\":{\"class\":\"solr.BasicAuthPlugin\"," + + "\"credentials\":{\"solr\":\"solr\"}}," + + "\"authorization\":{\"class\":\"solr.RuleBasedAuthorizationPlugin\",\"user-role\":{}}}"; + } + + private static CoreContainer mockCoreContainer(Path solrHome) { + CoreContainer cc = mock(CoreContainer.class); + when(cc.getSolrHome()).thenReturn(solrHome); + when(cc.getAuthorizationPlugin()).thenReturn(new RuleBasedAuthorizationPlugin()); + return cc; + } + + private static void assertNoTemporaryFiles(Path solrHome) throws IOException { + try (Stream files = Files.list(solrHome)) { + assertEquals( + "temporary files left behind", + List.of(), + files.filter(f -> f.getFileName().toString().endsWith(".tmp")).toList()); + } + } + + private static void postAuthorizationEdit(SecurityConfHandler handler, String command) + throws Exception { + SolrQueryRequestBase req = new SolrQueryRequestBase(null, new ModifiableSolrParams()); + req.getContext().put("httpMethod", SolrRequest.METHOD.POST); + req.getContext().put("path", "/admin/authorization"); + req.setContentStreams( + List.of( + new ContentStreamBase.ByteArrayStream(command.getBytes(StandardCharsets.UTF_8), ""))); + handler.handleRequestBody(req, new SolrQueryResponse()); + } + + private static boolean isSecurityJson(Path path) { + return path.getFileName().toString().equals("security.json"); + } + + private static boolean isTemporary(Path path) { + return path.getFileName().toString().endsWith(".tmp"); + } + + private static final class FaultyFileSystemProvider extends FilterFileSystemProvider { + final Set faults = EnumSet.noneOf(Fault.class); + final List events = new ArrayList<>(); + final List groupsSet = new ArrayList<>(); + + FaultyFileSystemProvider(FileSystem delegate) { + super("faulty", delegate); + } + + @Override + public FileChannel newFileChannel( + Path path, Set options, FileAttribute... attrs) + throws IOException { + FileChannel channel = super.newFileChannel(path, options, attrs); + boolean temporary = isTemporary(path); + boolean directory = !temporary && Files.isDirectory(path); + return new FilterFileChannel(channel) { + @Override + public void force(boolean metaData) throws IOException { + if (temporary) { + events.add("sync"); + if (faults.contains(Fault.SYNC_TEMP_FILE)) { + throw new IOException("injected sync failure"); + } + } else if (directory) { + events.add("dirSync"); + if (faults.contains(Fault.SYNC_DIRECTORY)) { + throw new IOException("injected directory sync failure"); + } + } + super.force(metaData); + } + }; + } + + @Override + public void move(Path source, Path target, CopyOption... options) throws IOException { + events.add("move"); + if (faults.contains(Fault.MOVE) && isSecurityJson(target)) { + throw new FileSystemException(target.toString(), null, "injected refused move"); + } + super.move(source, target, options); + } + + @Override + public A readAttributes( + Path path, Class type, LinkOption... options) throws IOException { + A attributes = super.readAttributes(path, type, options); + if (type == PosixFileAttributes.class + && faults.contains(Fault.GROUP_COPY) + && isSecurityJson(path)) { + return type.cast(new OtherGroupAttributes((PosixFileAttributes) attributes)); + } + return attributes; + } + + @Override + public V getFileAttributeView( + Path path, Class type, LinkOption... options) { + V view = super.getFileAttributeView(path, type, options); + if (type == PosixFileAttributeView.class) { + return type.cast(new PosixView((PosixFileAttributeView) view, path)); + } + if (type == FileOwnerAttributeView.class) { + return type.cast(new OwnerView((FileOwnerAttributeView) view, path)); + } + return view; + } + + private final class PosixView implements PosixFileAttributeView { + private final PosixFileAttributeView view; + private final Path path; + + PosixView(PosixFileAttributeView view, Path path) { + this.view = view; + this.path = path; + } + + @Override + public String name() { + return view.name(); + } + + @Override + public PosixFileAttributes readAttributes() throws IOException { + return view.readAttributes(); + } + + @Override + public void setTimes(FileTime lastModifiedTime, FileTime lastAccessTime, FileTime createTime) + throws IOException { + view.setTimes(lastModifiedTime, lastAccessTime, createTime); + } + + @Override + public void setPermissions(Set perms) throws IOException { + if (faults.contains(Fault.PERMISSION_COPY) && isTemporary(path)) { + throw new IOException("injected permission copy failure"); + } + view.setPermissions(perms); + } + + @Override + public void setOwner(UserPrincipal owner) throws IOException { + view.setOwner(owner); + } + + @Override + public UserPrincipal getOwner() throws IOException { + return view.getOwner(); + } + + @Override + public void setGroup(GroupPrincipal group) throws IOException { + if (isTemporary(path)) { + groupsSet.add(group.getName()); + if (faults.contains(Fault.GROUP_SET_FAILS)) { + throw new IOException("injected group failure"); + } + } + if (Objects.equals(group, FAKE_GROUP)) { + return; + } + view.setGroup(group); + } + } + + private final class OwnerView implements FileOwnerAttributeView { + private final FileOwnerAttributeView view; + private final Path path; + + OwnerView(FileOwnerAttributeView view, Path path) { + this.view = view; + this.path = path; + } + + @Override + public String name() { + return view.name(); + } + + @Override + public UserPrincipal getOwner() throws IOException { + if (faults.contains(Fault.OWNER_MISMATCH) && isSecurityJson(path)) { + return FAKE_OWNER; + } + return view.getOwner(); + } + + @Override + public void setOwner(UserPrincipal owner) throws IOException { + view.setOwner(owner); + } + } + } + + private static final class OtherGroupAttributes implements PosixFileAttributes { + private final PosixFileAttributes attributes; + + OtherGroupAttributes(PosixFileAttributes attributes) { + this.attributes = attributes; + } + + @Override + public GroupPrincipal group() { + return FAKE_GROUP; + } + + @Override + public UserPrincipal owner() { + return attributes.owner(); + } + + @Override + public Set permissions() { + return attributes.permissions(); + } + + @Override + public FileTime lastModifiedTime() { + return attributes.lastModifiedTime(); + } + + @Override + public FileTime lastAccessTime() { + return attributes.lastAccessTime(); + } + + @Override + public FileTime creationTime() { + return attributes.creationTime(); + } + + @Override + public boolean isRegularFile() { + return attributes.isRegularFile(); + } + + @Override + public boolean isDirectory() { + return attributes.isDirectory(); + } + + @Override + public boolean isSymbolicLink() { + return attributes.isSymbolicLink(); + } + + @Override + public boolean isOther() { + return attributes.isOther(); + } + + @Override + public long size() { + return attributes.size(); + } + + @Override + public Object fileKey() { + return attributes.fileKey(); + } + } +} diff --git a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerReaderTest.java b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerReaderTest.java new file mode 100644 index 000000000000..f11d8a026cd9 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerReaderTest.java @@ -0,0 +1,128 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.handler.admin; + +import static org.apache.solr.SolrTestCaseJ4.assumeWorkingMockito; +import static org.apache.solr.handler.admin.SecurityConfHandler.SecurityConfig; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import org.apache.lucene.tests.util.LuceneTestCase.SuppressFileSystems; +import org.apache.solr.SolrTestCase; +import org.apache.solr.common.util.Utils; +import org.apache.solr.core.CoreContainer; +import org.apache.solr.security.RuleBasedAuthorizationPlugin; + +/** + * Reads security.json while edits replace it. The reader keeps the file open while the writer + * renames a new file over it. The Windows-like test filesystem cannot track that case, so this + * class runs without it. + */ +@SuppressFileSystems({"ExtrasFS", "WindowsFS"}) +public class SecurityConfHandlerReaderTest extends SolrTestCase { + + /** A reader of security.json never sees a partial document while edits are written. */ + public void testReaderNeverSeesPartialSecurityJsonDuringEdits() throws Exception { + assumeWorkingMockito(); + Path solrHome = createTempDir("securityConfHandlerLocalReaders"); + assumeTrue( + "atomic replacement needs POSIX file attributes", + solrHome.getFileSystem().supportedFileAttributeViews().contains("posix")); + Path securityJson = solrHome.resolve("security.json"); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + Map small = shortSecurityData("x"); + Map large = largeSecurityData(); + byte[] smallBytes = Utils.toJSON(small); + byte[] largeBytes = Utils.toJSON(large); + Files.write(securityJson, smallBytes); + AtomicReference writerErr = new AtomicReference<>(); + AtomicBoolean writing = new AtomicBoolean(true); + Thread writer = + new Thread( + () -> { + try { + for (int i = 0; i < 100; i++) { + handler.persistConf(new SecurityConfig().setData(i % 2 == 0 ? large : small)); + } + } catch (Throwable t) { + writerErr.set(t); + } finally { + writing.set(false); + } + }); + writer.start(); + int reads = 0; + try { + while (writing.get()) { + byte[] got; + try { + got = Files.readAllBytes(securityJson); + } catch (IOException e) { + // NFS reports a stale handle when the file is replaced during a read + continue; + } + assertTrue( + "reader saw a partial security.json of " + got.length + " bytes", + Arrays.equals(got, smallBytes) || Arrays.equals(got, largeBytes)); + reads++; + } + } finally { + writer.join(TimeUnit.SECONDS.toMillis(60)); + } + assertTrue("reader made no reads while the writer ran", reads > 0); + assertNull("writer failed: " + writerErr.get(), writerErr.get()); + } + + private static Map largeSecurityData() { + Map creds = new LinkedHashMap<>(); + creds.put("large", "L".repeat(1_000_000)); + Map authc = new LinkedHashMap<>(); + authc.put("class", "solr.BasicAuthPlugin"); + authc.put("credentials", creds); + Map data = new LinkedHashMap<>(); + data.put("authentication", authc); + return data; + } + + private static Map shortSecurityData(CharSequence credValue) { + Map creds = new LinkedHashMap<>(); + creds.put("u", credValue); + Map authc = new LinkedHashMap<>(); + authc.put("class", "solr.BasicAuthPlugin"); + authc.put("credentials", creds); + Map data = new LinkedHashMap<>(); + data.put("authentication", authc); + return data; + } + + private static CoreContainer mockCoreContainer(Path solrHome) { + CoreContainer cc = mock(CoreContainer.class); + when(cc.getSolrHome()).thenReturn(solrHome); + when(cc.getAuthorizationPlugin()).thenReturn(new RuleBasedAuthorizationPlugin()); + return cc; + } +} diff --git a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java index 5b5f4556c17e..19e3c783a843 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java @@ -17,11 +17,33 @@ package org.apache.solr.handler.admin; import static org.apache.solr.handler.admin.SecurityConfHandler.SecurityConfig; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.json.JsonMapper; +import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.GroupPrincipal; +import java.nio.file.attribute.PosixFileAttributes; +import java.nio.file.attribute.PosixFilePermission; +import java.util.Arrays; +import java.util.EnumSet; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Stream; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.common.SolrErrorWrappingException; @@ -30,6 +52,7 @@ import org.apache.solr.common.util.CommandOperation; import org.apache.solr.common.util.ContentStreamBase; import org.apache.solr.common.util.Utils; +import org.apache.solr.core.CoreContainer; import org.apache.solr.request.SolrQueryRequestBase; import org.apache.solr.response.SolrQueryResponse; import org.apache.solr.security.BasicAuthPlugin; @@ -178,6 +201,343 @@ public void testEdit() throws Exception { handler.close(); } + /** + * Two overlapping writes of security.json must never leave a torn file behind: the document on + * disk afterwards is always exactly one of the written documents, never the beginning of one + * followed by the tail of a longer one (the corruption shape in SOLR-18010). The first write is + * held open inside serialization while the second write runs, which is the interleave that + * produced that shape. + */ + public void testConcurrentPersistConfLeavesOneWholeDocument() throws Exception { + assumeWorkingMockito(); + Path solrHome = createTempDir("securityConfHandlerLocalPersist"); + Path securityJson = solrHome.resolve("security.json"); + CoreContainer cc = mock(CoreContainer.class); + when(cc.getSolrHome()).thenReturn(solrHome); + SecurityConfHandlerLocalForTesting handler = new SecurityConfHandlerLocalForTesting(cc); + + byte[] shortBytes = Utils.toJSON(shortSecurityData("x")); + byte[] longBytes = Utils.toJSON(longSecurityData()); + Files.write(securityJson, Utils.toJSON(shortSecurityData("seed"))); + + CountDownLatch entered = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + AtomicReference firstErr = new AtomicReference<>(); + Thread first = + new Thread( + () -> { + try { + handler.persistConf( + new SecurityConfig() + .setData(shortSecurityData(new LatchingValue("x", entered, release)))); + } catch (Throwable t) { + firstErr.set(t); + } + }); + first.start(); + assertTrue("first write reached serialization", entered.await(60, TimeUnit.SECONDS)); + handler.persistConf(new SecurityConfig().setData(longSecurityData())); + release.countDown(); + first.join(TimeUnit.SECONDS.toMillis(60)); + assertFalse("first write still running", first.isAlive()); + assertNull("first write failed: " + firstErr.get(), firstErr.get()); + + byte[] fileBytes = Files.readAllBytes(securityJson); + assertSingleJsonDocument(fileBytes); + assertTrue( + "security.json holds neither written document (" + + fileBytes.length + + " bytes; the writes were " + + shortBytes.length + + " and " + + longBytes.length + + " bytes)", + Arrays.equals(fileBytes, shortBytes) || Arrays.equals(fileBytes, longBytes)); + } + + /** An edit keeps the POSIX permissions of the existing file. */ + public void testEditKeepsExistingSecurityJsonPermissions() throws Exception { + assumeWorkingMockito(); + Path solrHome = createTempDir("securityConfHandlerLocalPerms"); + assumeTrue( + "POSIX file permissions are not supported on this file system", + solrHome.getFileSystem().supportedFileAttributeViews().contains("posix")); + Path securityJson = solrHome.resolve("security.json"); + Files.write(securityJson, seedSecurityJson().getBytes(StandardCharsets.UTF_8)); + EnumSet expected = + EnumSet.of( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, + PosixFilePermission.GROUP_READ); + Files.setPosixFilePermissions(securityJson, expected); + Object fileKey = Files.readAttributes(securityJson, BasicFileAttributes.class).fileKey(); + GroupPrincipal group = Files.readAttributes(securityJson, PosixFileAttributes.class).group(); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + postAuthorizationEdit(handler, "{'set-user-role': {'alice': 'admin'}}"); + assertTrue(Files.readString(securityJson, StandardCharsets.UTF_8).contains("alice")); + assertEquals(expected, Files.getPosixFilePermissions(securityJson)); + assertEquals(group, Files.readAttributes(securityJson, PosixFileAttributes.class).group()); + assertNotEquals( + "security.json was written in place, not replaced", + fileKey, + Files.readAttributes(securityJson, BasicFileAttributes.class).fileKey()); + assertNoTemporaryFiles(solrHome); + } + + /** While an edit is written, the temporary file is no wider than the existing security.json. */ + public void testTemporaryFileIsNoWiderThanExistingSecurityJson() throws Exception { + assumeWorkingMockito(); + Path solrHome = createTempDir("securityConfHandlerLocalTmpMode"); + assumeTrue( + "POSIX file permissions are not supported on this file system", + solrHome.getFileSystem().supportedFileAttributeViews().contains("posix")); + Path securityJson = solrHome.resolve("security.json"); + Files.write(securityJson, seedSecurityJson().getBytes(StandardCharsets.UTF_8)); + Set existing = + EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE); + Files.setPosixFilePermissions(securityJson, existing); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + Map bulk = bulkSecurityData(); + Set> seen = ConcurrentHashMap.newKeySet(); + AtomicBoolean writing = new AtomicBoolean(true); + Thread watcher = + new Thread( + () -> { + while (writing.get()) { + try (Stream files = Files.list(solrHome)) { + for (Path file : + files.filter(f -> f.getFileName().toString().endsWith(".tmp")).toList()) { + seen.add(Files.getPosixFilePermissions(file)); + } + } catch (IOException e) { + // the temporary file was renamed or deleted between the listing and the read + } + } + }); + watcher.start(); + try { + for (int i = 0; i < 3; i++) { + handler.persistConf(new SecurityConfig().setData(bulk)); + } + } finally { + writing.set(false); + watcher.join(TimeUnit.SECONDS.toMillis(60)); + } + assertFalse("the watcher never saw a temporary file", seen.isEmpty()); + for (Set mode : seen) { + assertTrue( + "temporary file mode " + mode + " is wider than " + existing, existing.containsAll(mode)); + } + assertNoTemporaryFiles(solrHome); + } + + /** An edit to a hard-linked security.json is written in place, so the other name sees it. */ + public void testEditKeepsHardLinkedSecurityJsonShared() throws Exception { + assumeWorkingMockito(); + Path solrHome = createTempDir("securityConfHandlerLocalHardLink"); + Path securityJson = solrHome.resolve("security.json"); + Path other = solrHome.resolve("other.json"); + Files.write(securityJson, seedSecurityJson().getBytes(StandardCharsets.UTF_8)); + boolean linked = true; + try { + Files.createLink(other, securityJson); + } catch (IOException | UnsupportedOperationException e) { + linked = false; + } + assumeTrue("hard links are not supported on this file system", linked); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + postAuthorizationEdit(handler, "{'set-user-role': {'alice': 'admin'}}"); + assertTrue(Files.readString(other, StandardCharsets.UTF_8).contains("alice")); + assertNoTemporaryFiles(solrHome); + } + + /** A read-only SOLR_HOME still allows an edit when security.json itself is writable. */ + public void testEditInPlaceWhenSolrHomeIsReadOnly() throws Exception { + assumeWorkingMockito(); + Path solrHome = createTempDir("securityConfHandlerLocalReadOnly"); + assumeTrue( + "POSIX file permissions are not supported on this file system", + solrHome.getFileSystem().supportedFileAttributeViews().contains("posix")); + Path securityJson = solrHome.resolve("security.json"); + Files.write(securityJson, seedSecurityJson().getBytes(StandardCharsets.UTF_8)); + Files.setPosixFilePermissions( + solrHome, EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_EXECUTE)); + try { + assumeFalse("the directory is writable for this user", Files.isWritable(solrHome)); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + postAuthorizationEdit(handler, "{'set-user-role': {'alice': 'admin'}}"); + assertTrue(Files.readString(securityJson, StandardCharsets.UTF_8).contains("alice")); + } finally { + Files.setPosixFilePermissions( + solrHome, + EnumSet.of( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, + PosixFilePermission.OWNER_EXECUTE)); + } + } + + /** A security.json created by a write gets the mode a newly created file gets. */ + public void testPersistCreatesSecurityJsonWithDefaultMode() throws Exception { + assumeWorkingMockito(); + Path solrHome = createTempDir("securityConfHandlerLocalCreate"); + assumeTrue( + "POSIX file permissions are not supported on this file system", + solrHome.getFileSystem().supportedFileAttributeViews().contains("posix")); + Path control = solrHome.resolve("control.json"); + Files.write(control, new byte[] {'{', '}'}); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + assertTrue(handler.persistConf(new SecurityConfig().setData(shortSecurityData("x")))); + Path securityJson = solrHome.resolve("security.json"); + assertEquals( + Files.getPosixFilePermissions(control), Files.getPosixFilePermissions(securityJson)); + assertNoTemporaryFiles(solrHome); + } + + /** Atomic replacement needs a POSIX view and a file that is not a symbolic link. */ + public void testAtomicReplaceNeedsPosixViewAndNoSymbolicLink() { + assertTrue( + SecurityConfHandlerLocal.supportsAtomicReplace( + Set.of("basic", "posix", "unix", "owner"), false)); + assertFalse( + SecurityConfHandlerLocal.supportsAtomicReplace( + Set.of("basic", "owner", "dos", "acl", "user"), false)); + assertFalse( + SecurityConfHandlerLocal.supportsAtomicReplace(Set.of("basic", "posix", "owner"), true)); + } + + private static String seedSecurityJson() { + return "{\"authentication\":{\"class\":\"solr.BasicAuthPlugin\"," + + "\"credentials\":{\"solr\":\"solr\"}}," + + "\"authorization\":{\"class\":\"solr.RuleBasedAuthorizationPlugin\",\"user-role\":{}}}"; + } + + private static CoreContainer mockCoreContainer(Path solrHome) { + CoreContainer cc = mock(CoreContainer.class); + when(cc.getSolrHome()).thenReturn(solrHome); + when(cc.getAuthorizationPlugin()).thenReturn(new RuleBasedAuthorizationPlugin()); + return cc; + } + + private static void assertNoTemporaryFiles(Path solrHome) throws IOException { + try (Stream files = Files.list(solrHome)) { + assertEquals( + "temporary files left behind", + List.of(), + files.filter(f -> f.getFileName().toString().endsWith(".tmp")).toList()); + } + } + + private static void postAuthorizationEdit(SecurityConfHandler handler, String command) + throws Exception { + SolrQueryRequestBase req = new SolrQueryRequestBase(null, new ModifiableSolrParams()); + req.getContext().put("httpMethod", SolrRequest.METHOD.POST); + req.getContext().put("path", "/admin/authorization"); + req.setContentStreams( + List.of( + new ContentStreamBase.ByteArrayStream(command.getBytes(StandardCharsets.UTF_8), ""))); + handler.handleRequestBody(req, new SolrQueryResponse()); + } + + private static void assertSingleJsonDocument(byte[] fileBytes) { + ObjectMapper mapper = + JsonMapper.builder().enable(DeserializationFeature.FAIL_ON_TRAILING_TOKENS).build(); + try { + mapper.readValue(fileBytes, Map.class); + } catch (Exception e) { + fail("security.json is not a single JSON document: " + e); + } + } + + private static Map shortSecurityData(CharSequence credValue) { + Map creds = new LinkedHashMap<>(); + creds.put("u", credValue); + Map authc = new LinkedHashMap<>(); + authc.put("class", "solr.BasicAuthPlugin"); + authc.put("credentials", creds); + Map data = new LinkedHashMap<>(); + data.put("authentication", authc); + return data; + } + + private static Map longSecurityData() { + StringBuilder pad = new StringBuilder(); + for (int i = 0; i < 6000; i++) { + pad.append('L'); + } + Map creds = new LinkedHashMap<>(); + creds.put("longuser", pad.toString()); + Map authc = new LinkedHashMap<>(); + authc.put("class", "solr.BasicAuthPlugin"); + authc.put("credentials", creds); + Map data = new LinkedHashMap<>(); + data.put("authentication", authc); + return data; + } + + private static Map bulkSecurityData() { + Map creds = new LinkedHashMap<>(); + creds.put("bulk", "B".repeat(8_000_000)); + Map authc = new LinkedHashMap<>(); + authc.put("class", "solr.BasicAuthPlugin"); + authc.put("credentials", creds); + Map data = new LinkedHashMap<>(); + data.put("authentication", authc); + return data; + } + + /** + * A {@link CharSequence} whose first read blocks until released. Serialization evaluates it, so a + * write whose data carries one is held open at a known point inside the write. + */ + private static final class LatchingValue implements CharSequence { + private final String delegate; + private final CountDownLatch entered; + private final CountDownLatch release; + private final AtomicBoolean armed = new AtomicBoolean(true); + + LatchingValue(String delegate, CountDownLatch entered, CountDownLatch release) { + this.delegate = delegate; + this.entered = entered; + this.release = release; + } + + private void gate() { + if (armed.compareAndSet(true, false)) { + entered.countDown(); + try { + release.await(60, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + } + + @Override + public int length() { + gate(); + return delegate.length(); + } + + @Override + public char charAt(int index) { + gate(); + return delegate.charAt(index); + } + + @Override + public CharSequence subSequence(int start, int end) { + gate(); + return delegate.subSequence(start, end); + } + + @Override + public String toString() { + gate(); + return delegate; + } + } + public static class MockSecurityHandler extends SecurityConfHandler { private Map m; final BasicAuthPlugin basicAuthPlugin = new BasicAuthPlugin(); diff --git a/solr/solr-ref-guide/modules/deployment-guide/pages/authentication-and-authorization-plugins.adoc b/solr/solr-ref-guide/modules/deployment-guide/pages/authentication-and-authorization-plugins.adoc index 61abd3fc8a75..8fadfbe2e8bb 100644 --- a/solr/solr-ref-guide/modules/deployment-guide/pages/authentication-and-authorization-plugins.adoc +++ b/solr/solr-ref-guide/modules/deployment-guide/pages/authentication-and-authorization-plugins.adoc @@ -119,6 +119,10 @@ With a user-managed cluster, you will need to place `security.json` on each node You can use the authentication and authorization APIs, but with a user-managed cluster you will need to make the same API requests on each node separately. You can also edit `security.json` by hand if you prefer. +In standalone mode on a POSIX file system, Solr saves `security.json` by writing a new file and renaming it over the old one, so a reader never sees a partial file. +The new file does not keep POSIX ACLs or SELinux labels from the old file, so set them again after an edit. +A hard-linked `security.json` is written in place instead, so its links keep sharing the same content. + [#configuring-authentication] == Authentication