Skip to content
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

feat: add Extended request operations #516

Merged
merged 9 commits into from
Oct 31, 2024

Conversation

cpuschma
Copy link
Member

This implements the base for "Extended Requests" as defined in RFC2251 Section 4.12. I'm having trouble getting the requestValue to work, as this seems to get ignored by the server and Wireshark showing errors regarding unexpected fields. Additionally, submitting the requestName as LDAPOID (basically an alias for an BER-encoded OCTET STRING), the server immediately closes the connection without returning an error (tested against Active Directory and OpenLDAP). It works when switching to ber.TagEOC.

I'm open for ideas or anyone who has experience with implementing the "Extended Request" operation!

@cpuschma cpuschma force-pushed the feat/extend_request branch from 4dba82f to efdaca2 Compare April 23, 2024 21:24
@cpuschma
Copy link
Member Author

@Denis-shl
Copy link

@cpuschma hi, I ran into the same problem and I'm working on it now.Tell me, did you manage to implement it?

@JesseCoretta
Copy link

This implements the base for "Extended Requests" as defined in RFC2251 Section 4.12.

Hi @cpuschma -- I'm going to try and reproduce this behavior on my end and see if I can discern what is going on.

Also, just in case you didn't already realize it, RFC2251 was obsoleted by RFC4511. The section number in question, however, remains 4.12.

Jesse 😃

@JesseCoretta
Copy link

JesseCoretta commented Oct 18, 2024

This implements the base for "Extended Requests" as defined in RFC2251 Section 4.12. I'm having trouble getting the requestValue to work, as this seems to get ignored by the server and Wireshark showing errors regarding unexpected fields. Additionally, submitting the requestName as LDAPOID (basically an alias for an BER-encoded OCTET STRING), the server immediately closes the connection without returning an error (tested against Active Directory and OpenLDAP). It works when switching to ber.TagEOC.

I'm open for ideas or anyone who has experience with implementing the "Extended Request" operation!

OK, to begin I cloned the feat/extend_request branch from your repo. I decided I'd just add a new Unit Test to be lazy. I decided to start with LDAP "Who Am I?" per RFC 4532.

As the hostname implies, I am using OpenDJ on my local system.

func TestExtendedRequest_Jesse(t *testing.T) {                          
        l, err := DialURL("ldap://opendj.example.com:389")              
        if err != nil {                                                 
                t.Errorf("%s failed: %v", t.Name(), err)                
                return                                                  
        }                                                               
        defer l.Close()

        binduser := ``
        bindpass := ``

        if err = l.Bind(binduser,bindpass); err != nil {
                 t.Errorf("%s failed: %v", t.Name(), err)
                 return
        }

        rfc4532req := NewExtendedRequest(`1.3.6.1.4.1.4203.1.11.3`, nil)  // request value is <nil>

        var rfc4532resp *ExtendResponse
        if rfc4532resp, err = l.Extended(rfc4532req); err != nil {
                t.Errorf("%s failed: %v", t.Name(), err)
                return
        }

        // I find it awkward to check errors of UNBIND ops,
        // but the linter probably won't accept anything less ...
        if err = l.Unbind(); err != nil {
                t.Errorf("%s failed: %v", t.Name(), err)
                return
        }

        // can probably comment this out when diagnostics/testing
        // has been completed.
        t.Logf("%#v\n", rfc4532resp)
}
=== RUN   TestExtendedRequest_Jesse
    examples_test.go:45: &ldap.ExtendResponse{Name:"dn:", Value:(*ber.Packet)(nil)}
--- PASS: TestExtendedRequest_Jesse (0.13s)
PASS
ok  	github.com/go-ldap/ldap	0.132s

If I swap the Bind user to someone not anonymous, the response is also correct (for OpenDJ):

=== RUN   TestExtendedRequest_Jesse
    examples_test.go:49: &ldap.ExtendResponse{Name:"dn:cn=Directory Manager,cn=Root DNs,cn=config", Value:(*ber.Packet)(nil)}
