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

macOS: Support intermediate certificates #5

Closed

Conversation

timja
Copy link
Contributor

@timja timja commented Jan 6, 2025

Fixes #3

This works now, remaining before ready for review:

  • Change test to not be specific to my machine
  • Try change to SSL policy from basic x509
  • Review CF objects to ensure they are being correctly released

@shalupov
Copy link
Collaborator

shalupov commented Feb 19, 2025

current state: I was able to call SecItemCopyMatching without errors and it even returns something. See ef3ec66

Basically, two things:

  • kCFBooleanFalse/True are not CFString, they're CFBoolean. Those unique constants could not be easily created, so I've imported them from framework symbols
  • For some reason, SecItemCopyMatching do not accepted constants created via new strings, so I've also imported them from symbols

Could you please continue on PR and I'll try to help with JNA when required?

@timja
Copy link
Contributor Author

timja commented Feb 19, 2025

Thanks for the help, its working now, I'll take a look at tests and any refactoring needed later

@timja timja marked this pull request as ready for review February 20, 2025 16:07
@timja timja requested a review from shalupov February 20, 2025 16:07
@shalupov shalupov self-assigned this Feb 24, 2025
return false;
}

return SecurityFramework.INSTANCE.SecTrustEvaluateWithError(secTrustRefByReference.getSecTrustRef(), null);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a WARN message that certificated is not trusted thus not imported

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its valid for a certificate to be not trusted, its logged elsewhere:

LOGGER.fine("Certificate '" + certificateDescription + "' has no trust settings and failed to validate against trusted roots");

@shalupov
Copy link
Collaborator

From failing tests, looks like system certificates are not imported

@timja
Copy link
Contributor Author

timja commented Feb 25, 2025

From failing tests, looks like system certificates are not imported

Right (annoying github action not trusting me -.-)

From stackoverflow:
https://stackoverflow.com/a/35095195

OpenJDK took the above approach:
https://github.com/openjdk/jdk/blob/master/src/java.base/macosx/native/libosxsecurity/KeystoreImpl.m#L525-L527

Although SecKeychainOpen is deprecated.

This looks like the supported way to do it though:
https://developer.apple.com/documentation/security/sectrustcopyanchorcertificates(_:)?language=objc

I'll give SecTrustCopyAnchorCertificates a go.

@timja
Copy link
Contributor Author

timja commented Feb 25, 2025

I implemented SecTrustCopyAnchorCertificates but it contains non system certificates which is why openjdk went the other route.

I've implemented it using the key chain file system APIs. deprecated but still seems to be the recommended approach if you need to do this.

@timja timja requested a review from shalupov February 25, 2025 21:02
@shalupov
Copy link
Collaborator

squashed and pushed manually 477763d

@shalupov
Copy link
Collaborator

also added more checks: 96702ad

@shalupov shalupov closed this Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

macOS: Custom chains that include intermediate certificates do not work
2 participants