Fixed
Status Update
Comments
ge...@gmail.com <ge...@gmail.com> #2
[Empty comment from Monorail migration]
ge...@gmail.com <ge...@gmail.com> #3
The main features are shown on the main index of the home page. Should the about page include the same information that is there, or be more detailed?
ge...@gmail.com <ge...@gmail.com> #4
I would say more details. Alternatively we could just rename it to "Project History" and move it to a less prominent place.
al...@gmail.com <al...@gmail.com> #5
[Monorail components: Homepage]
al...@gmail.com <al...@gmail.com> #6
[Monorail components: Documentation]
ge...@gmail.com <ge...@gmail.com> #7
[Monorail components: -docs]
al...@gmail.com <al...@gmail.com> #8
[Empty comment from Monorail migration]
al...@gmail.com <al...@gmail.com> #9
See [1] for difference between active directory and samba, they should have allowed anonymous authentication over plain transport, but it seems like they have enforced this for all, making the server incompatible with startTLS.
[1]https://www.samba.org/samba/security/CVE-2016-2112.html
[1]
ge...@gmail.com <ge...@gmail.com> #10
I see what you're saying and referring to, but I have to disagree on your conclusion.
StartTLS, to the very same LDAP server having "ldap server require strong auth = yes"...
1. works fine with 'ldapsearch -ZZ' inside the same Debian Buster Docker image:
$ LDAPTLS_CACERT=/usr/local/share/ca-certificates/mycert.crt ldapsearch -v -ZZ \
-H ldap://ldap.mydomain.tld -x -D 'service-gerrit@MYDOMAIN.TLD' -w '<password>' \
-b 'CN=Users,DC=mydomain,DC=tld'
ldap_initialize( ldap://ldap.mydomain.tld:389/??base )
filter: (objectclass=*)
[...]
# search result
search: 3
result: 0 Success
# numResponses: 29
# numEntries: 28
2. works fine in JRE 8 from Debian Stretch and Gerrit (this part is unchanged > 1 year in my setup).
So this is either a bug in OpenJDK JRE 11 with Debian Buster, or an omission in how Gerrit initializes the LDAP connection with StartTLS.
Also:
> startTLS requires:
> 1. bind over plain transport
Why? It can upgrade to use TLS before binding, no? Perhaps I misunderstand something here.
Have you actually tried this too on JRE 11 to verify?
StartTLS, to the very same LDAP server having "ldap server require strong auth = yes"...
1. works fine with 'ldapsearch -ZZ' inside the same Debian Buster Docker image:
$ LDAPTLS_CACERT=/usr/local/share/ca-certificates/mycert.crt ldapsearch -v -ZZ \
-H ldap://ldap.mydomain.tld -x -D 'service-gerrit@MYDOMAIN.TLD' -w '<password>' \
-b 'CN=Users,DC=mydomain,DC=tld'
ldap_initialize( ldap://ldap.mydomain.tld:389/??base )
filter: (objectclass=*)
[...]
# search result
search: 3
result: 0 Success
# numResponses: 29
# numEntries: 28
2. works fine in JRE 8 from Debian Stretch and Gerrit (this part is unchanged > 1 year in my setup).
So this is either a bug in OpenJDK JRE 11 with Debian Buster, or an omission in how Gerrit initializes the LDAP connection with StartTLS.
Also:
> startTLS requires:
> 1. bind over plain transport
Why? It can upgrade to use TLS before binding, no? Perhaps I misunderstand something here.
Have you actually tried this too on JRE 11 to verify?
al...@gmail.com <al...@gmail.com> #11
Binding must be performed somewhere, interesting, maybe there is a difference in how jdk works, not sure.
Can you please try the attached patch? Let's see if something changes.
Can you please try the attached patch? Let's see if something changes.
ge...@gmail.com <ge...@gmail.com> #12
Thanks for looking into this. I've tried the patch and it behaves the same way for me, with or without setting 'supportAnonymous = false'. :-(
al...@gmail.com <al...@gmail.com> #13
Again, the supportAnonymous is valid only for ldaps:// and not startTLS.
Can you please attach the ldapsearch debug output (-v)?
Can you please attach the ldapsearch debug output (-v)?
al...@gmail.com <al...@gmail.com> #14
Sorry debug (-d 255) and maybe -n so I can see how initial connection is performed.
al...@gmail.com <al...@gmail.com> #15
Can you please try this configuration with the new patch?
[ldap]
server = ldaps://ldap.mydomain.tld
startTls = true
initialAuthentication = ANONYMOUS
Maybe SASL will be accepted in your case and maybe anonymous will work without additional configuration.
Thanks!
[ldap]
server = ldaps://ldap.mydomain.tld
startTls = true
initialAuthentication = ANONYMOUS
Maybe SASL will be accepted in your case and maybe anonymous will work without additional configuration.
Thanks!
ge...@gmail.com <ge...@gmail.com> #16
> debug (-d 255) and maybe -n so I can see how initial connection is performed.
Okay, I will, in a moment.
> Again, the supportAnonymous is valid only for ldaps:// and not startTLS
(I see that, with supportAnonymous=false and startTls=true it seems not to do any StartTLS attempt even.)
I have found something else interesting (leaving supportAnonymous to default, with startTLS=true). I performed a tcpdump and observed:
- plaintext bindRequest(1) "<ROOT>" simple, then extendedReq(2) LDAP_START_TLS_OID, etc. OK
- TLS seems to be built up successfully (Change Cipher Spec, Encrypted Handshake Message), etc. OK
- Gerrit initiates the close of the connection with the very small packet size (66 bytes TCP FIN). HUH?
- Gerrit initiates a fully new connection. ??
- Gerrit now talks in plaintext with a bindRequest() for the configured username. huh, that is really not okay!
bindRequest(1) "service-gerrit@MYDOMAIN.TLD" simple.
It seems to lose the StartTLS connection context and just performs the bind on a new plaintext connection. Something seems horribly broken here.
(This was without your patch.)
Okay, I will, in a moment.
> Again, the supportAnonymous is valid only for ldaps:// and not startTLS
(I see that, with supportAnonymous=false and startTls=true it seems not to do any StartTLS attempt even.)
I have found something else interesting (leaving supportAnonymous to default, with startTLS=true). I performed a tcpdump and observed:
- plaintext bindRequest(1) "<ROOT>" simple, then extendedReq(2) LDAP_START_TLS_OID, etc. OK
- TLS seems to be built up successfully (Change Cipher Spec, Encrypted Handshake Message), etc. OK
- Gerrit initiates the close of the connection with the very small packet size (66 bytes TCP FIN). HUH?
- Gerrit initiates a fully new connection. ??
- Gerrit now talks in plaintext with a bindRequest() for the configured username. huh, that is really not okay!
bindRequest(1) "service-gerrit@MYDOMAIN.TLD" simple.
It seems to lose the StartTLS connection context and just performs the bind on a new plaintext connection. Something seems horribly broken here.
(This was without your patch.)
al...@gmail.com <al...@gmail.com> #17
What you say is very disturbing... it suggests that there is a bug in java-11 for the same sequence that cause leak of information. If I will have time tomorrow I will send you a small test program to perform the ldap bind with starttls so you can play with it without gerrit. I just lost my test environment so I cannot perform many tests.
It has nothing to do with the patches, so if this is the behaviour you experiencing, actually seeing in wireshark the plaintext in java-11 and not in java-8 then the problem is probably somewhere else.
It has nothing to do with the patches, so if this is the behaviour you experiencing, actually seeing in wireshark the plaintext in java-11 and not in java-8 then the problem is probably somewhere else.
al...@gmail.com <al...@gmail.com> #18
You can try this[1] example. Just add ctx.reconnect(null) before "do something useful", please run this with jdk-8 and jdk-11 and try to reproduce the issue.
[1]https://docs.oracle.com/javase/jndi/tutorial/ldap/ext/src/StartTlsSimple.java
[1]
ge...@gmail.com <ge...@gmail.com> #19
I've tried the patch in #comment14 , with the configuration (ldaps schema with startTLS, are you sure?)
http://paste.openstack.org/show/778461/
This does not use StartTLS, but only talks on tcp port 636 in plain TLS.
same patch in #comment14 , with the configuration (but ldap schema)
http://paste.openstack.org/show/778460/
> debug (-d 255) and maybe -n so I can see how initial connection is performed
I had to sanitize a lot in the details here, not sure what data can be decoded into what. I hope it still provides enough data...
http://paste.openstack.org/show/778462/
> What you say is very disturbing... it suggests that there is a bug in java-11 for the same sequence that cause leak of information.
I agree here. Yikes.
Thanks for your further links so far, I'll play around more tomorrow too! :-)
This does not use StartTLS, but only talks on tcp port 636 in plain TLS.
same patch in
> debug (-d 255) and maybe -n so I can see how initial connection is performed
I had to sanitize a lot in the details here, not sure what data can be decoded into what. I hope it still provides enough data...
> What you say is very disturbing... it suggests that there is a bug in java-11 for the same sequence that cause leak of information.
I agree here. Yikes.
Thanks for your further links so far, I'll play around more tomorrow too! :-)
ge...@gmail.com <ge...@gmail.com> #20
I have found this OpenJDK bug report that described the exact observations I see on Java 11 - also describes the use of a method (ctx.getAttributes) we also invoke [2] - and it was closed with the comment "not a bug" [1] (it was reported against version Java 5). Without any specific reasoning, I presume this was close because it specified a new server with 'ldap://' in that "line A".
Also, I notice that the code path with ctx.getAttributes in [2] is only chosen in Gerrit if ldap.accountMemberField is set, which is positive for my case with this being set in case of Active Directory.
I've tried to reproduce it simple code examples and I was unable to reproduce this on Ubuntu 18.04 JRE 11, Debian Buster JRE 11 (as in production where I noticed it) and remote-JDK11 from Bazel. :-(
Now here's a theory I have, that may explain why I do see this "out in the field". That "line A" from the bug report [1]. Could it be that the resolution of the Group DN as entered in [3] includes a full 'ldap://' URL again because of LDAP referrals [4] which happens due to a Gerrit code path I haven't found yet? That would *totally* make sense and reproduce the behavior. (Also it would explain why I see many more connections being opened on JRE 11!)
I'll try to debug that more later.
(Although I'm running a single Samba AD server and it should not refer to other servers, I noticed 'weird' behavior like this too using Apache Directory Studio.)
Thank you so much so far for the pointers, Alon!
[1]:https://bugs.openjdk.java.net/browse/JDK-6880657
[2]:https://gerrit.googlesource.com/gerrit/+/350a451832fc53c71c33e5387793b30250db6a41/java/com/google/gerrit/server/auth/ldap/Helper.java#348
[3]:https://gerrit.googlesource.com/gerrit/+/350a451832fc53c71c33e5387793b30250db6a41/java/com/google/gerrit/server/auth/ldap/Helper.java#346
[4]:https://docs.oracle.com/javase/jndi/tutorial/ldap/referral/jndi.html
Also, I notice that the code path with ctx.getAttributes in [2] is only chosen in Gerrit if ldap.accountMemberField is set, which is positive for my case with this being set in case of Active Directory.
I've tried to reproduce it simple code examples and I was unable to reproduce this on Ubuntu 18.04 JRE 11, Debian Buster JRE 11 (as in production where I noticed it) and remote-JDK11 from Bazel. :-(
Now here's a theory I have, that may explain why I do see this "out in the field". That "line A" from the bug report [1]. Could it be that the resolution of the Group DN as entered in [3] includes a full 'ldap://' URL again because of LDAP referrals [4] which happens due to a Gerrit code path I haven't found yet? That would *totally* make sense and reproduce the behavior. (Also it would explain why I see many more connections being opened on JRE 11!)
I'll try to debug that more later.
(Although I'm running a single Samba AD server and it should not refer to other servers, I noticed 'weird' behavior like this too using Apache Directory Studio.)
Thank you so much so far for the pointers, Alon!
[1]:
[2]:
[3]:
[4]:
ge...@gmail.com <ge...@gmail.com> #21
Also, the behavior of reconnect() seems changed in JRE 11 when comparing sources, git-blamed on this: [1].
The time of day does not allow me to oversee the full consequences of this diff, but it looks like it does change the way the TLS connection is fully re-initiated.
[1]:https://github.com/AdoptOpenJDK/openjdk-jdk11u/commit/7b7d6755b5eb173c7f567997b869615cbbc8c7e5
The time of day does not allow me to oversee the full consequences of this diff, but it looks like it does change the way the TLS connection is fully re-initiated.
[1]:
ge...@gmail.com <ge...@gmail.com> #22
*Found the bugger.*
We seem to (ab)use LdapContext.reconnect() as a check whether we can bind with the given username+password, but other than that it's supposed to be a no-op at that point of the code. I believe reconnect() was never supposed to be used as a test like that from reading the java documentation. All code examples use lazy-connecting, ie. connection will be set up by JNDI when it's needed on actual operations being done.
So it worked well for us up to and including Java 8, but unfortunately, for improved TLS functionality supported with JDK 9+, it will do a full reconnect and TLS handshake [1] (reason there: re-evaluating a CRL from the CA cert).
This not only uncovers this StartTLS issue, but it also explains why I see an explosion of LDAP connections from Gerrit to my LDAP server when run under Java 11, which I still experienced with StartTLS disabled.
I believe we should find a simple replacement for a non-lazy evaluation of the security context parameters set.
A WIP change uploaded here: [2]. @Alon: do you have a better idea to perform a non-lazy LDAP bind? The best I can find online is "use reconnect()" or try to find the "root DSE" (?). For regular LDAP auth we need this as-is, but for the HTTP_LDAP case we could do it lazily and catch the exception when it happens, I guess?
[1]:https://github.com/AdoptOpenJDK/openjdk-jdk11u/commit/7b7d6755b5eb173c7f567997b869615cbbc8c7e5
[2]:https://gerrit-review.googlesource.com/c/gerrit/+/238832
[3]:https://stackoverflow.com/q/7348867/1254292
We seem to (ab)use LdapContext.reconnect() as a check whether we can bind with the given username+password, but other than that it's supposed to be a no-op at that point of the code. I believe reconnect() was never supposed to be used as a test like that from reading the java documentation. All code examples use lazy-connecting, ie. connection will be set up by JNDI when it's needed on actual operations being done.
So it worked well for us up to and including Java 8, but unfortunately, for improved TLS functionality supported with JDK 9+, it will do a full reconnect and TLS handshake [1] (reason there: re-evaluating a CRL from the CA cert).
This not only uncovers this StartTLS issue, but it also explains why I see an explosion of LDAP connections from Gerrit to my LDAP server when run under Java 11, which I still experienced with StartTLS disabled.
I believe we should find a simple replacement for a non-lazy evaluation of the security context parameters set.
A WIP change uploaded here: [2]. @Alon: do you have a better idea to perform a non-lazy LDAP bind? The best I can find online is "use reconnect()" or try to find the "root DSE" (?). For regular LDAP auth we need this as-is, but for the HTTP_LDAP case we could do it lazily and catch the exception when it happens, I guess?
[1]:
[2]:
[3]:
ed...@gmail.com <ed...@gmail.com> #23
[Empty comment from Monorail migration]
ed...@gmail.com <ed...@gmail.com> #24
Adding this to the ESC component.
The ESC should discuss the severity of this security issue, agree on who's driving the investigation and which releases need fixes.
[Monorail components: ESC]
The ESC should discuss the severity of this security issue, agree on who's driving the investigation and which releases need fixes.
[Monorail components: ESC]
al...@gmail.com <al...@gmail.com> #25
As much as I appreciate the willing to solve this, I do not agree with you conclusions. It is not a matter of lazy, but the need to force use changes in the environment. Otherwise each jndi call will have to compare the entire environment to implicitly perform som fixing activity. The use of explicit reconnect was designed explicitly for notification to the driver to re-evaluate the environment and reconnect using these settings. I trust you that you see different behavior in java11, however, before jumping into conclusions I wold have waited until a bug agains ido with reproduction use case is discussed and closed without a fix. Not only gerrit is affected by this break (assuming there is one), many other applications are using the same pattern in the last 15 years or so. This is at least backward compatibility issue that should be formally announced by upstream, along with explicit alternate method.
ge...@gmail.com <ge...@gmail.com> #26
Thanks Edwin for handling my request via email the same day, much appreciated.
And thanks Alon, for your view again. :-)
And thanks Alon, for your view again. :-)
Okay. Do you have any reference for this requirement to re-evaluate the environment? Java 8 docs on reconnect() say "Reconnects to the LDAP server [...]" [5] which seem to indicate that it does what the method is called, and that it effectively rebinds as well looks like more of a side effect to me (this is just my interpretation). The only thing that is stated to remain is the _controls_.
Aside StackOverflow answers, I find mostly code *not* calling reconnect() - and it appears to re-evaluate the environment just fine without that. Examples: [4], [1].
> Not only gerrit is affected by this break (assuming there is one), many other applications are using the
> same pattern in the last 15 years or so.
Could you provide some examples? I found only a handful of seemingly personal projects and one major project "Spring LDAP", which has a PR pending [1] for the same issue. Perhaps my "find code online" skills are not the best or perhaps most of it is closed source. :-)
By the way, I think I have found better references from the JDK project:
- JDK 9+ behavioral change of reconnect() reported recently in [2].
- Change causing this was partially reverted in a commit ~ 6 weeks ago for JDK 14 in [3].
This would support your statement that reconnect() *should* indeed not open a new connection, but OTOH also does not imply this is the appropriate way to check for a successful LDAP bind, IIUC.
> This is at least backward compatibility issue that should be formally announced by upstream, along with explicit
> alternate method.
Completely agree with you on this. This is a full surprise with nasty side effects.
I will do more testing and research in the coming days.
(It seems notoriously hard to find credible sources/info with the fragmentation of JDK development going on and Oracle seemingly to have taken down some links for 9+ docs.)
I guess we can use some integration/E2E tests on LDAP as well, to make it easier to reproduce.
[1]:
[2]:
[3]:
[4]:
[5]:
al...@gmail.com <al...@gmail.com> #27
Once again, in my humble opinion and with respect, guessing nor bible interpretation of the Internet is the way to go. I suggest you move the discussion to upstream with minimal reproduction.code and get definite response, someone may realize if this is intentional, bug or intentional with known backward compatibility, or even unaware of this.
I trust you can find reconnect being used by many popular projects sources as I have.
I am sorry I cannot help at this time with the process, I had lost my development machine recently.
I trust you can find reconnect being used by many popular projects sources as I have.
I am sorry I cannot help at this time with the process, I had lost my development machine recently.
hi...@google.com <hi...@google.com> #28
Thank you Gert and Alon for digging into this. We discussed this in today's ESC meeting.
Gert, you seem to have emerged into this issue very far already - which is great. Can you own the resolution?
I can offer to help with reviews or a little bit of thinking on the side if desired.
Gert, you seem to have emerged into this issue very far already - which is great. Can you own the resolution?
I can offer to help with reviews or a little bit of thinking on the side if desired.
ge...@gmail.com <ge...@gmail.com> #29
> Can you own the resolution?
I think the most important thing now is to make a test plan (more below), so that we can show what's broken and what works before and after a patch.
For the approach to the fix, I still *think* we should not call reconnect() and I'd suggest to go for the 'creative' approach as in the spring-ldap PR linked earlier. However, I also have to weigh in Alon's response - who clearly has much more experience in this field.
Either way, we have to be super careful not to introduce another issue for other combinations of JRE/LDAP server (or even a another security issue like letting authenticate() pass without providing the right credentials in auth.type=LDAP/LDAP_BIND scenarios).
For testing, I have in mind to make a matrix of combinations of software to ensure that we don't break something else. And I'm afraid it's huge with lots of scope creep... :(
- JREs: OpenJDK & Oracle. (Maybe IBM's one, or OpenJ9 or others?)
- JRE versions: 8, 11. (Maybe include a latest JDK 14 with the "new" behavior as linked before?)
- Gerrit, 2.15, 2.16, 3.0, master. (2.15 does not support StartTLS, but might still be affected to the connection bug)
- LDAP: OpenLDAP, Samba 4. (Maybe FreeIPA too? I don't think it will be easy for me to get a real MS AD server spun up and I can't think of a reason for why it would behave differently.)
Configurations:
- Gerrit with: ldap.startTls on/off, ldap.supportAnonymous on/off.
- Gerrit with auth.type=LDAP, auth.type=LDAP_BIND and auth.type=HTTP_LDAP (CLIENT_SSL_CERT_LDAP not so relevant perhaps because same external auth code path as HTTP_LDAP).
- Gerrit with LDAP connection pooling on/off.
- LDAP server with StartTLS off, opportunistic StartTLS, mandatory StartTLS (= modern default).
Tests to perform / observations to verify: (e.g. with the use of a packet capture and ngrep/tshark automation?)
- Whether connections made are actually enforcing TLS (connection not established if LDAP server StartTLS off with Gerrit ldap.startTls=on and Gerrit 2.16+).
- With auth.type=LDAP, auth.type=LDAP_BIND, check if authentication passed with correct password, is rejected with wrong.
- Verify other regular use still works: regular TLS.
- Number of connections made. (the performance aspect of this bug)
> I can offer to help with reviews or a little bit of thinking on the side if desired.
Thanks! First question: WDYT of the above parameters? In my head it's enormous now... any ideas appreciated. I'd love to hear I'm overthinking this! :-)
Due to the nature of these tests leaving the scope of Gerrit as Java application, I think it will be not so straightforward to test them properly with Bazel integration tests (challenges: packet captures, JRE keystore configuration for custom CA certificate to trust LDAP server). And as far as I know we don't have much in place currently to start from.
I think the most important thing now is to make a test plan (more below), so that we can show what's broken and what works before and after a patch.
For the approach to the fix, I still *think* we should not call reconnect() and I'd suggest to go for the 'creative' approach as in the spring-ldap PR linked earlier. However, I also have to weigh in Alon's response - who clearly has much more experience in this field.
Either way, we have to be super careful not to introduce another issue for other combinations of JRE/LDAP server (or even a another security issue like letting authenticate() pass without providing the right credentials in auth.type=LDAP/LDAP_BIND scenarios).
For testing, I have in mind to make a matrix of combinations of software to ensure that we don't break something else. And I'm afraid it's huge with lots of scope creep... :(
- JREs: OpenJDK & Oracle. (Maybe IBM's one, or OpenJ9 or others?)
- JRE versions: 8, 11. (Maybe include a latest JDK 14 with the "new" behavior as linked before?)
- Gerrit, 2.15, 2.16, 3.0, master. (2.15 does not support StartTLS, but might still be affected to the connection bug)
- LDAP: OpenLDAP, Samba 4. (Maybe FreeIPA too? I don't think it will be easy for me to get a real MS AD server spun up and I can't think of a reason for why it would behave differently.)
Configurations:
- Gerrit with: ldap.startTls on/off, ldap.supportAnonymous on/off.
- Gerrit with auth.type=LDAP, auth.type=LDAP_BIND and auth.type=HTTP_LDAP (CLIENT_SSL_CERT_LDAP not so relevant perhaps because same external auth code path as HTTP_LDAP).
- Gerrit with LDAP connection pooling on/off.
- LDAP server with StartTLS off, opportunistic StartTLS, mandatory StartTLS (= modern default).
Tests to perform / observations to verify: (e.g. with the use of a packet capture and ngrep/tshark automation?)
- Whether connections made are actually enforcing TLS (connection not established if LDAP server StartTLS off with Gerrit ldap.startTls=on and Gerrit 2.16+).
- With auth.type=LDAP, auth.type=LDAP_BIND, check if authentication passed with correct password, is rejected with wrong.
- Verify other regular use still works: regular TLS.
- Number of connections made. (the performance aspect of this bug)
> I can offer to help with reviews or a little bit of thinking on the side if desired.
Thanks! First question: WDYT of the above parameters? In my head it's enormous now... any ideas appreciated. I'd love to hear I'm overthinking this! :-)
Due to the nature of these tests leaving the scope of Gerrit as Java application, I think it will be not so straightforward to test them properly with Bazel integration tests (challenges: packet captures, JRE keystore configuration for custom CA certificate to trust LDAP server). And as far as I know we don't have much in place currently to start from.
hi...@google.com <hi...@google.com> #30
> For testing, I have in mind to make a matrix of combinations of software to ensure that we don't break something else. And I'm afraid it's huge with lots of scope creep... :(
Yes, that would be a lot of work with lots of potential to miss cases. In the end, this would also mean that we test behavior in Gerrit that should be tested in the respective other projects - be it any JDKs or LDAP implementations.
My blunt question here is: Is there a way for the operator of the LDAP server to prevent this from happening? If - for example - the LDAP server is configured to only listen to secure ports using a secure protocol, Gerrit's connection attempt should just fail. Is this correct?
If not, my understanding is that Gerrit's connection attempt if it's configured to use LDAPS would just fail. The security vulnerability then comes from users trying to fix this by applying advice from the internet. If that is correct, Gerrit could be more chatty in the error message and pro-actively inform admins about the situation.
Yes, that would be a lot of work with lots of potential to miss cases. In the end, this would also mean that we test behavior in Gerrit that should be tested in the respective other projects - be it any JDKs or LDAP implementations.
My blunt question here is: Is there a way for the operator of the LDAP server to prevent this from happening? If - for example - the LDAP server is configured to only listen to secure ports using a secure protocol, Gerrit's connection attempt should just fail. Is this correct?
If not, my understanding is that Gerrit's connection attempt if it's configured to use LDAPS would just fail. The security vulnerability then comes from users trying to fix this by applying advice from the internet. If that is correct, Gerrit could be more chatty in the error message and pro-actively inform admins about the situation.
al...@gmail.com <al...@gmail.com> #31
I am sorry, but I will repeat and the mute myself. This issue is not directly relevant to gerrit and should be discussed upstream as a regression. Solution should come from upstream directly and explicitly. Any attempt to guess or find a “working” hack will result in future breakage.
Per statements above it is clearly you are very far away from idm slot if you think gerrit admin may change enterprise idm settings.
Kind regards,
Per statements above it is clearly you are very far away from idm slot if you think gerrit admin may change enterprise idm settings.
Kind regards,
hi...@google.com <hi...@google.com> #32
> I am sorry, but I will repeat and the mute myself. This issue is not directly relevant to gerrit and should be discussed upstream as a regression. Solution should come from upstream directly and explicitly. Any attempt to guess or find a “working” hack will result in future breakage.
Sure. However, I see no reason why Gerrit can't be pro-active in informing users about a potential issue. May it be through documentation or a more helpful error message. That effort is separate from any fix upstream.
> Per statements above it is clearly you are very far away from idm slot if you think gerrit admin may change enterprise idm settings.
I think if the enterprise identity server allows connections on insecure ports or using otherwise insecure protocols, that should be changed irrespective of this issue.
Sure. However, I see no reason why Gerrit can't be pro-active in informing users about a potential issue. May it be through documentation or a more helpful error message. That effort is separate from any fix upstream.
> Per statements above it is clearly you are very far away from idm slot if you think gerrit admin may change enterprise idm settings.
I think if the enterprise identity server allows connections on insecure ports or using otherwise insecure protocols, that should be changed irrespective of this issue.
ge...@gmail.com <ge...@gmail.com> #33
> Is there a way for the operator of the LDAP server to prevent this from happening? If - for example -
> the LDAP server is configured to only listen to secure ports using a secure protocol, Gerrit's connection
> attempt should just fail. Is this correct?
No, in TLS clients should enforce use of TLS when configured to do so. Analogy: if you typehttps://google.com in your browser's URL bar, and it would make an attempt to do plaintext HTTP to google.com , that is an issue as this can be MitM'd, regardless of the server configuration.
Hardened server configurations can mitigate only passive attackers (sniffers) from obtaining the secrets & data over the wire, with Gerrit showing the failure I had initially in this issue. It would still expose the username (where this should be hidden in a proper StartTLS connection).
> Gerrit's connection attempt if it's configured to use LDAPS would just fail.
LDAP with StartTLS enabled will fail without further consequences IF the connection is free-of-malicious-users. But that's exactly the purpose of using TLS here, to not having to assume that.
Potentially, and essentially the main concern of me is the following. A MitM user can observe the behavior of Gerrit and MitM the plaintext connection attempts, accepting them, passing it onto a secure connection to the LDAP server transparently to Gerrit, thus also hiding the failure from Gerrit and snooping on the traffic.
> The security vulnerability then comes from users trying to fix this by applying advice from the internet.
I'm not so sure if I follow that part. Are you referring to how to _run_ Gerrit in Java 9+ and following comments fromhttps://crbug.com/gerrit/7843 ? Then, yeah, the scenario how this can happen in the wild on Gerrit installations is users deploying/upgrading to modern OSs with Java 11, failing to start Gerrit, finding that issue or the docs where it's hidden a bit, get it to work or not and run into the same as I did.
> I see no reason why Gerrit can't be pro-active in informing users about a potential issue. May it be
> through documentation or a more helpful error message. That effort is separate from any fix upstream.
I think I have to do more testing before I can give my opinion/verdict whether it's "Gerrit doing something wrong" or the JDK. As far as I can see for now, there's no way to control StartTLS on a reconnect() and how reconnect() behaves with a extendedOperation() call prior remains undocumented in JNDI.
I've attempted to follow "TLS with Simple Authentication" from the JNDI tutorial [1] (where StartTLS is used without reconnect()) to create a small test, outside of Gerrit code, and that works OK in both JRE 8 and 11 for me.
So far I have spent quite some time in the straightforward baseline test to get regular auth.type=LDAP/LDAP_BIND to work with Gerrit and I haven't been very successful somehow. Preliminary results on my specific case with auth.type=HTTP_LDAP w/ ldap.supportAnonymous=true & ldap.username=set seems to work properly with a change from reconnect() to lookup("") in the LdapHelper's open().
My next steps:
- Trying to make a small reproducer outside of Gerrit context, like the JNDI tutorial but with reconnect(), assuming that this will indeed show the same issue, supplying PCAPs with it. Plus PCAPs & debug output of command line ldap-utils. We can use this to discuss it with JDK people. (?)
- More testing with auth.type=LDAP/LDAP_BIND combinations replacing reconnect(). If that works, I'll post a preliminary patch for Gerrit (here?). I don't have the time on the short term to test all of the combinations, though.
[1]:https://docs.oracle.com/javase/jndi/tutorial/ldap/ext/starttls.html
> the LDAP server is configured to only listen to secure ports using a secure protocol, Gerrit's connection
> attempt should just fail. Is this correct?
No, in TLS clients should enforce use of TLS when configured to do so. Analogy: if you type
Hardened server configurations can mitigate only passive attackers (sniffers) from obtaining the secrets & data over the wire, with Gerrit showing the failure I had initially in this issue. It would still expose the username (where this should be hidden in a proper StartTLS connection).
> Gerrit's connection attempt if it's configured to use LDAPS would just fail.
LDAP with StartTLS enabled will fail without further consequences IF the connection is free-of-malicious-users. But that's exactly the purpose of using TLS here, to not having to assume that.
Potentially, and essentially the main concern of me is the following. A MitM user can observe the behavior of Gerrit and MitM the plaintext connection attempts, accepting them, passing it onto a secure connection to the LDAP server transparently to Gerrit, thus also hiding the failure from Gerrit and snooping on the traffic.
> The security vulnerability then comes from users trying to fix this by applying advice from the internet.
I'm not so sure if I follow that part. Are you referring to how to _run_ Gerrit in Java 9+ and following comments from
> I see no reason why Gerrit can't be pro-active in informing users about a potential issue. May it be
> through documentation or a more helpful error message. That effort is separate from any fix upstream.
I think I have to do more testing before I can give my opinion/verdict whether it's "Gerrit doing something wrong" or the JDK. As far as I can see for now, there's no way to control StartTLS on a reconnect() and how reconnect() behaves with a extendedOperation() call prior remains undocumented in JNDI.
I've attempted to follow "TLS with Simple Authentication" from the JNDI tutorial [1] (where StartTLS is used without reconnect()) to create a small test, outside of Gerrit code, and that works OK in both JRE 8 and 11 for me.
So far I have spent quite some time in the straightforward baseline test to get regular auth.type=LDAP/LDAP_BIND to work with Gerrit and I haven't been very successful somehow. Preliminary results on my specific case with auth.type=HTTP_LDAP w/ ldap.supportAnonymous=true & ldap.username=set seems to work properly with a change from reconnect() to lookup("") in the LdapHelper's open().
My next steps:
- Trying to make a small reproducer outside of Gerrit context, like the JNDI tutorial but with reconnect(), assuming that this will indeed show the same issue, supplying PCAPs with it. Plus PCAPs & debug output of command line ldap-utils. We can use this to discuss it with JDK people. (?)
- More testing with auth.type=LDAP/LDAP_BIND combinations replacing reconnect(). If that works, I'll post a preliminary patch for Gerrit (here?). I don't have the time on the short term to test all of the combinations, though.
[1]:
lu...@gmail.com <lu...@gmail.com> #34
@Gert thanks a lot for reporting the security issue.
In our latest release (v3.0.2) we explicitly say:
"Gerrit is not yet compatible with Java 9 or newer at this time."
That means that we *DO NOT* recommend or support Java 11 as yet.
However, I believe that it is *VERY IMPORTANT* to fix the security issues before releasing and supporting Java 11.
Can we say *for sure* that the problem does not happen on Java 8?
[2]https://gerrit-documentation.storage.googleapis.com/Documentation/3.0.2/install.html
In our latest release (v3.0.2) we explicitly say:
"Gerrit is not yet compatible with Java 9 or newer at this time."
That means that we *DO NOT* recommend or support Java 11 as yet.
However, I believe that it is *VERY IMPORTANT* to fix the security issues before releasing and supporting Java 11.
Can we say *for sure* that the problem does not happen on Java 8?
[2]
al...@gmail.com <al...@gmail.com> #35
Seems to be properly reported and resolved at upstream last month or so[1], let’s wait for release before jumping in.
[1]https://bugs.openjdk.java.net/browse/JDK-8217606 ?
[1]
lu...@gmail.com <lu...@gmail.com> #36
Yes, thanks, Alon for your pointer.
I do agree: let's see how things evolve on [1]. We never said that Java 9+ is supported, so we should be fine for now.
The next forthcoming v3.1 of Gerrit (due in Mid-November) won't officially offer support for Java 9+ (even if technically, yes, it would work) and we won't suggest to upgrade to Java 9+ as yet.
I do agree: let's see how things evolve on [1]. We never said that Java 9+ is supported, so we should be fine for now.
The next forthcoming v3.1 of Gerrit (due in Mid-November) won't officially offer support for Java 9+ (even if technically, yes, it would work) and we won't suggest to upgrade to Java 9+ as yet.
da...@gmail.com <da...@gmail.com> #37
Per discussion in yesterday's ESC meeting, removing the security status of this issue.
lu...@gmail.com <lu...@gmail.com> #38
[Empty comment from Monorail migration]
hi...@google.com <hi...@google.com> #39
The upstream bug was closed. Can we close this one?
lu...@gmail.com <lu...@gmail.com> #40
Sure, let me do a manual verification today, using the Gerrit master (with Java 11) and an LDAPS server.
lu...@gmail.com <lu...@gmail.com> #41
This will be done with the aws-gerrit environment setup on Java11
lu...@gmail.com <lu...@gmail.com> #42
We have validated this use-case with aws-gerrit:
https://gerrit.googlesource.com/aws-gerrit/+/refs/heads/master/gerrit/etc/gerrit.config.template
The aws-gerrit setup has been used for Gerrit v3.3 release plan and will also be used for Gerrit v3.4.
Marking this as resolved.
The aws-gerrit setup has been used for Gerrit v3.3 release plan and will also be used for Gerrit v3.4.
Marking this as resolved.
ek...@google.com <ek...@google.com> #43
[Monorail components: Backend]
ek...@google.com <ek...@google.com> #44
[Monorail components: -LDAP]
ek...@google.com <ek...@google.com> #45
[Monorail components: SteeringCommittee]
ek...@google.com <ek...@google.com> #46
[Monorail components: -ESC]
ek...@google.com <ek...@google.com> #47
[Empty comment from Monorail migration]
is...@google.com <is...@google.com> #48
Edits were made to reflect the following in Monorail: auto-CCs.
Description
Affected Version: probably 2.16+, tested with master/3.0.2-2996-g6b99fd0dbd
What steps will reproduce the problem?
0. Configure Gerrit with LDAP/HTTP_LDAP authentication and StartTLS enabled + required.
1. Start Gerrit in JRE 11 (either JDK 11 build or JDK 8 build, doesn't matter).
What is the expected output?
Same configuration to work in both Java 8 and Java 11 runtime.
What do you see instead?
Client logged in is not a member of any group any more.
Server logs error:
javax.naming.AuthenticationNotSupportedException: [LDAP: error code 8 - BindSimple: Transport encryption required.]
Full traceback:http://paste.openstack.org/show/778456/
But I have transport encryption enabled using StartTLS, via unchanged configuration:
Please provide any additional information below.
I can clearly reproduce this by *only* changing the JRE from 8 to 11, so I suspect there's something Gerrit should do to properly initialize the LDAP connector on JRE 11 vs 8.
I'm using the plain default JREs in Debian:
Setting the 'ldaps' schema as an attempt to work around this, e.g. "server = ldaps://...", we have a separate issue:https://crbug.com/gerrit/11566 .
I'm using Samba 4 AD from Ubuntu 18.04 as LDAP server, which is supposed to behave like MS AD 2008, IIUC.
Current work-around: use plain old TLS LDAP on port 636:
I believe this also rules out custom CA certificate management that changed in JRE 11 (with a different type of key store and whatnot that I had to adjust for), because the work-around without StartTLS works fine with sslVerify = true.