lkml.org 
[lkml]   [2018]   [Jan]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.14 088/118] apparmor: fix ptrace label match when matching stacked labels
    Date
    4.14-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: John Johansen <john.johansen@canonical.com>

    commit 0dda0b3fb255048a221f736c8a2a24c674da8bf3 upstream.

    Given a label with a profile stack of
    A//&B or A//&C ...

    A ptrace rule should be able to specify a generic trace pattern with
    a rule like

    ptrace trace A//&**,

    however this is failing because while the correct label match routine
    is called, it is being done post label decomposition so it is always
    being done against a profile instead of the stacked label.

    To fix this refactor the cross check to pass the full peer label in to
    the label_match.

    Fixes: 290f458a4f16 ("apparmor: allow ptrace checks to be finer grained than just capability")
    Reported-by: Matthew Garrett <mjg59@google.com>
    Tested-by: Matthew Garrett <mjg59@google.com>
    Signed-off-by: John Johansen <john.johansen@canonical.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    security/apparmor/include/perms.h | 3 ++
    security/apparmor/ipc.c | 53 ++++++++++++++++++++++----------------
    2 files changed, 35 insertions(+), 21 deletions(-)

    --- a/security/apparmor/include/perms.h
    +++ b/security/apparmor/include/perms.h
    @@ -133,6 +133,9 @@ extern struct aa_perms allperms;
    #define xcheck_labels_profiles(L1, L2, FN, args...) \
    xcheck_ns_labels((L1), (L2), xcheck_ns_profile_label, (FN), args)

    +#define xcheck_labels(L1, L2, P, FN1, FN2) \
    + xcheck(fn_for_each((L1), (P), (FN1)), fn_for_each((L2), (P), (FN2)))
    +

    void aa_perm_mask_to_str(char *str, const char *chrs, u32 mask);
    void aa_audit_perm_names(struct audit_buffer *ab, const char **names, u32 mask);
    --- a/security/apparmor/ipc.c
    +++ b/security/apparmor/ipc.c
    @@ -64,40 +64,48 @@ static void audit_ptrace_cb(struct audit
    FLAGS_NONE, GFP_ATOMIC);
    }

    +/* assumes check for PROFILE_MEDIATES is already done */
    /* TODO: conditionals */
    static int profile_ptrace_perm(struct aa_profile *profile,
    - struct aa_profile *peer, u32 request,
    - struct common_audit_data *sa)
    + struct aa_label *peer, u32 request,
    + struct common_audit_data *sa)
    {
    struct aa_perms perms = { };

    - /* need because of peer in cross check */
    - if (profile_unconfined(profile) ||
    - !PROFILE_MEDIATES(profile, AA_CLASS_PTRACE))
    - return 0;
    -
    - aad(sa)->peer = &peer->label;
    - aa_profile_match_label(profile, &peer->label, AA_CLASS_PTRACE, request,
    + aad(sa)->peer = peer;
    + aa_profile_match_label(profile, peer, AA_CLASS_PTRACE, request,
    &perms);
    aa_apply_modes_to_perms(profile, &perms);
    return aa_check_perms(profile, &perms, request, sa, audit_ptrace_cb);
    }

    -static int cross_ptrace_perm(struct aa_profile *tracer,
    - struct aa_profile *tracee, u32 request,
    - struct common_audit_data *sa)
    +static int profile_tracee_perm(struct aa_profile *tracee,
    + struct aa_label *tracer, u32 request,
    + struct common_audit_data *sa)
    {
    + if (profile_unconfined(tracee) || unconfined(tracer) ||
    + !PROFILE_MEDIATES(tracee, AA_CLASS_PTRACE))
    + return 0;
    +
    + return profile_ptrace_perm(tracee, tracer, request, sa);
    +}
    +
    +static int profile_tracer_perm(struct aa_profile *tracer,
    + struct aa_label *tracee, u32 request,
    + struct common_audit_data *sa)
    +{
    + if (profile_unconfined(tracer))
    + return 0;
    +
    if (PROFILE_MEDIATES(tracer, AA_CLASS_PTRACE))
    - return xcheck(profile_ptrace_perm(tracer, tracee, request, sa),
    - profile_ptrace_perm(tracee, tracer,
    - request << PTRACE_PERM_SHIFT,
    - sa));
    - /* policy uses the old style capability check for ptrace */
    - if (profile_unconfined(tracer) || tracer == tracee)
    + return profile_ptrace_perm(tracer, tracee, request, sa);
    +
    + /* profile uses the old style capability check for ptrace */
    + if (&tracer->label == tracee)
    return 0;

    aad(sa)->label = &tracer->label;
    - aad(sa)->peer = &tracee->label;
    + aad(sa)->peer = tracee;
    aad(sa)->request = 0;
    aad(sa)->error = aa_capable(&tracer->label, CAP_SYS_PTRACE, 1);

    @@ -115,10 +123,13 @@ static int cross_ptrace_perm(struct aa_p
    int aa_may_ptrace(struct aa_label *tracer, struct aa_label *tracee,
    u32 request)
    {
    + struct aa_profile *profile;
    + u32 xrequest = request << PTRACE_PERM_SHIFT;
    DEFINE_AUDIT_DATA(sa, LSM_AUDIT_DATA_NONE, OP_PTRACE);

    - return xcheck_labels_profiles(tracer, tracee, cross_ptrace_perm,
    - request, &sa);
    + return xcheck_labels(tracer, tracee, profile,
    + profile_tracer_perm(profile, tracee, request, &sa),
    + profile_tracee_perm(profile, tracer, xrequest, &sa));
    }



    \
     
     \ /
      Last update: 2018-01-15 13:56    [W:4.241 / U:0.128 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site