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

subKey signature for unknown key gives NullPointer #72

Closed
ruud-de-jong opened this issue Feb 18, 2020 · 5 comments
Closed

subKey signature for unknown key gives NullPointer #72

ruud-de-jong opened this issue Feb 18, 2020 · 5 comments
Labels
bug Something isn't working.
Milestone

Comments

@ruud-de-jong
Copy link
Contributor

ruud-de-jong commented Feb 18, 2020

One of my dependencies is DerbyClient 10.10.1.1.
It's key is: https://hkps.pool.sks-keyservers.net/pks/lookup?op=get&options=mr&search=0x3D8B00E198E21827

3D8B00E198E21827.zip

I've attached the key(ring) it downloads from the keyserver.
It fails with the following stacktrace:

java.lang.NullPointerException
	at org.bouncycastle.openpgp.operator.bc.BcPGPKeyConverter.getPublicKey(Unknown Source)
	at org.bouncycastle.openpgp.operator.bc.BcPGPContentVerifierBuilderProvider$BcPGPContentVerifierBuilder.build(Unknown Source)
	at org.bouncycastle.openpgp.PGPSignature.init(Unknown Source)
	at org.simplify4u.plugins.PublicKeyUtils.lambda$null$4(PublicKeyUtils.java:179)
	at io.vavr.control.Try.run(Try.java:118)
	at org.simplify4u.plugins.PublicKeyUtils.lambda$verifySigForSubKey$5(PublicKeyUtils.java:176)
@ruud-de-jong
Copy link
Contributor Author

If I add this test:

    @Test
    public void invalidDerbyClient() throws IOException, PGPException {

        try (InputStream inputStream = getClass().getResourceAsStream("/3D8B00E198E21827.asc")) {
            PGPPublicKeyRing publicKeyRing = PublicKeyUtils.loadPublicKeyRing(inputStream, 0x3D8B00E198E21827L);

            assertNotNull(publicKeyRing);

            assertEquals(PublicKeyUtils.getUserIDs(publicKeyRing.getPublicKey(0x3D8B00E198E21827L), publicKeyRing),
                    Collections.singletonList("Rick Hillegas <rhillegas@apache.org>"));
        }
    }

And change the main code from

        subKey.getSignatures().forEachRemaining(s -> Try.run(() -> {
                    PGPSignature sig = (PGPSignature) s;
                    PGPPublicKey masterKey = publicKeyRing.getPublicKey(sig.getKeyID());
                    sig.init(new BcPGPContentVerifierBuilderProvider(), masterKey);
                    if (!sig.verifyCertification(masterKey, subKey)) {
                        throw new PGPException(
                                String.format("Failed signature type: %d for subKey: %s in key: %s",
                                        sig.getSignatureType(),
                                        fingerprint(subKey), fingerprint(masterKey)));
                    }
                }).get()
        );

to

        subKey.getSignatures().forEachRemaining(s -> Try.run(() -> {
                    PGPSignature sig = (PGPSignature) s;
                    PGPPublicKey masterKey = publicKeyRing.getPublicKey(sig.getKeyID());
                    if (masterKey != null) {
                        sig.init(new BcPGPContentVerifierBuilderProvider(), masterKey);
                        if (!sig.verifyCertification(masterKey, subKey)) {
                            throw new PGPException(
                                    String.format("Failed signature type: %d for subKey: %s in key: %s",
                                            sig.getSignatureType(),
                                            fingerprint(subKey), fingerprint(masterKey)));
                        }
                    }
                }).get()
        );

it works.
But, is this the desired behaviour?

@ruud-de-jong
Copy link
Contributor Author

I've added a pull request, but cannot seem to link it to this issue

@slawekjaranowski
Copy link
Member

PR: #73

@slawekjaranowski slawekjaranowski added the bug Something isn't working. label Feb 19, 2020
@slawekjaranowski
Copy link
Member

NullPointer occurs because SubKey for this key is signed by someone more, and in key ring we don't have all keys.

For SubKey we should verify only "Binding Signature" not all signature.

thx for test, I will do some more fix.

@slawekjaranowski
Copy link
Member

Key is ok

@slawekjaranowski slawekjaranowski changed the title Invalid Public Key gives NullPointer subKey signature for unknown key gives NullPointer Feb 19, 2020
@slawekjaranowski slawekjaranowski added this to the NextRelease milestone Feb 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working.
Development

No branches or pull requests

2 participants