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.

Sign up for GitHub

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

Jump to bottom

[fido] Clear sensitive data after use #101

Merged
merged 2 commits into from
Aug 31, 2023
Merged

[fido] Clear sensitive data after use #101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6d7aa36
Clear sensitive data from memory
AdamVe Aug 28, 2023
ee2cf6c
use finally correctly
AdamVe Aug 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 80 additions & 62 deletions fido/src/main/java/com/yubico/yubikit/fido/client/BasicWebAuthnClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@
import com.yubico.yubikit.fido.ctap.PinUvAuthDummyProtocol;
import com.yubico.yubikit.fido.ctap.PinUvAuthProtocolV1;
import com.yubico.yubikit.fido.webauthn.AttestationObject;
import com.yubico.yubikit.fido.webauthn.AttestedCredentialData;
import com.yubico.yubikit.fido.webauthn.AuthenticatorAttestationResponse;
import com.yubico.yubikit.fido.webauthn.AuthenticatorData;
import com.yubico.yubikit.fido.webauthn.AuthenticatorSelectionCriteria;
import com.yubico.yubikit.fido.webauthn.PublicKeyCredential;
import com.yubico.yubikit.fido.webauthn.PublicKeyCredentialCreationOptions;
Expand All @@ -40,10 +38,10 @@

