Skip to content

Commit

Permalink
added validation of new keys, therefore refactored KeyParser a little…
Browse files Browse the repository at this point in the history 
… and fixed some warnings
  • Loading branch information
@wolpi
wolpi committed May 11, 2024
1 parent 696a9ea commit 24dce9c
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 54 deletions.
85 changes: 43 additions & 42 deletions pftpd-pojo-lib/src/main/java/org/primftpd/pojo/KeyParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import java.nio.ByteBuffer;
import java.security.KeyFactory;
import java.security.NoSuchAlgorithmException;
import java.security.NoSuchProviderException;
import java.security.PublicKey;
import java.security.spec.DSAPublicKeySpec;
import java.security.spec.InvalidKeySpecException;
Expand Down Expand Up @@ -49,9 +48,9 @@ public class KeyParser {

static {
Map<String, Integer> tmpCoordSize = new HashMap<>();
tmpCoordSize.put(NAME_ECDSA_256, Integer.valueOf(32));
tmpCoordSize.put(NAME_ECDSA_384, Integer.valueOf(48));
tmpCoordSize.put(NAME_ECDSA_521, Integer.valueOf(66));
tmpCoordSize.put(NAME_ECDSA_256, 32);
tmpCoordSize.put(NAME_ECDSA_384, 48);
tmpCoordSize.put(NAME_ECDSA_521, 66);
EC_NAME_TO_COORD_SIZE = Collections.unmodifiableMap(tmpCoordSize);

// see org.bouncycastle.asn1.nist.NISTNamedCurves
Expand All @@ -74,35 +73,9 @@ public static List<PublicKey> parsePublicKeys(InputStream is, Base64Decoder base
try {
String keyLine = reader.readLine();
lineCounter++;
String[] parts = keyLine.split(" ");

String name = null;
String keyEncoded = null;
if (parts.length >= 2) {
name = parts[0];
keyEncoded = parts[1];
}

if (keyEncoded != null) {
byte[] keyBytes = base64Decoder.decode(keyEncoded);

PublicKey key = null;
if (NAME_RSA.equals(name)) {
key = parsePublicKeyRsa(keyBytes);
} else if (NAME_DSA.equals(name)) {
key = parsePublicKeyDsa(keyBytes);
} else if (NAME_ECDSA_256.equals(name)) {
key = parsePublicKeyEcdsa(name, keyBytes);
} else if (NAME_ECDSA_384.equals(name)) {
key = parsePublicKeyEcdsa(name, keyBytes);
} else if (NAME_ECDSA_521.equals(name)) {
key = parsePublicKeyEcdsa(name, keyBytes);
} else if (NAME_ED25519.equals(name)) {
key = parsePublicKeyEd25519(keyBytes);
}
if (key != null) {
keys.add(key);
}
PublicKey key = parseKeyLine(keyLine, base64Decoder);
if (key != null) {
keys.add(key);
}
} catch (Exception e) {
errors.add("could not read key at line " + lineCounter + ": " + e.getClass().getName() + ", " + e.getMessage());
Expand All @@ -112,6 +85,37 @@ public static List<PublicKey> parsePublicKeys(InputStream is, Base64Decoder base
return keys;
}

public static PublicKey parseKeyLine(String keyLine, Base64Decoder base64Decoder) throws Exception {
PublicKey key = null;
String[] parts = keyLine.split(" ");

String name = null;
String keyEncoded = null;
if (parts.length >= 2) {
name = parts[0];
keyEncoded = parts[1];
}

if (keyEncoded != null) {
byte[] keyBytes = base64Decoder.decode(keyEncoded);

if (NAME_RSA.equals(name)) {
key = parsePublicKeyRsa(keyBytes);
} else if (NAME_DSA.equals(name)) {
key = parsePublicKeyDsa(keyBytes);
} else if (NAME_ECDSA_256.equals(name)) {
key = parsePublicKeyEcdsa(name, keyBytes);
} else if (NAME_ECDSA_384.equals(name)) {
key = parsePublicKeyEcdsa(name, keyBytes);
} else if (NAME_ECDSA_521.equals(name)) {
key = parsePublicKeyEcdsa(name, keyBytes);
} else if (NAME_ED25519.equals(name)) {
key = parsePublicKeyEd25519(keyBytes);
}
}
return key;
}

protected static PublicKey parsePublicKeyRsa(byte[] keyBytes)
throws InvalidKeySpecException, NoSuchAlgorithmException {
// name is also included in bytes
Expand Down Expand Up @@ -164,29 +168,27 @@ protected static PublicKey createPubKeyDsa(
return kf.generatePublic(keySpec);
}

protected static PublicKey parsePublicKeyEcdsa(String name, byte[] keyBytes)
throws InvalidKeySpecException, NoSuchAlgorithmException, IOException, NoSuchProviderException {
protected static PublicKey parsePublicKeyEcdsa(String name, byte[] keyBytes) {

// use as Buffer to avoid issue with java 8, see GH #226
Buffer byteBuffer = ByteBuffer.wrap(keyBytes);
ByteBuffer byteBuffer = ByteBuffer.wrap(keyBytes);

// https://security.stackexchange.com/questions/129910/ecdsa-why-do-ssh-keygen-and-java-generated-public-keys-have-different-sizes
final int coordLength = EC_NAME_TO_COORD_SIZE.get(name);
byteBuffer.position(keyBytes.length - 2*coordLength);
byte[] xBytes = new byte[coordLength];
((ByteBuffer)byteBuffer).get(xBytes);
byteBuffer.get(xBytes);
BigInteger x = new BigInteger(1, xBytes);

byteBuffer.position(keyBytes.length - coordLength);
byte[] yBytes = new byte[coordLength];
((ByteBuffer)byteBuffer).get(yBytes);
byteBuffer.get(yBytes);
BigInteger y = new BigInteger(1, yBytes);

return createPubKeyEcdsa(name, x, y);
}

public static PublicKey createPubKeyEcdsa(String name, BigInteger x, BigInteger y)
throws NoSuchAlgorithmException, InvalidKeySpecException {
public static PublicKey createPubKeyEcdsa(String name, BigInteger x, BigInteger y) {
final String curveName = EC_NAME_TO_CURVE_NAME.get(name);
// note: this bouncy castle helper class is awesome!
// it justifies the additional dependency even though api-level 28 comes with conscrypt
Expand All @@ -204,8 +206,7 @@ public static PublicKey parsePublicKeyEd25519(byte[] keyBytes)
if (keyBytes.length > KEY_SIZE_ED25519) {
int startIndex = keyBytes.length-KEY_SIZE_ED25519;
int endIndex = keyBytes.length;
byte[] copy = Arrays.copyOfRange(keyBytes, startIndex, endIndex);
keyBytes = copy;
keyBytes = Arrays.copyOfRange(keyBytes, startIndex, endIndex);
}

KeyFactorySpi factory = new KeyFactorySpi.Ed25519();
Expand Down
1 change: 1 addition & 0 deletions primitiveFTPd/res/values/strings.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
<string name="cancel">Cancel</string>
<string name="pubkeyAuthKeysHeading">Keys for public key auth</string>
<string name="addPubkeyForAuth">Add public key for auth</string>
<string name="pubkeyInvalid">Input is not a valid key, has been ignored.</string>
<string name="state">State</string>
<string name="serverStarted">Running …</string>
<string name="serverStopped">Stopped</string>
Expand Down
52 changes: 40 additions & 12 deletions primitiveFTPd/src/org/primftpd/ui/PubKeyAuthKeysFragment.java
View file
Original file line number Diff line number Diff line change
@@ -1,24 +1,30 @@
package org.primftpd.ui;

import android.os.Bundle;
import android.util.Base64;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import android.widget.LinearLayout;
import android.widget.TextView;
import android.widget.Toast;

import com.google.android.material.floatingactionbutton.FloatingActionButton;

import org.jetbrains.annotations.NotNull;
import org.primftpd.R;
import org.primftpd.pojo.KeyParser;
import org.primftpd.util.Defaults;
import org.primftpd.util.ServersRunningBean;
import org.primftpd.util.ServicesStartStopUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.BufferedReader;
import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;
import java.security.PublicKey;
import java.util.ArrayList;
import java.util.List;

Expand Down Expand Up @@ -102,20 +108,42 @@ private void displayKeys(View view, List<String> keys) {

protected void addKeyToFile(CharSequence key) {
final String path = Defaults.pubKeyAuthKeyPath(getContext());
try {
try (FileWriter writer = new FileWriter(path, true)) {
writer.append("\n");
writer.append(key);
}
if (validateKey(key)) {
try {
try (FileWriter writer = new FileWriter(path, true)) {
writer.append("\n");
writer.append(key);
}

View view = getView();
if (view != null) {
List<String> keys = loadKeysForDisplay();
displayKeys(view, keys);
View view = getView();
if (view != null) {
List<String> keys = loadKeysForDisplay();
displayKeys(view, keys);
}

ServersRunningBean serversRunningBean = ServicesStartStopUtil.checkServicesRunning(
requireContext());
if (serversRunningBean.atLeastOneRunning()) {
Toast.makeText(getContext(), R.string.restartServer, Toast.LENGTH_SHORT).show();
}
} catch (IOException e) {
logger.info("could not store key in file '{}': {}, {}",
new Object[]{path, e.getClass().getName(), e.getMessage()});
}
} catch (IOException e) {
logger.info("could not store key in file '{}': {}, {}",
new Object[]{path, e.getClass().getName(), e.getMessage()});
} else {
Toast.makeText(getContext(), R.string.pubkeyInvalid, Toast.LENGTH_SHORT).show();
}
}

protected boolean validateKey(CharSequence key) {
PublicKey pubKey = null;
try {
pubKey = KeyParser.parseKeyLine(
key.toString(),
str -> Base64.decode(str, Base64.DEFAULT));
} catch (Exception e) {
// handled by having pubKey equal null
}
return pubKey != null;
}
}

0 comments on commit 24dce9c

Please sign in to comment.