--- PASS: TestExtendedRequest_Jesse (0.01s)

Now ... these are the correct results. The requestValue is supposed to be empty in the context of this particular extended request. You mentioned the server ignores it ... I suspect that is intentional and expected in some cases. For instance, note the following excerpt from RFC 4532 section 2.1:

   The whoami request is an ExtendedRequest with a requestName field
   containing the whoamiOID OID and an absent requestValue field.  For
   example, a whoami request could be encoded as the sequence of octets
   (in hex):

      30 1e 02 01 02 77 19 80  17 31 2e 33 2e 36 2e 31
      2e 34 2e 31 2e 34 32 30  33 2e 31 2e 31 31 2e 33

Note that specific detail:

... and an absent requestValue field ...

        ExtendedRequest ::= [APPLICATION 23] SEQUENCE {
             requestName      [0] LDAPOID,
             requestValue     [1] OCTET STRING OPTIONAL }

Note it is OPTIONAL, as indicated. I am wondering if you were getting unexpected results because the OID you were testing with was perhaps of a similar use to LDAP "Who Am I?", in that NO value is expected? This is a wild, wild guess, but I'm offering it nonetheless.

Jesse 😃

EDIT: @cpuschma -- I see there are linter errors. I did not think my code would make it into your tests since I figured it was just a demo. Sorry about my lack of error checks for bind/unbind 😨.

I've updated my example to check those errors and I created dedicated user/pass vars for easy context-switching. Just a heads-up, no reply needed. Thank you!

@JesseCoretta
Copy link