import java.io.Closeable;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -340,74 +338,89 @@ protected Ctap2Session.CredentialData ctapMakeCredential(
@Nullable CommandState state
) throws IOException, CommandException, ClientError {

if (options.getExtensions() != null) {
throw new ClientError(ClientError.Code.CONFIGURATION_UNSUPPORTED, "Extensions not supported");
}

Map<String, ?> rp = options.getRp().toMap();
String rpId = options.getRp().getId();
if (rpId == null) {
((Map<String, Object>) rp).put("id", effectiveDomain);
} else if (!(effectiveDomain.equals(rpId) || effectiveDomain.endsWith("." + rpId))) {
throw new ClientError(ClientError.Code.BAD_REQUEST, "RP ID is not valid for effective domain");
}

byte[] pinUvAuthParam = null;
int pinUvAuthProtocol = 0;
byte[] pinToken = null;
try {
if (options.getExtensions() != null) {
throw new ClientError(
ClientError.Code.CONFIGURATION_UNSUPPORTED,
"Extensions not supported");
}

Map<String, Boolean> ctapOptions = new HashMap<>();
AuthenticatorSelectionCriteria authenticatorSelection = options.getAuthenticatorSelection();
if (authenticatorSelection != null) {
String residentKeyRequirement = authenticatorSelection.getResidentKey();
if (ResidentKeyRequirement.REQUIRED.equals(residentKeyRequirement) ||
(ResidentKeyRequirement.PREFERRED.equals(residentKeyRequirement) &&
(pinSupported || uvSupported)
)
) {
ctapOptions.put(OPTION_RESIDENT_KEY, true);
Map<String, ?> rp = options.getRp().toMap();
String rpId = options.getRp().getId();
if (rpId == null) {
((Map<String, Object>) rp).put("id", effectiveDomain);
} else if (!(effectiveDomain.equals(rpId) || effectiveDomain.endsWith("." + rpId))) {
throw new ClientError(
ClientError.Code.BAD_REQUEST,
"RP ID is not valid for effective domain");
}
if (getCtapUv(authenticatorSelection.getUserVerification(), pin != null)) {
ctapOptions.put(OPTION_USER_VERIFICATION, true);

byte[] pinUvAuthParam = null;
int pinUvAuthProtocol = 0;

Map<String, Boolean> ctapOptions = new HashMap<>();
AuthenticatorSelectionCriteria authenticatorSelection =
options.getAuthenticatorSelection();
if (authenticatorSelection != null) {
String residentKeyRequirement = authenticatorSelection.getResidentKey();
if (ResidentKeyRequirement.REQUIRED.equals(residentKeyRequirement) ||
(ResidentKeyRequirement.PREFERRED.equals(residentKeyRequirement) &&
(pinSupported || uvSupported)
)
) {
ctapOptions.put(OPTION_RESIDENT_KEY, true);
}
if (getCtapUv(authenticatorSelection.getUserVerification(), pin != null)) {
ctapOptions.put(OPTION_USER_VERIFICATION, true);
}
} else {
if (getCtapUv(UserVerificationRequirement.PREFERRED, pin != null)) {
ctapOptions.put(OPTION_USER_VERIFICATION, true);
}
}
} else {
if (getCtapUv(UserVerificationRequirement.PREFERRED, pin != null)) {
ctapOptions.put(OPTION_USER_VERIFICATION, true);

if (pin != null) {
pinToken = clientPin.getPinToken(pin);
pinUvAuthParam = clientPin.getPinUvAuth().authenticate(pinToken, clientDataHash);
pinUvAuthProtocol = clientPin.getPinUvAuth().getVersion();
} else if (pinConfigured && !ctapOptions.containsKey(OPTION_USER_VERIFICATION)) {
throw new PinRequiredClientError();
}
}

if (pin != null) {
byte[] pinToken = clientPin.getPinToken(pin);
pinUvAuthParam = clientPin.getPinUvAuth().authenticate(pinToken, clientDataHash);
pinUvAuthProtocol = clientPin.getPinUvAuth().getVersion();
} else if (pinConfigured && !ctapOptions.containsKey(OPTION_USER_VERIFICATION)) {
throw new PinRequiredClientError();
}
final List<PublicKeyCredentialDescriptor> excludeCredentials =
removeUnsupportedCredentials(
options.getExcludeCredentials()
);

final List<PublicKeyCredentialDescriptor> excludeCredentials = removeUnsupportedCredentials(
options.getExcludeCredentials()
);
final Map<String, Object> user = CredentialManager.credentialUserEntityToMap(
options.getUser()
);

final Map<String, Object> user = CredentialManager.credentialUserEntityToMap(options.getUser());
List<Map<String, ?>> pubKeyCredParams = new ArrayList<>();
for (PublicKeyCredentialParameters param : options.getPubKeyCredParams()) {
if (isPublicKeyCredentialTypeSupported(param.getType())) {
pubKeyCredParams.add(param.toMap());
}
}

List<Map<String, ?>> pubKeyCredParams = new ArrayList<>();
for (PublicKeyCredentialParameters param : options.getPubKeyCredParams()) {
if (isPublicKeyCredentialTypeSupported(param.getType())) {
pubKeyCredParams.add(param.toMap());
return ctap.makeCredential(
clientDataHash,
rp,
user,
pubKeyCredParams,
getCredentialList(excludeCredentials),
null,
ctapOptions.isEmpty() ? null : ctapOptions,
pinUvAuthParam,
pinUvAuthProtocol,
state
);
} finally {
if (pinToken != null) {
Arrays.fill(pinToken, (byte) 0);
}
}

return ctap.makeCredential(
clientDataHash,
rp,
user,
pubKeyCredParams,
getCredentialList(excludeCredentials),
null,
ctapOptions.isEmpty() ? null : ctapOptions,
pinUvAuthParam,
pinUvAuthProtocol,
state
);
}

/**
Expand Down Expand Up @@ -454,9 +467,10 @@ protected List<Ctap2Session.AssertionData> ctapGetAssertions(

byte[] pinUvAuthParam = null;
int pinUvAuthProtocol = 0;
byte[] pinToken = null;
try {
if (pin != null) {
byte[] pinToken = clientPin.getPinToken(pin);
pinToken = clientPin.getPinToken(pin);
pinUvAuthParam = clientPin.getPinUvAuth().authenticate(pinToken, clientDataHash);
pinUvAuthProtocol = clientPin.getPinUvAuth().getVersion();
}
Expand All @@ -480,6 +494,10 @@ protected List<Ctap2Session.AssertionData> ctapGetAssertions(
throw new PinInvalidClientError(e, clientPin.getPinRetries());
}
throw ClientError.wrapCtapException(e);
} finally {
if (pinToken != null) {
Arrays.fill(pinToken, (byte) 0);
}
}
}

Expand Down
29 changes: 24 additions & 5 deletions fido/src/main/java/com/yubico/yubikit/fido/ctap/ClientPin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,11 @@ public int getPinRetries() throws IOException, CommandException {
*/
public byte[] getPinToken(char[] pin) throws IOException, CommandException {
Pair<Map<Integer, ?>, byte[]> pair = getSharedSecret();
byte[] pinHash = null;
try {
byte[] pinHash = Arrays.copyOf(MessageDigest.getInstance("SHA-256").digest(preparePin(pin, false)), PIN_HASH_LEN);
pinHash = Arrays.copyOf(
MessageDigest.getInstance("SHA-256").digest(preparePin(pin, false)),
PIN_HASH_LEN);
byte[] pinHashEnc = pinUvAuth.encrypt(pair.second, pinHash);

Logger.debug(logger, "Getting PIN token");
Expand All @@ -144,6 +147,10 @@ public byte[] getPinToken(char[] pin) throws IOException, CommandException {
} catch (NoSuchAlgorithmException e) {
Logger.error(logger, "Failure getting PIN token: ", e);
throw new IllegalStateException(e);
} finally {
if (pinHash != null) {
Arrays.fill(pinHash, (byte) 0);
}
}
}

Expand Down Expand Up @@ -178,12 +185,18 @@ public void setPin(char[] pin) throws IOException, CommandException {
* @throws IOException A communication error in the transport layer.
* @throws CommandException A communication in the protocol layer.
*/
public void changePin(char[] currentPin, char[] newPin) throws IOException, CommandException {
public void changePin(char[] currentPin, char[] newPin)
throws IOException, CommandException
{
byte[] newPinBytes = preparePin(newPin, true);
Pair<Map<Integer, ?>, byte[]> pair = getSharedSecret();

byte[] pinHash = null;
try {
byte[] pinHash = Arrays.copyOf(MessageDigest.getInstance("SHA-256").digest(preparePin(currentPin, false)), PIN_HASH_LEN);
pinHash = Arrays.copyOf(
MessageDigest.getInstance("SHA-256").digest(preparePin(currentPin, false)),
PIN_HASH_LEN
);
byte[] pinHashEnc = pinUvAuth.encrypt(pair.second, pinHash);
byte[] newPinEnc = pinUvAuth.encrypt(pair.second, newPinBytes);

Expand All @@ -207,6 +220,10 @@ public void changePin(char[] currentPin, char[] newPin) throws IOException, Comm
} catch (NoSuchAlgorithmException e) {
Logger.error(logger, "Failure changing PIN: ", e);
throw new IllegalStateException(e);
} finally {
if (pinHash != null) {
Arrays.fill(pinHash, (byte) 0);
}
}
}

Expand All @@ -215,13 +232,15 @@ public void changePin(char[] currentPin, char[] newPin) throws IOException, Comm
*/
static byte[] preparePin(char[] pin, boolean pad) {
if (pin.length < MIN_PIN_LEN) {
throw new IllegalArgumentException("PIN must be at least " + MIN_PIN_LEN + " characters");
throw new IllegalArgumentException(
"PIN must be at least " + MIN_PIN_LEN + " characters");
}
ByteBuffer byteBuffer = StandardCharsets.UTF_8.encode(CharBuffer.wrap(pin));
try {
int byteLen = byteBuffer.limit() - byteBuffer.position();
if (byteLen > MAX_PIN_LEN) {
throw new IllegalArgumentException("PIN must be no more than " + MAX_PIN_LEN + " bytes");
throw new IllegalArgumentException(
"PIN must be no more than " + MAX_PIN_LEN + " bytes");
}
byte[] pinBytes = new byte[pad ? PIN_BUFFER_LEN : byteLen];
System.arraycopy(byteBuffer.array(), byteBuffer.position(), pinBytes, 0, byteLen);
Expand Down