Skip to content

Commit

Permalink
[fix] stop assuming (JDK) EC key identifier
Browse files Browse the repository at this point in the history
it's "EC" with Sun provider but "ECDSA" with BC
  • Loading branch information
kares committed Apr 11, 2024
1 parent f7f1009 commit c197ac8
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 110 deletions.
17 changes: 1 addition & 16 deletions src/main/java/org/jruby/ext/openssl/NetscapeSPKI.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import org.jruby.RubyClass;
import org.jruby.RubyModule;
import org.jruby.RubyObject;
import org.jruby.RubyString;
import org.jruby.anno.JRubyMethod;
import org.jruby.exceptions.RaiseException;
import org.jruby.ext.openssl.impl.Base64;
Expand All @@ -57,8 +56,6 @@
// org.bouncycastle.jce.netscape.NetscapeCertRequest emulator:
import org.jruby.ext.openssl.impl.NetscapeCertRequest;

import static org.jruby.ext.openssl.PKeyDSA._DSA;
import static org.jruby.ext.openssl.PKeyRSA._RSA;
import static org.jruby.ext.openssl.OpenSSL.*;

/**
Expand Down Expand Up @@ -102,19 +99,7 @@ public IRubyObject initialize(final ThreadContext context, final IRubyObject[] a
catch (GeneralSecurityException e) { throw newSPKIError(e); }
catch (IllegalArgumentException e) { throw newSPKIError(e); }

final PublicKey publicKey = cert.getPublicKey();
final String algorithm = publicKey.getAlgorithm();
final RubyString pub_key = RubyString.newString(runtime, publicKey.getEncoded());

if ( "RSA".equalsIgnoreCase(algorithm) ) {
this.public_key = _RSA(runtime).callMethod(context, "new", pub_key);
}
else if ( "DSA".equalsIgnoreCase(algorithm) ) {
this.public_key = _DSA(runtime).callMethod(context, "new", pub_key);
}
else {
throw runtime.newLoadError("not implemented algo for public key: " + algorithm);
}
this.public_key = PKey.newInstance(runtime, cert.getPublicKey());
}
return this;
}
Expand Down
49 changes: 28 additions & 21 deletions src/main/java/org/jruby/ext/openssl/PKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.security.*;
import java.security.interfaces.DSAPrivateKey;
import java.security.interfaces.DSAPublicKey;
import java.security.interfaces.ECPublicKey;
import java.security.interfaces.RSAPrivateCrtKey;
import java.security.interfaces.RSAPublicKey;
import java.security.spec.InvalidKeySpecException;
Expand Down Expand Up @@ -88,6 +89,20 @@ public static RaiseException newPKeyError(Ruby runtime, String message) {
return Utils.newError(runtime, (RubyClass) _PKey(runtime).getConstantAt("PKeyError"), message);
}

public static PKey newInstance(final Ruby runtime, final PublicKey publicKey) {
if (publicKey instanceof RSAPublicKey) {
return new PKeyRSA(runtime, (RSAPublicKey) publicKey);
}
if (publicKey instanceof DSAPublicKey) {
return new PKeyDSA(runtime, (DSAPublicKey) publicKey);
}
if (publicKey instanceof ECPublicKey) {
return new PKeyEC(runtime, publicKey);
}
assert publicKey != null;
throw runtime.newNotImplementedError("public key algorithm: " + (publicKey != null ? publicKey.getAlgorithm() : "nil"));
}

static RubyModule _PKey(final Ruby runtime) {
return (RubyModule) runtime.getModule("OpenSSL").getConstantAt("PKey");
}
Expand Down Expand Up @@ -122,20 +137,15 @@ public static IRubyObject read(final ThreadContext context, IRubyObject recv, IR
if (keyPair != null) {
final String alg = getAlgorithm(keyPair);
if ( "RSA".equals(alg) ) {
return new PKeyRSA(runtime, _PKey(runtime).getClass("RSA"),
(RSAPrivateCrtKey) keyPair.getPrivate(), (RSAPublicKey) keyPair.getPublic()
);
return new PKeyRSA(runtime, _PKey(runtime).getClass("RSA"), (RSAPrivateCrtKey) keyPair.getPrivate(), (RSAPublicKey) keyPair.getPublic());
}
if ( "DSA".equals(alg) ) {
return new PKeyDSA(runtime, _PKey(runtime).getClass("DSA"),
(DSAPrivateKey) keyPair.getPrivate(), (DSAPublicKey) keyPair.getPublic()
);
return new PKeyDSA(runtime, _PKey(runtime).getClass("DSA"), (DSAPrivateKey) keyPair.getPrivate(), (DSAPublicKey) keyPair.getPublic());
}
if ( "EC".equals(alg) ) {
return new PKeyEC(runtime, _PKey(runtime).getClass("EC"),
keyPair.getPrivate(), keyPair.getPublic()
);
if ( "EC".equals(alg) || "ECDSA".equals(alg) ) { // Sun vs BC provider naming
return new PKeyEC(runtime, _PKey(runtime).getClass("EC"), keyPair.getPrivate(), keyPair.getPublic());
}
debug(runtime, "PKey readPrivateKey unexpected key pair algorithm: " + alg);
}

PublicKey pubKey = null;
Expand Down Expand Up @@ -168,16 +178,14 @@ public static IRubyObject read(final ThreadContext context, IRubyObject recv, IR
}
}

if (pubKey != null) {
if ( "RSA".equals(pubKey.getAlgorithm()) ) {
return new PKeyRSA(runtime, (RSAPublicKey) pubKey);
}
if ( "DSA".equals(pubKey.getAlgorithm()) ) {
return new PKeyDSA(runtime, (DSAPublicKey) pubKey);
}
if ( "EC".equals(pubKey.getAlgorithm()) ) {
return new PKeyEC(runtime, pubKey);
}
if (pubKey instanceof RSAPublicKey) {
return new PKeyRSA(runtime, (RSAPublicKey) pubKey);
}
if (pubKey instanceof DSAPublicKey) {
return new PKeyDSA(runtime, (DSAPublicKey) pubKey);
}
if (pubKey instanceof ECPublicKey) {
return new PKeyEC(runtime, pubKey);
}

throw newPKeyError(runtime, "Could not parse PKey: unsupported");
Expand All @@ -188,7 +196,6 @@ private static String getAlgorithm(final KeyPair key) {
if ( key.getPublic() != null ) return key.getPublic().getAlgorithm();
return null;
}

}

public PKey(Ruby runtime, RubyClass type) {
Expand Down
6 changes: 0 additions & 6 deletions src/main/java/org/jruby/ext/openssl/PKeyDSA.java
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,6 @@ private static PKeyDSA dsaGenerate(final Ruby runtime,
}
}

static PKeyDSA newInstance(final Ruby runtime, final PublicKey publicKey) {
//if ( publicKey instanceof DSAPublicKey ) {
return new PKeyDSA(runtime, (DSAPublicKey) publicKey);
//}
}

@JRubyMethod(rest = true, visibility = Visibility.PRIVATE)
public IRubyObject initialize(final ThreadContext context, final IRubyObject[] args, final Block block) {
final Ruby runtime = context.runtime;
Expand Down
4 changes: 0 additions & 4 deletions src/main/java/org/jruby/ext/openssl/PKeyEC.java
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,6 @@ public final class PKeyEC extends PKey {
public PKeyEC allocate(Ruby runtime, RubyClass klass) { return new PKeyEC(runtime, klass); }
};

static PKeyEC newInstance(final Ruby runtime, final PublicKey publicKey) {
return new PKeyEC(runtime, publicKey);
}

static void createPKeyEC(final Ruby runtime, final RubyModule PKey, final RubyClass PKeyPKey, final RubyClass OpenSSLError) {
RubyClass EC = PKey.defineClassUnder("EC", PKeyPKey, ALLOCATOR);

Expand Down
6 changes: 0 additions & 6 deletions src/main/java/org/jruby/ext/openssl/PKeyRSA.java
Original file line number Diff line number Diff line change
Expand Up @@ -213,12 +213,6 @@ private static PKeyRSA rsaGenerate(final Ruby runtime,
return rsa;
}

static PKeyRSA newInstance(final Ruby runtime, final PublicKey publicKey) {
//if ( publicKey instanceof RSAPublicKey ) {
return new PKeyRSA(runtime, (RSAPublicKey) publicKey);
//}
}

@JRubyMethod(rest = true, visibility = Visibility.PRIVATE)
public IRubyObject initialize(final ThreadContext context, final IRubyObject[] args, final Block block) {
final Ruby runtime = context.runtime;
Expand Down
9 changes: 6 additions & 3 deletions src/main/java/org/jruby/ext/openssl/SecurityHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -606,23 +606,26 @@ static boolean verify(final X509CRL crl, final PublicKey publicKey, final boolea
try {
final DigestAlgorithmIdentifierFinder digestAlgFinder = new DefaultDigestAlgorithmIdentifierFinder();
final ContentVerifierProvider verifierProvider;
if ( "DSA".equalsIgnoreCase( publicKey.getAlgorithm() )) {
if (publicKey instanceof DSAPublicKey) {
BigInteger y = ((DSAPublicKey) publicKey).getY();
DSAParams params = ((DSAPublicKey) publicKey).getParams();
DSAParameters parameters = new DSAParameters(params.getP(), params.getQ(), params.getG());
AsymmetricKeyParameter dsaKey = new DSAPublicKeyParameters(y, parameters);
verifierProvider = new BcDSAContentVerifierProviderBuilder(digestAlgFinder).build(dsaKey);
}
else if ( "EC".equalsIgnoreCase( publicKey.getAlgorithm() )) {
else if (publicKey instanceof ECPublicKey) {
AsymmetricKeyParameter ecKey = ECUtil.generatePublicKeyParameter(publicKey);
verifierProvider = new BcECContentVerifierProviderBuilder(digestAlgFinder).build(ecKey);
}
else {
else if (publicKey instanceof RSAPublicKey) {
BigInteger mod = ((RSAPublicKey) publicKey).getModulus();
BigInteger exp = ((RSAPublicKey) publicKey).getPublicExponent();
AsymmetricKeyParameter rsaKey = new RSAKeyParameters(false, mod, exp);
verifierProvider = new BcRSAContentVerifierProviderBuilder(digestAlgFinder).build(rsaKey);
}
else {
throw new IllegalStateException("unsupported public key type: " + (publicKey != null ? publicKey.getClass() : null));
}
return new X509CRLHolder(crl.getEncoded()).isSignatureValid( verifierProvider );
}
catch (OperatorException e) {
Expand Down
34 changes: 5 additions & 29 deletions src/main/java/org/jruby/ext/openssl/X509Cert.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@
import java.security.cert.CertificateEncodingException;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;

import java.security.interfaces.DSAPublicKey;
import java.security.interfaces.RSAPublicKey;

import java.util.ArrayList;
import java.util.Collection;
import java.util.LinkedHashMap;
Expand Down Expand Up @@ -82,7 +82,6 @@
import org.jruby.ext.openssl.x509store.PEMInputOutput;
import org.jruby.ext.openssl.x509store.X509AuxCertificate;
import org.jruby.runtime.Block;
import org.jruby.runtime.ObjectAllocator;
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.Visibility;
import org.jruby.runtime.builtin.IRubyObject;
Expand Down Expand Up @@ -359,7 +358,7 @@ public IRubyObject to_text(final ThreadContext context) {
final PublicKey publicKey = getPublicKey();
text.append(S20,0,12).append("Public Key Algorithm: ").append(publicKey.getAlgorithm()).append('\n');

if ( "RSA".equals( publicKey.getAlgorithm() ) ) {
if (publicKey instanceof RSAPublicKey) {
final RSAPublicKey rsaKey = ((RSAPublicKey) publicKey);
text.append(S20,0,16).append("Public-Key: (").append( rsaKey.getModulus().bitLength() ).append(" bit)\n");

Expand All @@ -370,13 +369,13 @@ public IRubyObject to_text(final ThreadContext context) {
text.append(S20,0,16).append("Exponent: ").append(exponent).
append(" (0x").append( exponent.toString(16) ).append(")\n");
}
else if ( "DSA".equals( publicKey.getAlgorithm() ) ) {
else if (publicKey instanceof DSAPublicKey) {
final DSAPublicKey dsaKey = ((DSAPublicKey) publicKey);
text.append(S20,0,16).append("Public-Key: (").append( dsaKey.getY().bitLength() ).append(" bit)\n");

text.append(S20,0,16).append("TODO: not-implemented (PR HOME-WORK)").append('\n'); // left-TODO
}
else {
else { // "EC" or "ECDSA"
text.append(S20,0,16).append("TODO: not-implemented (PRs WELCOME!)").append('\n'); // left-TODO
}

Expand Down Expand Up @@ -540,30 +539,7 @@ private void initializePublicKey() throws RaiseException {
throw newCertificateError(runtime, "no certificate");
}

final PublicKey publicKey = cert.getPublicKey();

final String algorithm = publicKey.getAlgorithm();

if ( "RSA".equalsIgnoreCase(algorithm) ) {
//if ( public_key == null ) {
// throw new IllegalStateException("no public key encoded data");
//}
set_public_key( PKeyRSA.newInstance(runtime, publicKey) );
}
else if ( "DSA".equalsIgnoreCase(algorithm) ) {
//if ( public_key == null ) {
// throw new IllegalStateException("no public key encoded data");
//}
set_public_key( PKeyDSA.newInstance(runtime, publicKey) );
}
else if ( "EC".equalsIgnoreCase(algorithm) ) {
set_public_key( PKeyEC.newInstance(runtime, publicKey) );
}
else {
String message = "unsupported algorithm";
if ( algorithm != null ) message += " '" + algorithm + "'";
throw newCertificateError(runtime, message);
}
set_public_key(PKey.newInstance(runtime, cert.getPublicKey()));

this.changed = changed;
}
Expand Down
28 changes: 3 additions & 25 deletions src/main/java/org/jruby/ext/openssl/X509Request.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import org.bouncycastle.asn1.pkcs.Attribute;
import org.bouncycastle.asn1.x500.X500Name;
import org.bouncycastle.asn1.x509.AlgorithmIdentifier;
import org.bouncycastle.operator.DefaultSignatureNameFinder;

import org.jruby.Ruby;
import org.jruby.RubyArray;
Expand Down Expand Up @@ -111,30 +110,14 @@ public IRubyObject initialize(final ThreadContext context, final IRubyObject[] a
throw newRequestError(runtime, "invalid certificate request data", e);
}

final String algorithm;
final byte[] encoded;
final PublicKey publicKey;
try {
final PublicKey pkey = request.generatePublicKey();
algorithm = pkey.getAlgorithm();
encoded = pkey.getEncoded();
publicKey = request.generatePublicKey();
}
catch (IOException|GeneralSecurityException e) {
throw newRequestError(runtime, e);
}

final RubyString enc = RubyString.newString(runtime, encoded);
if ( "RSA".equalsIgnoreCase(algorithm) ) {
this.public_key = newPKeyImplInstance(context, "RSA", enc);
}
else if ( "DSA".equalsIgnoreCase(algorithm) ) {
this.public_key = newPKeyImplInstance(context, "DSA", enc);
}
else if ( "EC".equalsIgnoreCase(algorithm) ) {
this.public_key = newPKeyImplInstance(context, "EC", enc);
}
else {
throw runtime.newNotImplementedError("public key algorithm: " + algorithm);
}
this.public_key = PKey.newInstance(runtime, publicKey);

this.subject = newName( context, request.getSubject() );

Expand All @@ -155,11 +138,6 @@ else if ( "EC".equalsIgnoreCase(algorithm) ) {
return this;
}

private static PKey newPKeyImplInstance(final ThreadContext context,
final String className, final RubyString encoded) { // OpenSSL::PKey::RSA.new(encoded)
return (PKey) _PKey(context.runtime).getClass(className).callMethod(context, "new", encoded);
}

private static X509Attribute newAttribute(final ThreadContext context,
final ASN1ObjectIdentifier type, final ASN1Set values) throws IOException {
return X509Attribute.newAttribute(context.runtime, type, values);
Expand Down

0 comments on commit c197ac8

Please sign in to comment.