Skip to content

Commit a9543a3

Browse files
krutonandroid code review
authored andcommitted
Merge "Pass additional inputs when spawning apps via the Zygote and add SELinux permission checks."
2 parents 20d6caf + 83d9eda commit a9543a3

File tree

3 files changed

+116
-11
lines changed

3 files changed

+116
-11
lines changed

core/java/android/os/Process.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,7 @@ public class Process {
274274
* @param gids Additional group-ids associated with the process.
275275
* @param debugFlags Additional flags.
276276
* @param targetSdkVersion The target SDK version for the app.
277+
* @param seInfo null-ok SE Android information for the new process.
277278
* @param zygoteArgs Additional arguments to supply to the zygote process.
278279
*
279280
* @return An object that describes the result of the attempt to start the process.
@@ -285,10 +286,11 @@ public static final ProcessStartResult start(final String processClass,
285286
final String niceName,
286287
int uid, int gid, int[] gids,
287288
int debugFlags, int targetSdkVersion,
289+
String seInfo,
288290
String[] zygoteArgs) {
289291
try {
290292
return startViaZygote(processClass, niceName, uid, gid, gids,
291-
debugFlags, targetSdkVersion, zygoteArgs);
293+
debugFlags, targetSdkVersion, seInfo, zygoteArgs);
292294
} catch (ZygoteStartFailedEx ex) {
293295
Log.e(LOG_TAG,
294296
"Starting VM process through Zygote failed");
@@ -451,6 +453,7 @@ private static ProcessStartResult zygoteSendArgsAndGetResult(ArrayList<String> a
451453
* new process should setgroup() to.
452454
* @param debugFlags Additional flags.
453455
* @param targetSdkVersion The target SDK version for the app.
456+
* @param seInfo null-ok SE Android information for the new process.
454457
* @param extraArgs Additional arguments to supply to the zygote process.
455458
* @return An object that describes the result of the attempt to start the process.
456459
* @throws ZygoteStartFailedEx if process start failed for any reason
@@ -460,6 +463,7 @@ private static ProcessStartResult startViaZygote(final String processClass,
460463
final int uid, final int gid,
461464
final int[] gids,
462465
int debugFlags, int targetSdkVersion,
466+
String seInfo,
463467
String[] extraArgs)
464468
throws ZygoteStartFailedEx {
465469
synchronized(Process.class) {
@@ -510,6 +514,10 @@ private static ProcessStartResult startViaZygote(final String processClass,
510514
argsForZygote.add("--nice-name=" + niceName);
511515
}
512516

517+
if (seInfo != null) {
518+
argsForZygote.add("--seinfo=" + seInfo);
519+
}
520+
513521
argsForZygote.add(processClass);
514522

515523
if (extraArgs != null) {

core/java/com/android/internal/os/ZygoteConnection.java

Lines changed: 106 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import dalvik.system.PathClassLoader;
2727
import dalvik.system.Zygote;
2828

29+
import android.os.SELinux;
30+
2931
import java.io.BufferedReader;
3032
import java.io.DataInputStream;
3133
import java.io.DataOutputStream;
@@ -73,6 +75,7 @@ class ZygoteConnection {
7375
private final DataOutputStream mSocketOutStream;
7476
private final BufferedReader mSocketReader;
7577
private final Credentials peer;
78+
private final String peerSecurityContext;
7679

7780
/**
7881
* A long-lived reference to the original command socket used to launch
@@ -109,6 +112,8 @@ class ZygoteConnection {
109112
Log.e(TAG, "Cannot read peer credentials", ex);
110113
throw ex;
111114
}
115+
116+
peerSecurityContext = SELinux.getPeerContext(mSocket.getFileDescriptor());
112117
}
113118

114119
/**
@@ -207,10 +212,11 @@ boolean runOnce() throws ZygoteInit.MethodAndArgsCaller {
207212
try {
208213
parsedArgs = new Arguments(args);
209214

210-
applyUidSecurityPolicy(parsedArgs, peer);
211-
applyRlimitSecurityPolicy(parsedArgs, peer);
212-
applyCapabilitiesSecurityPolicy(parsedArgs, peer);
213-
applyInvokeWithSecurityPolicy(parsedArgs, peer);
215+
applyUidSecurityPolicy(parsedArgs, peer, peerSecurityContext);
216+
applyRlimitSecurityPolicy(parsedArgs, peer, peerSecurityContext);
217+
applyCapabilitiesSecurityPolicy(parsedArgs, peer, peerSecurityContext);
218+
applyInvokeWithSecurityPolicy(parsedArgs, peer, peerSecurityContext);
219+
applyseInfoSecurityPolicy(parsedArgs, peer, peerSecurityContext);
214220

215221
applyDebuggerSystemProperty(parsedArgs);
216222
applyInvokeWithSystemProperty(parsedArgs);
@@ -229,7 +235,8 @@ boolean runOnce() throws ZygoteInit.MethodAndArgsCaller {
229235
}
230236

231237
pid = Zygote.forkAndSpecialize(parsedArgs.uid, parsedArgs.gid,
232-
parsedArgs.gids, parsedArgs.debugFlags, rlimits);
238+
parsedArgs.gids, parsedArgs.debugFlags, rlimits,
239+
parsedArgs.seInfo, parsedArgs.niceName);
233240
} catch (IOException ex) {
234241
logAndPrintError(newStderr, "Exception creating pipe", ex);
235242
} catch (ErrnoException ex) {
@@ -352,6 +359,10 @@ static class Arguments {
352359
long permittedCapabilities;
353360
long effectiveCapabilities;
354361

362+
/** from --seinfo */
363+
boolean seInfoSpecified;
364+
String seInfo;
365+
355366
/** from all --rlimit=r,c,m */
356367
ArrayList<int[]> rlimits;
357368

@@ -429,6 +440,13 @@ private void parseArgs(String args[])
429440
peerWait = true;
430441
} else if (arg.equals("--runtime-init")) {
431442
runtimeInit = true;
443+
} else if (arg.startsWith("--seinfo=")) {
444+
if (seInfoSpecified) {
445+
throw new IllegalArgumentException(
446+
"Duplicate arg specified");
447+
}
448+
seInfoSpecified = true;
449+
seInfo = arg.substring(arg.indexOf('=') + 1);
432450
} else if (arg.startsWith("--capabilities=")) {
433451
if (capabilitiesSpecified) {
434452
throw new IllegalArgumentException(
@@ -591,7 +609,8 @@ private String[] readArgumentList()
591609
* @param peer non-null; peer credentials
592610
* @throws ZygoteSecurityException
593611
*/
594-
private static void applyUidSecurityPolicy(Arguments args, Credentials peer)
612+
private static void applyUidSecurityPolicy(Arguments args, Credentials peer,
613+
String peerSecurityContext)
595614
throws ZygoteSecurityException {
596615

597616
int peerUid = peer.getUid();
@@ -624,6 +643,17 @@ private static void applyUidSecurityPolicy(Arguments args, Credentials peer)
624643
}
625644
}
626645

646+
if (args.uidSpecified || args.gidSpecified || args.gids != null) {
647+
boolean allowed = SELinux.checkSELinuxAccess(peerSecurityContext,
648+
peerSecurityContext,
649+
"zygote",
650+
"specifyids");
651+
if (!allowed) {
652+
throw new ZygoteSecurityException(
653+
"Peer may not specify uid's or gid's");
654+
}
655+
}
656+
627657
// If not otherwise specified, uid and gid are inherited from peer
628658
if (!args.uidSpecified) {
629659
args.uid = peer.getUid();
@@ -664,7 +694,7 @@ public static void applyDebuggerSystemProperty(Arguments args) {
664694
* @throws ZygoteSecurityException
665695
*/
666696
private static void applyRlimitSecurityPolicy(
667-
Arguments args, Credentials peer)
697+
Arguments args, Credentials peer, String peerSecurityContext)
668698
throws ZygoteSecurityException {
669699

670700
int peerUid = peer.getUid();
@@ -676,6 +706,17 @@ private static void applyRlimitSecurityPolicy(
676706
"This UID may not specify rlimits.");
677707
}
678708
}
709+
710+
if (args.rlimits != null) {
711+
boolean allowed = SELinux.checkSELinuxAccess(peerSecurityContext,
712+
peerSecurityContext,
713+
"zygote",
714+
"specifyrlimits");
715+
if (!allowed) {
716+
throw new ZygoteSecurityException(
717+
"Peer may not specify rlimits");
718+
}
719+
}
679720
}
680721

681722
/**
@@ -689,7 +730,7 @@ private static void applyRlimitSecurityPolicy(
689730
* @throws ZygoteSecurityException
690731
*/
691732
private static void applyCapabilitiesSecurityPolicy(
692-
Arguments args, Credentials peer)
733+
Arguments args, Credentials peer, String peerSecurityContext)
693734
throws ZygoteSecurityException {
694735

695736
if (args.permittedCapabilities == 0
@@ -698,6 +739,15 @@ private static void applyCapabilitiesSecurityPolicy(
698739
return;
699740
}
700741

742+
boolean allowed = SELinux.checkSELinuxAccess(peerSecurityContext,
743+
peerSecurityContext,
744+
"zygote",
745+
"specifycapabilities");
746+
if (!allowed) {
747+
throw new ZygoteSecurityException(
748+
"Peer may not specify capabilities");
749+
}
750+
701751
if (peer.getUid() == 0) {
702752
// root may specify anything
703753
return;
@@ -747,14 +797,61 @@ private static void applyCapabilitiesSecurityPolicy(
747797
* @param peer non-null; peer credentials
748798
* @throws ZygoteSecurityException
749799
*/
750-
private static void applyInvokeWithSecurityPolicy(Arguments args, Credentials peer)
800+
private static void applyInvokeWithSecurityPolicy(Arguments args, Credentials peer,
801+
String peerSecurityContext)
751802
throws ZygoteSecurityException {
752803
int peerUid = peer.getUid();
753804

754805
if (args.invokeWith != null && peerUid != 0) {
755806
throw new ZygoteSecurityException("Peer is not permitted to specify "
756807
+ "an explicit invoke-with wrapper command");
757808
}
809+
810+
if (args.invokeWith != null) {
811+
boolean allowed = SELinux.checkSELinuxAccess(peerSecurityContext,
812+
peerSecurityContext,
813+
"zygote",
814+
"specifyinvokewith");
815+
if (!allowed) {
816+
throw new ZygoteSecurityException("Peer is not permitted to specify "
817+
+ "an explicit invoke-with wrapper command");
818+
}
819+
}
820+
}
821+
822+
/**
823+
* Applies zygote security policy for SEAndroid information.
824+
*
825+
* @param args non-null; zygote spawner arguments
826+
* @param peer non-null; peer credentials
827+
* @throws ZygoteSecurityException
828+
*/
829+
private static void applyseInfoSecurityPolicy(
830+
Arguments args, Credentials peer, String peerSecurityContext)
831+
throws ZygoteSecurityException {
832+
int peerUid = peer.getUid();
833+
834+
if (args.seInfo == null) {
835+
// nothing to check
836+
return;
837+
}
838+
839+
if (!(peerUid == 0 || peerUid == Process.SYSTEM_UID)) {
840+
// All peers with UID other than root or SYSTEM_UID
841+
throw new ZygoteSecurityException(
842+
"This UID may not specify SEAndroid info.");
843+
}
844+
845+
boolean allowed = SELinux.checkSELinuxAccess(peerSecurityContext,
846+
peerSecurityContext,
847+
"zygote",
848+
"specifyseinfo");
849+
if (!allowed) {
850+
throw new ZygoteSecurityException(
851+
"Peer may not specify SEAndroid info");
852+
}
853+
854+
return;
758855
}
759856

760857
/**

services/java/com/android/server/am/ActivityManagerService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1967,7 +1967,7 @@ private final void startProcessLocked(ProcessRecord app,
19671967
// the PID of the new process, or else throw a RuntimeException.
19681968
Process.ProcessStartResult startResult = Process.start("android.app.ActivityThread",
19691969
app.processName, uid, uid, gids, debugFlags,
1970-
app.info.targetSdkVersion, null);
1970+
app.info.targetSdkVersion, null, null);
19711971

19721972
BatteryStatsImpl bs = app.batteryStats.getBatteryStats();
19731973
synchronized (bs) {

0 commit comments

Comments
 (0)