Implementation suggestions:

  • I realize this is nit-picky, but type ExtendResponse struct should probably be type ExtendedResponse struct
  • Given that the requestValue MAY be nil, consider changing the NewExtendedRequest function signature to allow variadic input, e.g.: NewExtendedRequest(name string, value ...*ber.Packet) *ExtendedRequest { for convenience

@cpuschma
Copy link
Member Author

@JesseCoretta Thank you for pointing out the obsolete RFC document, didn't notice that! Also thank you for your suggestions regarding the function signatures 👍

@JesseCoretta
Copy link

@JesseCoretta Thank you for pointing out the obsolete RFC document, didn't notice that! Also thank you for your suggestions regarding the function signatures 👍

@cpuschma ... happy to help. Did you see my second comment, regarding the "Who Am I?" operation? If you tell me what OID you were testing with, I can reproduce on my end. I only used RFC 4532 because it is well known, no idea if thats what you were testing....

@JesseCoretta
Copy link

@cpuschma -- I had asked if you could let me know what OID you were testing with, not sure if you're just not seeing my replies or what, but I have to move onto other projects, so you can disregard the above (large) reply.

@cpuschma
Copy link
Member Author

@JesseCoretta. Be mindful that this is not a project that is supported by any organization or large group of people. I would really like to put more time into this, but I don't have much time at the moment due to a big change at work and also health problems. So please keep that in mind.

About the OID: I started the pull request in April because of another feature request in issue #514, which would be “3.1.1.3.4.2.1 LDAP_SERVER_FAST_BIND_OID”. See https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-adts/58bbd5c4-b5c4-41e2-b12c-cdaad1223d6a?redirectedfrom=MSDN

I will try my best to continue working on the two pull requests next weekend. I appreciate your input, of course, but the tone should remain appropriate.

@JesseCoretta
Copy link

@cpuschma

@JesseCoretta. Be mindful that this is not a project that is supported by any organization or large group of people. I would really like to put more time into this, but I don't have much time at the moment due to a big change at work and also health problems. So please keep that in mind.

Yes, I am aware -- that is the reason I originally wanted to help this project. You had asked if anyone with experience with extended operations could assist you. Sorry ...

About the OID: I started the pull request in April because of another feature request in issue #514, which would be “3.1.1.3.4.2.1 LDAP_SERVER_FAST_BIND_OID”. See https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-adts/58bbd5c4-b5c4-41e2-b12c-cdaad1223d6a?redirectedfrom=MSDN

AHA! "3.1.1.3.4.2.1" is not a valid ASN.1 Object Identifier -- OIDs can only begin with 0, 1 or 2.

I believe that is actually a section number (see the remark here).

Could that be why your request was failing? I suggest you try another ExOp OID, such as LDAP WhoAmI OID, or maybe the StartTransaction (RFC 5805) if the DSA supports it. Any valid ExOp OID, whether or not it expects a value.

Also keep in mind that a DSA that does not recognize the OID will exhibit the behavior you described earlier, though normally its supposed to actually SAY "Unknown ExOp OID", etc ...

@cpuschma
Copy link
Member Author

cpuschma commented Oct 23, 2024

@JesseCoretta

AHA! "3.1.1.3.4.2.1" is not a valid ASN.1 Object Identifier -- OIDs can only begin with 0, 1 or 2.
I believe that is actually a section number (see the remark here).

You're right! I most likely used the section number instead of the correct OID. The correct one would be “1.2.840.113556.1.4.1781” as shown by you according to https://learn.microsoft.com/en-us/previous-versions/windows/desktop/ldap/ldap-server-fast-bind-oid . Thank you for pointing this out.

Yes, I am aware -- that is the reason I originally wanted to help this project. You had asked if anyone with experience with extended operations could assist you. Sorry ...

I really appreciate your input! I checked out your Github profile and am glad to get feedback from someone who obviously knows something about the subject 👍 During the week it's mostly just stressful at the moment for me. I'll try again at the weekend with the “right” OID this time 😄

@johnweldon
Copy link
Member

johnweldon commented Oct 23, 2024 via email

@JesseCoretta
Copy link

@JesseCoretta

AHA! "3.1.1.3.4.2.1" is not a valid ASN.1 Object Identifier -- OIDs can only begin with 0, 1 or 2.
I believe that is actually a section number (see the remark here).

You're right! I most likely used the section number instead of the correct OID. The correct one would be “1.2.840.113556.1.4.1781” as shown by you according to https://learn.microsoft.com/en-us/previous-versions/windows/desktop/ldap/ldap-server-fast-bind-oid . Thank you for pointing this out.

Yes, I am aware -- that is the reason I originally wanted to help this project. You had asked if anyone with experience with extended operations could assist you. Sorry ...

I really appreciate your input! I checked out your Github profile and am glad to get feedback from someone who obviously knows something about the subject 👍 During the week it's mostly just stressful at the moment, for me I'll try again at the weekend with the “right” OID this time 😄

Very good. I had formally offered my assistance to one of your teammates via email but I don't think there was any interest in my joining.

Nevertheless if you guys ever need SME consultation, I'm happy to participate in an ad hoc manner, at least.

Also, feel better :)

@JesseCoretta
Copy link

Yes, I am aware -- that is the reason I originally wanted to help this
project. You had asked if anyone with experience with extended operations could assist you. Sorry ... Thank you Jesse Should I interpret this to mean you'd like to be considered as a contributor to this project? Christopher had been handling almost all of the support and maintenance for a long time and may welcome some help.

On Wed, Oct 23, 2024, 1:00 PM Christopher Puschmann < @.> wrote: @JesseCoretta https://github.com/JesseCoretta AHA! "3.1.1.3.4.2.1" is not a valid ASN.1 Object Identifier -- OIDs can only begin with 0, 1 or 2. I believe that is actually a section number (see the remark here https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-adts/dc4eb502-fb94-470c-9ab8-ad09fa720ea6 ). You're right! I most likely used the section number instead of the correct OID. The correct one would be “1.2.840.113556.1.4.1781” as shown by you according to https://learn.microsoft.com/en-us/previous-versions/windows/desktop/ldap/ldap-server-fast-bind-oid . Thank you for pointing this out. Yes, I am aware -- that is the reason I originally wanted to help this project. You had asked if anyone with experience with extended operations could assist you. Sorry ... I really appreciate your input! I checked out your Github profile and am glad to get feedback from someone who obviously knows something about the subject 👍 During the week it's mostly just stressful at the moment, for me I'll try again at the weekend with the “right” OID this time 😄 — Reply to this email directly, view it on GitHub <#516 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAMOWQSHQZZPPPKFODOBY3Z4755XAVCNFSM6AAAAABGVOXY3GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZTGMZDCNRXGQ . You are receiving this because you are subscribed to this thread.Message ID: @.>

