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

#153 Delegated management token support #155

Merged
merged 10 commits into from
May 3, 2019
Merged

#153 Delegated management token support #155

merged 10 commits into from
May 3, 2019

Conversation

gregorjohannson
Copy link
Contributor

  • Add universal token generation support (by assigning a path to private key with "-token")
  • Add DAP properties to -install command CAP loading part (extract the same functionality from -load to new method)
  • Some newbie-friendly refactoring

@gregorjohannson
martinpaljak#153: Add initial functionality of including DM token cal…
f99b873 
…culation for INSTALL [for load]. Minor refactoring for more clarity.
@gregorjohannson
martinpaljak#153: Extract DM token addition to separate universal han…
21b1865 
…dler, use in all commands that require DM token. Replace private key loading implementation.
@gregorjohannson
martinpaljak#153: Simple tests for DelegatedManagementHandler
d3ba331
@gregorjohannson
martinpaljak#153: Refactor DAP properties assignment to separate clas…
d480e4e 
…s. Handle DAP in --install. Extract duplicated CAP loading to separate method. Declare DM token in GlobalPlatform and load it in transitDM. Refactoring.
@gregorjohannson
martinpaljak#153: Sign APDU data for token with crypto.Cipher (RSA/EC…
3338d5c 
…B/PKCS1Padding) instead of security.Signature
@martinpaljak
Copy link
Owner

This pull request introduces 1 alert when merging 3338d5c into f94d7f5 - view on LGTM.com

new alerts:

  • 1 for Boxed variable is never null

Comment posted by LGTM.com

@gregorjohannson
martinpaljak#153: Fix introduced LGTM alert. Remove DAPProperties set…
a0a18a8 
…ters. Cipher back to Signature. Swap -token to -token-key.
@martinpaljak
Copy link
Owner

This pull request introduces 1 alert and fixes 1 when merging a0a18a8 into f94d7f5 - view on LGTM.com

new alerts:

  • 1 for Boxed variable is never null

fixed alerts:

  • 1 for Dereferenced variable may be null

Comment posted by LGTM.com

@martinpaljak
Copy link
Owner

This pull request fixes 1 alert when merging dfce1b7 into f94d7f5 - view on LGTM.com

fixed alerts:

  • 1 for Dereferenced variable may be null

Comment posted by LGTM.com

@gregorjohannson
martinpaljak#153: Fix false assumption that INS_DELETE does not requi…
11e6b9d 
…re zero-length of DM token to be transmitted
@martinpaljak
Copy link
Owner

This pull request fixes 1 alert when merging 11e6b9d into 1551b30 - view on LGTM.com

fixed alerts:

  • 1 for Dereferenced variable may be null

Comment posted by LGTM.com

@gregorjohannson
martinpaljak#153: Include required LFDB hash calculation for load if…
53b9781 
… token-key exists (as per specification). Respect SHA256 option for load if DAP not required.
@martinpaljak
Copy link
Owner

This pull request fixes 1 alert when merging 53b9781 into 73e0db9 - view on LGTM.com

fixed alerts:

  • 1 for Dereferenced variable may be null

Comment posted by LGTM.com

@martinpaljak
Copy link
Owner

This pull request fixes 1 alert when merging 8f2788e into 73e0db9 - view on LGTM.com

fixed alerts:

  • 1 for Dereferenced variable may be null

Comment posted by LGTM.com

@gregorjohannson gregorjohannson marked this pull request as ready for review April 23, 2019 10:48
@martinpaljak martinpaljak merged commit c8dad37 into martinpaljak:master May 3, 2019
@martinpaljak
Copy link
Owner

martinpaljak commented May 3, 2019 •
edited
Loading

Thanks! It will undergo some generic rewrites.

martinpaljak
martinpaljak reviewed May 3, 2019
View reviewed changes
Copy link
Owner

@martinpaljak martinpaljak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinpaljak martinpaljak martinpaljak left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gregorjohannson @martinpaljak