2005-05-03 15:28:35

by Benjamin Reed

[permalink] [raw]
Subject: [PATCH] dynamic wep keys for airo.c

I'm resubmitting a patch for the 2.6.11.8 aironet driver (airo.c) that will
enable dynamic wep keying without disabling the MAC. It allows
us to use xsupplicant with the driver.

Aironet provides the ability to set WEP keys permanently or
temporarily. There is a special IW_ENCODE_TEMP flag for selecting
which type of key you are setting. However, apart from iwconfig,
nobody seems to use this flag. When a permanent WEP key is set,
the MAC must be disabled. I have added a module parameter to skip
disabling the MAC even if a permanent WEP key is set. Using this
flag allows xsupplicant to work without modification.

ben

Signed-off-by: Benjamin Reed <[email protected]>

--- drivers/net/wireless/airo.c.orig 2005-05-02 10:15:24.000000000 -0700
+++ drivers/net/wireless/airo.c 2005-05-02 11:16:43.000000000 -0700
@@ -224,6 +224,8 @@ static
int maxencrypt /* = 0 */; /* The highest rate that the card can encrypt at.
0 means no limit. For old cards this was 4 */

+static int perm_key_support = 1; /* If set, the MAC will be disabled when
+ permanent wep keys are set. */
static int auto_wep /* = 0 */; /* If set, it tries to figure out the wep mode */
static int aux_bap /* = 0 */; /* Checks to see if the aux ports are needed to read
the bap, needed on some older cards and buses. */
@@ -251,6 +253,13 @@ module_param(basic_rate, int, 0);
module_param_array(rates, int, NULL, 0);
module_param_array(ssids, charp, NULL, 0);
module_param(auto_wep, int, 0);
+module_param(perm_key_support, int, 1);
+MODULE_PARM_DESC(perm_key_support, "The MAC is supposed to be disabled before \
+a permanent WEP key (the default) is set. Applications that want to set the \
+temporary keys, and thus not disable the MAC, are supposed to use the \
+IW_ENCODE_TEMP flag. Unfortunately, life doesn't always work the way it is \
+supposed to. If the IW_ENCODE_TEMP flag is not used and the MAC should not be \
+disabled, set this flag to 0.");
MODULE_PARM_DESC(auto_wep, "If non-zero, the driver will keep looping through \
the authentication options until an association is made. The value of \
auto_wep is number of the wep keys to check. A value of 2 will try using \
@@ -1738,9 +1747,12 @@ static int writeWepKeyRid(struct airo_in
rc = PC4500_writerid(ai, RID_WEP_TEMP, &wkr, sizeof(wkr), lock);
if (rc!=SUCCESS) printk(KERN_ERR "airo: WEP_TEMP set %x\n", rc);
if (perm) {
+ // We make these messages debug. They really should be error,
+ // but no one seems to use the TEMP flag and writing a PERM key
+ // with the MAC disable throws this error
rc = PC4500_writerid(ai, RID_WEP_PERM, &wkr, sizeof(wkr), lock);
if (rc!=SUCCESS) {
- printk(KERN_ERR "airo: WEP_PERM set %x\n", rc);
+ printk(KERN_DEBUG "airo: WEP_PERM set %x\n", rc);
}
}
return rc;
@@ -3813,11 +3825,14 @@ static u16 issuecommand(struct airo_info
pRsp->rsp1 = IN4500(ai, RESP1);
pRsp->rsp2 = IN4500(ai, RESP2);
if ((pRsp->status & 0xff00)!=0 && pCmd->cmd != CMD_SOFTRESET) {
- printk (KERN_ERR "airo: cmd= %x\n", pCmd->cmd);
- printk (KERN_ERR "airo: status= %x\n", pRsp->status);
- printk (KERN_ERR "airo: Rsp0= %x\n", pRsp->rsp0);
- printk (KERN_ERR "airo: Rsp1= %x\n", pRsp->rsp1);
- printk (KERN_ERR "airo: Rsp2= %x\n", pRsp->rsp2);
+ /* These really should be error, but supplicants don't seem
+ * to use the TEMP flag when setting the keys, so this
+ * error is common */
+ printk (KERN_DEBUG "airo: cmd= %x\n", pCmd->cmd);
+ printk (KERN_DEBUG "airo: status= %x\n", pRsp->status);
+ printk (KERN_DEBUG "airo: Rsp0= %x\n", pRsp->rsp0);
+ printk (KERN_DEBUG "airo: Rsp1= %x\n", pRsp->rsp1);
+ printk (KERN_DEBUG "airo: Rsp2= %x\n", pRsp->rsp2);
}

// clear stuck command busy if necessary
@@ -4046,10 +4061,12 @@ static int PC4500_writerid(struct airo_i
Cmd cmd;
Resp rsp;

+#if 0 /* This check is to catch bugs, not needed for WepRid with temp key */
if (test_bit(FLAG_ENABLED, &ai->flags))
printk(KERN_ERR
"%s: MAC should be disabled (rid=%04x)\n",
__FUNCTION__, rid);
+#endif
memset(&cmd, 0, sizeof(cmd));
memset(&rsp, 0, sizeof(rsp));

@@ -5094,7 +5111,7 @@ static int set_wep_key(struct airo_info
wkr.len = sizeof(wkr);
wkr.kindex = 0xffff;
wkr.mac[0] = (char)index;
- if (perm) printk(KERN_INFO "Setting transmit key to %d\n", index);
+ if (perm) printk(KERN_DEBUG "Setting transmit key to %d\n", index);
if (perm) ai->defindex = (char)index;
} else {
// We are actually setting the key
@@ -5103,12 +5120,11 @@ static int set_wep_key(struct airo_info
wkr.klen = keylen;
memcpy( wkr.key, key, keylen );
memcpy( wkr.mac, macaddr, ETH_ALEN );
- printk(KERN_INFO "Setting key %d\n", index);
}

- disable_MAC(ai, lock);
+ if (perm_key_support && perm) disable_MAC(ai, lock);
writeWepKeyRid(ai, &wkr, perm, lock);
- enable_MAC(ai, &rsp, lock);
+ if (perm_key_support && perm) enable_MAC(ai, &rsp, lock);
return 0;
}

@@ -5562,9 +5578,9 @@ static int __init airo_init_module( void
}

#ifdef CONFIG_PCI
- printk( KERN_INFO "airo: Probing for PCI adapters\n" );
+ printk( KERN_DEBUG "airo: Probing for PCI adapters\n" );
pci_register_driver(&airo_driver);
- printk( KERN_INFO "airo: Finished probing for PCI adapters\n" );
+ printk( KERN_DEBUG "airo: Finished probing for PCI adapters\n" );
#endif

/* Always exit with success, as we are a library module
@@ -5576,7 +5592,7 @@ static int __init airo_init_module( void
static void __exit airo_cleanup_module( void )
{
while( airo_devices ) {
- printk( KERN_INFO "airo: Unregistering %s\n", airo_devices->dev->name );
+ printk( KERN_DEBUG "airo: Unregistering %s\n", airo_devices->dev->name );
stop_airo_card( airo_devices->dev, 1 );
}
#ifdef CONFIG_PCI
@@ -6166,6 +6182,7 @@ static int airo_set_encode(struct net_de
{
struct airo_info *local = dev->priv;
CapabilityRid cap_rid; /* Card capability info */
+ u16 oldAuthType;

/* Is WEP supported ? */
readCapabilityRid(local, &cap_rid, 1);
@@ -6208,7 +6225,8 @@ static int airo_set_encode(struct net_de
/* Copy the key in the driver */
memcpy(key.key, extra, dwrq->length);
/* Send the key to the card */
- set_wep_key(local, index, key.key, key.len, 1, 1);
+ set_wep_key(local, index, key.key, key.len,
+ !(dwrq->flags&IW_ENCODE_TEMP), 1);
}
/* WE specify that if a valid key is set, encryption
* should be enabled (user may turn it off later)
@@ -6222,13 +6240,15 @@ static int airo_set_encode(struct net_de
/* Do we want to just set the transmit key index ? */
int index = (dwrq->flags & IW_ENCODE_INDEX) - 1;
if ((index >= 0) && (index < ((cap_rid.softCap & 0x80)?4:1))) {
- set_wep_key(local, index, NULL, 0, 1, 1);
+ set_wep_key(local, index, NULL, 0,
+ !(dwrq->flags&IW_ENCODE_TEMP), 1);
} else
/* Don't complain if only change the mode */
if(!dwrq->flags & IW_ENCODE_MODE) {
return -EINVAL;
}
}
+ oldAuthType = local->config.authType;
/* Read the flags */
if(dwrq->flags & IW_ENCODE_DISABLED)
local->config.authType = AUTH_OPEN; // disable encryption
@@ -6237,7 +6257,7 @@ static int airo_set_encode(struct net_de
if(dwrq->flags & IW_ENCODE_OPEN)
local->config.authType = AUTH_ENCRYPT; // Only Wep
/* Commit the changes to flags if needed */
- if(dwrq->flags & IW_ENCODE_MODE)
+ if(oldAuthType != local->config.authType)
set_bit (FLAG_COMMIT, &local->flags);
return -EINPROGRESS; /* Call commit handler */
}


2005-05-03 18:34:23

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [PATCH] dynamic wep keys for airo.c

Benjamin Reed wrote :
>
> I'm resubmitting a patch for the 2.6.11.8 aironet driver (airo.c) that will
> enable dynamic wep keying without disabling the MAC. It allows
> us to use xsupplicant with the driver.
>
> Aironet provides the ability to set WEP keys permanently or
> temporarily. There is a special IW_ENCODE_TEMP flag for selecting
> which type of key you are setting. However, apart from iwconfig,
> nobody seems to use this flag. When a permanent WEP key is set,
> the MAC must be disabled. I have added a module parameter to skip
> disabling the MAC even if a permanent WEP key is set. Using this
> flag allows xsupplicant to work without modification.

Hmm... I don't know why you are so afraid of submitting a
patch to xsupplicant (and wpa_supplicant). Having xsupplicant *always*
setting IW_ENCODE_TEMP would be worthwhile. Have you even try to
submit a patch to the author of xsupplicant ?
There was other driver having issues with with not reseting
the MAC while changing the WEP key. For example, some Lucent firmware
may lock up if you set WEP without fully reseting the MAC. So, having
IW_ENCODE_TEMP in xsupplicant would mean that the driver could support
802.1x properly if the firmware is sane and remain conservative while
setting static WEP keys, i.e. the best of both words. Driver that are
sane (moder hardware) can just ignore IW_ENCODE_TEMP.
With respect to airo, I think it would be nice if xsupplicant
did not pollute the WEP EEprom, so that next time you roam to a static
WEP AP, you can resuse the EEprom keys.

In summary, I believe that a correct solution is not so
difficult to implement (just a 2 line patch for xsupplicant and
wpa_supplicant), therefore I don't beleive we should go with
workarounds.

Have fun...

Jean

2005-05-03 19:59:59

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [PATCH] dynamic wep keys for airo.c

On Tue, May 03, 2005 at 12:54:02PM -0600, Chris Hessing wrote:
>
> I agree completely with Jean. In fact, the needed patch to use
> IW_ENCODE_TEMP is already in Xsupplicant. However, to remain on the
> conservative side I have provided a command line switch to disable the
> use of IW_ENCODE_TEMP. (Although I don't know of anything that would
> require it.)
>
> There was a new pre-release of Xsupplicant yesterday that will make use
> of IW_ENCODE_TEMP. Please let me know if anything additional is needed.

Wow ! That's even better ! Thanks a lot !

> Thanks!

Jean

2005-05-03 22:37:40

by Benjamin Reed

[permalink] [raw]
Subject: Re: [PATCH] dynamic wep keys for airo.c

Jean,

There is no patch to xsupplicant that will work without patching the
airo.c driver. The current airo.c driver always disables the MAC before
setting the WEP key whether it is temporary or permanent. This is
incorrect. When the MAC is disabled the card disassociates causing the
whole handshake to start over again.

ben

> Benjamin Reed wrote :
>>
>> I'm resubmitting a patch for the 2.6.11.8 aironet driver (airo.c) that
>> will
>> enable dynamic wep keying without disabling the MAC. It allows
>> us to use xsupplicant with the driver.
>>
>> Aironet provides the ability to set WEP keys permanently or
>> temporarily. There is a special IW_ENCODE_TEMP flag for selecting
>> which type of key you are setting. However, apart from iwconfig,
>> nobody seems to use this flag. When a permanent WEP key is set,
>> the MAC must be disabled. I have added a module parameter to skip
>> disabling the MAC even if a permanent WEP key is set. Using this
>> flag allows xsupplicant to work without modification.
>
> Hmm... I don't know why you are so afraid of submitting a
> patch to xsupplicant (and wpa_supplicant). Having xsupplicant *always*
> setting IW_ENCODE_TEMP would be worthwhile. Have you even try to
> submit a patch to the author of xsupplicant ?
> There was other driver having issues with with not reseting
> the MAC while changing the WEP key. For example, some Lucent firmware
> may lock up if you set WEP without fully reseting the MAC. So, having
> IW_ENCODE_TEMP in xsupplicant would mean that the driver could support
> 802.1x properly if the firmware is sane and remain conservative while
> setting static WEP keys, i.e. the best of both words. Driver that are
> sane (moder hardware) can just ignore IW_ENCODE_TEMP.
> With respect to airo, I think it would be nice if xsupplicant
> did not pollute the WEP EEprom, so that next time you roam to a static
> WEP AP, you can resuse the EEprom keys.
>
> In summary, I believe that a correct solution is not so
> difficult to implement (just a 2 line patch for xsupplicant and
> wpa_supplicant), therefore I don't beleive we should go with
> workarounds.
>
> Have fun...
>
> Jean
>
>

2005-05-03 23:13:27

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [PATCH] dynamic wep keys for airo.c

On Tue, May 03, 2005 at 05:34:31PM -0500, [email protected] wrote:
> Jean,
>
> There is no patch to xsupplicant that will work without patching the
> airo.c driver. The current airo.c driver always disables the MAC before
> setting the WEP key whether it is temporary or permanent. This is
> incorrect. When the MAC is disabled the card disassociates causing the
> whole handshake to start over again.

Yes, I know perfectly. A patch is needed, however I don't
think your current patch is the most appropriate solution.

The current solution is :
1) set perm key -> goes in the eeprom, reset MAC
2) set temp key -> not in the eeprom, reset MAC
o /proc can set (1) and (2)
o iwconfig can only set (1)
o xsupplicant always set (1) - broken

