aircrack-ng / aircrack-ng-archive

Pre-migration repository. New repository ➙
https://github.com/aircrack-ng/aircrack-ng
GNU General Public License v2.0
857 stars 306 forks source link

broken SKA #125

Closed rofl0r closed 7 years ago

rofl0r commented 7 years ago

i was googling around for "broken ska" message in airodump and i've read on several sites about a possible aircrack bug...

and indeed, looking at the source we have this weird conditional: https://github.com/aircrack-ng/aircrack-ng/blob/master/src/airodump-ng.c#L510 and https://github.com/aircrack-ng/aircrack-ng/blob/master/src/airodump-ng.c#L528

so the "not encrypted" vs "encrypted" branches are not on the same indentation level which suggests a bug i.e. the "not encrypted" branch is taken when the packet is not an auth packet since that is the spot that sets G.sk_len2 which is later used to check if the SKA is "broken"

also:

commit d12d0803e625e6c0d5cef2be8b91009e8041eaee Author: Martin Beck hirte@aircrack-ng.org

this commit looks suspicious... there's no info whatsoever about what G.sk_len2 might be, or what criteria makes SKA broken or not broken

i'd naively suspect that this commit broke xor file generation instead of reporting broken SKA correctly

rofl0r commented 7 years ago

i've had some success patching out the "broken ska" check introduced in the commit mentioned above, and a xor file was created. however i couldnt associate with the prga later on ("challenge failure"). i checked out commit 3fb427926409a93db00e86fe57321fe039e0383e which was a few revisions before hirte's change (should be about equivalent to release 1.0-beta2) , and it created a different .xor file - which works! so apart from the bogus "broken SKA" check, the xor-file generation is broken in current aircrack-ng too.

aircrack-ng commented 7 years ago

And does it still work on open auth wep with your changes?

On 2017-07-08 08:56, rofl0r wrote:

i've had some success patching out the "broken ska" check introduced in the commit mentioned above, and a xor file was created. however i couldnt associate with the prga later on ("challenge failure"). i checked out commit 3fb427926409a93db00e86fe57321fe039e0383e which was a few revisions before hirte's change (should be about equivalent to release 1.0-beta2) , and it created a different .xor file - which works! so apart from the bogus "broken SKA" check, the xor-file generation is broken in current aircrack-ng too.

rofl0r commented 7 years ago

i don't see why it shouldn't. my commits touch only stuff that's related to SKA. but i suggest you take a look yourself at my PR.

rofl0r commented 7 years ago

btw, i also tried to reorder the 2 branches i talked about earlier:

so the "not encrypted" vs "encrypted" branches are not on the same indentation level which suggests a bug

this is what i tried:

    /* is auth packet */ 
    if( (h80211[1] & 0x40) != 0x40 )
    {           
        /* not encrypted */
        if( ( h80211[24] + (h80211[25] << 8) ) == 1 )
        {
            /* Shared-Key Authentication */
            if( ( h80211[26] + (h80211[27] << 8) ) == 2 )
            {   
                /* sequence == 2 */ 
                memcpy(G.sharedkey[0], h80211, caplen); 
                G.sk_len = caplen-24;
            }
            if( ( h80211[26] + (h80211[27] << 8) ) == 4 )
            {
                /* sequence == 4 */
                memcpy(G.sharedkey[2], h80211, caplen);
            }
        }
    else
    {
        /* encrypted */
        memcpy(G.sharedkey[1], h80211, caplen);
        G.sk_len2 = caplen-24-4;
    }

    }
  else return 1;

which seems logical according to the comments, however with that in place the .xor generation broke again in a different way, so i think that should be left as is... maybe its only the comment that's misplaced, but i suppose you should take a look at it.