Document valid port range.
Switch the port parameters to long, since Java int can't hold the
largest u32 value.
Throw an exception if out of range values are passed.
Also enforce the >= 1024 restriction in VS, just in case we start
using privileged vsock ports for something.
Use a port number > 2^31 in one of our tests just to make sure it
works.
BYPASS_INCLUSIVE_LANGUAGE_REASON='man' used to refer to the manual command.
Bug: 261037705
Test: atest MicrodroidTests
Change-Id: Icf22d9c2e746f300b184e66469fb6b62c574b6ff
diff --git a/javalib/api/system-current.txt b/javalib/api/system-current.txt
index 04283c2..0fb83ac 100644
--- a/javalib/api/system-current.txt
+++ b/javalib/api/system-current.txt
@@ -4,8 +4,8 @@
public class VirtualMachine implements java.lang.AutoCloseable {
method public void clearCallback();
method public void close();
- method @NonNull public android.os.IBinder connectToVsockServer(int) throws android.system.virtualmachine.VirtualMachineException;
- method @NonNull public android.os.ParcelFileDescriptor connectVsock(int) throws android.system.virtualmachine.VirtualMachineException;
+ method @NonNull public android.os.IBinder connectToVsockServer(@IntRange(from=android.system.virtualmachine.VirtualMachine.MIN_VSOCK_PORT, to=android.system.virtualmachine.VirtualMachine.MAX_VSOCK_PORT) long) throws android.system.virtualmachine.VirtualMachineException;
+ method @NonNull public android.os.ParcelFileDescriptor connectVsock(@IntRange(from=android.system.virtualmachine.VirtualMachine.MIN_VSOCK_PORT, to=android.system.virtualmachine.VirtualMachine.MAX_VSOCK_PORT) long) throws android.system.virtualmachine.VirtualMachineException;
method @NonNull public android.system.virtualmachine.VirtualMachineConfig getConfig();
method @NonNull public java.io.InputStream getConsoleOutput() throws android.system.virtualmachine.VirtualMachineException;
method @NonNull public java.io.InputStream getLogOutput() throws android.system.virtualmachine.VirtualMachineException;
@@ -17,6 +17,8 @@
method public void stop() throws android.system.virtualmachine.VirtualMachineException;
method @NonNull public android.system.virtualmachine.VirtualMachineDescriptor toDescriptor() throws android.system.virtualmachine.VirtualMachineException;
field public static final String MANAGE_VIRTUAL_MACHINE_PERMISSION = "android.permission.MANAGE_VIRTUAL_MACHINE";
+ field public static final long MAX_VSOCK_PORT = 4294967295L; // 0xffffffffL
+ field public static final long MIN_VSOCK_PORT = 1024L; // 0x400L
field public static final int STATUS_DELETED = 2; // 0x2
field public static final int STATUS_RUNNING = 1; // 0x1
field public static final int STATUS_STOPPED = 0; // 0x0
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachine.java b/javalib/src/android/system/virtualmachine/VirtualMachine.java
index 1cf808a..87f9f6a 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachine.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachine.java
@@ -45,9 +45,11 @@
import android.annotation.CallbackExecutor;
import android.annotation.IntDef;
+import android.annotation.IntRange;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.annotation.RequiresPermission;
+import android.annotation.SuppressLint;
import android.annotation.SystemApi;
import android.annotation.TestApi;
import android.content.ComponentCallbacks2;
@@ -121,6 +123,24 @@
"android.permission.USE_CUSTOM_VIRTUAL_MACHINE";
/**
+ * The lowest port number that can be used to communicate with the virtual machine payload.
+ *
+ * @see #connectToVsockServer
+ * @see #connectVsock
+ */
+ @SuppressLint("MinMaxConstant") // Won't change: see man 7 vsock.
+ public static final long MIN_VSOCK_PORT = 1024;
+
+ /**
+ * The highest port number that can be used to communicate with the virtual machine payload.
+ *
+ * @see #connectToVsockServer
+ * @see #connectVsock
+ */
+ @SuppressLint("MinMaxConstant") // Won't change: see man 7 vsock.
+ public static final long MAX_VSOCK_PORT = (1L << 32) - 1;
+
+ /**
* Status of a virtual machine
*
* @hide
@@ -998,9 +1018,13 @@
*/
@SystemApi
@NonNull
- public IBinder connectToVsockServer(int port) throws VirtualMachineException {
+ public IBinder connectToVsockServer(
+ @IntRange(from = MIN_VSOCK_PORT, to = MAX_VSOCK_PORT) long port)
+ throws VirtualMachineException {
+
synchronized (mLock) {
- IBinder iBinder = nativeConnectToVsockServer(getRunningVm().asBinder(), port);
+ IBinder iBinder =
+ nativeConnectToVsockServer(getRunningVm().asBinder(), validatePort(port));
if (iBinder == null) {
throw new VirtualMachineException("Failed to connect to vsock server");
}
@@ -1016,10 +1040,12 @@
*/
@SystemApi
@NonNull
- public ParcelFileDescriptor connectVsock(int port) throws VirtualMachineException {
+ public ParcelFileDescriptor connectVsock(
+ @IntRange(from = MIN_VSOCK_PORT, to = MAX_VSOCK_PORT) long port)
+ throws VirtualMachineException {
synchronized (mLock) {
try {
- return getRunningVm().connectVsock(port);
+ return getRunningVm().connectVsock(validatePort(port));
} catch (RemoteException e) {
throw e.rethrowAsRuntimeException();
} catch (ServiceSpecificException e) {
@@ -1028,6 +1054,16 @@
}
}
+ private int validatePort(long port) {
+ // Ports below 1024 are "privileged" (payload code can't bind to these), and port numbers
+ // are 32-bit unsigned numbers at the OS level, even though we pass them as 32-bit signed
+ // numbers internally.
+ if (port < MIN_VSOCK_PORT || port > MAX_VSOCK_PORT) {
+ throw new IllegalArgumentException("Bad port " + port);
+ }
+ return (int) port;
+ }
+
/**
* Returns the root directory where all files related to this {@link VirtualMachine} (e.g.
* {@code instance.img}, {@code apk.idsig}, etc) are stored.
diff --git a/tests/aidl/com/android/microdroid/testservice/ITestService.aidl b/tests/aidl/com/android/microdroid/testservice/ITestService.aidl
index 077c74f..a4ecc45 100644
--- a/tests/aidl/com/android/microdroid/testservice/ITestService.aidl
+++ b/tests/aidl/com/android/microdroid/testservice/ITestService.aidl
@@ -17,9 +17,9 @@
/** {@hide} */
interface ITestService {
- const int SERVICE_PORT = 5678;
+ const long SERVICE_PORT = 5678;
- const int ECHO_REVERSE_PORT = 6789;
+ const long ECHO_REVERSE_PORT = 0x80000001L; // Deliberately chosen to be > 2^31, < 2^32
/* add two integers. */
int addInteger(int a, int b);
diff --git a/tests/testapk/src/native/testbinary.cpp b/tests/testapk/src/native/testbinary.cpp
index 8a0019d..b6a7aa2 100644
--- a/tests/testapk/src/native/testbinary.cpp
+++ b/tests/testapk/src/native/testbinary.cpp
@@ -112,7 +112,7 @@
}
struct sockaddr_vm server_sa = (struct sockaddr_vm){
.svm_family = AF_VSOCK,
- .svm_port = BnTestService::ECHO_REVERSE_PORT,
+ .svm_port = static_cast<uint32_t>(BnTestService::ECHO_REVERSE_PORT),
.svm_cid = VMADDR_CID_ANY,
};
int ret = TEMP_FAILURE_RETRY(bind(server_fd, (struct sockaddr*)&server_sa, sizeof(server_sa)));
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index 072afec..186052d 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -973,13 +973,16 @@
if !matches!(&*self.instance.vm_state.lock().unwrap(), VmState::Running { .. }) {
return Err(Status::new_service_specific_error_str(-1, Some("VM is not running")));
}
- let stream =
- VsockStream::connect_with_cid_port(self.instance.cid, port as u32).map_err(|e| {
- Status::new_service_specific_error_str(
- -1,
- Some(format!("Failed to connect: {:?}", e)),
- )
- })?;
+ let port = port as u32;
+ if port < 1024 {
+ return Err(Status::new_service_specific_error_str(
+ -1,
+ Some(format!("Can't connect to privileged port {port}")),
+ ));
+ }
+ let stream = VsockStream::connect_with_cid_port(self.instance.cid, port).map_err(|e| {
+ Status::new_service_specific_error_str(-1, Some(format!("Failed to connect: {:?}", e)))
+ })?;
Ok(vsock_stream_to_pfd(stream))
}
}