You solution is :
1) set temp key -> not in the eeprom, not reset MAC
2a) set perm key -> goes in the eeprom, reset MAC
2b) set perm key -> goes in the eeprom, not reset MAC
o module parameter select (2a) or (2b)
o iwconfig can set (2) {default} or (1) {temp keyword}
o old xsupplicant set (2) - may work, depend on module parameter
o new xsupplicant set (1) - always work

First, do we know for sure that all Aironet firmware will
accept to change the perm WEP key without having to reset the MAC ?
Maybe the Cisco driver does it this way, but it only target the latest
firware rev, whereas up to know we have included in the driver code
for very antique firmware rev. I don't have the answer to that one.
Second, I think that the module parameter is counter
productive. What we want there is people migrating to the new
xsupplicant that does the right thing. Also, module parameter is not
something that can be changed on the fly, and is one more thing to
configure. We want things to always work, all the time.

What I would think is better long term :
1) set temp key -> not in the eeprom, not reset MAC
2) set perm key -> goes in the eeprom, reset MAC
o iwconfig can set (2) {default} or (1) {temp keyword}
o old xsupplicant set (2) - always broken
o new xsupplicant set (1) - always work

But, that's only my personal opinion, and you may want to
check what the new xsupplicant is doing before making the final
decision.

> ben

Have fun...

Jean

2005-05-06 08:04:36

by Domen Puncer

[permalink] [raw]
Subject: Re: [PATCH] dynamic wep keys for airo.c

On 03/05/05 08:26 -0700, Benjamin Reed wrote:
> module_param_array(ssids, charp, NULL, 0);
> module_param(auto_wep, int, 0);
> +module_param(perm_key_support, int, 1);

^ permissions in sysfs

> if (rc!=SUCCESS) printk(KERN_ERR "airo: WEP_TEMP set %x\n", rc);
> if (perm) {
> + // We make these messages debug. They really should be error,
> + // but no one seems to use the TEMP flag and writing a PERM key
> + // with the MAC disable throws this error

/* */?


Domen