[PATCH] pdb_set_*

Andrew Bartlett abartlet at samba.org
Mon Oct 7 11:52:00 GMT 2002


"Stefan (metze) Metzmacher" wrote:
> 
> Hi Andrew,
> 
> here're the first steps of my pdb_set_* patch (THIS is NOT ready!!!)
> 
> every pdb_set function gets a uint8 flag (DEFAULT | SET | CHANGED)
> 
> then the backends can decide to only store changed values...
> 
> metze

Looks like a good start - a few comments below...

-----------------------------------------------------------------------------
> Stefan "metze" Metzmacher <metze at metzemix.de>
> 
>   ------------------------------------------------------------------------
> diff -Npur --exclude=CVS --exclude=*.bak --exclude=*.o --exclude=*.po --exclude=.#* HEAD/source/include/smb.h HEAD-pdb/source/include/smb.h
> --- HEAD/source/include/smb.h   Mon Oct  7 06:58:17 2002
> +++ HEAD-pdb/source/include/smb.h       Mon Oct  7 09:55:35 2002
> @@ -569,25 +569,67 @@ typedef struct {
>  /*
>   * bit flags representing initialized fields in SAM_ACCOUNT
>   */
> -#define FLAG_SAM_UNINIT                0x00000000
> -#define FLAG_SAM_UID           0x00000001
> -#define FLAG_SAM_GID           0x00000002
> -#define FLAG_SAM_SMBHOME       0x00000004
> -#define FLAG_SAM_PROFILE       0x00000008
> -#define FLAG_SAM_DRIVE          0x00000010
> -#define FLAG_SAM_LOGONSCRIPT   0x00000020
> -#define FLAG_SAM_LOGONTIME     0x00000040
> -#define FLAG_SAM_LOGOFFTIME    0x00000080
> -#define FLAG_SAM_KICKOFFTIME   0x00000100
> -#define FLAG_SAM_CANCHANGETIME 0x00000200
> -#define FLAG_SAM_MUSTCHANGETIME        0x00000400
> -#define FLAG_SAM_PLAINTEXT_PW   0x00000800
> +#define FLAG_SAM_UNINIT                        0x00000000
> +#define FLAG_SAM_USERNAME_CHANGED      0x00000001
> +#define FLAG_SAM_FULLNAME_CHANGED      0x00000002
> +#define FLAG_SAM_DOMAIN_DEFAULT                0x00000003
> +#define FLAG_SAM_DOMAIN_CHANGED                0x00000004
> +#define FLAG_SAM_NTUSERNAME_CHANGED    0x00000005
> +#define FLAG_SAM_ACCTDESC_CHANGED      0x00000006

Why not make this an enum?  Then create 3 bitmasks.

I think every element should have a 'default', 'set' and 'changed'
state.

> +
> +/* if you add a flag increment FLAG_SAM_COUNT */
> +#define FLAG_SAM_COUNT                 0x00000031
> +
> +enum(DEFAULT,SET,CHANGED);
> 
>  #define IS_SAM_UNIX_USER(x) \
> -       ((pdb_get_init_flag(x) & FLAG_SAM_UID) \
> -        && (pdb_get_init_flag(x) & FLAG_SAM_GID))
> +       ((pdb_get_init_flag(x,FLAG_SAM_UID_SET) \
> +        && (pdb_get_init_flag(x,FLAG_SAM_GID_SET)))
> 
> -#define IS_SAM_SET(x, flag)    ((x)->private.init_flag & (flag))
> +#define IS_SAM_SET(x, flag)    pdb_get_init_flag(x, flag)
> 
>  typedef struct sam_passwd
>  {
> @@ -599,7 +641,7 @@ typedef struct sam_passwd
> 
>         struct user_data {
>                 /* initiailization flags */
> -               uint32 init_flag;
> +               struct bitmap *init_flag;
> 
>                 time_t logon_time;            /* logon time */
>                 time_t logoff_time;           /* logoff time */
> diff -Npur --exclude=CVS --exclude=*.bak --exclude=*.o --exclude=*.po --exclude=.#* HEAD/source/include/stamp-h HEAD-pdb/source/include/stamp-h
> --- HEAD/source/include/stamp-h Thu Jan  1 01:00:00 1970
> +++ HEAD-pdb/source/include/stamp-h     Thu Sep 19 08:33:52 2002
> @@ -0,0 +1 @@
> +Sun Jul 18 20:32:29 UTC 1999
> diff -Npur --exclude=CVS --exclude=*.bak --exclude=*.o --exclude=*.po --exclude=.#* HEAD/source/passdb/pdb_get_set.c HEAD-pdb/source/passdb/pdb_get_set.c
> --- HEAD/source/passdb/pdb_get_set.c    Fri Sep 27 07:40:04 2002
> +++ HEAD-pdb/source/passdb/pdb_get_set.c        Mon Oct  7 09:22:01 2002
> @@ -178,12 +178,12 @@ const DOM_SID *pdb_get_group_sid(const S
>   * @return the flags indicating the members initialised in the struct.
>   **/
> 
> -uint32 pdb_get_init_flag (const SAM_ACCOUNT *sampass)
> +BOOL pdb_get_init_flag (const SAM_ACCOUNT *sampass, uint32 flag)
>  {
> -        if (sampass)
> -               return sampass->private.init_flag;
> -       else
> -                return FLAG_SAM_UNINIT;
> +        if (!sampass || !sampass->private.init_flags)
> +               return False;
> +
> +        return bitmap_query(sampass->private.init_flags, flag);
>  }
> 
>  uid_t pdb_get_uid (const SAM_ACCOUNT *sampass)
> @@ -334,109 +334,279 @@ uint32 pdb_get_unknown6 (const SAM_ACCOU
>   Collection of set...() functions for SAM_ACCOUNT_INFO.
>   ********************************************************************/
> 
> -BOOL pdb_set_acct_ctrl (SAM_ACCOUNT *sampass, uint16 flags)
> +BOOL pdb_set_acct_ctrl (SAM_ACCOUNT *sampass, uint16 acct_ctrl, uint8 flag)
>  {
> +       uint32 sam_flag_changed = FLAG_SAM_ACCTCTRL_CHANGED;
> +
>         if (!sampass)
>                 return False;
> 
> -       if (sampass) {
> -               sampass->private.acct_ctrl = flags;
> -               return True;
> +       sampass->private.acct_ctrl = acct_ctrl;
> +
> +       switch(flag) {
> +               case CHANGED:
> +                       if (!pdb_set_init_flag(sampass, sam_flag_changed)
> +                               return False;
> +                       break;
> +               case DEFAULT:
> +               case SET:
> +                       if (!pdb_unset_init_flag(sampass, sam_flag_changed)
> +                               return False;
> +                       break;

I'm not sure if this logic looks quite right.  As I mentioned above, I
think it's easist to make this identical for all the attributes.  This
would allow us to make a new procecure:

BOOL set_init_flags(uint32 element, flag);

That would do this swtich 'in general'.  I would set every element as
'default' when we create a SAM_ACCOUNT, and make the changes when we
actually modify it.  So 'changed' would mark both 'set' and 'changed',
while 'default' would mark none (clear both), and 'set' would just mark
'set' (clear changed).  Hmm, I think this means we only need 2
bitmaps...

> @@ -1041,12 +1527,12 @@ BOOL pdb_set_pass_changed_now (SAM_ACCOU
> 
>         if (!account_policy_get(AP_MAX_PASSWORD_AGE, &expire)
>             || (expire==(uint32)-1)) {
> -               if (!pdb_set_pass_must_change_time (sampass, get_time_t_max(), False))
> +               if (!pdb_set_pass_must_change_time (sampass, get_time_t_max(), SET))

Hmm, shouldn't this be 'changed'?

>                         return False;
>         } else {
>                 if (!pdb_set_pass_must_change_time (sampass,
>                                                     pdb_get_pass_last_set_time(sampass)
> -                                                   + expire, True))
> +                                                   + expire, CHANGED))
>                         return False;
>         }
> 
> @@ -1068,13 +1554,13 @@ BOOL pdb_set_plaintext_passwd (SAM_ACCOU
> 
>         nt_lm_owf_gen (plaintext, new_nt_p16, new_lanman_p16);
> 
> -       if (!pdb_set_nt_passwd (sampass, new_nt_p16))
> +       if (!pdb_set_nt_passwd (sampass, new_nt_p16, CHANGED))
>                 return False;
> 
> -       if (!pdb_set_lanman_passwd (sampass, new_lanman_p16))
> +       if (!pdb_set_lanman_passwd (sampass, new_lanman_p16, CHANGED))
>                 return False;
> 
> -       if (!pdb_set_plaintext_pw_only (sampass, plaintext))
> +       if (!pdb_set_plaintext_pw_only (sampass, plaintext, CHANGED))
>                 return False;
> 
>         if (!pdb_set_pass_changed_now (sampass))

These look right.

Andrew Bartlett

-- 
Andrew Bartlett                                 abartlet at pcug.org.au
Manager, Authentication Subsystems, Samba Team  abartlet at samba.org
Student Network Administrator, Hawker College   abartlet at hawkerc.net
http://samba.org     http://build.samba.org     http://hawkerc.net



More information about the samba-technical mailing list