cloud-gov / shibboleth-boshrelease

IdP using Shibboleth as a BOSH release
Other
2 stars 1 forks source link

Upgrade Shibboleth to a newer release #82

Closed ccostino closed 3 years ago

ccostino commented 3 years ago

We need to update Shibboleth to a more recent release, which entails a few things:

Once we figure these things out, we then need to make sure we fully test the upgrades once we start rolling things into the dev and staging environments before shifting things to production.

Security considerations

ccostino commented 3 years ago

@ChrisMcGowan, did I miss anything from our conversation earlier today? Please feel free to make edits directly; thanks!

pburkholder commented 3 years ago

Please also update Java - it's a few releases behind. I think Java 8 may be EOL. Should this be a separate issue?

onelittlebecca commented 3 years ago

From Steve: I updated tomcat, shibby and java. The plugin we use is throwing some null pointer exception but the stack traces and logs seem to be getting truncated.

You could see the line of the null pointer and I was at the point of trying to figure out why “the thing” (I can’t remember exactly) was null

onelittlebecca commented 3 years ago

And logs are in /var/vcap/sys/log

karareinsel commented 3 years ago

@onelittlebecca @Dark-Knight-999 I'm moving this to doing since work has begun.

pburkholder commented 3 years ago

While we're working on this, let's bump openJDK to Latest GA release: 8u302 per https://wiki.openjdk.java.net/display/jdk8u. Can we?

pburkholder commented 3 years ago

I've created #94 for the Java update

ccostino commented 3 years ago

In reviewing the IDP logs for Shibboleth some more tonight I realized that the errors are happening in multiple spots. The "good" news is it appears consistent (meaning the same error, a NullPointerException is being thrown) and for the same reason in terms of trying to retrieve an object but not. Now it's a matter of figuring out why; what changed in the Tomcat upgrade that makes the code behave differently now (at least, this is what is suspected now as the reason behind the error being thrown).

ccostino commented 3 years ago

I'm also trying to understand the differences and behaviors of the getSubcontext method - there's 4 versions of it: http://shibboleth.net/sites/release/java-opensaml/4.1.1/apidocs/org/opensaml/messaging/context/BaseContext.html#getSubcontext(java.lang.Class)

ccostino commented 3 years ago

@ChrisMcGowan, I'm not seeing anything out of the ordinary here with Shibboleth's Tomcat 8 page, but is there a way we can adjust the server settings within the BOSH release itself or just that they're set and working correctly?

Part of the confusion here (at least for me) is which piece is actually being impacted by the Tomcat upgrade - Shibboleth itself, just the plugin, or both? Is there some way to see the Tomcat logs themselves in the container? Barring that, is there an easy way to setup and run a BOSH release locally? Or will I just have to get Shibboleth and this plugin running on their own on my machine?

ccostino commented 3 years ago

One last thought: it seems odd that the TokenUserContext class continues to be set/instantiated correctly, but the UsernamePasswordContext class does not, and both are tied with the AuthenticationContext class and derived off of the same profileRequestContext object. I've been trying to dig more into the UsernamePasswordContext class to see how it's all wired up and where the breakdown might be but this has been tough so far, it's a part of Shibboleth itself and the plugin is just hooking into it. I think I did find the corresponding documentation for developers, though: https://shibboleth.atlassian.net/wiki/spaces/IDP30/pages/2483847274/ProfileHandling

Found the following docs for v3.4.8:

But I can't find the TokenUserContext, and now I know partially why: it's import path is net.kvak.shibboleth.totpauth.api.authn.context.TokenUserContext (which has no docs here? https://shibboleth.net/sites/release/java-identity-provider/3.4.8/apidocs/) but the other two come from net.shibboleth.idp.authn.context - now I'm wondering what the kvak directory/module is or if it's anything other than what's set in the plugin itself, whereas the other two classes come from Shibboleth itself.

Looks like I answered my own question; TokenUserContext is from the plugin itself, defined here: https://github.com/cloud-gov/Shibboleth-IdP3-TOTP-Auth/blob/master/totpauth-api/src/main/java/net/kvak/shibboleth/totpauth/api/authn/context/TokenUserContext.java

Presumably then the plugin itself might actually be playing nice with Tomcat given that this object can still be found and is set properly as the code runs, but Shibboleth is not since the UsernamePasswordContext (or its... parent? the AuthenticationContext class) returns null, which then causes the NPE (NullPointerException) to be thrown because we're trying to call a method off of a null object instead of a properly instantiated one.

The other thing I'll add is that when retrieving a TokenUserContext object with these getSubcontext methods it's also passing in True, which will instantiate the object if it doesn't already exist. Attempting to do the same with the UsernamePasswordContext object throws a different error.

ccostino commented 3 years ago

I confirmed today that the plugin and Shibboleth are talking to the database just fine, it really is just a matter of the app loading or instantiating the data properly, specifically the UsernamePasswordContext object.

ccostino commented 3 years ago

I've been trying to add another dependency to help with debugging, specifically the ReflectionsToStringBuilder and ToStringStyle dependencies, but I cannot get it to import correctly even after adding it to the Gradle dependencies. This results in a nested exception is java.lang.NoClassDefFoundError: org/apache/commons/lang3/builder/ToStringStyle error being thrown, and I'm stumped as to how to fix it at the moment.

After researching this some it seems to be an issue with finding the *.jar file that should hold these classes/objects and that perhaps the CLASSPATH environment variable needs to be updated, but I'm not sure how that is all wired up with building a BOSH release; none of the other external dependencies have this issue.

Adding this would be a huge help in seeing what's in the UsernamePasswordContext object and if it's able to load any information or if it's completely blank.

ccostino commented 3 years ago

While poking around in the VM/container itself where the IDP package is installed and trying to figure out why I'm not seeing the Apache Commons JAR file installed when it's now listed as a dependency within the plugin, I noticed something else: the configuration changes outlined in the plugin's repo don't appear to be present in the config/idp.properties file or the conf/authn/general-authn.xml file, yet the code is clearly being touched because we see it in the logs, and I do see the two JAR files for the plugin in the idp package (in idp/edit-webapp/WEB-INF/lib).

The config discrepancy doesn't make any sense to me though, shouldn't we be seeing those?

ccostino commented 3 years ago

I just checked production and the files I was looking at do not have this configuration setup either, so it must not be necessary. Admittedly, this adds to my confusion of how this plugin is actually configured with Shibboleth... ¯\_(ツ)_/¯

ccostino commented 3 years ago

I think I may have found our culprit! Taken from the release notes page for the V3 IdP, specifically version 3.3.0:

By default, the IdP now removes the in-memory copy of the user's password from the internal state of the request after validating it. If you need to retain the UsernamePasswordContext for the life of the request, you will need to add a bean to conf/authn/password-authn-config.xml named shibboleth.authn.Password.RemoveAfterValidation and set it to java.boolean.FALSE.

How do we check/set this within the Shibboleth config in a BOSH release?

ccostino commented 3 years ago

I did some more research today to try and figure out what to do and I think it comes down to these steps:

  1. Update the Shibboleth config with this RemoveAfterValidation bean: https://shibboleth.atlassian.net/wiki/spaces/IDP30/pages/2494726322/PasswordAuthnConfiguration#PasswordAuthnConfiguration-Beans
  2. Update the BOSH release
  3. Undo a few of the last changes I had made in the TOTP plugin
  4. Release it all into dev and test it

If someone wants to pick this up while I'm out, by all means please have at it! Otherwise I will resume when I return and look for help on setting the bean and creating the BOSH release as neither of those steps are entirely clear to me.

ccostino commented 3 years ago

We have verified the fix and it is indeed the configuration change! Changing the configuration in Shibboleth to revert back to the older behavior and reverting the recent changes in the plugin got everything working again.

The plugin updates have already been merged, too! The next step is to get the submodule reference updated here.

ccostino commented 3 years ago

This is now fully deployed in dev and ready for full testing!

ccostino commented 3 years ago

We have now shifted this to staging for more testing and to prep for release!

ccostino commented 3 years ago

It has been a long time in coming, but this update is now live in production! A huge thank you to Steve, Becca, Chris, Van, Ben, and whoever else worked on this to get everything in order. 🎉