I had intended to offer my X.500 knowledge in any way that may help or advance the objectives or roadmap of this project, whether or not coding was involved. With products like this, historically speaking, most of the time I am sought out for SME involvement -- research, testing and PoC are some "asks", just to name a few.

Is there a particular aspect of this project with which you would benefit from such assistance?

@johnweldon
Copy link
Member

I had intended to offer my X.500 knowledge in any way that may help or advance the objectives or roadmap of this project, whether or not coding was involved. With products like this, historically speaking, most of the time I am sought out for SME involvement -- research, testing and PoC are some "asks", just to name a few.

Is there a particular aspect of this project with which you would benefit from such assistance?

Thanks Jesse - I think pulling on your subject matter expertise is very helpful.

I was thinking more general contribution (vetting requests, managing releases, improving our versioning, etc.) but after re-reading your comments I see the offer was X.500 specific.

@JesseCoretta
Copy link

I had intended to offer my X.500 knowledge in any way that may help or advance the objectives or roadmap of this project, whether or not coding was involved. With products like this, historically speaking, most of the time I am sought out for SME involvement -- research, testing and PoC are some "asks", just to name a few.
Is there a particular aspect of this project with which you would benefit from such assistance?

Thanks Jesse - I think pulling on your subject matter expertise is very helpful.

I was thinking more general contribution (vetting requests, managing releases, improving our versioning, etc.) but after re-reading your comments I see the offer was X.500 specific.

Well I mentioned X.500 only because I have not limited my focus to LDAP alone. X.500 is still quite relevant, and it is a "larger umbrella" than simply citing LDAP alone. Sufficed to say, both are applicable.

@cpuschma
Copy link
Member Author

cpuschma commented Oct 23, 2024

Nevertheless if you guys ever need SME consultation, I'm happy to participate in an ad hoc manner, at least.

Also, feel better :)

It's always good to have someone who has much better knowledge of X.500 and OIDs. I will definitely come back to this!
And thank you :)

@JesseCoretta
Copy link

Just a diagnostics update -- no response needed at this time, @cpuschma. Everything is totally fine.

To broaden test coverage, I've installed the newest 389ds from source on my test host (Ubuntu 22) and have been conducting other tests to see if the EXOPs submitted behave as expected.

So far:

WHOAMI? continues to work, in that proper credentials return the confirmed DN, and invalid credentials return nothing. I'd also like to expand this test case to include SASL/EXTERNAL mutual auth (PKI) with and without proxy auth just for fun.

RFC 5805 "Start Transaction" fails (the correct behavior, as 389DS does not support this feature). We also receive the correct response: TestExtendedRequest_Jesse failed: LDAP Result Code 2 "Protocol Error": unsupported extended operation

Next, I tried to send Netscape's "Start Replication" EXOP, and the log events confirm the OID was both received and recognized.

[28/Oct/2024:18:26:26.854307716 -0700] conn=12 fd=64 slot=64 connection from 127.0.0.1 to 127.0.0.1
[28/Oct/2024:18:26:26.854423980 -0700] conn=12 op=0 BIND dn="cn=Directory Manager" method=128 version=3
[28/Oct/2024:18:26:26.862731292 -0700] conn=12 op=0 RESULT err=0 tag=97 nentries=0 wtime=0.000045757 optime=0.008316495 etime=0.008359580 dn="cn=directory manager"
[28/Oct/2024:18:26:26.862968921 -0700] conn=12 op=1 EXT oid="2.16.840.1.113730.3.5.3" name="replication-multisupplier-extop"
[28/Oct/2024:18:26:26.870506843 -0700] conn=12 op=1 RESULT err=0 tag=120 nentries=0 wtime=0.000064626 optime=0.007532201 etime=0.007586861
[28/Oct/2024:18:26:26.871463342 -0700] conn=12 op=2 UNBIND
[28/Oct/2024:18:26:26.871517271 -0700] conn=12 op=2 fd=64 Disconnect - Cleanly Closed Connection - U1

