-
Notifications
You must be signed in to change notification settings - Fork 155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Now MAC filter deny is supported. (Only accept was supported.) #43
base: master
Are you sure you want to change the base?
Conversation
02a153b
to
ee858a8
Compare
Any updates regarding this? @garywill |
Hi, @zaibaq . Your commit has 300+ lines changes, which is too hard for us to review. You can use some GUI tool to choose lines to put into a commit, like |
@@ -179,8 +179,9 @@ define_global_variables(){ | |||
WIFI_IFACE= | |||
CHANNEL=default | |||
WPA_VERSION=2 | |||
MAC_FILTER=0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're changing the meaning of this variable (from "enable mac filter or not" to "macaddr_acl in hostapd.conf" ),
we should use a new variable name MACADDR_ACL
@@ -179,8 +179,9 @@ define_global_variables(){ | |||
WIFI_IFACE= | |||
CHANNEL=default | |||
WPA_VERSION=2 | |||
MAC_FILTER=0 | |||
MAC_FILTER=3 # 3 is not valid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leave it blank (or use -1 or -99) is better. hostapd may add 3 as a valid value in the future
shift | ||
if [ "$MAC_FILTER_TYPE" == "deny" ] | ||
then | ||
printf "ERROR: Can't use --mac-filter-accept and --mac-filter-deny together.\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need >&2
shift | ||
MAC_FILTER_ACCEPT="$1" | ||
if [ "$MAC_FILTER_TYPE" == "accept" ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This variable MAC_FILTER_TYPE
would be unnecessary. Use that MACADDR_ACL
if [[ $MAC_FILTER -eq 0 ]]; then | ||
cat <<- EOF >> "$CONFDIR/hostapd.conf" | ||
macaddr_acl=0 | ||
deny_mac_file=${MAC_FILTER_FILE} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking ...
maybe we can make the script option like this:
--mac-filter-accept <accept_file>
--mac-filter-deny <deny_file>
then variable MAC_FILTER_FILE
and option --mac-filter-file
would be unnecessary .
--mac-filter-accept -
will apply the default path /etc/hostapd/hostapd.accept
.
--mac-filter-deny -
similar to above
What do you think?
This is to support
--mac-filter-deny
after only--mac-filter-accept
was supported.Use:
If you want to use deny acl, use the option
--mac-filter-deny
. Similarly for accept acl, use--mac-filter-accept
.Obviously, you cannot use both options at the same time.
If you want to set a specific path for the mac addresses file, whether you're using deny or accept, you need to set the option
--mac-filter-file <MAC_ADDRESSES_FILE>
.