Bug 6624 - SPNEGO negTokenInit reqFlags is BIT STRING not INTEGER
Summary: SPNEGO negTokenInit reqFlags is BIT STRING not INTEGER
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.0
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: unspecified
Hardware: Other Linux
: P3 normal (vote)
Target Milestone: ---
Assignee: Guenther Deschner
QA Contact: Matthias Dieter Wallnöfer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-08-09 21:18 UTC by Kouhei Sutou
Modified: 2009-09-17 13:11 UTC (History)
2 users (show)

See Also:


Attachments
a packet capture screenshot (137.56 KB, image/png)
2009-08-12 23:34 UTC, Kouhei Sutou
no flags Details
PCAP (1.91 KB, application/cap)
2009-08-13 00:30 UTC, Kouhei Sutou
no flags Details
an implementation. (6.11 KB, patch)
2009-08-13 01:07 UTC, Kouhei Sutou
no flags Details
revised patch. (6.47 KB, patch)
2009-08-13 01:13 UTC, Kouhei Sutou
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kouhei Sutou 2009-08-09 21:18:33 UTC
RFC4178 says SPNEGO negTokenInit reqFlags is BIT STRING:
  NegTokenInit ::= SEQUENCE {
    ...
    reqFlags        [1] ContextFlags  OPTIONAL,
      -- inherited from RFC 2478 for backward compatibility,
      -- RECOMMENDED to be left out
    ...
  }
  ContextFlags ::= BIT STRING {
    ...
  } (SIZE (32))
from http://tools.ietf.org/html/rfc4178#section-4.2.1

source4/auth/gensec/spnego_parse.c:read_negTokenInit() reads reqFlags as INTEGER (asn1_read_Integer() is used) not BIT STRING.

NOTE: Likewise(*1) sends reqFlags. Likewise can't join Samba4's Active Directory domain because Samba4 reports an error in asn1_read_Integer().

FYI: Samba3 doesn't have this bug. source3/libsmb/clispnego.c:parse_negTokenTarg() handles reqFlags as BIT STRING not INTEGER.
Comment 1 Kouhei Sutou 2009-08-09 21:24:42 UTC
I have some solution ideas. Which solution do you like?

(1) We implement asn1_{read,write}_BitString() and use them in {read,write}_negTokenInit().
(2) We just ignore reqFlags because RFC4178 says "RECOMMENDED to be left out
". This is the same solution as source3/libsmb/clispnego.c:parse_negTokenTarg().

(2) will be easier rather than (1).
Comment 2 Stefan Metzmacher 2009-08-12 04:43:48 UTC
I'd like to see solution 1.

Can you attach a network capture please.

metze
Comment 3 Kouhei Sutou 2009-08-12 23:34:27 UTC
Created attachment 4551 [details]
a packet capture screenshot

I lined a target packet with red.

0xa1: reqFlags tag
0x04: reqFlags packet length
0x03: BIT STRING tag (not INTEGER = 0x02)
0x02: BIT STRING packet length
0x01: number of unused bit
0x7c: flags (the last bit is unused)
Comment 4 Andrew Bartlett 2009-08-12 23:41:53 UTC
By packet capture we do mean the PCAP formatted file.  Please save the capture and attach it to this bug.  Screen-shots are not helpful, sorry. 
Comment 5 Kouhei Sutou 2009-08-13 00:30:17 UTC
Created attachment 4552 [details]
PCAP
Comment 6 Kouhei Sutou 2009-08-13 01:07:21 UTC
Created attachment 4553 [details]
an implementation.
Comment 7 Kouhei Sutou 2009-08-13 01:13:55 UTC
Created attachment 4554 [details]
revised patch.

Oops. The previous patch has a memory leak...
Comment 8 Stefan Metzmacher 2009-08-13 05:13:04 UTC
can you point me to the documentation where the encoding of BIT Strings is defined. I can't find a definition that has understandable examples.

As heimdal does this correct I looked add asn1_compile and its output,
but I didn't understand it...

Are more that 32 bits allowed in ASN.1?

MIT has this #define»BIT_STRING_PADDING»·····0x01

Is this real padding (why has it a non zero value then)
or does that have a meaning?

metze
Comment 9 Kouhei Sutou 2009-08-13 06:26:14 UTC
(In reply to comment #8)
> can you point me to the documentation where the encoding of BIT Strings is
> defined. I can't find a definition that has understandable examples.

I showed it in Bug #6625.

Please see section 8.6 in
http://www.itu.int/ITU-T/studygroups/com17/languages/X.690-0207.pdf
Comment 10 Matthias Dieter Wallnöfer 2009-09-07 15:03:03 UTC
Metze, could you comment on the latest patch? So we could apply it and close this one.
Comment 11 Guenther Deschner 2009-09-17 12:30:26 UTC
looking into this.
Comment 12 Guenther Deschner 2009-09-17 13:11:50 UTC
Ok, a slightly modified version has been pushed to master.

Thanks!