Skip to content

Commit

Permalink
Bugfix/yesdk 1071 oath credential (#556)
Browse files Browse the repository at this point in the history
* Reorganized tests, and added tests to cover bugs related to the handling of optional credential properties.

* Issuer property uses null when there is no issuer.

* Issuer handles white space.

* Constructors accept null issuer, and parser returns a nullable string.

* Strings are trimmed, null used only as a default value on optional properties, updated property validation checks, updated name builder and length calculation.

* Cleaned up formatting.

* Fixed asserts on non-nullable property.

* Converted GetName() into expression-bodied member, Name, to preserve existing API.

* Rolled back changes not directly related to the optional Issuer property and Name construction. Updated URI parsing to pass existing unit test and added test to check correct handling of URI without Issuer.

* Small cosmetic changes.

* Updated documentation.

* Removed white space trimming of issuer and account properties.
  • Loading branch information
coonsd authored and Greg Domzalski committed Feb 2, 2023
1 parent 7c99692 commit a9b390c
Show file tree
Hide file tree
Showing 12 changed files with 671 additions and 200 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ CLA | INS | P1 | P2 | Lc | Data |
| IMF data(o) | Imf |

Notes:
- Name data is presented as "period/issuer:account", if period is standard (30s) then it's presented as "issuer:account".
- Minimal length of the Key data (secret) is 14 bytes, if the lentgth is less then pad with 0s.
- Name data is typically presented as "period/issuer:account", but the "period/" and "issuer:" are optional under certain configurations.
- Minimal length of the Key data (secret) is 14 bytes, if the length is less then pad with 0s.
- Key (secret) is arbitary key value encoded in Base32 according to RFC 3548.
- Imf data is a Counter, which counts the number of iterations for HOTP.

Expand Down Expand Up @@ -218,7 +218,7 @@ CLA | INS | P1 | P2 | Lc | Data |

Notes:
- This command is only available on YubiKeys with firmware version 5.3.0 and later.
- Name data is presented as `period/issuer:account`. If period is standard (30s) or if credential is HOTP then it's presented as `issuer:account`.
- Name data is presented as `period/issuer:account`, but the "period/" and "issuer:" are optional under certain configurations.
- The new issuer can be an empty string.

### Response APDU info
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,34 +26,34 @@ The credential has a set of parameters.

| Name | Description |
| :--- | :--- |
| Issuer | The issuer parameter is a string value indicating the provider or service this account is associated with. |
| Issuer | The issuer parameter is an optional string value indicating the provider or service this account is associated with. |
| Account Name | The account name is a string that usually is the user's email address. |
| Type | Indicates the type of the credential as either HOTP or TOTP. |
| Algorithm | The hash algorithm used by the credential. |
| Secret | The secret parameter is an arbitrary key value encoded in Base32 according to RFC 3548. |
| Digits | The number of digits in a one-time password (OTP). |
| Requires Touch | The credential requires the user to touch the key to generate a one-time password (OTP). |
| Name | Only get property witch serves as the unique identifier for the credential.|
| Name | Only get property which serves as the unique identifier for the credential. |

The Name is created from Period, Issue and Account Name with the following format:

```
"period/issuer:account"
```

If period is a default value - 30 seconds, or the credential's type is HOTP, then the format will be:
If the credential is TOTP with the default period (30 seconds) or the credential's type is HOTP, then the format will be:

```
"issuer:account"
```

Also, if Issuer is not specified, the format will be:
If Issuer is not specified, the format will be:

```
"period/account"
```

Or just an Account Name for TOTP with default period or HOTP credentials:
Or just an Account Name for TOTP with default period (30 seconds) or HOTP credentials:

```
"account"
Expand Down Expand Up @@ -121,12 +121,12 @@ The URI specification [RFC 3986](https://datatracker.ietf.org/doc/html/rfc3986).

2. Specifying each parameter

If you are unable to capture the QR code and use a URI string, you can manually create the credential by adding the account information, the provider (Amazon, Google, Microsoft, etc.) and the shared secret.
If you are unable to capture the QR code and use a URI string, you can manually create the credential by adding the account information. The Issuer is recommended, but not required.

```
// create TOTP credential
var credential = new Credential {
Issuer = Yubico,
Issuer = "Yubico",
AccountName = "[email protected]",
Type = Totp,
Period = 30,
Expand All @@ -137,12 +137,11 @@ var credential = new Credential {
// create HOTP credential
var credential = new Credential {
Issuer = Yubico,
Issuer = "Yubico",
AccountName = "[email protected]",
Type = Hotp,
Period = 0,
Digits = 6,
Counter =
Counter = 0,
Secret = "HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ",
RequireTouch = false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ var credentialHotp = new Credential
Issuer = "Yubico",
AccountName = "[email protected]",
Type = Hotp,
Period = 0,
Algorithm = Sha256,
Digits = 8,
Counter = 10,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,11 @@ The label is created from:

| Name | Description |
| :--- | :--- |
| Issuer | A string value indicating the provider or service this account is associated with. It can be absent. |
| Issuer | An optional string value indicating the provider or service this account is associated with. |
| Account Name | A URI-encoded string that usually is the user's email address. |

It is formatted as "Issuer:Account" when both parameters are present. It is formatted as "Account" when there is no Issuer.

The label prevents collisions between different accounts with different providers that might be identified using the same account name, e.g. the user's email address.

The issuer and account name should be separated by a literal or url-encoded colon, and optional spaces may precede the account name. Neither issuer nor account name may themselves contain a colon. According to [RFC 5234](https://www.rfc-editor.org/rfc/rfc5234.txt) a valid label might look like:
Expand All @@ -67,6 +69,7 @@ Example:[email protected]
ACME%20Co:[email protected]
```

## Parameters

### Secret
Expand All @@ -81,7 +84,7 @@ There is Base32 helper class in the Yubico.Core library.

### Issuer

The issuer parameter is a string value indicating the provider or service the credential is associated with. It is URL-encoded according to [RFC 3986](https://datatracker.ietf.org/doc/html/rfc3986).
The issuer parameter is an optional string value indicating the provider or service the credential is associated with. It is URL-encoded according to [RFC 3986](https://datatracker.ietf.org/doc/html/rfc3986).

Valid values corresponding to the label examples above would be:

Expand Down
13 changes: 11 additions & 2 deletions Yubico.YubiKey/src/Resources/ExceptionMessages.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions Yubico.YubiKey/src/Resources/ExceptionMessages.resx
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@
<value>A credential cannot be null.</value>
</data>
<data name="InvalidCredentialAccount" xml:space="preserve">
<value>A credential's account cannot be null.</value>
<value>A credential's account cannot be empty or white space.</value>
</data>
<data name="InvalidCredentialNameLength" xml:space="preserve">
<value>A credential's name cannot be more than 64 bytes in length.</value>
Expand Down Expand Up @@ -488,7 +488,7 @@
<value>Failed to select the smart card application. 0x{0:X4}</value>
</data>
<data name="InvalidCredentialType" xml:space="preserve">
<value>A credential type cannot be null.</value>
<value>A credential type must be specified.</value>
</data>
<data name="InvalidKeyboardInstruction" xml:space="preserve">
<value>A YubiOTP APDU was received with instruction number {0}. This instruction is not supported.</value>
Expand Down Expand Up @@ -853,4 +853,7 @@
<data name="YubiHsmAuthTouchTimeout" xml:space="preserve">
<value>The operation timed out while waiting for touch.</value>
</data>
<data name="InvalidUriQuery" xml:space="preserve">
<value>The URI query cannot be null or empty.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ private static Credential FromLabelAndType(string label, CredentialType type)
throw new ArgumentNullException(nameof(label));
}

(CredentialPeriod period, string issuer, string account) = Credential.ParseLabel(label, type);
(CredentialPeriod period, string? issuer, string account) = Credential.ParseLabel(label, type);
return new Credential(issuer, account, type, period);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ private static Credential _GetCredential(ReadOnlyMemory<byte> value)
var algorithm = (HashAlgorithm)(algorithmType & 0x0F);
var type = (CredentialType)(algorithmType & 0xF0);
string label = Encoding.UTF8.GetString(value.Slice(1).ToArray());
(CredentialPeriod period, string issuer, string account) = Credential.ParseLabel(label, type);
(CredentialPeriod period, string? issuer, string account) = Credential.ParseLabel(label, type);

return new Credential(issuer, account, period, type, algorithm);
}
Expand Down
Loading

0 comments on commit a9b390c

Please sign in to comment.