Skip to content

Commit

Permalink
Fix decoding signature bytes (Fixes #355, #354) (#361)
Browse files Browse the repository at this point in the history 
* Fix for signature verify in DSA

* Cleaned up signature verification

* Fixed import

* Ignored erroneous pmd warnings

* Updated JavaDoc
  • Loading branch information
@hierynomus
hierynomus authored Sep 29, 2017
1 parent 762d088 commit ec46a7a
Show file tree
Hide file tree
Showing 6 changed files with 193 additions and 138 deletions.
67 changes: 11 additions & 56 deletions src/main/java/com/hierynomus/sshj/signature/SignatureEdDSA.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,16 @@
package com.hierynomus.sshj.signature;

import net.i2p.crypto.eddsa.EdDSAEngine;
import net.schmizz.sshj.common.Buffer;
import net.schmizz.sshj.common.KeyType;
import net.schmizz.sshj.common.SSHRuntimeException;
import net.schmizz.sshj.signature.AbstractSignature;
import net.schmizz.sshj.signature.Signature;

import java.security.*;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.SignatureException;

public class SignatureEdDSA implements Signature {
public class SignatureEdDSA extends AbstractSignature {
public static class Factory implements net.schmizz.sshj.common.Factory.Named<Signature> {

@Override
Expand All @@ -37,53 +39,14 @@ public Signature create() {
}
}

final EdDSAEngine engine;

protected SignatureEdDSA() {
try {
engine = new EdDSAEngine(MessageDigest.getInstance("SHA-512"));
} catch (NoSuchAlgorithmException e) {
throw new SSHRuntimeException(e);
}
}

@Override
public void initVerify(PublicKey pubkey) {
try {
engine.initVerify(pubkey);
} catch (InvalidKeyException e) {
throw new SSHRuntimeException(e);
}
SignatureEdDSA() {
super(getEngine());
}

@Override
public void initSign(PrivateKey prvkey) {
private static EdDSAEngine getEngine() {
try {
engine.initSign(prvkey);
} catch (InvalidKeyException e) {
throw new SSHRuntimeException(e);
}
}

@Override
public void update(byte[] H) {
update(H, 0, H.length);
}

@Override
public void update(byte[] H, int off, int len) {
try {
engine.update(H, off, len);
} catch (SignatureException e) {
throw new SSHRuntimeException(e);
}
}

@Override
public byte[] sign() {
try {
return engine.sign();
} catch (SignatureException e) {
return new EdDSAEngine(MessageDigest.getInstance("SHA-512"));
} catch (NoSuchAlgorithmException e) {
throw new SSHRuntimeException(e);
}
}
Expand All @@ -96,17 +59,9 @@ public byte[] encode(byte[] signature) {
@Override
public boolean verify(byte[] sig) {
try {
Buffer.PlainBuffer plainBuffer = new Buffer.PlainBuffer(sig);
String algo = plainBuffer.readString();
if (!"ssh-ed25519".equals(algo)) {
throw new SSHRuntimeException("Expected 'ssh-ed25519' key algorithm, but was: " + algo);
}
byte[] bytes = plainBuffer.readBytes();
return engine.verify(bytes);
return signature.verify(extractSig(sig, "ssh-ed25519"));
} catch (SignatureException e) {
throw new SSHRuntimeException(e);
} catch (Buffer.BufferException e) {
throw new SSHRuntimeException(e);
}
}
}
64 changes: 36 additions & 28 deletions src/main/java/net/schmizz/sshj/signature/AbstractSignature.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,41 +15,48 @@
*/
package net.schmizz.sshj.signature;

import net.schmizz.sshj.common.Buffer;
import net.schmizz.sshj.common.SSHRuntimeException;
import net.schmizz.sshj.common.SecurityUtils;

import java.security.GeneralSecurityException;
import java.security.PrivateKey;
import java.security.PublicKey;
import java.security.SignatureException;
import java.security.*;

/** An abstract class for {@link Signature} that implements common functionality. */
/**
* An abstract class for {@link Signature} that implements common functionality.
*/
public abstract class AbstractSignature
implements Signature {

protected final String algorithm;
protected java.security.Signature signature;
@SuppressWarnings("PMD.UnnecessaryFullyQualifiedName")
protected final java.security.Signature signature;

protected AbstractSignature(String algorithm) {
this.algorithm = algorithm;
try {
this.signature = SecurityUtils.getSignature(algorithm);
} catch (GeneralSecurityException e) {
throw new SSHRuntimeException(e);
}
}

protected AbstractSignature(@SuppressWarnings("PMD.UnnecessaryFullyQualifiedName")
java.security.Signature signatureEngine) {
this.signature = signatureEngine;
}

@Override
public void initVerify(PublicKey publicKey) {
try {
signature = SecurityUtils.getSignature(algorithm);
signature.initVerify(publicKey);
} catch (GeneralSecurityException e) {
} catch (InvalidKeyException e) {
throw new SSHRuntimeException(e);
}
}

@Override
public void initSign(PrivateKey privateKey) {
try {
signature = SecurityUtils.getSignature(algorithm);
signature.initSign(privateKey);
} catch (GeneralSecurityException e) {
} catch (InvalidKeyException e) {
throw new SSHRuntimeException(e);
}
}
Expand Down Expand Up @@ -77,23 +84,24 @@ public byte[] sign() {
}
}

protected byte[] extractSig(byte[] sig) {
if (sig[0] == 0 && sig[1] == 0 && sig[2] == 0) {
int i = 0;
int j = sig[i++] << 24 & 0xff000000
| sig[i++] << 16 & 0x00ff0000
| sig[i++] << 8 & 0x0000ff00
| sig[i++] & 0x000000ff;
i += j;
j = sig[i++] << 24 & 0xff000000
| sig[i++] << 16 & 0x00ff0000
| sig[i++] << 8 & 0x0000ff00
| sig[i++] & 0x000000ff;
byte[] newSig = new byte[j];
System.arraycopy(sig, i, newSig, 0, j);
return newSig;
/**
* Check whether the signature is generated using the expected algorithm, and if so, return the signature blob
*
* @param sig The full signature
* @param expectedKeyAlgorithm The expected key algorithm
* @return The blob part of the signature
*/
protected byte[] extractSig(byte[] sig, String expectedKeyAlgorithm) {
Buffer.PlainBuffer buffer = new Buffer.PlainBuffer(sig);
try {
String algo = buffer.readString();
if (!expectedKeyAlgorithm.equals(algo)) {
throw new SSHRuntimeException("Expected '" + expectedKeyAlgorithm + "' key algorithm, but got: " + algo);
}
return buffer.readBytes();
} catch (Buffer.BufferException e) {
throw new SSHRuntimeException(e);
}
return sig;
}

}
53 changes: 27 additions & 26 deletions src/main/java/net/schmizz/sshj/signature/SignatureDSA.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,23 @@

import net.schmizz.sshj.common.KeyType;
import net.schmizz.sshj.common.SSHRuntimeException;
import org.bouncycastle.asn1.*;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.math.BigInteger;
import java.security.SignatureException;
import java.util.Arrays;

/** DSA {@link Signature} */
/**
* DSA {@link Signature}
*/
public class SignatureDSA
extends AbstractSignature {

/** A named factory for DSA signature */
/**
* A named factory for DSA signature
*/
public static class Factory
implements net.schmizz.sshj.common.Factory.Named<Signature> {

Expand Down Expand Up @@ -73,33 +82,25 @@ public byte[] encode(byte[] sig) {
}

@Override
public boolean verify(byte[] sig) {
sig = extractSig(sig);

// ASN.1
int frst = (sig[0] & 0x80) != 0 ? 1 : 0;
int scnd = (sig[20] & 0x80) != 0 ? 1 : 0;

int length = sig.length + 6 + frst + scnd;
byte[] tmp = new byte[length];
tmp[0] = (byte) 0x30;
tmp[1] = (byte) 0x2c;
tmp[1] += frst;
tmp[1] += scnd;
tmp[2] = (byte) 0x02;
tmp[3] = (byte) 0x14;
tmp[3] += frst;
System.arraycopy(sig, 0, tmp, 4 + frst, 20);
tmp[4 + tmp[3]] = (byte) 0x02;
tmp[5 + tmp[3]] = (byte) 0x14;
tmp[5 + tmp[3]] += scnd;
System.arraycopy(sig, 20, tmp, 6 + tmp[3] + scnd, 20);
sig = tmp;

public boolean verify(byte[] incomingSig) {
byte[] extractSig = extractSig(incomingSig, "ssh-dss");
try {
return signature.verify(sig);
// ASN.1
ByteArrayOutputStream os = new ByteArrayOutputStream();
ASN1OutputStream asn1OutputStream = new ASN1OutputStream(os);
ASN1EncodableVector vector = new ASN1EncodableVector();
BigInteger bigInteger = new BigInteger(1, Arrays.copyOfRange(extractSig, 0, 20));
vector.add(new ASN1Integer(bigInteger));
BigInteger bigInteger2 = new BigInteger(1, Arrays.copyOfRange(extractSig, 20, 40));
vector.add(new ASN1Integer(bigInteger2));
asn1OutputStream.writeObject(new DERSequence(vector));
asn1OutputStream.close();
byte[] finalSig = os.toByteArray();
return signature.verify(finalSig);
} catch (SignatureException e) {
throw new SSHRuntimeException(e);
} catch (IOException e) {
throw new SSHRuntimeException(e);
}
}

Expand Down
29 changes: 2 additions & 27 deletions src/main/java/net/schmizz/sshj/signature/SignatureECDSA.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,7 @@ public boolean verify(byte[] sig) {
byte[] r;
byte[] s;
try {
Buffer sigbuf = new Buffer.PlainBuffer(sig);
final String algo = new String(sigbuf.readBytes());
if (!keyTypeName.equals(algo)) {
throw new SSHRuntimeException(String.format("Signature :: " + keyTypeName + " expected, got %s", algo));
}
final int rsLen = sigbuf.readUInt32AsInt();
if (sigbuf.available() != rsLen) {
throw new SSHRuntimeException("Invalid key length");
}
Buffer sigbuf = new Buffer.PlainBuffer(extractSig(sig, keyTypeName));
r = sigbuf.readBytes();
s = sigbuf.readBytes();
} catch (Exception e) {
Expand All @@ -135,28 +127,11 @@ public boolean verify(byte[] sig) {
}

private byte[] asnEncode(byte[] r, byte[] s) throws IOException {
int rLen = r.length;
int sLen = s.length;

/*
* We can't have the high bit set, so add an extra zero at the beginning
* if so.
*/
if ((r[0] & 0x80) != 0) {
rLen++;
}
if ((s[0] & 0x80) != 0) {
sLen++;
}

/* Calculate total output length */
int length = 6 + rLen + sLen;

ASN1EncodableVector vector = new ASN1EncodableVector();
vector.add(new ASN1Integer(r));
vector.add(new ASN1Integer(s));

ByteArrayOutputStream baos = new ByteArrayOutputStream(length);
ByteArrayOutputStream baos = new ByteArrayOutputStream();
ASN1OutputStream asnOS = new ASN1OutputStream(baos);

asnOS.writeObject(new DERSequence(vector));
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/net/schmizz/sshj/signature/SignatureRSA.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public byte[] encode(byte[] signature) {

@Override
public boolean verify(byte[] sig) {
sig = extractSig(sig);
sig = extractSig(sig, "ssh-rsa");
try {
return signature.verify(sig);
} catch (SignatureException e) {
Expand Down
Loading

0 comments on commit ec46a7a

Please sign in to comment.