audit-kernel icon indicating copy to clipboard operation
audit-kernel copied to clipboard

RFE: Improve supported feature reporting

Open WOnder93 opened this issue 7 years ago • 1 comments

I was thinking about the problematic situation with the current feature bitmap approach and I think I came up with a viable long-term solution.

High-level description

My idea is to supersede the current 32-bit bitmap field with two fields:

  1. A 32-bit base feature counter (n). This number would represent the highest feature ID for which it holds that all features with lower and equal ID are implemented in the kernel.
  2. A 32-bit extra feature bitmap, where bit of order k would signal the presence of feature with ID n + 2 + k (we add two instead of one because if feature n + 1 is also present, then we can simply increment n).

In the upstream kernel we would simply increment n on every new feature, because it will always have all features implemented. Whenever we would branch-off the upstream kernel (e.g. for a new RHEL version), we would start with a clean feature bitmap, enabling us to backport up to 33 future features (less if some bits are reserved for special purpose). Even better, every time we would backport the oldest not-yet-backported feature, we would simply increment n (and shift the bitmap), effectively extending the window of features we can backport by one.

Notes / drawbacks

  1. We need to add two new 32-bit fields to the audit_status UAPI structure. We need to keep the old feature_bitmap (and just freeze it at the current list of features) for backwards compatibility. The userspace will need to be able to handle the two possible sizes of the audit_status structure, but this should be easily doable (and old versions should already correctly ignore any extra bytes sent by the kernel).
  2. We won't be able to easily remove (deprecate) a feature as we can now. (However I think this is just an 'accidental' feature of the current system – we probably don't need that ability.)
  3. The userspace 'formula' to determine if a feature is available will be more complex. Something like (example code):
    int has_feature(struct audit_status *status, u32 f)
    {
    	/* AUDIT_FEATURE_BITMAP_V2 set in the old bitmap signals
    	 * the use of the new system: */
    	if (!(status->feature_bitmap & AUDIT_FEATURE_BITMAP_V2))
    		/* Legacy feature system: */
    		return status->feature_bitmap & (1U << f);
    
    	if (f <= status->feature_base)
    		return 1; /* feature implemented */
    
    	if (f == status->feature_base + 1)
    		return 0; /* first feature after the last implemented one */
    
    	if (f > status->feature_base + 32 + 1 /* minus reserved bits */)
    		return 0; /* feature beyond window */
    
    	/* is feature backported? */
    	return status->feature_bitmap_v2 & (1U << (f - status->feature_base - 2));
    }
    
    //...
    if (has_feature(status, AUDIT_STATIC_FEATURE_SESSIONID_FILTER))
    	// do something
    
    vs. the current approach:
    if (status->feature_bitmap & AUDIT_FEATURE_BITMAP_SESSIONID_FILTER)
    	// do something
    
  4. We need to add new macros for the current features, this time represented by incrementing sequence of numbers (not by the sequence of powers of two). For example, we could add the following to audit.h:
    #define AUDIT_STATIC_FEATURE_BACKLOG_LIMIT	0
    #define AUDIT_STATIC_FEATURE_BACKLOG_WAIT_TIME	1
    #define AUDIT_STATIC_FEATURE_EXECUTABLE_PATH	2
    #define AUDIT_STATIC_FEATURE_EXCLUDE_EXTEND	3
    #define AUDIT_STATIC_FEATURE_SESSIONID_FILTER	4
    #define AUDIT_STATIC_FEATURE_LOST_RESET		5
    #define AUDIT_STATIC_FEATURE_FILTER_FS		6
    //...
    
    (I use STATIC here to distinguish these features from the struct audit_features features, which can be dynamically enabled/disabled as I understand it.)

@pcmoore, @rgbriggs, @stevegrubb: What do you think about this idea?

WOnder93 avatar May 22 '18 11:05 WOnder93

On 2018-05-22 04:18, Ondrej Mosnáček wrote:

@pcmoore, @rgbriggs, @stevegrubb: What do you think about this idea?

Brilliant, complex, limiting, difficult to explain to distros, testing permutation explosion...

rgbriggs avatar May 31 '18 21:05 rgbriggs