My next goal is to fabricate some BER packets for EXOPs which require this. As it stands, most of the EXOPs supported by my test host are Netscape-specific (not derived from RFCs), thus I may have to start digging through some docs or source code to discern what it is expecting.

@cpuschma
Copy link
Member Author

I finally got the time to try the extended implementation again and indeed: I did use the wrong OID in the past.. 🥹

As suggested, I renamed the struct and functions to "ExtendedRequest" and "ExtendedResponse", but I would avoid a variadic function signature for now, because all other constructors like “NewSearchRequest” don't use it either. However, it would not interfere with any existing code or linter if we change this later.

Regarding your test case suggestion "Golang LDAPv3 SASL/EXTERNAL + Proxy Authorization Example": It seems that the public DS we test against (ldap.itd.umich.edu) does not support test case. Ideally, we would need a temporary LDAP server for a higher coverage, which is pulled up and filled before the tests...

@JesseCoretta
Copy link

I finally got the time to try the extended implementation again and indeed: I did use the wrong OID in the past.. 🥹

Glad it was something simple!

Regarding your test case suggestion "Golang LDAPv3 SASL/EXTERNAL + Proxy Authorization Example": It seems that the public DS we test against (ldap.itd.umich.edu) does not support test case. Ideally, we would need a temporary LDAP server for a higher coverage, which is pulled up and filled before the tests...

Oh I just meant on my end (using my own server), not as an official packaged test case. I should have been clearer, sorry.

Regardless, duly noted :) ... thank you @cpuschma!

@cpuschma cpuschma marked this pull request as ready for review October 31, 2024 00:06
@cpuschma
Copy link
Member Author

cpuschma commented Oct 31, 2024

I'm going to fix the failed lint in our Github Actions in another PR. Looks like other unrelated tests will be affected as well like conn_test.go. They seem to come from #527.

@cpuschma cpuschma merged commit 601814b into go-ldap:master Oct 31, 2024
22 of 24 checks passed
@cpuschma cpuschma deleted the feat/extend_request branch October 31, 2024 14:56
@Denis-shl
Copy link

I am very pleased with this revision. Tell me, I want to test extended operations in FreeIPA (https://www.freeipa.org/page/V4/Keytab_Retrieval#new-extended-operation ) But I don't understand how I can build a ber package. Could you help me with an example?

@cpuschma
Copy link
Member Author

cpuschma commented Nov 1, 2024

@Denis-shl Looking at the document you linked you could try an implementation like this. Bear in mind that this is untested as I don't have an FreeIPA at hand to test this and this ExOp is exclusive to FreeIPA

The documentation also wrongly specificed the KeytabGetRequest ASN1 definition. Each option within a CHOICE must have a unique tag to ensure unambiguous identification of the selected option. I suppose they meant "1" instead of the 2nd 0 here:

KeytabGetRequest ::= CHOICE {
    newkeys  [0] NewKeys,
    curkeys  [0] CurrentKeys, # <<-- invalid, should be a 1
    reply    [2] Reply
}
func main() {
	conn, _ := ldap.DialURL("")

	packet := ber.NewString(ber.ClassUniversal, ber.TypePrimitive, 1, "<your input here>", "serviceIdentity")
	request := ldap.NewExtendedRequest("2.16.840.1.113730.3.8.10.5", packet)
	response, err := conn.Extended(request)
	if err != nil {
		// ...
	}
	fmt.Printf("%#v\n", response.Value)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants