| Message ID | alpine.DEB.2.00.1104051458350.25803@peruna (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 5 Apr 2011 14:59:09 -0700 (PDT) John Calixto <john.calixto@modsystems.com> wrote: > Some ACMDs might actually damage the card. This check ensures the > caller has CAP_SYS_ADMIN before allowing such an activity. CAP_SYS_RAWIO for "can damage stuff". -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 5 Apr 2011, Alan Cox wrote: > CAP_SYS_RAWIO for "can damage stuff". > Will do. Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 5, 2011 at 4:59 PM, John Calixto <john.calixto@modsystems.com> wrote: > Some ACMDs might actually damage the card. This check ensures the > caller has CAP_SYS_ADMIN before allowing such an activity. > > Signed-off-by: John Calixto <john.calixto@modsystems.com> > --- > drivers/mmc/card/block.c | 29 +++++++++++++++++++++++++++++ > 1 files changed, 29 insertions(+), 0 deletions(-) > > diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c > index c2e107c..2ed8c57 100644 > --- a/drivers/mmc/card/block.c > +++ b/drivers/mmc/card/block.c > @@ -31,6 +31,7 @@ > #include <linux/mutex.h> > #include <linux/scatterlist.h> > #include <linux/string_helpers.h> > +#include <linux/capability.h> > #include <linux/compat.h> > #include <linux/delay.h> > > @@ -205,6 +206,34 @@ static int mmc_blk_ioctl_acmd(struct block_device *bdev, > goto acmd_done; > } > > + /* > + * The following ACMDs are known to be nondestructive. They are used > + * by SD security applications (ref: SD Specifications, Part 1, > + * Physical Layer Simplified Specification, Version 3.01, Table 4-27). > + * Any other commands require CAP_SYS_ADMIN because they may destroy > + * the card. > + */ > + switch (sdic.opcode) { > + case SD_APP_SD_STATUS: > + case 18: > + case 25: > + case 26: > + case 38: > + case 43: > + case 44: > + case 45: > + case 46: > + case 47: > + case 48: > + case 49: > + break; > + default: > + if (!capable(CAP_SYS_ADMIN)) { > + err = -EPERM; > + goto acmd_done; > + } > + } > + > cmd.opcode = sdic.opcode; > cmd.arg = sdic.arg; > cmd.flags = sdic.flags; > -- > 1.7.4.1 Hi John, Just to clarify, were the commands supposed to exclude commands that do writes? In MMC-land: CMD18 is PROGRAM_CID (which is a once-in-a-lifetime operation). CMD38 is erase CMD25 is write_multiple_block - this can give a non-root user full control over a disk, bypassing security. You should definitely check if the the target device is MMC or SD. Because the MMC and SD have slightly differing command sets. A -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 5, 2011 at 6:04 PM, Andrei Warkentin <andreiw@motorola.com> wrote: > On Tue, Apr 5, 2011 at 4:59 PM, John Calixto > <john.calixto@modsystems.com> wrote: >> Some ACMDs might actually damage the card. This check ensures the >> caller has CAP_SYS_ADMIN before allowing such an activity. >> >> Signed-off-by: John Calixto <john.calixto@modsystems.com> >> --- >> drivers/mmc/card/block.c | 29 +++++++++++++++++++++++++++++ >> 1 files changed, 29 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c >> index c2e107c..2ed8c57 100644 >> --- a/drivers/mmc/card/block.c >> +++ b/drivers/mmc/card/block.c >> @@ -31,6 +31,7 @@ >> #include <linux/mutex.h> >> #include <linux/scatterlist.h> >> #include <linux/string_helpers.h> >> +#include <linux/capability.h> >> #include <linux/compat.h> >> #include <linux/delay.h> >> >> @@ -205,6 +206,34 @@ static int mmc_blk_ioctl_acmd(struct block_device *bdev, >> goto acmd_done; >> } >> >> + /* >> + * The following ACMDs are known to be nondestructive. They are used >> + * by SD security applications (ref: SD Specifications, Part 1, >> + * Physical Layer Simplified Specification, Version 3.01, Table 4-27). >> + * Any other commands require CAP_SYS_ADMIN because they may destroy >> + * the card. >> + */ >> + switch (sdic.opcode) { >> + case SD_APP_SD_STATUS: >> + case 18: >> + case 25: >> + case 26: >> + case 38: >> + case 43: >> + case 44: >> + case 45: >> + case 46: >> + case 47: >> + case 48: >> + case 49: >> + break; >> + default: >> + if (!capable(CAP_SYS_ADMIN)) { >> + err = -EPERM; >> + goto acmd_done; >> + } >> + } >> + >> cmd.opcode = sdic.opcode; >> cmd.arg = sdic.arg; >> cmd.flags = sdic.flags; >> -- >> 1.7.4.1 > > Hi John, > > Just to clarify, were the commands supposed to exclude commands that do writes? > > In MMC-land: > CMD18 is PROGRAM_CID (which is a once-in-a-lifetime operation). > CMD38 is erase > CMD25 is write_multiple_block - this can give a non-root user full > control over a disk, bypassing security. > > You should definitely check if the the target device is MMC or SD. > Because the MMC and SD have slightly differing command sets. > > A > Let me clarify, this is what the MMC spec says: The only effect of the APP_CMD is that if the command index of the, immediately, following command has an ACMD overloading, the non standard version will used. If, as an example, a card has a definition for ACMD13 but not for ACMD7 then, if received immediately after APP_CMD command, Command 13 will be interpreted as the non standard ACMD13 but, command 7 as the standard CMD7. In order to use one of the manufacturer specific ACMD’s the host will: * Send APP_CMD. The response will have the APP_CMD bit (new status bit) set signaling ... ... ... If a non valid command is sent (neither ACMD nor CMD) then it will be handled as a standard MultiMediaCard illegal command error. This means, since an MMC card doesn't support ACMD25 or ACMD38, these will be executed as CMD25 or CMD38. I.e. - it's an attack vector. A -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 5 Apr 2011, Andrei Warkentin wrote: > On Tue, Apr 5, 2011 at 4:59 PM, John Calixto > <john.calixto@modsystems.com> wrote: > > Some ACMDs might actually damage the card. This check ensures the > > caller has CAP_SYS_ADMIN before allowing such an activity. > > > > Signed-off-by: John Calixto <john.calixto@modsystems.com> > > --- > > drivers/mmc/card/block.c | 29 +++++++++++++++++++++++++++++ > > 1 files changed, 29 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c > > index c2e107c..2ed8c57 100644 > > --- a/drivers/mmc/card/block.c > > +++ b/drivers/mmc/card/block.c > > @@ -31,6 +31,7 @@ > > #include <linux/mutex.h> > > #include <linux/scatterlist.h> > > #include <linux/string_helpers.h> > > +#include <linux/capability.h> > > #include <linux/compat.h> > > #include <linux/delay.h> > > > > @@ -205,6 +206,34 @@ static int mmc_blk_ioctl_acmd(struct block_device *bdev, > > goto acmd_done; > > } > > > > + /* > > + * The following ACMDs are known to be nondestructive. They are used > > + * by SD security applications (ref: SD Specifications, Part 1, > > + * Physical Layer Simplified Specification, Version 3.01, Table 4-27). > > + * Any other commands require CAP_SYS_ADMIN because they may destroy > > + * the card. > > + */ > > + switch (sdic.opcode) { > > + case SD_APP_SD_STATUS: > > + case 18: > > + case 25: > > + case 26: > > + case 38: > > + case 43: > > + case 44: > > + case 45: > > + case 46: > > + case 47: > > + case 48: > > + case 49: > > + break; > > + default: > > + if (!capable(CAP_SYS_ADMIN)) { > > + err = -EPERM; > > + goto acmd_done; > > + } > > + } > > + > > cmd.opcode = sdic.opcode; > > cmd.arg = sdic.arg; > > cmd.flags = sdic.flags; > > -- > > 1.7.4.1 > > Hi John, > > Just to clarify, were the commands supposed to exclude commands that do writes? > > In MMC-land: > CMD18 is PROGRAM_CID (which is a once-in-a-lifetime operation). > CMD38 is erase > CMD25 is write_multiple_block - this can give a non-root user full > control over a disk, bypassing security. Hi Andrei, I have CMD18 as READ_MULTIPLE_BLOCK... Regardless, this ioctl is specifically for ACMD opcodes (application-specific; preceeded by CMD55), not CMD opcodes. > > You should definitely check if the the target device is MMC or SD. > Because the MMC and SD have slightly differing command sets. > Given that this patch is a conservative opcode filter (it demands CAP_SYS_ADMIN for all opcodes *except* those listed explicitly), do you think I still need to add another check for SD vs. MMC? John
On Tue, 5 Apr 2011, Andrei Warkentin wrote: > Let me clarify, this is what the MMC spec says: > > > The only effect of the APP_CMD is that if the command index of > the, immediately, following command > has an ACMD overloading, the non standard version will used. If, > as an example, a card has a definition > for ACMD13 but not for ACMD7 then, if received immediately after > APP_CMD command, Command 13 > will be interpreted as the non standard ACMD13 but, command 7 as > the standard CMD7. > In order to use one of the manufacturer specific ACMD’s the host will: > * Send APP_CMD. The response will have the APP_CMD bit (new > status bit) set signaling > ... > ... > ... > If a non valid command is sent (neither ACMD nor CMD) then it > will be handled as a standard > MultiMediaCard illegal command error. > > This means, since an MMC card doesn't support ACMD25 or ACMD38, these > will be executed as CMD25 or CMD38. I.e. - it's an attack vector. > Ah I see. OK, that makes sense. I'll check to make sure to check that the allowed ACMDs are just for SD cards. Thanks for catching that! John
On Tue, Apr 5, 2011 at 6:40 PM, John Calixto <john.calixto@modsystems.com> wrote: >> >> In MMC-land: >> CMD18 is PROGRAM_CID (which is a once-in-a-lifetime operation). >> CMD38 is erase >> CMD25 is write_multiple_block - this can give a non-root user full >> control over a disk, bypassing security. > > Hi Andrei, > > I have CMD18 as READ_MULTIPLE_BLOCK... Regardless, this ioctl is > specifically for ACMD opcodes (application-specific; preceeded by > CMD55), not CMD opcodes. I'm sorry, I meant CMD26 instead of 18. Could you check the SD behavior for undefined ACMDs? If I do ACMD25, and ACMD25 is not defined, will it be executed as CMD25? This is the MMC behavior as I have mentioned. If so, that means you will be able to bypass access control and be able to (at the very least) read/write block as non-root. Is there a way for SD to verify which ACMDs the card actually supports? As far as MMC is concerned - no. I really wish ACMD had their own classes as well. A -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 5 Apr 2011, Andrei Warkentin wrote: > Could you check the SD behavior for undefined ACMDs? If I do ACMD25, > and ACMD25 is not defined, will it be executed as CMD25? This is the > MMC behavior as I have mentioned. The SD spec has a similar paragraph about unspecified ACMD opcodes - they should be treated as the equivalent CMD opcodes. However, all of the opcodes that I listed are indeed specified. So, if the card identifies itself as an SD card, it must implement those opcodes. If it identifies itself as an SD card, and it does *not* implement the ACMD opcodes I listed, then it likely "fell out the back door of the manufacturing facility and landed on a store shelf" ;) ... its quality/correct performance/reliability is unlikely. > If so, that means you will be able to bypass access control and be > able to (at the very least) read/write block as non-root. > In my opinion, this is where we should defer to the UNIX file permissions on the device node. I know that my normal user account does not have permission to open() the device node directly (do distros really do that???). I think that if you are allowing a non-root user to open() the device node for your mmcblk device, you take on the all the risk that entails. > Is there a way for SD to verify which ACMDs the card actually > supports? As far as MMC is concerned - no. I really wish ACMD had > their own classes as well. > Unfortunately not. Would it help to also check (mode & FMODE_WRITE) before allowing these operations? At least then, the actions are limited to what you could already do with plain old read(). John -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2011/4/6 John Calixto <john.calixto@modsystems.com>: > On Tue, 5 Apr 2011, Andrei Warkentin wrote: >> Could you check the SD behavior for undefined ACMDs? If I do ACMD25, >> and ACMD25 is not defined, will it be executed as CMD25? This is the >> MMC behavior as I have mentioned. > The SD spec has a similar paragraph about unspecified ACMD opcodes - > they should be treated as the equivalent CMD opcodes. However, all of > the opcodes that I listed are indeed specified. So, if the card > identifies itself as an SD card, it must implement those opcodes. If > it identifies itself as an SD card, and it does *not* implement the ACMD > opcodes I listed, then it likely "fell out the back door of the > manufacturing facility and landed on a store shelf" ;) ... its > quality/correct performance/reliability is unlikely. Not all specified ACMDs are mandatory. SD spec 2.00 says that only 6,13,41,42,51 are mandatory for all cards. This is less than the list you prepared in the patch. It's better not to make those exceptions and instead prepare an userspace daemon that opens mmcblkX (non-exclusively) and forwards the requests from unprivileged users to the device (or even implements application specific API). Best Regards, Micha? Miros?aw -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 6 Apr 2011, Micha? Miros?aw wrote: > 2011/4/6 John Calixto <john.calixto@modsystems.com>: > > On Tue, 5 Apr 2011, Andrei Warkentin wrote: > >> Could you check the SD behavior for undefined ACMDs? If I do ACMD25, > >> and ACMD25 is not defined, will it be executed as CMD25? This is the > >> MMC behavior as I have mentioned. > > The SD spec has a similar paragraph about unspecified ACMD opcodes - > > they should be treated as the equivalent CMD opcodes. However, all of > > the opcodes that I listed are indeed specified. So, if the card > > identifies itself as an SD card, it must implement those opcodes. If > > it identifies itself as an SD card, and it does *not* implement the ACMD > > opcodes I listed, then it likely "fell out the back door of the > > manufacturing facility and landed on a store shelf" ;) ... its > > quality/correct performance/reliability is unlikely. > > Not all specified ACMDs are mandatory. SD spec 2.00 says that only > 6,13,41,42,51 are mandatory for all cards. This is less than the list > you prepared in the patch. > Hmm... In several places of the full SD spec, and even in the redacted "SD Specifications, Part 1, Physical Layer, Simplified Specification, Version 3.01", it states the security functions are mandatory for all SD cards except ROM cards. For ROM cards, the security ACMDs are expected to be treated as illegal commands, and do not fall back to their CMD equivalents. > It's better not to make those exceptions and instead prepare an > userspace daemon that opens mmcblkX (non-exclusively) and forwards the > requests from unprivileged users to the device (or even implements > application specific API). > OK, this sounds reasonable to me (although I still do not see why non-root, non-privileged users have open() access to /dev/mmcblk0). I will replace the opcode filter with an unconditional check for CAP_SYS_RAWIO. With this capability check, I think I can skip checking for SD vs. MMC. It would be nice for this ioctl to have more general purpose than just enabling secure operations on SD cards (see Arnd Bergmann's intended use as a passthrough for virtual machines). Regarding permissions, the next iteration of this patch will: - Be a single patch containing the main ioctl functionality *and* this capability verification (checking "CAP_SYS_RAWIO", per Alan Cox in this thread). - Only have the check for CAP_SYS_RAWIO. It will *not* have an opcode-specific filter, nor a filter limiting it to just SD cards. Is that satisfactory? John
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c index c2e107c..2ed8c57 100644 --- a/drivers/mmc/card/block.c +++ b/drivers/mmc/card/block.c @@ -31,6 +31,7 @@ #include <linux/mutex.h> #include <linux/scatterlist.h> #include <linux/string_helpers.h> +#include <linux/capability.h> #include <linux/compat.h> #include <linux/delay.h> @@ -205,6 +206,34 @@ static int mmc_blk_ioctl_acmd(struct block_device *bdev, goto acmd_done; } + /* + * The following ACMDs are known to be nondestructive. They are used + * by SD security applications (ref: SD Specifications, Part 1, + * Physical Layer Simplified Specification, Version 3.01, Table 4-27). + * Any other commands require CAP_SYS_ADMIN because they may destroy + * the card. + */ + switch (sdic.opcode) { + case SD_APP_SD_STATUS: + case 18: + case 25: + case 26: + case 38: + case 43: + case 44: + case 45: + case 46: + case 47: + case 48: + case 49: + break; + default: + if (!capable(CAP_SYS_ADMIN)) { + err = -EPERM; + goto acmd_done; + } + } + cmd.opcode = sdic.opcode; cmd.arg = sdic.arg; cmd.flags = sdic.flags;
Some ACMDs might actually damage the card. This check ensures the caller has CAP_SYS_ADMIN before allowing such an activity. Signed-off-by: John Calixto <john.calixto@modsystems.com> --- drivers/mmc/card/block.c | 29 +++++++++++++++++++++++++++++ 1 files changed, 29 insertions(+), 0 deletions(-)