Archived

This forum has been archived. Please start a new discussion on GitHub.

IceSSL: Additional failure info accessible from the low level SSL engine?

We are looking to enhance the user experience around certification validation failures in our application. IceSSL::verify() does provide verification for a number of items ( e.g. cert chain provided, chain valid, chain depth, and trust ), and potentially more. But if any of those fail, the return is simply a true/false. We understand that we could write our own logic to do these verifications up front, but if Ice already has this logic build in, it would be nice to be able to access it so we don't have to have our own. Currently we do install a certificate verifier as well, which we use to do extra validation, but that also won't be called unless the low level Ice SSL checks pass. If this information could be obtained, we could enhance our certificate dialog prompt back to the user that might help them more accurately understand the failure and how to fix it.
Thanks!

«13

Comments

  • xdm
    xdm La Coruña, Spain

    Hi Jeremy,

    I see two ways you can do this, disable the IceSSL verification with IceSSL.VerifyPeer=0 and
    handle all the verification in your custom certificate verifier.

    An alternate way would be to catch the SecurityException throw by IceSSL and inspect the "reason"
    member. Your own certificate verifier can also throw SecurityException to indicate failures so you
    would be able to handle both the low-level IceSSL checks and your custom certificate verifier checks
    in the same way.

    Cheers,
    Jose

  • Hi Jose,

    Some additional information:

    We do have our own certificate verifier installed. When a certificate issue happens during a connection, we display a warning dialog. We then allow the user to either proceed with or deny the connection.

    For the user to do that, we need to inform him about the certificate issues. By inspecting the object provided by Ice we can see if the Ice certificate validation failed or not but we don't know why it failed.

    We could write code that finds the certificate issue by checking the certificate expiration date, the certificate chain structure, etc. We could also write code to call SChannel / SecureTransport / OpenSSL to do this validation for us.

    We would like to avoid writing that code if we could simply get the certificate issues from ice. It seems you do have that information, since that is available when the exception is thrown.

    However, inside the certificate verifier I don't think we have access to this information because no exception has been thrown yet.

    Thanks for the help.
    Fábio

  • xdm
    xdm La Coruña, Spain
    edited March 2021

    Hi Fábio,

    If you can work with an updated version of Ice, we can patch IceSSL to provide some kind of error code,
    this can be the native SSL library error code, or we can map these errors to a new IceSSL error code that
    is independent of the native SSL library, and then make it accessible to the verifier by either adding a
    new field to the IceSSL::ConnectionInfo object (this is a binary incompatible change) or using a thread
    static variable.

    With the Ice 3.7 API, there is no way to get this error information. You can use the native APIs of each SSL
    library to validate the certificates again and get the reason for the failure, but this is not trivial. We
    can help you write this code if required.

    Let us know what would be a better fit for your project and we can work from there.

    Cheers,
    Jose

  • Hi Jose.

    Creating a new IceSSL error code and making it accessible from IceSSL::ConnectionInfo would be perfect for us. Is this something that can be implemented on a future Ice release (3.7.6)?

    We are currently using 3.7.2 and we build it, so it if is possible to patch it that would work for us as well. If making the error code accessible from a static variable is simpler to implement, that would be fine.

    Thanks again,
    Fábio

  • xdm
    xdm La Coruña, Spain

    Hi Fabio,

    I going to implement this and I will get back to you as soon as I have something working.

    Cheers,
    Jose

  • That's awesome.

    Thanks so much.

    Fábio

  • xdm
    xdm La Coruña, Spain
    edited March 2021

    Hi Fabio,

    I opened a PR with the required changes for 3.7 https://github.com/zeroc-ice/ice/pull/1270
    feel free to post any comments there, it is still going throw review and subject to changes.

    Cheers,
    Jose

  • Hi Jose.

    It looks good.

    I see there will be an option to get an error description string as well. That will be pretty useful.

    I'm assuming it will only be possible for the certificate verification routine to return a single error. Is that correct?

    Thanks again.

    Fábio

  • Hi Jose,
    I'm having trouble building the branch that has your patch. Any chance you have any built binaries for these changes that we could just try out? Only need them for Windows for now.
    Thanks
    Jeremy

  • Never mind, I was able to build it after installing the Windows UCRT SDK dependency. I can't use the branch directly in our code as it looks like headers might have changed? We're running on 3.7.2 now. Regardless, the number of files modified was small enough that I just patched our 3.7.2 source and got that to go.
    I see now that there's an ExtendedConnectionInfo class that derives from ConnectionInfo, and it addes the TrustError member. What I don't see is how to use it. That class is defined in PluginI.h, which looks to be an internal header? Our code only references Plugin.h and ConnectionInfo.h from IceSSL, and neither of those look to have this reference. I'm probably missing something obvious here. Anyway, do you have an example of a program that would use these changes with the certificate verifier?
    Thanks
    Jeremy

  • xdm
    xdm La Coruña, Spain

    Sorry for the delay, the notification ended in my spam folder, and I didn't saw your replies

    Anyway, do you have an example of a program that would use these changes with the certificate verifier?

    The internal ExtendedConnectionInfo class is to avoid breaking binary compatibility, to get the error code you can call getTrustError with the info object provided to the verifier. For the next major release we can move trustError member directly to IceSSL::ConnectionInfo but for a patch release we cannot break binary compatibility.

    I'm assuming it will only be possible for the certificate verification routine to return a single error. Is that correct?

    Yes that is correct

    I see there will be an option to get an error description string as well. That will be pretty useful.

    If that is useful we can keep it in the public API

  • xdm
    xdm La Coruña, Spain

    The changes have been merge into the 3.7 branch, let me know if you find any issues.

    Cheers,
    Jose

  • Hey Jose,
    I finally got a build to work so I could test this. I was able to verify that it didn't report errors for a valid cert and chain, and that it reported "partial chain" when I removed a root CA.
    Questions:

    • What other certificate error cases did you test?
    • What errors should we be looking for?
    • I see you've merged it into the 3.7 branch. I think we're interested in picking up in whatever your next release would be. We're currently at 3.7.2, but would have no issues with moving forward. When could we pick this up in an official release?

    Thanks
    Jeremy

  • xdm
    xdm La Coruña, Spain

    Hi Jeremy

    In our test suite, we have checked

    TrustError::HostNameMismatch
    TrustError::PartialChain
    TrustError::UntrustedRoot
    TrustError::InvalidTime
    

    You might want to also check TrustError::InvalidPurpose you can get this if the certificate is used for a purpose other than what it was created for, for example, CA certificates need to be marked as so, and the same for client and server certificates.

    The updates will be part of the 3.7.6 patch release, we do a patch release once every 6 months, 3.7.5 was released the last January, so expect 3.7.6 around July this year.

    Cheers,
    Jose

  • Hi Jose,
    Good to hear!
    As I previously stated, we're at 3.7.2. Waiting for 3.7.6 in July is a bit too far out for us. We are considering either (1) patching our 3.7.2 and using that, or (2) going to 3.7.5 and patching that. I'm not aware of what changes were made between these versions. Which option would you recommend based on your knowledge of what has changed? There's always risk with moving to a newer version, but it's still 3.7.x so maybe that is minimal.
    Thanks again!
    Jeremy

  • xdm
    xdm La Coruña, Spain

    There a no major changes between 3.7.2 and 3.7.5, it is mostly minor bug fixes and support for new compilers, so either option should work. One thing that changed in 3.7.5 is the handling of class cycles that are now disallowed by default, you can set Ice.AcceptClassCycles to 1 to use the old behavior and accept class cycles.

    I would say that patching 3.7.5 would be a bit simpler, as there have not been many changes since the release, getting the additional bug fixes shouldn't hurt.

  • Hi Jose,
    Our build scripts reference the ice.test.sln as part of the build. I can't get this solution to build. I am pulling down the tags/v3.7.5 branch of Ice. I'm loading Ice.test.sln in the cpp/msbuild folder. I choose x64/debug. I get the following error:
    2>D:\source\rgs-tools\lib_src\Ice\V3.7.5\icelivesocket-master\src\ice\cpp\msbuild\packages\zeroc.icebuilder.msbuild.5.0.6\build\zeroc.icebuilder.msbuild.cpp.targets(78,14): error MSB4131: The "GeneratedCompiledPaths" parameter is not supported by the "Slice2CppDependTask" task. Verify the parameter exists on the task, and it is a gettable public instance property.

    I don't see this on the 3.7.3 or 3.7.4 branches.
    Any ideas?
    Thanks
    Jeremy

  • xdm
    xdm La Coruña, Spain

    Seems like the build is picking an old version of zeroc.icebuilder.msbuild tasks, the GeneratedCompiledPaths parameter is new in version 5.0.6, which is the version used in 3.7.5 branch.

    Looking at icelivesocket it was still configured to use the 5.0.4 version of the builder, I updated icelivesocket main branch to use 5.0.6 I think this will solve your problem.

    You might want to kill MSBuild process before attempting a new build in case the old icebuilder tasks are still cached with MSbuild process.

    If you have other projects in this build that also use zeroc.icebuilder.msbuild you might want to update them to 5.0.6 too.

    Let me know if this solves your problem.

    Cheers,
    Jose

  • That makes sense. If I build that Ice.test.sln on a clean version of v3.7.5 it works fine. In this case I had used our IceLiveSocket project which references the test projects and must be using that older version of the builder. I currently don't have access to that repo but Greg Hughes is helping me get access. Thanks again for the help.
    Jeremy

  • fish4life
    edited April 2021

    I got the Windows build of Ice working using those updates you put on the iceLiveSocket branch. We ended up just mirroring that repo to something local internally for us so we could apply tweaks to it.

    I'm tackling MacOS now. I can't get it to build Ice. It's probably something simple, but I can't seem to figure it out. I get a compile time error about "ld: library not found for -lmcpp". I had installed homebrew and did a "brew install lmdb mcpp" as per your README. I'm not sure if I need to modify something to point to the installed mcpp files? I assumed this would just work after installing that dependency. I'm on macOS 11.2.3

    (one other question, is v3.7.5 uses a newer version of OpenSSL than 1.0.2? I saw in the changelog file that Ice uses 1.1.0. We're trying to figure out if we can consolidate as we use 1.1.1 in other places)

    Thanks!

  • xdm
    xdm La Coruña, Spain

    On macOS if brew is installed in default location /usr/local this should just work otherwise, you might need to set MCPP_HOME LMDB_HOME environment variables. If using the default location you should have /usr/local/lib/libmcpp.a file after installing mcpp.

    Regarding OpenSSL versions, you can use 1.1.1 everywhere, as long as you use the same version to build Ice and your application things should work.

  • fish4life
    edited April 2021

    Yep, I have a Apple ARM system, so default was /opt/homebrew. The environment variable fixed it, thanks!

    Question regarding getTrustError() API you added. It does exactly as we requested, it reports back a single certificate error. What about the case where multiple errors are found in the cert? Does Ice simply stop processing the cert as soon as a single failure is found? For example, we use self signed certs which will always have a untrusted root error. This is correctly reported from getTrustError() if no other errors exist. But if the user types a host (e.g. IP address) that's not in our self signed cert, it will report a host mismatch error. This is fine, but was just curious if there was any Ice capability for multiple errors at the point where the certificate verifier is called.

    Question regarding VerifyDepthMax. I've set this value to 1, in hopes that it would trigger a chain length failure for a chain of length 2. I have a rootCA and a normal cert in my chain. I don't see getTrustError() triggering a fault on this. Maybe this is an invalid test case?

    Thanks!

  • xdm
    xdm La Coruña, Spain

    I think in most cases the underlying API we use give a single error per certificate, for example with OpenSSL we call SSL_get_verify_result https://www.openssl.org/docs/man1.0.2/man3/SSL_get_verify_result.html

  • Your TrustError enum does have the InvalidPurpose enum, which you had commented on earlier as something we should be checking. Out of curiosity does Ice do any low level checking of Key Usage/Advanced Key Usage fields too? My assumption would be no, since you don't know what the certificate is intended for. It would seem this is exactly what the cert verifier callback is provided for.
    Thanks!

  • xdm
    xdm La Coruña, Spain

    My assumption would be no, since you don't know what the certificate is intended for. It would seem this is exactly what the cert verifier callback is provided for.

    Some platforms like macOS require that on recent macOS versions you cannot use a server certificate if it has not the expected key usage (TLS Web server authentication), we don't do additional checks besides the default of the platform engine.

    See https://support.apple.com/en-us/HT210176

  • Makes sense. Thanks.
    I hope you don't mind all the question. I do appreciate your fast and informative responses, this is really helping us improve our certificate verification.

    Question: I'm using IceSSL.VerifyPeer=0, as you are aware. The intent being that no matter what fails on the Ice verifiction end, we get a chance to present that info to the user and allow them to accept or decline. This has been working very well until I started testing the IceSSL.VerifyDepthMax. If I have a chain length that exceeds this value, the connection fails automatically without a call to certificate verifier first. I studied the flow chart for verification in your documents, and I don't see this case called out. It should be that all failures fall through the the verifier first. Am i missing something here?
    Thanks again!

  • xdm
    xdm La Coruña, Spain

    Hi Jeremy,

    I'm glad to help, don't hesitate to ask!

    Setting IceSSL.VerifyPeer doesn't bypass the verify depth max check, nor the trust check (related to IceSSL.TrustOnly properties) it just allows to continue upon a failure in the chain verification done by the underlying SSL engine.

    https://doc.zeroc.com/ice/3.7/ice-plugins/icessl/programming-icessl/programming-icessl-in-c++#id-.ProgrammingIceSSLinC++v3.7-InstallingaCertificateVerifierinC++

    The depth verification is done by Ice you can easily move that to your certificate verifier, you just need to check the length of the certificates provided in IceSSL::ConnectionInfo object

    https://github.com/zeroc-ice/ice/blob/3.7/cpp/src/IceSSL/SSLEngine.cpp#L231

  • Hey Jose,
    That's good to hear, the help has been invaluable.

    I didn't expect setting IceSSL.VerifyPeer=0 to bypass those checks. That said, the flowchart does show that if CheckTrust fails it will fail the connection regardless of VerifyPeer state. I haven't tested that yet.

    My specific issue is that I don't get a callback to my certificate verifier if the chain length exceeds the setting. It should be calling my verifier callback according to the docs, and what you've said above. It's acting just like I had set VerifyPeer=1, failing the connection immediately.

    Is that a defect or am I doing something wrong? The only way I can see out of this is to set IceSSL.VerifyDepthMax to 0 to prevent any Ice checking, and then do our own chain length in the cert verifier callback.

    THanks
    Jeremy

  • xdm
    xdm La Coruña, Spain

    Setting IceSSL.VerifyPeer only affects the first step "chain verification", both IceSSL.VerifyDepthMax and
    IceSSL.TrustOnly.* are not affected by it, are we looking at the same flowchart?

    https://doc.zeroc.com/ice/files/3.7/30218158/30218159/1/1526670064000/SSL.jpg

    VerifyDepthMax is not considered part of the "chain verification" here, maybe that is the confusing part, it is a separate check done by IceSSL, "chain verification" corresponds to the checks done by the underlying SSL engine for which we provide the new TrustError.

    The only way I can see out of this is to set IceSSL.VerifyDepthMax to 0 to prevent any Ice checking, and then do our own chain length in the cert verifier callback.

    Yes, I don't see another way around this with the current implementation.

  • Ah, I see - I assumed the "Chain Valid" step included the VerifyDepthMax portion . If that is separate that certainly explains the discrepancy. It won't be too hard for us to assume responsibility for checking chain length.
    To be clear, will we also face this issue too with the CheckTrust step? I'm actually not even sure what that is -- our cerificate verifier doesn't deal with anything trust related today.