On 8/4/21 10:53 PM, Lakshmi Ramasubramanian wrote:
> Hi Simon,
>
> On 8/4/2021 2:20 AM, THOBY Simon wrote:
>
>> Signed-off-by: Simon Thoby <simon.thoby@xxxxxxxxxx>
>> Reviewed-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
>> ---
>> Documentation/ABI/testing/ima_policy | 6 ++-
>> security/integrity/ima/ima_policy.c | 75 ++++++++++++++++++++++++++--
>> 2 files changed, 76 insertions(+), 5 deletions(-)
>>
>
>> +static unsigned int ima_parse_appraise_hash(char *arg)
>> +{
>> + unsigned int res = 0;
>> + int idx;
>> + char *token;
>> +
>> + while ((token = strsep(&arg, ",")) != NULL) {
>> + idx = match_string(hash_algo_name, HASH_ALGO__LAST, token);
>> +
>> + if (idx < 0) {
>> + pr_err("unknown hash algorithm \"%s\", ignoring",
>> + token);
>> + continue;
> Is it right to ignore an invalid digest algorithm given in the IMA policy rule? Should "invalid ima policy" error be reported instead?
Um no you're right, better be strict in what we accept, or users may start depending on it.
I'll update that.
Thanks,
Simon
>
> Other changes look good.
>
> Reviewed-by: Lakshmi Ramasubramanian <nramas@xxxxxxxxxxxxxxxxxxx>
>
> -lakshmi
>
>> + }
>> +
>> + /* Add the hash algorithm to the 'allowed' bitfield */
>> + res |= (1U << idx);
>> + }
>> +
>> + return res;
>> +}
>> +
>> static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>> {
>> struct audit_buffer *ab;
>> @@ -1522,6 +1546,26 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>> else
>> result = -EINVAL;
>> break;
>> + case Opt_appraise_hash:
>> + ima_log_string(ab, "appraise_hash", args[0].from);
>> +
>> + if (entry->allowed_hashes) {
>> + result = -EINVAL;
>> + break;
>> + }
>> +
>> + entry->allowed_hashes =
>> + ima_parse_appraise_hash(args[0].from);
>> +
>> + /* invalid or empty list of algorithms */
>> + if (!entry->allowed_hashes) {
>> + result = -EINVAL;
>> + break;
>> + }
>> +
>> + entry->flags |= IMA_VALIDATE_HASH;
>> +
>> + break;
>> case Opt_permit_directio:
>> entry->flags |= IMA_PERMIT_DIRECTIO;
>> break;
>> @@ -1714,6 +1758,23 @@ static void ima_show_rule_opt_list(struct seq_file *m,
>> seq_printf(m, "%s%s", i ? "|" : "", opt_list->items[i]);
>> }
>> +static void ima_policy_show_appraise_hash(struct seq_file *m,
>> + unsigned int allowed_hashes)
>> +{
>> + int idx, list_size = 0;
>> +
>> + for (idx = 0; idx < HASH_ALGO__LAST; idx++) {
>> + if (!(allowed_hashes & (1U << idx)))
>> + continue;
>> +
>> + /* only add commas if the list contains multiple entries */
>> + if (list_size++)
>> + seq_puts(m, ",");
>> +
>> + seq_puts(m, hash_algo_name[idx]);
>> + }
>> +}
>> +
>> int ima_policy_show(struct seq_file *m, void *v)
>> {
>> struct ima_rule_entry *entry = v;
>> @@ -1825,6 +1886,12 @@ int ima_policy_show(struct seq_file *m, void *v)
>> seq_puts(m, " ");
>> }
>> + if (entry->flags & IMA_VALIDATE_HASH) {
>> + seq_puts(m, "appraise_hash=");
>> + ima_policy_show_appraise_hash(m, entry->allowed_hashes);
>> + seq_puts(m, " ");
>> + }
>> +
>> for (i = 0; i < MAX_LSM_RULES; i++) {
>> if (entry->lsm[i].rule) {
>> switch (i) {
>>
